RE: [PATCH v10 04/12] iommu: Add attach/detach_dev_pasid iommu interface

From: Tian, Kevin
Date: Wed Jul 27 2022 - 23:07:14 EST


> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Wednesday, July 27, 2022 7:54 PM
>
> On Wed, Jul 27, 2022 at 03:20:25AM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > > Sent: Tuesday, July 26, 2022 9:57 PM
> > >
> > > On Tue, Jul 26, 2022 at 02:23:26PM +0800, Baolu Lu wrote:
> > > > On 2022/7/25 22:40, Jason Gunthorpe wrote:
> > > > > On Sun, Jul 24, 2022 at 03:03:16PM +0800, Baolu Lu wrote:
> > > > >
> > > > + * Block PASID attachment in all cases where the PCI fabric is
> > > > + * routing based on address. ACS disables it.
> > > > + */
> > > > + if (dev_is_pci(dev) &&
> > > > + !pci_acs_path_enabled(to_pci_dev(dev), NULL, REQ_ACS_FLAGS))
> > > > + return -ENODEV;
> > >
> > > I would probably still put this in a function just to be clear, and
> > > probably even a PCI layer funcion 'pci_is_pasid_supported' that
> > > clearly indicates that the fabric path can route a PASID packet
> > > without mis-routing it.
> >
> > But there is no single line in above check related to PASID...
>
> The question to answer here is if the device/fabric supports PASID,
> and on PCI that requires ACS on any switches. IMHO that is a PCI layer
> question and perhaps we shouldn't even succeed pci_enable_pasid() if
> ACS isn't on.

Yes, this sounds a better approach than inventing another function
for iommu core to check.

>
> Then we don't need this weirdo check in the core iommu code at all.
>

and then we could also move group->pasid_array to device->pasid_array
with this approach. Though the end result doesn't change i.e. still only
the singleton group can enable pasid the iommu core can just stick to
the device manner now.