Re: [PATCH v19 13/18] vfio/pci: Introduce VFIO_DEVICE_FEATURE_TPH family uapi for PCI TPH control

[fengchengwen <fengchengwen@huawei.com>]

Hi Alex,

On 7/9/2026 7:31 AM, Alex Williamson wrote:
> On Thu, 2 Jul 2026 20:42:19 +0800
> Chengwen Feng <fengchengwen@huawei.com> wrote:
> 
>> Add user API definitions for PCI TPH device feature set, including three
>> feature IDs:
>> - VFIO_DEVICE_FEATURE_TPH: Global TPH enable and capability query
>> - VFIO_DEVICE_FEATURE_TPH_RESOLVE: Resolve PH/ST from specified source
>>   (GET only)
>> - VFIO_DEVICE_FEATURE_TPH_ST: Batch program ST table entries (SET only)
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>>  include/uapi/linux/vfio.h | 62 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 62 insertions(+)
>>
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index 5de618a3a5ee..ed04b6280cd6 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -1534,6 +1534,68 @@ struct vfio_device_feature_dma_buf {
>>   */
>>  #define VFIO_DEVICE_FEATURE_MIG_PRECOPY_INFOv2  12
>>  
>> +/* PCIe TPH device feature definitions for VFIO_DEVICE_FEATURE ioctl */
>> +#define VFIO_DEVICE_FEATURE_TPH		13
>> +#define VFIO_DEVICE_FEATURE_TPH_RESOLVE	14
>> +#define VFIO_DEVICE_FEATURE_TPH_ST	15
>> +
>> +/*
>> + * VFIO_DEVICE_FEATURE_TPH - Control and query PCI TPH capabilities
>> + *
>> + * SET: Opt-in to TPH feature; flags must be zero.
> 
> We clearly need to define what it actually means to opt-in, as this
> series is not implementing it as it should be defined.  The user needs
> to opt-in via this feature before we modify the device ABI in any way.
> The extent to which the user may enable TPH features is bound both by
> the device capabilities and the module parameters set by the system
> admin.

I will add full documentation describing the mandatory opt-in rule: users
must call SET on this feature before any TPH virtualization or TPH_RESOLVE/
TPH_ST ioctl access is allowed. Without opt-in, TPH config space registers
remain raw read-only and related TPH feature ioctls return -ENOTSUPP.
Also clarify the dual capability bound: exposed TPH functions are limited by
both hardware native TPH capability and host-wide tph module policy.

> 
>> + * GET: Return supported TPH capability bits in flags.
>> + */
>> +struct vfio_device_feature_tph {
>> +	__u32 flags;
>> +};
>> +
>> +/* Capability bits returned by GET on VFIO_DEVICE_FEATURE_TPH */
>> +#define VFIO_DEVICE_TPH_CAP_DMABUF	(1u << 0) /* DMABUF sources resolvable */
>> +#define VFIO_DEVICE_TPH_CAP_CPU		(1u << 1) /* CPU sources resolvable via _DSM */
>> +#define VFIO_DEVICE_TPH_CAP_LITERAL	(1u << 2) /* LITERAL ST source supported */
>> +
>> +/*
>> + * VFIO_DEVICE_FEATURE_TPH_RESOLVE - Resolve PH/ST tag value from source
>> + * Note: GET only
>> + */
>> +struct vfio_device_feature_tph_resolve {
>> +	__u32 flags;	/* IN: source type + namespace modifiers */
>> +	__u32 src;	/* IN: dma-buf fd or CPU ID based on flags */
>> +	__u8 valid;	/* OUT: bitmap indicating valid output fields */
>> +#define VFIO_DEVICE_TPH_VALID_PH (1u << 0) /* ph holds valid processing hint */
>> +#define VFIO_DEVICE_TPH_VALID_ST (1u << 1) /* st holds valid steering tag */
>> +	__u8 ph;	/* OUT: TPH processing hint */
>> +	__u16 st;	/* OUT: raw steering tag value */
>> +};
>> +
>> +/*
>> + * VFIO_DEVICE_FEATURE_TPH_ST - Batch program architected ST table entries
>> + * Return value: number of successfully written entries
>> + * Note: SET only
>> + */
>> +struct vfio_device_feature_tph_st {
>> +	__u32 flags;		/* IN: source type + namespace modifiers */
>> +	__u16 start;		/* IN: first ST table index / MSI-X vector */
>> +	__u16 count;		/* IN: number of contiguous entries to program */
>> +	__aligned_u64 dests;	/* IN: user pointer to array of source u32 values */
>> +};
>> +
>> +/* Common flags shared by TPH_RESOLVE and TPH_ST features */
>> +#define VFIO_DEVICE_TPH_SRC_NONE           (1u << 0) /* TPH_ST only: set ST=0 */
>> +#define VFIO_DEVICE_TPH_SRC_DMABUF	   (1u << 1) /* src = dma-buf fd */
>> +#define VFIO_DEVICE_TPH_SRC_CPU_VOLATILE   (1u << 2) /* src = volatile memory CPU ID */
>> +#define VFIO_DEVICE_TPH_SRC_CPU_PERSISTENT (1u << 3) /* src = persistent memory CPU ID */
>> +#define VFIO_DEVICE_TPH_SRC_LITERAL        (1u << 4) /* TPH_ST only: src = raw literal ST */
>> +#define VFIO_DEVICE_TPH_EXTENDED           (1u << 30) /* Use extended 16-bit ST namespace */
>> +#define VFIO_DEVICE_TPH_REQUIRE_ST         (1u << 31) /* TPH_ST only: fail if resolved ST=0 */
> 
> I don't see the value in jumping to the end of the flags field for
> these last two.  Adding DMABUF is a good change from the proposal, but
> I wouldn't go out of our way to try to preserve some grouping of source
> flags from modifiers.

I will move VFIO_DEVICE_TPH_EXTENDED and VFIO_DEVICE_TPH_REQUIRE_ST down
from bits 30/31 to lower unused positions, remove artificial separation
between source-type flags and modifier flags for cleaner grouping.
> 
> We also seem to have avoided any statement or decision to what extent
> our uAPI is defined by the current implementation.  Particularly the
> ordering of TPH_ST versus manipulating the control bits in config space
> are entirely undefined here.
> 
> We're also not addressing that the TPH_ST policy counters the overall
> feature policy to return 0/-errno and not updating the documentation to
> bring that to the forefront.  Thanks,

Two undefined behaviors will be explicitly documented:
- Execution order and priority between VFIO_DEVICE_FEATURE_TPH_ST batch
  programming and direct userspace writes to TPH_CTRL config registers;
- Error return rules when requested operation exceeds current tph_policy
  level (Literal mode blocked under policy <= 2 returns -ENOTSUPP).

Both will be fix in v20.

Thanks

> 
> Alex
> 
>> +
>> +/* Mask for mutually exclusive source type bits, only one may be set */
>> +#define VFIO_DEVICE_TPH_SRC_MASK	(VFIO_DEVICE_TPH_SRC_NONE | \
>> +					 VFIO_DEVICE_TPH_SRC_DMABUF | \
>> +					 VFIO_DEVICE_TPH_SRC_CPU_VOLATILE | \
>> +					 VFIO_DEVICE_TPH_SRC_CPU_PERSISTENT | \
>> +					 VFIO_DEVICE_TPH_SRC_LITERAL)
>> +
>>  /* -------- API for Type1 VFIO IOMMU -------- */
>>  
>>  /**
>