RE: [PATCH RFC v2 02/11] iommu: Add iommu_group_singleton_lockdown()

[Tian, Kevin]

> From: Jason Gunthorpe
> Sent: Wednesday, March 30, 2022 10:30 PM
> 
> On Wed, Mar 30, 2022 at 02:12:57PM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe
> > > Sent: Wednesday, March 30, 2022 7:58 PM
> > >
> > > On Wed, Mar 30, 2022 at 06:50:11AM +0000, 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