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

[Chenbo Xia <chenbox@nvidia.com>]

Sorry I missed all previous versions…

+ARM guy

> On Nov 4, 2023, at 02:29, Abdullah Sevincer <abdullah.sevincer@intel.com> wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> 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>

Is PASID now part of PCIe spec? This APIs should both work for x86/arm?
Not sure ARM is OK with the naming, previously they are calling it more as
Sub Stream ID (SSID)

> ---
> 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)
> +{
> +       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..d45b7bf2ab 100644
> --- a/drivers/bus/pci/rte_bus_pci.h
> +++ b/drivers/bus/pci/rte_bus_pci.h
> @@ -295,6 +295,19 @@ 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 dev
> + *   A pointer to a rte_pci_device structure.
> + * @param offset
> + *   Offset of the PASID external capability.
> + * @param enable
> + *   Flag to enable or disable PASID.
> + */
> +__rte_internal
> +int rte_pci_pasid_ena_dis(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..01e6a09eb6 100644
> --- a/drivers/bus/pci/version.map
> +++ b/drivers/bus/pci/version.map
> @@ -36,6 +36,7 @@ INTERNAL {
>        global:
> 
>        rte_pci_get_sysfs_path;
> +       rte_pci_pasid_ena_dis;
>        rte_pci_register;
>        rte_pci_unregister;
> };
> diff --git a/lib/pci/rte_pci.h b/lib/pci/rte_pci.h
> index 69e932d910..d195f01950 100644
> --- a/lib/pci/rte_pci.h
> +++ b/lib/pci/rte_pci.h
> @@ -101,6 +101,10 @@ 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 */

Align with old definitions will looks better. Using TAB?

Thanks,
Chenbo

> 
> /* Advanced Error Reporting (RTE_PCI_EXT_CAP_ID_ERR) */
> #define RTE_PCI_ERR_UNCOR_STATUS       0x04    /* Uncorrectable Error Status */
> --
> 2.25.1
>