> -----Original Message-----
> From: Robin Murphy [mailto:[email protected]]
> Sent: 29 October 2020 15:54
> To: Shameerali Kolothum Thodi <[email protected]>;
> [email protected]; [email protected]
> Cc: Linuxarm <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; John Garry
> <[email protected]>; Song Bao Hua (Barry Song)
> <[email protected]>; Jonathan Cameron
> <[email protected]>
> 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 <[email protected]>
> > ---
> > -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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
