Re: [PATCH RFC v2 03/11] iommu/sva: Add iommu_domain type for SVA
On Sat, Apr 02, 2022 at 08:43:16AM +, Tian, Kevin wrote: > > This assumes any domain is interchangeable with any device, which is > > not the iommu model. We need a domain op to check if a device is > > compatiable with the domain for vfio an iommufd, this should do the > > same. > > This suggests that mm_struct needs to include the format information > of the CPU page table so the format can be checked by the domain op? No, Linux does not support multiple formats for CPU page tables, AFAICT, and creating the SVA domain in the first place should check this. > > It means each mm can have a list of domains associated with it and a > > new domain is auto-created if the device doesn't work with any of the > > existing domains. > > mm has only one page table and one format. If a device is incompatible > with an existing domain wrapping that page table, how come creating > another domain could make it compatible? Because domains wrap more than just the IOPTE format, they have additional data related to the IOMMU HW block itself. Imagine a SOC with two IOMMU HW blocks that can both process the CPU IOPTE format, but have different configuration. So if device A users IOMMU A it needs an iommu_domain from driver A and same for another device B, even if both iommu_domains are thin wrappers around the same mm_struct. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC v2 02/11] iommu: Add iommu_group_singleton_lockdown()
On Sat, Apr 02, 2022 at 07:12:12AM +, Tian, Kevin wrote: > That is one scenario of dma aliasing. Another is like Alex replied where > one device has an alias requestor ID due to PCI quirks. The alias RID > may or may not map to a real device but probably what we really care > here regarding to p2p are struct devices listed in the group. Those devices are just broken and not spec complaint. IMHO we can treat them as disabling ACS for their segment as well. > We may check following conditions to set the immutable flag when > a new group is created for a device in pci_device_group(): > > 1) ACS is enabled in the upstream path of the device; > 2) the device is single function or ACS is enabled on a multi-function device; > 3) the device type is PCI_EXP_TYPE_ENDPOINT (thus no hotplug); > 4) no 'dma aliasing' on this device; It makes sense to me Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/fsl_pamu: Prepare cleanup of powerpc's asm/prom.h
powerpc's asm/prom.h brings some headers that it doesn't need itself. In order to clean it up, first add missing headers in users of asm/prom.h Signed-off-by: Christophe Leroy --- drivers/iommu/fsl_pamu.c| 3 +++ drivers/iommu/fsl_pamu_domain.c | 1 + 2 files changed, 4 insertions(+) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index fc38b1fba7cf..0d03f837a5d4 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -11,6 +11,9 @@ #include #include #include +#include +#include +#include #include diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 69a4a62dc3b9..94b4589dc67c 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -9,6 +9,7 @@ #include "fsl_pamu_domain.h" +#include #include /* -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH RFC v2 03/11] iommu/sva: Add iommu_domain type for SVA
> From: Jason Gunthorpe
> Sent: Thursday, March 31, 2022 3:02 AM
>
> On Tue, Mar 29, 2022 at 01:37:52PM +0800, Lu Baolu wrote:
> > @@ -95,6 +101,7 @@ struct iommu_domain {
> > void *handler_token;
> > struct iommu_domain_geometry geometry;
> > struct iommu_dma_cookie *iova_cookie;
> > + struct iommu_sva_cookie *sva_cookie;
>
> Cookie is still the wrong word to use here
>
> > +struct iommu_sva_cookie {
> > + struct mm_struct *mm;
> > + ioasid_t pasid;
> > + refcount_t users;
>
> Really surprised to see a refcount buried inside the iommu_domain..
>
> This design seems inside out, the SVA struct should 'enclose' the domain, not
> be a pointer inside it.
>
> struct iommu_sva_domain {
>struct kref_t kref;
>struct mm_struct *mm;
>ioasid_t pasid;
>
>/* All the domains that are linked to this */
>struct xarray domain_list;
> };
>
> And then you could have a pointer to that inside the mm_struct instead
> of just the naked pasid.
>
> > +static __maybe_unused struct iommu_domain *
>
> Why maybe unused?
>
> > +iommu_sva_get_domain(struct device *dev, struct mm_struct *mm)
> > +{
> > + struct iommu_domain *domain;
> > + ioasid_t pasid = mm->pasid;
> > +
> > + if (pasid == INVALID_IOASID)
> > + return NULL;
> > +
> > + domain = xa_load(_domain_array, pasid);
> > + if (!domain)
> > + return iommu_sva_alloc_domain(dev, mm);
> > + iommu_sva_domain_get_user(domain);
>
> This assumes any domain is interchangeable with any device, which is
> not the iommu model. We need a domain op to check if a device is
> compatiable with the domain for vfio an iommufd, this should do the
> same.
This suggests that mm_struct needs to include the format information
of the CPU page table so the format can be checked by the domain op?
>
> It means each mm can have a list of domains associated with it and a
> new domain is auto-created if the device doesn't work with any of the
> existing domains.
>
mm has only one page table and one format. If a device is incompatible
with an existing domain wrapping that page table, how come creating
another domain could make it compatible?
Thanks
Kevin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH RFC v2 02/11] iommu: Add iommu_group_singleton_lockdown()
> From: Jason Gunthorpe > Sent: Wednesday, March 30, 2022 10:30 PM > > On Wed, Mar 30, 2022 at 02:12:57PM +, Tian, Kevin wrote: > > > From: Jason Gunthorpe > > > Sent: Wednesday, March 30, 2022 7:58 PM > > > > > > On Wed, Mar 30, 2022 at 06:50:11AM +, Tian, Kevin wrote: > > > > > > > One thing that I'm not very sure is about DMA alias. Even when > physically > > > > there is only a single device within the group the aliasing could lead > > > > to multiple RIDs in the group making it non-singleton. But probably we > > > > don't need support SVA on such device until a real demand comes? > > > > > > How can we have multiple RIDs in the same group and have only one > > > device in the group? > > > > Alex may help throw some insight here. Per what I read from the code > > looks like certain device can generate traffic with multiple RIDs. > > IIRC "dma alias" refers to things like legacy PCI to PCIe bridges that > do still have multiple PCI ID's behind the bridge used in > configuration cycles however the PCI to PCIe bridge will tag all PCIe > TLPs with its own RID because classic PCI has no way for the requestor > to convey a RID to the bridge. That is one scenario of dma aliasing. Another is like Alex replied where one device has an alias requestor ID due to PCI quirks. The alias RID may or may not map to a real device but probably what we really care here regarding to p2p are struct devices listed in the group. > > So, from a Linux perspective the group should have have multiple > struct devices behind the bridge, the bridge itself, and the RID the > IOMMU HW matches on is only the RID of the PCI bridge. > > But we know this because we know there is classic PCI stuff in the > heigharchy, so we can just mark that group as incompatible. Yes. > > > > Add a flag to the group that positively indicates the group can never > > > have more than one member, even after hot plug. eg because it is > > > impossible due to ACS, or lack of bridges, and so on. > > > > OK, I see your point. It essentially refers to a singleton group which > > is immutable to hotplug. > > Yes, known at creation time, not retroactively enforced because > someone used SVA > We may check following conditions to set the immutable flag when a new group is created for a device in pci_device_group(): 1) ACS is enabled in the upstream path of the device; 2) the device is single function or ACS is enabled on a multi-function device; 3) the device type is PCI_EXP_TYPE_ENDPOINT (thus no hotplug); 4) no 'dma aliasing' on this device; The last one is a bit conservative as it also precludes a device which aliasing dma due to quirks from being treated as a singleton group. But doing so saves the effort on trying to separate different aliasing scenarios as defined in pci_for_each_dma_alias(). Probably we can go this way as the first step. Once the flag is set on a group no other event can change it. If a new identified device hits an existing singleton group in pci_device_group() then it's a bug. Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
