DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v6 0/2] *** Disable PASID for DLB Device ***
@ 2023-11-03 18:29 Abdullah Sevincer
  2023-11-03 18:29 ` [PATCH v6 1/2] bus/pci: add function to enable/disable PASID Abdullah Sevincer
  2023-11-03 18:29 ` [PATCH v6 2/2] event/dlb2: fix disable PASID Abdullah Sevincer
  0 siblings, 2 replies; 11+ messages in thread
From: Abdullah Sevincer @ 2023-11-03 18:29 UTC (permalink / raw)
  To: dev; +Cc: jerinj, mike.ximing.chen, bruce.richardson, thomas, Abdullah Sevincer

*** BLURB HERE ***

Abdullah Sevincer (2):
  bus/pci: add function to enable/disable PASID
  event/dlb2: fix disable PASID

 drivers/bus/pci/pci_common.c      |  7 +++++++
 drivers/bus/pci/rte_bus_pci.h     | 13 +++++++++++++
 drivers/bus/pci/version.map       |  1 +
 drivers/event/dlb2/pf/dlb2_main.c | 11 +++++++++++
 lib/pci/rte_pci.h                 |  4 ++++
 5 files changed, 36 insertions(+)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
  2023-11-03 18:29 [PATCH v6 0/2] *** Disable PASID for DLB Device *** Abdullah Sevincer
@ 2023-11-03 18:29 ` Abdullah Sevincer
  2023-11-04  7:32   ` Jerin Jacob
                     ` (2 more replies)
  2023-11-03 18:29 ` [PATCH v6 2/2] event/dlb2: fix disable PASID Abdullah Sevincer
  1 sibling, 3 replies; 11+ messages in thread
From: Abdullah Sevincer @ 2023-11-03 18:29 UTC (permalink / raw)
  To: dev; +Cc: jerinj, mike.ximing.chen, bruce.richardson, thomas, Abdullah Sevincer

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)
+{
+	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 */
 
 /* Advanced Error Reporting (RTE_PCI_EXT_CAP_ID_ERR) */
 #define RTE_PCI_ERR_UNCOR_STATUS	0x04	/* Uncorrectable Error Status */
-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 2/2] event/dlb2: fix disable PASID
  2023-11-03 18:29 [PATCH v6 0/2] *** Disable PASID for DLB Device *** Abdullah Sevincer
  2023-11-03 18:29 ` [PATCH v6 1/2] bus/pci: add function to enable/disable PASID Abdullah Sevincer
@ 2023-11-03 18:29 ` Abdullah Sevincer
  1 sibling, 0 replies; 11+ messages in thread
From: Abdullah Sevincer @ 2023-11-03 18:29 UTC (permalink / raw)
  To: dev
  Cc: jerinj, mike.ximing.chen, bruce.richardson, thomas,
	Abdullah Sevincer, stable

In vfio-pci driver when PASID is enabled by default DLB hardware puts
DLB in SIOV mode. This breaks DLB PF-PMD mode. For DLB PF-PMD mode to
function properly PASID needs to be disabled.

In this commit this issue is addressed and PASID is disabled by writing
a zero to PASID control register.

Fixes: 5433956d5185 ("event/dlb2: add eventdev probe")
Cc: stable@dpdk.org

Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
---
 drivers/event/dlb2/pf/dlb2_main.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/event/dlb2/pf/dlb2_main.c b/drivers/event/dlb2/pf/dlb2_main.c
index aa03e4c311..05c2354515 100644
--- a/drivers/event/dlb2/pf/dlb2_main.c
+++ b/drivers/event/dlb2/pf/dlb2_main.c
@@ -26,6 +26,7 @@
 #define PF_ID_ZERO 0	/* PF ONLY! */
 #define NO_OWNER_VF 0	/* PF ONLY! */
 #define NOT_VF_REQ false /* PF ONLY! */
