Re: [PATCH 1/4] iommu: Add iommu_device_group callback and iommu_group sysfs entry

[Chris Wright <chrisw@redhat.com>]

* David Gibson (dwg@au1.ibm.com) wrote:
> On Wed, Nov 30, 2011 at 04:52:20PM -0800, Chris Wright wrote:
> > * David Gibson (dwg@au1.ibm.com) wrote:
> > > On Tue, Nov 29, 2011 at 10:25:51PM -0700, Alex Williamson wrote:
> > > > Note that iommu drivers are registered per bus_type, so the unique pair
> > > > is {bus_type, groupid}, which seems sufficient for vfio.
> > > 
> > > Hrm.  That's.. far from obvious.  And still breaks down if we have two
> > > separate iommus on the same bus type (e.g. two independent PCI host
> > > bridges with inbuilt IOMMUs).
> > 
> > Happens to still work for Intel IOMMU on x86 the way Alex wrote the
> > Intel VT-d patch in this series, as well as AMD IOMMU.  The caveat for
> > AMD IOMMU is that the groupid generation would break (as-is) once
> > there's support for multiple PCI segments.  This is not an inherent
> > shortcoming of the groupid mechanism though, just a current limitation
> > of AMD IOMMU's implementation.  Alex overloaded B:D.F for those which is
> > a convenient id since that maps to the device (or in the case of devices
> > behind a PCIe-to-PCI bridge, the requestor ID of all devices behind the
> > bridge, or "the group").
> 
> "Happens to still work" is not exactly a ringing endorsement.

Heh.  Put it another way.  Generating the group ID is left up to the
IOMMU.  This will break down when there's a system with multiple IOMMU's
on the same bus_type that don't have any awareness of one another.  This
is not the case for the existing series and x86 hw.

I'm not opposed to doing the allocation and ptr as id (taking care for
possibility that PCI hotplug/unplug/replug could reuse the same memory
for group id, however).  Just pointing out that the current system works
as is, and there's some value in it's simplicity (overloading ID ==
group structure + pretty printing ID in sysfs, for example).

thanks,
-chris