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

[Alex Williamson <alex@shazbot.org>]

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.

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