+#define DLB2_PCI_PASID_CAP_OFFSET        0x148   /* PASID capability offset */
 
 static int
 dlb2_pf_init_driver_state(struct dlb2_dev *dlb2_dev)
@@ -514,6 +515,16 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
 		}
 	}
 
+	/* Disable PASID if it is enabled by default, which
+	 * breaks the DLB if enabled.
+	 */
+	off = DLB2_PCI_PASID_CAP_OFFSET + RTE_PCI_PASID_CTRL;
+	if (rte_pci_pasid_ena_dis(pdev, off, false)) {
+		DLB2_LOG_ERR("[%s()] failed to write the pcie config space at offset %d\n",
+				__func__, (int)off);
+		return -1;
+	}
+
 	return 0;
 }
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
  2023-11-03 18:29 ` [PATCH v6 1/2] bus/pci: add function to enable/disable PASID Abdullah Sevincer
@ 2023-11-04  7:32   ` Jerin Jacob
  2023-11-04  9:19     ` Thomas Monjalon
  2023-11-04 14:01   ` Bruce Richardson
  2023-11-06  1:16   ` Chenbo Xia
  2 siblings, 1 reply; 11+ messages in thread
From: Jerin Jacob @ 2023-11-04  7:32 UTC (permalink / raw)
  To: Abdullah Sevincer; +Cc: dev, jerinj, mike.ximing.chen, bruce.richardson, thomas

On Sat, Nov 4, 2023 at 4:47 AM Abdullah Sevincer
<abdullah.sevincer@intel.com> wrote:
>
> This commit implements an internal api to enable and disable PASID for
> a device e.g. device driver event/dlb2.

git comment can be reworded when apply.

>
> 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>

Acked-by: Jerin Jacob <jerinj@marvell.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(+)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
  2023-11-04  7:32   ` Jerin Jacob
@ 2023-11-04  9:19     ` Thomas Monjalon
  2023-11-05  5:48       ` Jerin Jacob
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Monjalon @ 2023-11-04  9:19 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: Abdullah Sevincer, dev, jerinj, mike.ximing.chen, bruce.richardson

04/11/2023 08:32, Jerin Jacob:
> On Sat, Nov 4, 2023 at 4:47 AM Abdullah Sevincer
> <abdullah.sevincer@intel.com> wrote:
> >
> > This commit implements an internal api to enable and disable PASID for
> > a device e.g. device driver event/dlb2.
> 
> git comment can be reworded when apply.

What do you mean Jerin?


> > 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>
> 
> Acked-by: Jerin Jacob <jerinj@marvell.com>




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
  2023-11-03 18:29 ` [PATCH v6 1/2] bus/pci: add function to enable/disable PASID Abdullah Sevincer
  2023-11-04  7:32   ` Jerin Jacob
@ 2023-11-04 14:01   ` Bruce Richardson
  2023-11-05  5:43     ` Jerin Jacob
  2023-11-06  1:16   ` Chenbo Xia
  2 siblings, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2023-11-04 14:01 UTC (permalink / raw)
  To: Abdullah Sevincer; +Cc: dev, jerinj, mike.ximing.chen, thomas

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
  2023-11-04 14:01   ` Bruce Richardson
@ 2023-11-05  5:43     ` Jerin Jacob
  0 siblings, 0 replies; 11+ messages in thread
From: Jerin Jacob @ 2023-11-05  5:43 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: Abdullah Sevincer, dev, jerinj, mike.ximing.chen, thomas

On Sat, Nov 4, 2023 at 7:31 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> 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.

Yes. Above two functions are better than ena_dis().

Other option could be, rte_pci_pasid_ctrl() which is aligned with PCI
register name.

No strong opinion for the name. Feel free to pick one from above three options.


>
> /Bruce

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
  2023-11-04  9:19     ` Thomas Monjalon
