* [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).