[v1] bus/pci: add function to enable/disable PASID

  This commit implements an internal api to enable
and disable PASID for a device e.g. DLB Device.

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 | 11 +++++++++++
 drivers/bus/pci/version.map   |  1 +
 lib/pci/rte_pci.h             |  5 +++++
 4 files changed, 24 insertions(+)
  

On Fri, Nov 03, 2023 at 12:03:47PM -0500, Abdullah Sevincer wrote:
> This commit implements an internal api to enable
> and disable PASID for a device e.g. DLB Device.
> 
> 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.

Lines in the commit log can go up to 72 characters, so you can be a bit
less strict in wrapping. :-)
The term DLB may not be familiar to everyone, so I suggest referring to it
via proper name, or refer to the device driver event/dlb.

Few other minor comments inline.
/Bruce

> 
> Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> ---
>  drivers/bus/pci/pci_common.c  |  7 +++++++
>  drivers/bus/pci/rte_bus_pci.h | 11 +++++++++++
>  drivers/bus/pci/version.map   |  1 +
>  lib/pci/rte_pci.h             |  5 +++++
>  4 files changed, 24 insertions(+)
> 
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index 921d957bf6..ced072825e 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_set_pasid(const struct rte_pci_device *dev, off_t offset, bool enable)
> +{
> +	uint16_t pasid = enable;
> +	return rte_pci_write_config(dev, &pasid, sizeof(pasid), offset) < 0 ? -1 : 0;
> +}
> +
>  struct rte_pci_bus rte_pci_bus = {
>  	.bus = {
>  		.scan = rte_pci_scan,
> diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h
> index 21e234abf0..d97c8320a7 100644
> --- a/drivers/bus/pci/rte_bus_pci.h
> +++ b/drivers/bus/pci/rte_bus_pci.h
> @@ -295,6 +295,17 @@ void rte_pci_ioport_read(struct rte_pci_ioport *p,
>  void rte_pci_ioport_write(struct rte_pci_ioport *p,
>  		const void *data, size_t len, off_t offset);
>  
> +/**
> + * Enable/Disable PASID.
> + *
> + * @param offset
> + *   Offset of the PASID external capability.
> + * @param enable
> + *   Flag to enable or disable PASID.
> + */

Missing parameter "dev".

> +__rte_internal
> +int rte_pci_set_pasid(const struct rte_pci_device *dev, off_t offset, bool enable);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/drivers/bus/pci/version.map b/drivers/bus/pci/version.map
> index 74c5b075d5..329d386c85 100644
> --- a/drivers/bus/pci/version.map
> +++ b/drivers/bus/pci/version.map
> @@ -38,4 +38,5 @@ INTERNAL {
>  	rte_pci_get_sysfs_path;
>  	rte_pci_register;
>  	rte_pci_unregister;
> +	rte_pci_set_pasid;

Keep list in alphabetical order, so new entry goes above unregister.

>  };
> diff --git a/lib/pci/rte_pci.h b/lib/pci/rte_pci.h
> index 69e932d910..772a8d5622 100644
> --- a/lib/pci/rte_pci.h
> +++ b/lib/pci/rte_pci.h
> @@ -101,6 +101,11 @@ extern "C" {
>  #define RTE_PCI_EXT_CAP_ID_ACS		0x0d	/* Access Control Services */
>  #define RTE_PCI_EXT_CAP_ID_SRIOV	0x10	/* SR-IOV */
>  #define RTE_PCI_EXT_CAP_ID_PRI		0x13	/* Page Request Interface */
> +#define RTE_PCI_EXT_CAP_ID_PASID        0x1B    /* Process Address Space ID */
> +
> +/* Process Address Space ID */
> +#define RTE_PCI_PASID_CTRL		0x06    /* PASID control register */
> +#define RTE_PCI_PASID_CAP_OFFSET        0x148   /* PASID capability offset */
>  
>  /* Advanced Error Reporting (RTE_PCI_EXT_CAP_ID_ERR) */
>  #define RTE_PCI_ERR_UNCOR_STATUS	0x04	/* Uncorrectable Error Status */
> -- 
> 2.25.1
>
  

> -----Original Message-----
> From: Abdullah Sevincer <abdullah.sevincer@intel.com>
> Sent: Friday, November 3, 2023 10:34 PM
> To: dev@dpdk.org
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> mike.ximing.chen@intel.com; bruce.richardson@intel.com;
> thomas@monjalon.net; Abdullah Sevincer <abdullah.sevincer@intel.com>
> Subject: [EXT] [PATCH v1] bus/pci: add function to enable/disable PASID
> 
> External Email
> 
> ----------------------------------------------------------------------
> This commit implements an internal api to enable and disable PASID for a
> device e.g. DLB Device.
> 
> 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>
> ---
> +/**
> + * Enable/Disable PASID.
> + *
> + * @param offset
> + *   Offset of the PASID external capability.
> + * @param enable
> + *   Flag to enable or disable PASID.
> + */
> +__rte_internal
> +int rte_pci_set_pasid(const struct rte_pci_device *dev, off_t offset,
> +bool enable);

May be rte_pci_pasid_ena_dis(const struct rte_pci_device *dev, off_t offset,
bool enable) could be better name as it NOT setting the pasid

> +#define RTE_PCI_PASID_CAP_OFFSET        0x148   /* PASID capability offset */

Is this fixed for all PCIe device? Offset will vary based on number of capabilities present in a given device, if so,
move this to event/dlb.