Re: [PATCH v3 02/11] PCI: Add pci_bus_isolated()

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Sep 09, 2025 at 02:54:09PM -0500, Bjorn Helgaas wrote:
> On Fri, Sep 05, 2025 at 03:06:17PM -0300, Jason Gunthorpe wrote:
> > Prepare to move the calculation of the bus P2P isolation out of the iommu
> > code and into the PCI core. This allows using the faster list iteration
> > under the pci_bus_sem, and the code has a kinship with the logic in
> > pci_for_each_dma_alias().
> > 
> > Bus isolation is the concept that drives the iommu_groups for the purposes
> > of VFIO. Stated simply, if device A can send traffic to device B then they
> > must be in the same group.
> > 
> > Only PCIe provides isolation. The multi-drop electrical topology in
> > classical PCI allows any bus member to claim the transaction.
> > 
> > In PCIe isolation comes out of ACS. If a PCIe Switch and Root Complex has
> > ACS flags that prevent peer to peer traffic and funnel all operations to
> > the IOMMU then devices can be isolated.
> 
> I guess a device being isolated means that peer-to-peer Requests from
> a different bus can't reach it?

peer-to-peer requests from a different *device*

> Did you mean "Root Port" instead of "Root Complex"?  Or are you
> assuming an ACS Capability in an RCRB?  (I don't think Linux supports
> RCRBs, except maybe for CXL)

Can't really say, the interaction of ACS within the Root Port, Root
Complex and so on is not really fully specified. Something within the
Root Complex routes to the TA/IOMMU.

Linux has, and continues with this series, to assume that the Root
Port is routing to the TA because we don't accumulate other Root
Complex devices into shared groups.

> > Multi-function devices also have an isolation concern with self loopback
> > between the functions, though pci_bus_isolated() does not deal with
> > devices.
> 
> It looks like multi-function devices *can* implement ACS and can
> isolate functions from each other (PCIe r7.0, sec 6.12.1.2).  But it
> sounds like we're ignoring peer-to-peer on the same bus for now and
> assuming devices on the same bus can't be isolated from each other?

Yes, MFD has ACS, but no it is not ignored for now. The iommu grouping
code has two parts, one for busses/switches and another for MFDs.

This couple of patches switches the busses/switches to use the new
mechanism and leaves MFD alone. Later patches correct the issues in
MFD as well. The above comment is trying to explain this split in the
patch series.

> If we ignore ACS on non-bridge multi-function devices, I think the
> only way to isolate things is bridge ACS that controls forwarding
> between buses.  

Yes

> If everything on the bus must be in the same group, it
> makes sense for pci_bus_isolated() to take a pci_bus pointer and not
> deal with individual devices.

> Below, it seems like sometimes we refer to *buses* being isolated and
> other times *devices* (Root Port, Switch Port, Switch, etc), so I'm a
> little confused.

They are different things, and have different treatment. I've tried to
keep them separated by having code that works on busses and different
code that works on devices. 

A bus is isolating if upstream travelling transactions reaching the
bus only go upstream.

A device is isolated if its bus and all upstream busses are isolating,
and the device itself has no internal loopback.

In this code it sometimes talks about the device in terms of a
bridge, USP, DSP, or Root Port. All of these are a bit special because
a upstream travelling transaction is permitted to internal loopback to
the bridge device without touching a bus.

So while the bridge device may be on an isolating bus, the bridge
device itself is not isolated from its downstream bus.

> > As a property of a bus, there are several positive cases:
> > 
> >  - The point to point "bus" on a physical PCIe link is isolated if the
> >    bridge/root device has something preventing self-access to its own
> >    MMIO.
> >
> >  - A Root Port is usually isolated
> >
> >  - A PCIe switch can be isolated if all it's Down Stream Ports have good
> >    ACS flags
> 
> I guess this is saying that a switch's internal bus is isolated if all
> the DSPs have the ACS flags we need?

Yes

> > pci_bus_isolated() implements these rules and returns an enum indicating
> > the level of isolation the bus has, with five possibilies:
> > 
> >  PCIE_ISOLATED: Traffic on this PCIE bus can not do any P2P.
> 
> Is this saying that peer-to-peer Requests can't reach devices on this
> bus?  Or Requests *from* this bus can only go to the IOMMU?

Transactions mastered on this bus travelling in the upstream direction
are only received by the upstream bridge and are never received by any
other device on the bus.

Or Transactions reaching this bus travelling in the upstream direction
continue upstream and never go back downstream.

I would not use the words 'from' or 'to' when talking about busses,
they don't originate or terminate transactions they are just
transports.

> > + * pci_bus_isolated() does not consider loopback internal to devices, like
> > + * multi-function devices performing a self-loopback. The caller must check
> > + * this separately. It does not considering alasing within the bus.
> 
> s/alasing/aliasing/ (I guess this refers to the
> PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS thing where a bridge takes ownership?)

Yes

> > +	/*
> > +	 * bus->self is only NULL for SRIOV VFs, it represents a "virtual" bus
> > +	 * within Linux to hold any bus numbers consumed by VF RIDs. Caller must
> > +	 * use pci_physfn() to get the bus for calling this function.
> 
> s/VF RIDs/VFs/  I think?  I think we allocate these virtual bus
> numbers when enabling the VFs.

Maybe BDF instead of RID

> > +	/*
> > +	 * bus is the interior bus of a PCI-E switch where ACS rules apply.
> 
> s/interior/internal/ to match use above
> s/PCI-E/PCIe/
> 
> I'm not sure what this is saying.  A USP can't have an ACS Capability
> unless it's part of a multi-function device.

So it can have ACS :)

Not sure what is unclear?

I will fix up all the notes

Thanks,
Jason