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

[Alex Williamson <alex@shazbot.org>]

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
don't provide direct write access to the TPH capability, but backdoors
to config space are not uncommon.  So if we consider TPH itself to be
unsafe, all forms of it present some degree of the same attack vector.

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.

> > Prior discussion with Jason (
> > ref: https://lore.kernel.org/all/20260414151125.GF2577880@ziepe.ca/)
> > indicated VFIO-PCI is the correct component for per-device TPH
> > implementation.  
> 
> > > The host-only opt-in issue above also doesn't appear to be addressed or
> > > rebutted.  
> > 
> > enable_unsafe_tph host opt-in already fully gates all new TPH ABI:
> > TPH cap + feature ioctl are completely hidden from userspace when off,
> > no userspace opt-in needed.  
> 
> 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.

> But I don't really get what is going on with all the new uAPI
> proposed.
> 
> I don't understand what VFIO_DEVICE_FEATURE_TPH_ST_CONFIG is supposed
> to be for, userspace should not be able to directly program registers
> that are kernel controled. In this case it is 'safe' and it should be
> only programmed by specifying abstract information like TPH_CPU_ST
> does.
> 
> I'm also wondering why it needs a shadow entry, that doesn't seem
> normal for config space tables..

As above, the user owns the device and even in the ~safer~ storage
locations we can't really guarantee that the user cannot overwrite
MSI-X vector table entries or manipulate the device to insert values
into config space.

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.

The shadow table comes about because we don't know until the mode is
selected whether we're using the DS (effective) location or one of the
architected locations.  In fact, the internal kernel API doesn't even
let us even set a TPH value without TPH enabled, which presumes a
specific programming model that may not align with userspace.

I had envisioned that a shadow would also provide symmetry in the
interface for the DS implementation, but I was never really able to
make that stick with this rapid succession, no pause for clarification
or discussion, development style.  Thanks,

Alex