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

[Alex Williamson <alex@shazbot.org>]

On Tue, 2 Jun 2026 22:46:13 +0800
fengchengwen <fengchengwen@huawei.com> wrote:
> On 6/1/2026 11:58 PM, Alex Williamson wrote:
> > 
> > 
> >  - Patch 2/ sets tph_cap to zero if TPH cannot be enabled, but this is
> >    never propagated to vfio-pci to drop the TPH capability from the
> >    capability chain.
> > 
> >  - Patch 3/ exposing the enum to callers is a bit ugly, maybe we should
> >    retain pci_enable_tph() as the "auto" wrapper, resulting in no
> >    change to existing users, and add _ext and _std wrappers.
> > 
> >  - Patch 4/ same, multiple wrappers keeps the "auto" api cleaner.  Also
> >    note that auto is broken before TPH is enabled when tph_req_type is
> >    zero.
> > 
> >  - Patch 6/:
> >    - mutex_init/destroy belong in vfio_pci_core_init_dev/release_dev()
> >    - Re-order to avoid forward declarations.
> >    - vfio_check_feature() does not require declaring PROBE as a
> >      supported op.
> >    - pcie_tph_set_st_entry() can write to MMIO space without testing
> >      for or enabling device memory or power state.
> >    - tph_init allocates shadow table regardless of opt-in for tph
> >      support.
> >    - Redundant tests for count/index in feature function.
> >    - Duplicate memcpy/kfree in feature function.
> > 
> >  - Patch 7/:
> >    - Unclear why it's vfio-pci's responsibility to provide this
> >      translation interface.
> >    - PROBE is unnecessary for vfio_check_feature().
> >    - Array size conversion is strange.
> > 
> >  - Patch 8/:
> >    - default config read handles emulated bits.  Clear the bit if
> >      necessary in vconfig, use p_setd() to mark PCI_TPH_CAP_EXT_TPH
> >      virtualized, read-only.
> >    - Follow examples like vfio_pm_config_write, vfio_vpd_config_write,
> >      vfio_exp_config_write, etc.  Set the virtualized and writable
> >      bits, perform default config write, evaluate if anything was
> >      enabled, effect the change in hardware via kernel APIs.
> >    - pcie_disable_tph() happens after the restore state is recorded.
> >    - TPH capability becomes read/write, an ABI change to the user, with
> >      only a host-based opt-in.  Seems like the userspace driver also
> >      needs to opt-in to the ABI change.
> >    - Memory enable not assured for pcie_tph_set_st_entry().
> >    - Sashiko notes there are existing hazards in the tph_enabled
> >      storage unit relative to multiple bitfields that can be updated
> >      concurrently.  The problem is worse with this exposed through
> >      vfio-pci.  
> 
> Thanks for review, all your comments are resolved in v15:

Not everything.  At a glance I can see that v15 is still pushing for
vfio-pci to provide the interface to translate CPU IDs to steering
tags, despite my push back above that it doesn't seem within vfio-pci's
scope to provide a CPU ID -> ST interface.

The host-only opt-in issue above also doesn't appear to be addressed or
rebutted.

Typically v15 would be an indication that a series is coming to
maturity, but with the quick cadence and lack of discussion throughout
this development, I'm not even sure we've arrived at a proper uAPI for
this feature.  Thanks,

Alex