> -----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