Re: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Nov 03, 2023 at 01:29:32PM -0500, Abdullah Sevincer wrote:
> This commit implements an internal api to enable and disable PASID for
> a device e.g. device driver event/dlb2.
> 
> For kernels when PASID enabled by default it breaks DLB functionality,
> hence disabling PASID is required for DLB to function properly.
> 
> PASID capability is not exposed to users hence offset can not be
> retrieved by rte_pci_find_ext_capability() api. Therefore, api
> implemented in this commit accepts an offset for PASID with an enable
> flag which is used to enable/disable PASID.
> 
> Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> ---
>  drivers/bus/pci/pci_common.c  |  7 +++++++
>  drivers/bus/pci/rte_bus_pci.h | 13 +++++++++++++
>  drivers/bus/pci/version.map   |  1 +
>  lib/pci/rte_pci.h             |  4 ++++
>  4 files changed, 25 insertions(+)
> 
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index 921d957bf6..5aac2406f1 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -938,6 +938,13 @@ rte_pci_set_bus_master(const struct rte_pci_device *dev, bool enable)
>  	return 0;
>  }
>  
> +int
> +rte_pci_pasid_ena_dis(const struct rte_pci_device *dev, off_t offset, bool enable)

While I realise we are now at v6 of this patchset, and the name was
suggested on v4, seeing it implemented I'm afraid I think
rte_pci_pasid_ena_dis is not a great name! I also agree that the pasid_set
name was a bit misleading too, leaving us with a naming problem.
I have two suggestions:

* if we want to keep one function - "rte_pci_pasid_set_state", which makes
  it clear we are not setting the pasid, but the pasid state.
* separate this explicitly into rte_pci_pasid_enable() and
  rte_pci_pasid_disable() functions. This is the cleanest solution but it
  doesn't align with some of the other functions in pci lib which set the
  state.

Jerin, any further thoughts? and sorry for late feedback.

/Bruce