Re: [PATCH v14 0/8] vfio/pci: Add PCIe TPH support

[Alex Williamson <alex@shazbot.org>]

On Thu, 4 Jun 2026 15:33:02 -0300
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Wed, Jun 03, 2026 at 12:53:08PM -0600, Alex Williamson wrote:
> > On Tue, 2 Jun 2026 21:45:24 -0300
> > Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >   
> > > On Wed, Jun 03, 2026 at 08:34:53AM +0800, fengchengwen wrote:
> > >   
> > > > This translation helper is required to handle varied TPH configurations,
> > > > including cases where ST entries live inside device private registers
> > > > instead of MSI-X or TPH capability space.    
> > > 
> > > I thought we agreed vfio had to block that feature in config space due
> > > to security concerns? That's what enable_unsafe_tph is about, right?  
> > 
> > Steering tags can effectively live in one of three places.  The
> > architected locations are the TPH capability itself and the MSI-X
> > table, where support is mutually exclusive.  However the device can
> > support either of these and still enable DS mode, thus essentially
> > a third location.
> > 
> > None of these are particularly more or less unsafe than another.  It's
> > inadvisable for drivers to write to the MSI-X vector table, versus
> > using the SET_IRQS uAPI, but it's not prevented anymore via mmap.   
> 
> We'd have to go back to preventing it..

Disabling mmaps across vector tables obviously re-introduces the
performance issues on devices that don't account for a 64K exclusion
zone around the MSI-X vector table.
 
> > We don't provide direct write access to the TPH capability, but
> > backdoors to config space are not uncommon.    
> 
> I think we can discount this, if a device exposes TPH through config
> then it should keep it secure.

We have plenty of other caveats for userspace drivers that demand
"well behaved devices", so I agree we might be able to ignore the
backdoor vector.  That still means we can't control STs in DS mode, and
IV mode requires an opt-in to protect the vector table, at a potential
performance penalty, depending on the host and device.
 
> > I'm actually still wrestling with the question whether any of this
> > is really unsafe beyond the scope of ownership of the device at
> > all.  
> 
> Well there is certainly some kind of DOS argument for calling it
> unsafe, and we don't actually know what effects the TPH value even
> has.
> 
> I'd be much more worried about some "smart" accelerator device doing
> something impactful based on TPH than insecure config space for
> example.

Is that just a general sense of FUD to add weight to push the whole
thing behind an opt-in?

We could spin the effective storage locations a few different ways,
allowing TPH without an opt-in if we could limit it to the capability
location, requiring MSI-X mmap exclusion to enable devices that support
IV, and unblocking DS with another opt-in.  But if we have a general
sense that this is scary in all cases, then we might as well just have
one global opt-in.

> > > I agree in general that the presence of enable_unsafe_tph VFIO has to
> > > report the per-device steering tags to userspace so it can program the
> > > device specific registers, nothing else can do this.  
> > 
> > What's the scope of "nothing else" here?  The raw ST value for a CPU is
> > derived from an ACPI _DSM associated to a root port.  So if the scope
> > is that the kernel needs to provide that translation, I agree, but
> > where in the kernel it's provided may still require some thought.  
> 
> The APIs accept a PCI device:
> 
> int pcie_tph_get_cpu_st(struct pci_dev *pdev, enum tph_mem_type mem_type,
> 			unsigned int cpu, u16 *tag)
> 
> 
> IIRC the ACPI allows it to vary based on the source. Nothing else
> could provide the PCI device owned by VFIO to these functions..

The function immediately translates the endpoint function to a root
bridge, so I could certainly imagine some pcie port driver that creates
an interface to provide ST translations.  It could really be exposed
through sysfs too, except the entries are a multiple of NR_CPUS, so the
attribute count would explode if we adhere to a single value per
attribute.  Regardless, it's an ACPI lookup, so it doesn't rely on
device ownership, only using the device to lookup the ACPI companion to
invoke the DSM.

> > We also need to consider how this incorporates Zhiping's series, where
> > TPH values can be associated to a dmabuf.  Would userspace get access
> > to those raw TPH values to program the initiator?  It must for DS
> > support.  Therefore the interface became one of the user handling raw
> > ST values and the CPU ID to ST translation became this ill-fitting
> > sidecar.  
> 
> Arguably yes, likely iommufd would need an API to return the ST of the
> dmabuf when it maps it.

Right, so hosting a CPU ID to ST translation interface in the vfio-pci
device feature interface feels pretty shortsighted.  I don't know what
it should look like though.  Thanks,

Alex