Re: [PATCH v17 08/12] PCI/TPH: Add sysfs binary file to export CPU to steering-tag mapping

[fengchengwen <fengchengwen@huawei.com>]

Hi Alex,

On 6/17/2026 12:57 AM, Alex Williamson wrote:
> On Tue, 16 Jun 2026 11:42:24 -0300
> Jason Gunthorpe <jgg@ziepe.ca> wrote:
> 
>> On Tue, Jun 16, 2026 at 06:46:17PM +0800, Chengwen Feng wrote:
>>> Add per-device sysfs binary attribute tph_cpu_st to expose ACPI DSM CPU
>>> to steering-tag data to userspace, resolving the concern that VFIO should
>>> not host CPU-to-ST translation interfaces.
>>>
>>> Follow PCI standard binattr framework: dynamic visible group, fixed-size
>>> 8-byte packed uapi entry, aligned offset read, root-only 0400 permission.
>>> Refactor duplicate ACPI DSM logic into shared tph_get_cpu_st_info helper.
>>>
>>> ABI: /sys/bus/pci/devices/<BDF>/tph_cpu_st  
>>
>> I'm sorry, I really dislike this :(
>>
>> Structured binary sysfs attributes are pretty much against the rules,
>> I think using sysfs at all for this interface is a bad idea.
>>
>> IMHO the VFIO version was much better.
> 
> There are some deficiencies in this implementation:
> 
>  - The ABI needs to be documented in
>    Documentation/ABI/testing/sysfs-bus-pci
> 
>  - The attribute is at the wrong place, all endpoints would just
>    replicate the root port values.  Place it at the root port.
> 
>  - Corollary, is_visible should key on whether we have CPU to ST
>    mappings (_DSM) and the root port is a TPH completer (DevCap2), not
>    the TPH capabilities of an endpoint - a userspace driver can already
>    discover the endpoint TPH requester support.
> 
>  - The 8-byte aligned read requirement should be removed, perform a
>    sub-8-byte read from the slot offset.  !cpu_possible() should be
>    filled with zeros.  This allows userspace to dump the entire bin
>    file or read only a set of fields.
> 
>  - Probably cleaner to zero-initialize the buffer rather than memset()
>    and (redundant) reserved = 0.

I’ve sent v18-resend of the PCI TPH sysfs patchset with all prior review
comments fully addressed.

I remain open to both discussed implementation options. This v18-resend
implements the sysfs binary blob approach on Root Ports as you previously
suggested.

Please kindly review v18-resend and share your thoughts.

Thanks

> 
> IMO, this implementation in sysfs more so proves that vfio is the wrong
> place for the interface.  vfio has a use case to consume STs, but it
> doesn't produce them or own any of the mechanism by which they're
> generated.  This proposal, with the above improvements, provides
> effectively an ioctl-like interface when using a properly offset and
> sized pread().
> 
> The weak point is whether this bin attribute, exposing an array of
> structures, fits within the socially acceptable norms of sysfs.  There
> is some precedent for this, for example cc_settings_bin in infiniband,
> but these might also be considered legacy.  So I don't know if this
> sort of usage is a grey area that fits social norms or if it's promoting
> legacy use cases that we don't want to repeat.  Thanks,
> 
> Alex