Re: [PATCH v19 10/18] vfio/pci: Introduce tph policy parameter for staged TPH feature enablement

[Alex Williamson <alex@shazbot.org>]

On Thu, 2 Jul 2026 20:42:16 +0800
Chengwen Feng <fengchengwen@huawei.com> wrote:

> Add a module parameter 'tph' to implement staged TPH feature enabling
> policy. The parameter accepts a value from 0 to 3 to progressively enable
> TPH capabilities:
> 
> 0 = No-ST mode: RESOLVE only returns PH for DMABUF source;
>                 ST programming unavailable
> 1 = Add IV mode: RESOLVE only returns PH for DMABUF source;
>                  ST program supports NONE/DMABUF/CPU sources
> 2 = Add DS mode: RESOLVE returns PH+ST for DMABUF/CPU sources;
>                  ST program supports NONE/DMABUF/CPU sources
> 3 = Add LITERAL: RESOLVE returns PH+ST for DMABUF/CPU sources;
>                  ST program supports NONE/DMABUF/CPU/LITERAL sources
> 
> This commit only adds parameter plumbing and state storage, no
> functional TPH logic is introduced yet.

This adds module options for a feature that doesn't exist yet.  That's
the wrong ordering.  The module option is the only introspection that
the administrator has to the capabilities of the driver.

The intention of the proposal was also that support is added
incrementally, ie. "an incremental support path".

We should start with adding No-ST support to the driver, concluding
with the module parameter infrastructure that essentially advertises
the driver support on the host, 0 (default) is the only accepted value.

The next step would be incrementally revising the driver support to
include IV mode, concluding with exposing it and configuring it through
the driver option. Repeat for DS mode.  Repeat for LITERAL mode.

This is what allows us to decide where to stop in the implementation.
For example, we've designed LITERAL mode, but if no VMM intends to
support it, why implement it?

Also, this module option is being defined on vfio_pci.  Are we
intending that this is a feature that each variant driver would
opt-in to via their own module options?  LITERAL mode is incompatible
with migration, so it's possible each variant driver would define what
level of support they expose.  Thanks,

Alex

 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  drivers/vfio/pci/vfio_pci.c      | 25 +++++++++++++++++++++++++
>  drivers/vfio/pci/vfio_pci_priv.h |  7 +++++++
>  include/linux/vfio_pci_core.h    |  1 +
>  3 files changed, 33 insertions(+)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 830369ff878d..7ef82b445722 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -60,6 +60,28 @@ static bool disable_denylist;
>  module_param(disable_denylist, bool, 0444);
>  MODULE_PARM_DESC(disable_denylist, "Disable use of device denylist. Disabling the denylist allows binding to devices with known errata that may lead to exploitable stability or security issues when accessed by untrusted users.");
>  
> +static unsigned int tph_policy;
> +static int tph_set(const char *val, const struct kernel_param *kp)
> +{
> +	return param_set_uint_minmax(val, kp, VFIO_PCI_TPH_POLICY_NO_ST,
> +				     VFIO_PCI_TPH_POLICY_LITERAL);
> +}
> +static const struct kernel_param_ops tph_param_ops = {
> +	.set = tph_set,
> +	.get = param_get_uint,
> +};
> +module_param_cb(tph, &tph_param_ops, &tph_policy, 0644);
> +MODULE_PARM_DESC(tph,
> +	"Global TPH policy level (0=default No-ST):\n"
> +	"0 = No-ST mode: RESOLVE only returns PH for DMABUF source;\n"
> +	"                ST programming unavailable\n"
> +	"1 = Add IV mode: RESOLVE only returns PH for DMABUF source;\n"
> +	"                 ST program supports NONE/DMABUF/CPU sources\n"
> +	"2 = Add DS mode: RESOLVE returns PH+ST for DMABUF/CPU sources;\n"
> +	"                 ST program supports NONE/DMABUF/CPU sources\n"
> +	"3 = Add LITERAL: RESOLVE returns PH+ST for DMABUF/CPU sources;\n"
> +	"                 ST program supports NONE/DMABUF/CPU/LITERAL sources\n");
> +
>  static bool vfio_pci_dev_in_denylist(struct pci_dev *pdev)
>  {
>  	switch (pdev->vendor) {
> @@ -142,6 +164,9 @@ static int vfio_pci_init_dev(struct vfio_device *core_vdev)
>  #ifdef CONFIG_VFIO_PCI_VGA
>  	vdev->disable_vga = disable_vga;
>  #endif
> +#ifdef CONFIG_PCIE_TPH
> +	vdev->tph_policy = tph_policy;
> +#endif
>  
>  	return vfio_pci_core_init_dev(core_vdev);
>  }
> diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
> index fca9d0dfac90..c997cc9bf330 100644
> --- a/drivers/vfio/pci/vfio_pci_priv.h
> +++ b/drivers/vfio/pci/vfio_pci_priv.h
> @@ -11,6 +11,13 @@
>  /* Cap maximum number of ioeventfds per device (arbitrary) */
>  #define VFIO_PCI_IOEVENTFD_MAX		1000
>  
> +enum vfio_pci_tph_policy {
> +	VFIO_PCI_TPH_POLICY_NO_ST = 0,
> +	VFIO_PCI_TPH_POLICY_IV_ST = 1,
> +	VFIO_PCI_TPH_POLICY_DS_ST = 2,
> +	VFIO_PCI_TPH_POLICY_LITERAL = 3,
> +};
> +
>  struct vfio_pci_ioeventfd {
>  	struct list_head	next;
>  	struct vfio_pci_core_device	*vdev;
> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
> index 9a1674c152aa..a0641286dd90 100644
> --- a/include/linux/vfio_pci_core.h
> +++ b/include/linux/vfio_pci_core.h
> @@ -149,6 +149,7 @@ struct vfio_pci_core_device {
>  	struct notifier_block	nb;
>  	struct rw_semaphore	memory_lock;
>  	struct list_head	dmabufs;
> +	u8			tph_policy;
>  };
>  
>  enum vfio_pci_io_width {