> -----Original Message-----
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: 29 October 2020 15:54
> To: Shameerali Kolothum Thodi <shameerali.kolothum.th...@huawei.com>;
> linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org
> Cc: Linuxarm <linux...@huawei.com>; w...@kernel.org; j...@8bytes.org;
> jean-phili...@linaro.org; ashok....@intel.com; John Garry
> <john.ga...@huawei.com>; Song Bao Hua (Barry Song)
> <song.bao....@hisilicon.com>; Jonathan Cameron
> <jonathan.came...@huawei.com>
> Subject: Re: [PATCH] iommu/arm-smmu-v3: Add def_domain_type callback
> 
> On 2020-10-29 15:41, Shameer Kolothum wrote:
> > With the introduction of def_domain_type in iommu_ops, vendor
> > drivers can now inform the iommu generic layer about any specific
> > default domain requirement for a device. Any pci dev marked as
> > untrusted is now prevented from having an IDENTITY mapping
> > domain.
> >
> > The callback is also required when the support for dynamically
> > changing the default domain of a group is available.
> 
> Yes, we want to allow the group type control to work for all drivers,
> ideally...
> 
> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.th...@huawei.com>
> > ---
> >   -Only devices downstream from externally exposed PCIe hierarchies
> >    (such as Thunderbolt outside the platform) are currently marked
> >    as "untrusted". Not aware of any ARM64 platforms that may use
> >    this type of device.
> >
> >    Nevertheless, the main motivation for this patch is to have the
> >    flexibility of changing the iommu default domain for a group based
> >    on the series[1] "iommu: Add support to change default domain of an
> >    iommu group" and that mandates vendor iommu driver to provide this
> >    callback.
> >
> >   -This is tested along with [1] and was able to change the default
> >    domain of an iommu group on an HiSilicon D06 hardware.
> >
> > 1.
> https://lore.kernel.org/linux-iommu/20200925190620.18732-1-ashok.raj@intel
> .com/
> > ---
> >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 26
> +++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index e634bbe60573..d5dbcee995db 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -2567,6 +2567,31 @@ static int arm_smmu_dev_disable_feature(struct
> device *dev,
> >     }
> >   }
> >
> > +/*
> > + * Return the required default domain type for a specific device.
> > + *
> > + * @dev: the device in query
> > + *
> > + * Returns:
> > + *  - IOMMU_DOMAIN_DMA: device requires a dynamic mapping domain
> > + *  - 0: both identity and dynamic domains work for this device
> > + */
> > +static int arm_smmu_def_domain_type(struct device *dev)
> > +{
> > +   if (dev_is_pci(dev)) {
> > +           struct pci_dev *pdev = to_pci_dev(dev);
> > +
> > +           /*
> > +            * Prevent any device marked as untrusted from getting
> > +            * placed into the Identity mapping domain.
> > +            */
> > +           if (pdev->untrusted)
> > +                   return IOMMU_DOMAIN_DMA;
> > +   }
> 
> This should be somewhere in core code - it has nothing to do with SMMUv3.

Agree.

> > +
> > +   return 0;
> 
> I don't strictly object to adding a stub callback for that bit, but why
> can't iommu_change_dev_def_domain() simply assume it from a NULL
> callback? That works for everyone, for no extra cost ;)

Right.

Hi Ashok,
Do you have any plan to respin your series and is it possible to include
the above suggestions into that? Please let me know.

Thanks,
Shameer
 
> Robin.
> 
> > +}
> > +
> >   static struct iommu_ops arm_smmu_ops = {
> >     .capable                = arm_smmu_capable,
> >     .domain_alloc           = arm_smmu_domain_alloc,
> > @@ -2589,6 +2614,7 @@ static struct iommu_ops arm_smmu_ops = {
> >     .dev_feat_enabled       = arm_smmu_dev_feature_enabled,
> >     .dev_enable_feat        = arm_smmu_dev_enable_feature,
> >     .dev_disable_feat       = arm_smmu_dev_disable_feature,
> > +   .def_domain_type        = arm_smmu_def_domain_type,
> >     .pgsize_bitmap          = -1UL, /* Restricted during device attach */
> >   };
> >
> >
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to