Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1258943

Re: [PATCH 8/8] iommu: Move default domain allocation to iommu_group_get_for_dev()

Show all headers | View raw

Hi Joerg,

Looking at this from an SMMU perspective, I think there's an issue
with attaching to the default domain like this.

On Wed, Oct 21, 2015 at 11:51:43PM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Now that the iommu core support for iommu groups is not
> pci-centric anymore, we can move default domain allocation
> to the bus independent iommu_group_get_for_dev() function.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/iommu.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index e2b5526..abae363 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -810,14 +810,6 @@ struct iommu_group *pci_device_group(struct device *dev)
>  	if (IS_ERR(group))
>  		return NULL;
>  
> -	/*
> -	 * Try to allocate a default domain - needs support from the
> -	 * IOMMU driver.
> -	 */
> -	group->default_domain = __iommu_domain_alloc(pdev->dev.bus,
> -						     IOMMU_DOMAIN_DMA);
> -	group->domain = group->default_domain;
> -
>  	return group;
>  }
>  
> @@ -849,6 +841,16 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
>  	if (IS_ERR(group))
>  		return group;
>  
> +	/*
> +	 * Try to allocate a default domain - needs support from the
> +	 * IOMMU driver.
> +	 */
> +	if (!group->default_domain) {
> +		group->default_domain = __iommu_domain_alloc(dev->bus,
> +							     IOMMU_DOMAIN_DMA);
> +		group->domain = group->default_domain;
> +	}
> +
>  	ret = iommu_group_add_device(group, dev);
>  	if (ret) {
>  		iommu_group_put(group);

The call to iommu_group_get_for_dev in arm_smmu_add_device will end up
calling __iommu_attach_device, since group->domain will now be initialised
by the code above. This means the SMMU driver will see an ->attach_dev
call for a device that is part-way through an ->add_device callback and
will be missing the initialisation necessary for us to idenfity the SMMU
instance to which is corresponds. In fact, the iommudata for the group
won't be initialised at all, so the whole thing will fail afaict.

Note that I haven't actually taken this for a spin, so I could be missing
something.

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/8] iommu: Make core iommu-groups code more generic Joerg Roedel <joro@8bytes.org> - 2015-10-22 00:00 +0200
  [PATCH 3/8] iommu: Add generic_device_group() function Joerg Roedel <joro@8bytes.org> - 2015-10-22 00:00 +0200
  [PATCH 5/8] iommu/fsl: Convert to device_group call-back Joerg Roedel <joro@8bytes.org> - 2015-10-22 00:00 +0200
  [PATCH 2/8] iommu: Export and rename iommu_group_get_for_pci_dev() Joerg Roedel <joro@8bytes.org> - 2015-10-22 00:00 +0200
  [PATCH 7/8] iommu: Remove is_pci_dev() fall-back from iommu_group_get_for_dev Joerg Roedel <joro@8bytes.org> - 2015-10-22 00:00 +0200
  [PATCH 8/8] iommu: Move default domain allocation to iommu_group_get_for_dev() Joerg Roedel <joro@8bytes.org> - 2015-10-22 00:00 +0200
    Re: [PATCH 8/8] iommu: Move default domain allocation to  iommu_group_get_for_dev() Will Deacon <will.deacon@arm.com> - 2015-10-29 19:30 +0100
      Re: [PATCH 8/8] iommu: Move default domain allocation to  iommu_group_get_for_dev() Joerg Roedel <joro@8bytes.org> - 2015-10-30 15:20 +0100
  [PATCH 6/8] iommu/arm-smmu: Switch to device_group call-back Joerg Roedel <joro@8bytes.org> - 2015-10-22 00:00 +0200
  [PATCH 1/8] iommu: Revive device_group iommu-ops call-back Joerg Roedel <joro@8bytes.org> - 2015-10-22 00:00 +0200

csiph-web