@ 2023-11-05  5:48       ` Jerin Jacob
  0 siblings, 0 replies; 11+ messages in thread
From: Jerin Jacob @ 2023-11-05  5:48 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Abdullah Sevincer, dev, jerinj, mike.ximing.chen, bruce.richardson

On Sat, Nov 4, 2023 at 2:49 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 04/11/2023 08:32, Jerin Jacob:
> > On Sat, Nov 4, 2023 at 4:47 AM Abdullah Sevincer
> > <abdullah.sevincer@intel.com> wrote:
> > >
> > > This commit implements an internal api to enable and disable PASID for
> > > a device e.g. device driver event/dlb2.
> >
> > git comment can be reworded when apply.
>
> What do you mean Jerin?

Since I am not applying this patch, I thought you can reword something
like following

bus/pci: support PASID control

Add an internal API to control PASID for a given PCIe 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>
> >
> > Acked-by: Jerin Jacob <jerinj@marvell.com>
>
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
  2023-11-03 18:29 ` [PATCH v6 1/2] bus/pci: add function to enable/disable PASID Abdullah Sevincer
  2023-11-04  7:32   ` Jerin Jacob
  2023-11-04 14:01   ` Bruce Richardson
@ 2023-11-06  1:16   ` Chenbo Xia
  2023-11-06 15:44     ` Sevincer, Abdullah
  2023-11-07  8:17     ` Ruifeng Wang
  2 siblings, 2 replies; 11+ messages in thread
From: Chenbo Xia @ 2023-11-06  1:16 UTC (permalink / raw)
  To: Abdullah Sevincer
  Cc: dev, jerinj, mike.ximing.chen, bruce.richardson,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	ruifeng.wang

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
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
  2023-11-06  1:16   ` Chenbo Xia
@ 2023-11-06 15:44     ` Sevincer, Abdullah
  2023-11-07  8:17     ` Ruifeng Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Sevincer, Abdullah @ 2023-11-06 15:44 UTC (permalink / raw)
  To: Chenbo Xia
  Cc: dev, jerinj, Chen, Mike Ximing, Richardson, Bruce,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	ruifeng.wang


>+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)
For reference, Look for PASID definitions in the PCIe spec.
The API takes in offset which might be different for other devices. Part of the reason why we defined the API this way.

>+Align with old definitions will looks better. Using TAB?
I will align.

For v7 I will use naming rte_pci_pasid_set_state, is everyone okay with that?

Thanks.
Abdullah.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 1/2] bus/pci: add function to enable/disable PASID
  2023-11-06  1:16   ` Chenbo Xia
  2023-11-06 15:44     ` Sevincer, Abdullah
@ 2023-11-07  8:17     ` Ruifeng Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Ruifeng Wang @ 2023-11-07  8:17 UTC (permalink / raw)
  To: Chenbo Xia, Abdullah Sevincer
  Cc: dev, jerinj, mike.ximing.chen, bruce.richardson,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	nd



On 2023/11/6 9:16 AM, Chenbo Xia wrote:
> Sorry I missed all previous versions…
> 
> +ARM guy

Thanks for CCing.

> 
>> 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)

PASID is fine to ARM.
SSID is a term used in SMMU(IOMMU) which PASID is mapped to.

>  >> ---
>> 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
>>
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-11-07  8:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-03 18:29 [PATCH v6 0/2] *** Disable PASID for DLB Device *** Abdullah Sevincer
2023-11-03 18:29 ` [PATCH v6 1/2] bus/pci: add function to enable/disable PASID Abdullah Sevincer
2023-11-04  7:32   ` Jerin Jacob
2023-11-04  9:19     ` Thomas Monjalon
2023-11-05  5:48       ` Jerin Jacob
2023-11-04 14:01   ` Bruce Richardson
2023-11-05  5:43     ` Jerin Jacob
2023-11-06  1:16   ` Chenbo Xia
2023-11-06 15:44     ` Sevincer, Abdullah
2023-11-07  8:17     ` Ruifeng Wang
2023-11-03 18:29 ` [PATCH v6 2/2] event/dlb2: fix disable PASID Abdullah Sevincer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).