Re: [PATCH 03/11] iommu: Compute iommu_groups properly for PCIe switches

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Sep 23, 2025 at 03:29:52PM -0600, Alex Williamson wrote:
> > > Where?  Is this in reference to our handling of multi-function
> > > endpoints vs whether downstream switch ports are represented as
> > > multi-function vs multi-slot?  
> > 
> > If you have a MFD Linux with no ACS it will group the whole MFD if any
> > of it lacks ACS caps because it assumes there is an internal loopback
> > between functions.
> 
> Yes
> 
> > If the MFD has a single function with ACS then only that function is
> > removed from the group. The only way we can understand this as correct
> > by our grouping definition is to require the MFD have no internal
> > loopback. ACS is an egress control, not an ingress control.
> 
> Yes, current grouping is focused on creating sets of devices that
> cannot perform DMA outside of their group without passing through a
> translation agent.  It doesn't account for ingress from other devices.

That isn't the definition of groups at all. If any two devices can DMA
to each other they must be in the same group.

There is no ingress or egress rational with groups. The fact the
implementation is creating these inconsistencies with ingress vs
egress is a BUG and not in anyway part of the security definition.

> One of the few examples of this that seems to exist is something like
> you're describing where we have a MFD and one of the functions is
> quirked or reports an empty ACS capability to create another group.

Yes, one of my test systems has an intel NIC like this. 

> The ACS/quirk device is believed not to have the capability to DMA into
> the non-ACS/quirk devices, but the opposite is not guaranteed.  

I don't think so. The quirk should have been applied to the whole
MFD. Because the code has this behavior people got away with quirking
the NIC only. IMHO the way to understand that quirk is applying to the
whole MFD.

> In practice the ACS/quirk device is typically the only device that's
> worthwhile to assign, so the host is still isolated from the
> userspace driver.  Arguably the userspace device may not be isolated
> from the host devices, but without things like TDISP, there's
> already a degree of trust in other host drivers and devices.

No way! That isn't how any of this should work with hand waving around
"worthwhile to assign" guessing.

> I'm afraid that including the ingress potential in the group
> configuration is going to blow up existing groups, for not much
> practical gain.  

You are aruging both ways. The current design does not follow the
spec, and does not implement the security defintion for groups.

Dismissing that as "no practical gain" for correcting the security is
perhaps true, but then don't argue against this series that it
slightly weakens the security if it has "no practical gain". :(

> I wonder if there's an approach where a group split in
> this way might taint the non-ACS/quirk group to prevent vfio use cases
> and whether that would sufficiently close this gap with minimal
> breakage.

You want to build asymmetric groups now?? It is already too
complicated - this really brings no value IMHO.

> > If a MFD function is a bridge/port then the group doesn't propogate
> > the group downstream of the bridge - again this requires assuming
> > there is no internal loopback between functions.
> 
> I think that if we have a multi-function root port without ACS/quirks
> that all the functions and downstream devices are grouped together.

Yes, if the MFD bridge has ACS then yes it wrongly blocks the
propogation. Again the only way to understand this behavior as making
sense is if it is assuming there is no internal MFD loopback.

> > It is taking the undefined behavior in the spec and selectively making
> > both interpretations at once.
> 
> The intention is that undefined behavior should be considered
> non-isolated.

Sure, but it doesn't do that

> We try to define that boundary of a group based on provable egress
> DMA.

It's a bug. That logic doesn't match the security defintion of groups.

> > How about we answer the question "does this MFD have internal
> > loopback" as:
> >  - NO if any function has an appropriate ACS cap or quirk.
> 
> In this case rather than split the one ACS/quirk function into a group
> we split each function into a group.  Now we potentially have singleton
> groups for non-ACS/quirk functions that we really have no basis to
> believe are isolated from other similar devices.  

No, that's too strong. Given that ACS is an egress control it is
totally pointless for a vendor to make an ACS that egress controls one
function but the MFD has internal loopback allowing other functions to
ingress.

We are making the assumption, that Linux is already making, where if
some vendor has added/quirked ACS then it actually provides egress and
ingress isolation to that function. Meaning there is no MFD internal
loopback.

I'm propsing the consistently broaden this assumption to the whole MFD.

> >  - NO if any function is bridge/port
> 
> This would hand-wave away grouping multi-function downstream ports
> without ACS/quirks with no justification afaict.

We can drop this if the bridge functions have ACS already today to get
their groups split.

> >  - YES otherwise - all functions are end functions and no ACS declared
> > 
> > As above this is quite a bit closer to what Linux is doing now. It is
> > a practical estimation of the undefined spec behavior based on the
> > historical security posture of Linux.
> 
> It's really not what we're doing now.  We currently consider undefined
> behavior to be non-isolated, or we try to.  

Maybe it wanted to, but it isn't implemented right.

> The above makes broad and unwarranted (IMO) isolation claims.

I agree, it is trying to match the bugs in the current code with a
grounding in something explainable and understandable that matches our
security definition for groups.

> This puts data at risk more so than assuming undefined behavior is
> non-isolated.

I already sent a series that actually assumed undefined behavior was
non-isolated.

It breaks alot of systems because doing that *correctly* is extremely
pessimistic toward real world HW.

> Should we re-evaluate how we handle downstream switch ports exposed as
> separate slots, certainly.  

So lets start with that, we can stop this series after it fixes the
switch ports.

Jason