[PATCH v1] event/dlb2: add support for disabling PASID

[PATCH v1] event/dlb2: add support for disabling PASID

[Abdullah Sevincer]

vfio-pci driver in Linux kernel 6.2 enables PASID by default.
In DLB hardware, enabling PASID puts DLB in SIOV mode. This
breaks DLB PF-PMD mode. For DLB PF-PMD mode to function properly
PASID needs to be disabled for kernel 6.2.

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

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

diff --git a/drivers/event/dlb2/pf/dlb2_main.c b/drivers/event/dlb2/pf/dlb2_main.c
index 717aa4fc08..63868e2388 100644
--- a/drivers/event/dlb2/pf/dlb2_main.c
+++ b/drivers/event/dlb2/pf/dlb2_main.c
@@ -46,6 +46,7 @@
 #define DLB2_PCI_CAP_ID_MSIX      0x11
 #define DLB2_PCI_EXT_CAP_ID_PRI   0x13
 #define DLB2_PCI_EXT_CAP_ID_ACS   0xD
+#define DLB2_PCI_EXT_CAP_ID_PASID 0x1B	/* Process Address Space ID */
 
 #define DLB2_PCI_PRI_CTRL_ENABLE         0x1
 #define DLB2_PCI_PRI_ALLOC_REQ           0xC
@@ -64,6 +65,8 @@
 #define DLB2_PCI_ACS_CR                  0x8
 #define DLB2_PCI_ACS_UF                  0x10
 #define DLB2_PCI_ACS_EC                  0x20
+#define DLB2_PCI_PASID_CTRL              0x06    /* PASID control register */
+#define DLB2_PCI_PASID_CAP_OFFSET        0x148   /* PASID capability offset */
 
 static int dlb2_pci_find_capability(struct rte_pci_device *pdev, uint32_t id)
 {
@@ -257,12 +260,14 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
 	uint16_t rt_ctl_word;
 	uint32_t pri_reqs_dword;
 	uint16_t pri_ctrl_word;
+	uint16_t pasid_ctrl;
 
 	int pcie_cap_offset;
 	int pri_cap_offset;
 	int msix_cap_offset;
 	int err_cap_offset;
 	int acs_cap_offset;
+	int pasid_cap_offset;
 	int wait_count;
 
 	uint16_t devsta_busy_word;
@@ -582,6 +587,28 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
 		}
 	}
 
+	/* The current Linux kernel vfio driver does not expose PASID capability to
+	 * users. It also enables PASID by default, which breaks DLB PF PMD. We have
+	 * to use the hardcoded offset for now to disable PASID.
+	 */
+	pasid_cap_offset = DLB2_PCI_PASID_CAP_OFFSET;
+
+	off = pasid_cap_offset + DLB2_PCI_PASID_CTRL;
+	if (rte_pci_read_config(pdev, &pasid_ctrl, 2, off) != 2)
+		pasid_ctrl = 0;
+
+	if (pasid_ctrl) {
+		DLB2_INFO(dlb2_dev, "DLB2 disabling pasid...\n");
+
+		pasid_ctrl = 0;
+		ret = rte_pci_write_config(pdev, &pasid_ctrl, 2, off);
+		if (ret != 2) {
+			DLB2_LOG_ERR("[%s()] failed to write the pcie config space at offset %d\n",
+				__func__, (int)off);
+			return ret;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH v1] event/dlb2: add support for disabling PASID

[Jerin Jacob]

On Thu, Jun 8, 2023 at 2:31 AM Abdullah Sevincer
<abdullah.sevincer@intel.com> wrote:
>
> vfio-pci driver in Linux kernel 6.2 enables PASID by default.
> In DLB hardware, enabling PASID puts DLB in SIOV mode. This
> breaks DLB PF-PMD mode. For DLB PF-PMD mode to function properly
> PASID needs to be disabled for kernel 6.2.
>
> In this commit this issue is addressed and PASID is disabled
> by writing a zero to PASID control register.
>
> Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>

> +       /* The current Linux kernel vfio driver does not expose PASID capability to
> +        * users. It also enables PASID by default, which breaks DLB PF PMD. We have
> +        * to use the hardcoded offset for now to disable PASID.
> +        */
> +       pasid_cap_offset = DLB2_PCI_PASID_CAP_OFFSET;
> +
> +       off = pasid_cap_offset + DLB2_PCI_PASID_CTRL;

+++ additional folks.

Is make sense to move this helper function to PCI common for disabling
PASID for a PCI device so that other driver can use if needed
as the implementation is not specific to DLB2.



> +       if (rte_pci_read_config(pdev, &pasid_ctrl, 2, off) != 2)
> +               pasid_ctrl = 0;
> +
> +       if (pasid_ctrl) {
> +               DLB2_INFO(dlb2_dev, "DLB2 disabling pasid...\n");
> +
> +               pasid_ctrl = 0;
> +               ret = rte_pci_write_config(pdev, &pasid_ctrl, 2, off);
> +               if (ret != 2) {
> +                       DLB2_LOG_ERR("[%s()] failed to write the pcie config space at offset %d\n",
> +                               __func__, (int)off);
> +                       return ret;
> +               }
> +       }
> +
>         return 0;
>  }
>
> --
> 2.25.1
>

Re: [PATCH v1] event/dlb2: add support for disabling PASID

[Thomas Monjalon]

08/06/2023 07:38, Jerin Jacob:
> On Thu, Jun 8, 2023 at 2:31 AM Abdullah Sevincer
> <abdullah.sevincer@intel.com> wrote:
> >
> > vfio-pci driver in Linux kernel 6.2 enables PASID by default.
> > In DLB hardware, enabling PASID puts DLB in SIOV mode. This
> > breaks DLB PF-PMD mode. For DLB PF-PMD mode to function properly
> > PASID needs to be disabled for kernel 6.2.
> >
> > In this commit this issue is addressed and PASID is disabled
> > by writing a zero to PASID control register.
> >
> > Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> 
> > +       /* The current Linux kernel vfio driver does not expose PASID capability to
> > +        * users. It also enables PASID by default, which breaks DLB PF PMD. We have
> > +        * to use the hardcoded offset for now to disable PASID.
> > +        */
> > +       pasid_cap_offset = DLB2_PCI_PASID_CAP_OFFSET;
> > +
> > +       off = pasid_cap_offset + DLB2_PCI_PASID_CTRL;
> 
> +++ additional folks.
> 
> Is make sense to move this helper function to PCI common for disabling
> PASID for a PCI device so that other driver can use if needed
> as the implementation is not specific to DLB2.

I guess yes it makes sense.

Re: [PATCH v1] event/dlb2: add support for disabling PASID

[David Marchand]

On Thu, Jun 8, 2023 at 7:38 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Thu, Jun 8, 2023 at 2:31 AM Abdullah Sevincer
> <abdullah.sevincer@intel.com> wrote:
> >
> > vfio-pci driver in Linux kernel 6.2 enables PASID by default.
> > In DLB hardware, enabling PASID puts DLB in SIOV mode. This
> > breaks DLB PF-PMD mode. For DLB PF-PMD mode to function properly
> > PASID needs to be disabled for kernel 6.2.
> >
> > In this commit this issue is addressed and PASID is disabled
> > by writing a zero to PASID control register.
> >
> > Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
>
> > +       /* The current Linux kernel vfio driver does not expose PASID capability to
> > +        * users. It also enables PASID by default, which breaks DLB PF PMD. We have
> > +        * to use the hardcoded offset for now to disable PASID.
> > +        */
> > +       pasid_cap_offset = DLB2_PCI_PASID_CAP_OFFSET;
> > +
> > +       off = pasid_cap_offset + DLB2_PCI_PASID_CTRL;
>
> +++ additional folks.
>
> Is make sense to move this helper function to PCI common for disabling
> PASID for a PCI device so that other driver can use if needed
> as the implementation is not specific to DLB2.

Yes, having a helper sounds like a first step (and we probably have
more helpers to add seeing how drivers tend to redefine non vendor
specific pci configs, but that's another story).

Now, about PASID being enabled by default with Linux 6.2, is this
breaking of dlb PF something special? Or can we expect many (all?)
other devices to break too?
If so, maybe we should disable it in the pci common code.


-- 
David Marchand

Re: [PATCH v1] event/dlb2: add support for disabling PASID

[Jerin Jacob]

On Thu, Jun 8, 2023 at 12:59 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Thu, Jun 8, 2023 at 7:38 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >
> > On Thu, Jun 8, 2023 at 2:31 AM Abdullah Sevincer
> > <abdullah.sevincer@intel.com> wrote:
> > >
> > > vfio-pci driver in Linux kernel 6.2 enables PASID by default.
> > > In DLB hardware, enabling PASID puts DLB in SIOV mode. This
> > > breaks DLB PF-PMD mode. For DLB PF-PMD mode to function properly
> > > PASID needs to be disabled for kernel 6.2.
> > >
> > > In this commit this issue is addressed and PASID is disabled
> > > by writing a zero to PASID control register.
> > >
> > > Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> >
> > > +       /* The current Linux kernel vfio driver does not expose PASID capability to
> > > +        * users. It also enables PASID by default, which breaks DLB PF PMD. We have
> > > +        * to use the hardcoded offset for now to disable PASID.
> > > +        */
> > > +       pasid_cap_offset = DLB2_PCI_PASID_CAP_OFFSET;
> > > +
> > > +       off = pasid_cap_offset + DLB2_PCI_PASID_CTRL;
> >
> > +++ additional folks.
> >
> > Is make sense to move this helper function to PCI common for disabling
> > PASID for a PCI device so that other driver can use if needed
> > as the implementation is not specific to DLB2.
>
> Yes, having a helper sounds like a first step (and we probably have
> more helpers to add seeing how drivers tend to redefine non vendor
> specific pci configs, but that's another story).

@Abdullah Sevincer  Please move the implementation to code PCI code.

>
> Now, about PASID being enabled by default with Linux 6.2, is this
> breaking of dlb PF something special? Or can we expect many (all?)
> other devices to break too?
> If so, maybe we should disable it in the pci common code.

@Abdullah Sevincer Is implicitly enabling SIOV based on PASID
configuration DLB2 behavior or general PCI behavior that may affect
other NIC's?

>
>
> --
> David Marchand
>

RE: [PATCH v1] event/dlb2: add support for disabling PASID

[Chen, Mike Ximing]

> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Thursday, June 8, 2023 7:32 AM
> To: David Marchand <david.marchand@redhat.com>
> Cc: Sevincer, Abdullah <abdullah.sevincer@intel.com>; Gaetan Rivet <grive@u256.net>; Thomas Monjalon
> <thomas@monjalon.net>; dev@dpdk.org; jerinj@marvell.com; Chen, Mike Ximing
> <mike.ximing.chen@intel.com>
> Subject: Re: [PATCH v1] event/dlb2: add support for disabling PASID
> 
> On Thu, Jun 8, 2023 at 12:59 PM David Marchand <david.marchand@redhat.com> wrote:
> >
> > On Thu, Jun 8, 2023 at 7:38 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > >
> > > On Thu, Jun 8, 2023 at 2:31 AM Abdullah Sevincer
> > > <abdullah.sevincer@intel.com> wrote:
> > > >
> > > > vfio-pci driver in Linux kernel 6.2 enables PASID by default.
> > > > In DLB hardware, enabling PASID puts DLB in SIOV mode. This breaks
> > > > DLB PF-PMD mode. For DLB PF-PMD mode to function properly PASID
> > > > needs to be disabled for kernel 6.2.
> > > >
> > > > In this commit this issue is addressed and PASID is disabled by
> > > > writing a zero to PASID control register.
> > > >
> > > > Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> > >
> > > > +       /* The current Linux kernel vfio driver does not expose PASID capability to
> > > > +        * users. It also enables PASID by default, which breaks DLB PF PMD. We have
> > > > +        * to use the hardcoded offset for now to disable PASID.
> > > > +        */
> > > > +       pasid_cap_offset = DLB2_PCI_PASID_CAP_OFFSET;
> > > > +
> > > > +       off = pasid_cap_offset + DLB2_PCI_PASID_CTRL;
> > >
> > > +++ additional folks.
> > >
> > > Is make sense to move this helper function to PCI common for
> > > disabling PASID for a PCI device so that other driver can use if
> > > needed as the implementation is not specific to DLB2.
> >
> > Yes, having a helper sounds like a first step (and we probably have
> > more helpers to add seeing how drivers tend to redefine non vendor
> > specific pci configs, but that's another story).
> 
> @Abdullah Sevincer  Please move the implementation to code PCI code.
> 
> >
> > Now, about PASID being enabled by default with Linux 6.2, is this
> > breaking of dlb PF something special? Or can we expect many (all?)
> > other devices to break too?
> > If so, maybe we should disable it in the pci common code.
> 
> @Abdullah Sevincer Is implicitly enabling SIOV based on PASID configuration DLB2 behavior or general PCI
> behavior that may affect other NIC's?

PASID capability is required for SIOV, but not needed for PF mode.  
It makes sense to disable PASID in any PF mode that uses the vfio-pci driver (which enables PASID by
Default since kernel 6.2).

> 
> >
> >
> > --
> > David Marchand
> >

Re: [PATCH v1] event/dlb2: add support for disabling PASID

[David Marchand]

On Thu, Jun 8, 2023 at 9:28 AM David Marchand <david.marchand@redhat.com> wrote:
>
> On Thu, Jun 8, 2023 at 7:38 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >
> > On Thu, Jun 8, 2023 at 2:31 AM Abdullah Sevincer
> > <abdullah.sevincer@intel.com> wrote:
> > >
> > > vfio-pci driver in Linux kernel 6.2 enables PASID by default.
> > > In DLB hardware, enabling PASID puts DLB in SIOV mode. This
> > > breaks DLB PF-PMD mode. For DLB PF-PMD mode to function properly
> > > PASID needs to be disabled for kernel 6.2.
> > >
> > > In this commit this issue is addressed and PASID is disabled
> > > by writing a zero to PASID control register.
> > >
> > > Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
> >
> > > +       /* The current Linux kernel vfio driver does not expose PASID capability to
> > > +        * users. It also enables PASID by default, which breaks DLB PF PMD. We have
> > > +        * to use the hardcoded offset for now to disable PASID.
> > > +        */
> > > +       pasid_cap_offset = DLB2_PCI_PASID_CAP_OFFSET;
> > > +
> > > +       off = pasid_cap_offset + DLB2_PCI_PASID_CTRL;
> >
> > +++ additional folks.
> >
> > Is make sense to move this helper function to PCI common for disabling
> > PASID for a PCI device so that other driver can use if needed
> > as the implementation is not specific to DLB2.
>
> Yes, having a helper sounds like a first step (and we probably have
> more helpers to add seeing how drivers tend to redefine non vendor
> specific pci configs, but that's another story).

About this other story, I sent a cleanup series:
https://patchwork.dpdk.org/project/dpdk/list/?series=29101&state=%2A&archive=both

Comments/reviews welcome.


>
> Now, about PASID being enabled by default with Linux 6.2, is this
> breaking of dlb PF something special? Or can we expect many (all?)
> other devices to break too?
> If so, maybe we should disable it in the pci common code.

Is anybody looking into reworking this proposal and moving this code
to the pci bus driver?
Cc: pci bus maintainers.


-- 
David Marchand

RE: [PATCH v1] event/dlb2: add support for disabling PASID

[Sevincer, Abdullah]

>+Is anybody looking into reworking this proposal and moving this code to the pci bus driver?
>+Cc: pci bus maintainers.


>+--
>+David Marchand

We will work on this proposal, it is not finalized yet.
Its not DLB2 specific as commenters say, we are looking into if there is another way doing it.
If it will end up common and not DLB specific for other drivers as well we will add function to pci common. 

[PATCH v2] event/dlb2: disable PASID for kernel 6.2

[Abdullah Sevincer]

vfio-pci driver in Linux kernel 6.2 enables PASID by default.
In DLB hardware, enabling PASID puts DLB in SIOV mode. This
breaks DLB PF-PMD mode. For DLB PF-PMD mode to function properly
PASID needs to be disabled for kernel 6.2.

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

Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
---
 drivers/event/dlb2/pf/dlb2_main.c | 24 ++++++++++++++++++++++++
 lib/pci/rte_pci.h                 |  4 ++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/event/dlb2/pf/dlb2_main.c b/drivers/event/dlb2/pf/dlb2_main.c
index aa03e4c311..5065c0a9e4 100644
--- a/drivers/event/dlb2/pf/dlb2_main.c
+++ b/drivers/event/dlb2/pf/dlb2_main.c
@@ -190,6 +190,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
 	uint16_t rt_ctl_word;
 	uint32_t pri_reqs_dword;
 	uint16_t pri_ctrl_word;
+	uint16_t pasid_ctrl;
 
 	off_t pcie_cap_offset;
 	int pri_cap_offset;
@@ -197,6 +198,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
 	int err_cap_offset;
 	int acs_cap_offset;
 	int wait_count;
+	int pasid_cap_offset;
 
 	uint16_t devsta_busy_word;
 	uint16_t devctl_word;
@@ -514,6 +516,28 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
 		}
 	}
 
+	pasid_cap_offset = rte_pci_find_ext_capability(pdev, RTE_PCI_EXT_CAP_ID_PASID);
+
+	if (pasid_cap_offset >= 0) {
+		off = pasid_cap_offset + RTE_PCI_PASID_CTRL;
+
+		if (rte_pci_read_config(pdev, &pasid_ctrl, 2, off) != 2)
+			pasid_ctrl = 0;
+
+		if (pasid_ctrl) {
+
+			DLB2_INFO(dlb2_dev, "DLB2 disabling pasid...\n");
+			pasid_ctrl = 0;
+			ret = rte_pci_write_config(pdev, &pasid_ctrl, 2, off);
+
+			if (ret != 2) {
+				DLB2_LOG_ERR("[%s()] failed to write the pcie config space at offset %d\n",
+						__func__, (int)off);
+				return ret;
+			}
+		}
+	}
+
 	return 0;
 }
 
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

RE: [PATCH v1] event/dlb2: add support for disabling PASID

[Sevincer, Abdullah]

>+We will work on this proposal, it is not finalized yet.
>+Its not DLB2 specific as commenters say, we are looking into if there is another way doing it.
>+If it will end up common and not DLB specific for other drivers as well we will add function to pci common. 

As far as we know this problem is specific to DLB only, other drivers may have different design so they may not need to disable
PASID. I have moved the defines to rte_pci.h file(in case other drivers wants to disable PASID) and used function rte_pci_find_ext_capability to retrieve the offset.

[PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Abdullah Sevincer]

vfio-pci driver in Linux kernel 6.2 enables PASID by default.
In DLB hardware, enabling PASID puts DLB in SIOV mode. This
breaks DLB PF-PMD mode. For DLB PF-PMD mode to function properly
PASID needs to be disabled for kernel 6.2.

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 | 27 +++++++++++++++++++++++++++
 lib/pci/rte_pci.h                 |  5 +++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/event/dlb2/pf/dlb2_main.c b/drivers/event/dlb2/pf/dlb2_main.c
index aa03e4c311..34e47a4e33 100644
--- a/drivers/event/dlb2/pf/dlb2_main.c
+++ b/drivers/event/dlb2/pf/dlb2_main.c
@@ -190,6 +190,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
 	uint16_t rt_ctl_word;
 	uint32_t pri_reqs_dword;
 	uint16_t pri_ctrl_word;
+	uint16_t pasid_ctrl;
 
 	off_t pcie_cap_offset;
 	int pri_cap_offset;
@@ -197,6 +198,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
 	int err_cap_offset;
 	int acs_cap_offset;
 	int wait_count;
+	int pasid_cap_offset;
 
 	uint16_t devsta_busy_word;
 	uint16_t devctl_word;
@@ -514,6 +516,31 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
 		}
 	}
 
+	/* TODO - The current Linux kernel 6.2 vfio driver does not expose PASID capability to
+	 * users. It also enables PASID by default, which breaks DLB PF PMD. We have
+	 * to use the hardcoded offset for now to disable PASID. It may be different for
+	 * other device drivers since they may have different design. When PASID capability
+	 * is exposed to users, please revise this part and add api to disable PASID through
+	 * pci common code.
+	 */
+	pasid_cap_offset = RTE_PCI_PASID_CAP_OFFSET;
+
+	off = pasid_cap_offset + RTE_PCI_PASID_CTRL;
+	if (rte_pci_read_config(pdev, &pasid_ctrl, 2, off) != 2)
+		pasid_ctrl = 0;
+
+	if (pasid_ctrl) {
+		DLB2_INFO(dlb2_dev, "DLB2 disabling pasid...\n");
+
+		pasid_ctrl = 0;
+		ret = rte_pci_write_config(pdev, &pasid_ctrl, 2, off);
+		if (ret != 2) {
+			DLB2_LOG_ERR("[%s()] failed to write the pcie config space at offset %d\n",
+				__func__, (int)off);
+			return ret;
+		}
+	}
+
 	return 0;
 }
 
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

Re: [PATCH v1] event/dlb2: add support for disabling PASID

[Stephen Hemminger]

On Wed,  7 Jun 2023 16:00:50 -0500
Abdullah Sevincer <abdullah.sevincer@intel.com> wrote:

> From: Abdullah Sevincer <abdullah.sevincer@intel.com>
> To: dev@dpdk.org
> Cc: jerinj@marvell.com, mike.ximing.chen@intel.com,  Abdullah Sevincer <abdullah.sevincer@intel.com>
> Subject: [PATCH v1] event/dlb2: add support for disabling PASID
> Date: Wed,  7 Jun 2023 16:00:50 -0500
> X-Mailer: git-send-email 2.25.1
> 
> vfio-pci driver in Linux kernel 6.2 enables PASID by default.
> In DLB hardware, enabling PASID puts DLB in SIOV mode. This
> breaks DLB PF-PMD mode. For DLB PF-PMD mode to function properly
> PASID needs to be disabled for kernel 6.2.
> 
> In this commit this issue is addressed and PASID is disabled
> by writing a zero to PASID control register.
> 
> Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>

It is not clear if this is kernels 6.2 or later, or just that version.
Also, distribution kernel versions don't reflect upstream so if you are complaining about
RHEL etc. The code is version independent (good). It would be good to supply more
commit information (ideally upstream commit id) in this patch commit log.

Also 6.2 kernel is end-of-life (May 2023) so users should be using older or later
supported kernel version.

Re: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Jerin Jacob]

On Tue, Oct 31, 2023 at 4:12 AM Abdullah Sevincer
<abdullah.sevincer@intel.com> wrote:
>
> vfio-pci driver in Linux kernel 6.2 enables PASID by default.
> In DLB hardware, enabling PASID puts DLB in SIOV mode. This
> breaks DLB PF-PMD mode. For DLB PF-PMD mode to function properly
> PASID needs to be disabled for kernel 6.2.
>
> 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 | 27 +++++++++++++++++++++++++++
>  lib/pci/rte_pci.h                 |  5 +++++
>  2 files changed, 32 insertions(+)
>
> diff --git a/drivers/event/dlb2/pf/dlb2_main.c b/drivers/event/dlb2/pf/dlb2_main.c
> index aa03e4c311..34e47a4e33 100644
> --- a/drivers/event/dlb2/pf/dlb2_main.c
> +++ b/drivers/event/dlb2/pf/dlb2_main.c
> @@ -190,6 +190,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>         uint16_t rt_ctl_word;
>         uint32_t pri_reqs_dword;
>         uint16_t pri_ctrl_word;
> +       uint16_t pasid_ctrl;
>
>         off_t pcie_cap_offset;
>         int pri_cap_offset;
> @@ -197,6 +198,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>         int err_cap_offset;
>         int acs_cap_offset;
>         int wait_count;
> +       int pasid_cap_offset;
>
>         uint16_t devsta_busy_word;
>         uint16_t devctl_word;
> @@ -514,6 +516,31 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>                 }
>         }
>
> +       /* TODO - The current Linux kernel 6.2 vfio driver does not expose PASID capability to
> +        * users. It also enables PASID by default, which breaks DLB PF PMD. We have
> +        * to use the hardcoded offset for now to disable PASID. It may be different for
> +        * other device drivers since they may have different design. When PASID capability
> +        * is exposed to users, please revise this part and add api to disable PASID through
> +        * pci common code.
> +        */
> +       pasid_cap_offset = RTE_PCI_PASID_CAP_OFFSET;
> +
> +       off = pasid_cap_offset + RTE_PCI_PASID_CTRL;
> +       if (rte_pci_read_config(pdev, &pasid_ctrl, 2, off) != 2)
> +               pasid_ctrl = 0;
> +
> +       if (pasid_ctrl) {
> +               DLB2_INFO(dlb2_dev, "DLB2 disabling pasid...\n");
> +
> +               pasid_ctrl = 0;
> +               ret = rte_pci_write_config(pdev, &pasid_ctrl, 2, off);
> +               if (ret != 2) {
> +                       DLB2_LOG_ERR("[%s()] failed to write the pcie config space at offset %d\n",
> +                               __func__, (int)off);
> +                       return ret;
> +               }
> +       }
> +
>         return 0;
>  }

This patch can be splited as two,
1) Generic PCIe function to enable/disable PASID
2) Call generic function to disable PASID in drivers/event/dlb2/. Also
mention which Linux kernel commit
is introducing this issue in the git commit log.

RE: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Sevincer, Abdullah]

> +This patch can be splited as two,
> +1) Generic PCIe function to enable/disable PASID
> +2) Call generic function to disable PASID in drivers/event/dlb2/. Also mention which Linux kernel commit is introducing this issue in the git commit log.

Hi Jerrin,
I think I need to provide more information here, then we can decide which way we will go would be good for now. I agree to having 2 functions in pci common 
code to enable/disable PASID, but we need to have hardcoded PASID cap offset inside these functions as well since PASID capability is not exposed to user. Hence, to be more specific 
main reason to have hardcoded PASID is, rte_pci_find_ext_capability() function to retrieve the offset returns '0' since PASID is not exposed to user yet. 

We can see this is vfio_pci_config.c in kernel code where PASID is not exposed to user.
[PCI_EXT_CAP_ID_PASID]	=	0,	/* not yet */

So if it is okay to go with hardcoded offset now in these functions I will move the implementation to pci_common file.

Re: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Jerin Jacob]

On Tue, Oct 31, 2023 at 8:43 PM Sevincer, Abdullah
<abdullah.sevincer@intel.com> wrote:
>
>
> > +This patch can be splited as two,
> > +1) Generic PCIe function to enable/disable PASID
> > +2) Call generic function to disable PASID in drivers/event/dlb2/. Also mention which Linux kernel commit is introducing this issue in the git commit log.
>
> Hi Jerrin,
> I think I need to provide more information here, then we can decide which way we will go would be good for now. I agree to having 2 functions in pci common
> code to enable/disable PASID, but we need to have hardcoded PASID cap offset inside these functions as well since PASID capability is not exposed to user. Hence, to be more specific
> main reason to have hardcoded PASID is, rte_pci_find_ext_capability() function to retrieve the offset returns '0' since PASID is not exposed to user yet.
>
> We can see this is vfio_pci_config.c in kernel code where PASID is not exposed to user.
> [PCI_EXT_CAP_ID_PASID]  =       0,      /* not yet */
>
> So if it is okay to go with hardcoded offset now in these functions I will move the implementation to pci_common file.

I would suggest, add argument option to API whether to probe the
capability or not? - 0 means probe and- non zero means specific PASID
cap offset till Linux VFIO is exposing it.

Re: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Bruce Richardson]

On Tue, Oct 31, 2023 at 10:36:04PM +0530, Jerin Jacob wrote:
> On Tue, Oct 31, 2023 at 8:43 PM Sevincer, Abdullah
> <abdullah.sevincer@intel.com> wrote:
> >
> >
> > > +This patch can be splited as two,
> > > +1) Generic PCIe function to enable/disable PASID
> > > +2) Call generic function to disable PASID in drivers/event/dlb2/. Also mention which Linux kernel commit is introducing this issue in the git commit log.
> >
> > Hi Jerrin,
> > I think I need to provide more information here, then we can decide which way we will go would be good for now. I agree to having 2 functions in pci common
> > code to enable/disable PASID, but we need to have hardcoded PASID cap offset inside these functions as well since PASID capability is not exposed to user. Hence, to be more specific
> > main reason to have hardcoded PASID is, rte_pci_find_ext_capability() function to retrieve the offset returns '0' since PASID is not exposed to user yet.
> >
> > We can see this is vfio_pci_config.c in kernel code where PASID is not exposed to user.
> > [PCI_EXT_CAP_ID_PASID]  =       0,      /* not yet */
> >
> > So if it is okay to go with hardcoded offset now in these functions I will move the implementation to pci_common file.
> 
> I would suggest, add argument option to API whether to probe the
> capability or not? - 0 means probe and- non zero means specific PASID
> cap offset till Linux VFIO is exposing it.

That doesn't seem particularly useful to me. The calling-API in the DPDK
PMD (assuming it's PMD who use this), is no more likely to know whether
probing will work. Therefore, I think we just hard-code the offset for now.
We can decide what the best approach is later on once kernel actually
exposes the value to users. Only then will we know if it's possible to
detect that exposure or not.

/Bruce

Re: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Jerin Jacob]

On Tue, Oct 31, 2023 at 10:45 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, Oct 31, 2023 at 10:36:04PM +0530, Jerin Jacob wrote:
> > On Tue, Oct 31, 2023 at 8:43 PM Sevincer, Abdullah
> > <abdullah.sevincer@intel.com> wrote:
> > >
> > >
> > > > +This patch can be splited as two,
> > > > +1) Generic PCIe function to enable/disable PASID
> > > > +2) Call generic function to disable PASID in drivers/event/dlb2/. Also mention which Linux kernel commit is introducing this issue in the git commit log.
> > >
> > > Hi Jerrin,
> > > I think I need to provide more information here, then we can decide which way we will go would be good for now. I agree to having 2 functions in pci common
> > > code to enable/disable PASID, but we need to have hardcoded PASID cap offset inside these functions as well since PASID capability is not exposed to user. Hence, to be more specific
> > > main reason to have hardcoded PASID is, rte_pci_find_ext_capability() function to retrieve the offset returns '0' since PASID is not exposed to user yet.
> > >
> > > We can see this is vfio_pci_config.c in kernel code where PASID is not exposed to user.
> > > [PCI_EXT_CAP_ID_PASID]  =       0,      /* not yet */
> > >
> > > So if it is okay to go with hardcoded offset now in these functions I will move the implementation to pci_common file.
> >
> > I would suggest, add argument option to API whether to probe the
> > capability or not? - 0 means probe and- non zero means specific PASID
> > cap offset till Linux VFIO is exposing it.
>
> That doesn't seem particularly useful to me. The calling-API in the DPDK
> PMD (assuming it's PMD who use this), is no more likely to know whether
> probing will work. Therefore, I think we just hard-code the offset for now.

I think, there are three things here
1) Whether to have common API for dealing with generic function like
enabling PASID or not? - I think, we are in agreement to have common
public function(Implementation could be hard-coded or probe)
2) Since it is public PCIe API, I thought of adding probing in API as
it is just LINUX limitation now. No strong opinion on inclusion of
probe in on this as Linux is main EAL which supports PCIe now.
3) Since it is PCIe capability, In my understanding the offset will
change based on the number of capabilities available in PCIe config
space for a given device. _if so_, an additional argument for the
offset
needs to be passed from PMD to common PCI API(I.e it can not be
hard-coded in common PCI code)

> We can decide what the best approach is later on once kernel actually
> exposes the value to users. Only then will we know if it's possible to
> detect that exposure or not.
>
> /Bruce

Re: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Bruce Richardson]

On Wed, Nov 01, 2023 at 12:12:02AM +0530, Jerin Jacob wrote:
> On Tue, Oct 31, 2023 at 10:45 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Tue, Oct 31, 2023 at 10:36:04PM +0530, Jerin Jacob wrote:
> > > On Tue, Oct 31, 2023 at 8:43 PM Sevincer, Abdullah
> > > <abdullah.sevincer@intel.com> wrote:
> > > >
> > > >
> > > > > +This patch can be splited as two, +1) Generic PCIe function to
> > > > > enable/disable PASID +2) Call generic function to disable PASID
> > > > > in drivers/event/dlb2/. Also mention which Linux kernel commit is
> > > > > introducing this issue in the git commit log.
> > > >
> > > > Hi Jerrin, I think I need to provide more information here, then we
> > > > can decide which way we will go would be good for now. I agree to
> > > > having 2 functions in pci common code to enable/disable PASID, but
> > > > we need to have hardcoded PASID cap offset inside these functions
> > > > as well since PASID capability is not exposed to user. Hence, to be
> > > > more specific main reason to have hardcoded PASID is,
> > > > rte_pci_find_ext_capability() function to retrieve the offset
> > > > returns '0' since PASID is not exposed to user yet.
> > > >
> > > > We can see this is vfio_pci_config.c in kernel code where PASID is
> > > > not exposed to user.  [PCI_EXT_CAP_ID_PASID]  =       0,      /*
> > > > not yet */
> > > >
> > > > So if it is okay to go with hardcoded offset now in these functions
> > > > I will move the implementation to pci_common file.
> > >
> > > I would suggest, add argument option to API whether to probe the
> > > capability or not? - 0 means probe and- non zero means specific PASID
> > > cap offset till Linux VFIO is exposing it.
> >
> > That doesn't seem particularly useful to me. The calling-API in the
> > DPDK PMD (assuming it's PMD who use this), is no more likely to know
> > whether probing will work. Therefore, I think we just hard-code the
> > offset for now.
> 
> I think, there are three things here 1) Whether to have common API for
> dealing with generic function like enabling PASID or not? - I think, we
> are in agreement to have common public function(Implementation could be
> hard-coded or probe) 2) Since it is public PCIe API, I thought of adding
> probing in API as it is just LINUX limitation now. No strong opinion on
> inclusion of probe in on this as Linux is main EAL which supports PCIe
> now.  3) Since it is PCIe capability, In my understanding the offset will
> change based on the number of capabilities available in PCIe config space
> for a given device. _if so_, an additional argument for the offset needs
> to be passed from PMD to common PCI API(I.e it can not be hard-coded in
> common PCI code)
> 

Given these constraints and how late we are in the release cycle, I
therefore suggest we take the driver-specific bug fix for now. DLB seems
the only driver affected right now (and I can confirm the issue having
encountered it myself when running DPDK on Ubuntu 23.04).

I think a general function is a good thing to have, but it seems that such
a general function should wait until we really can make it generic in the
future.

Just my 2c. at this point.

/Bruce

Re: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Jerin Jacob]

On Wed, Nov 1, 2023 at 2:14 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Nov 01, 2023 at 12:12:02AM +0530, Jerin Jacob wrote:
> > On Tue, Oct 31, 2023 at 10:45 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > On Tue, Oct 31, 2023 at 10:36:04PM +0530, Jerin Jacob wrote:
> > > > On Tue, Oct 31, 2023 at 8:43 PM Sevincer, Abdullah
> > > > <abdullah.sevincer@intel.com> wrote:
> > > > >
> > > > >
> > > > > > +This patch can be splited as two, +1) Generic PCIe function to
> > > > > > enable/disable PASID +2) Call generic function to disable PASID
> > > > > > in drivers/event/dlb2/. Also mention which Linux kernel commit is
> > > > > > introducing this issue in the git commit log.
> > > > >
> > > > > Hi Jerrin, I think I need to provide more information here, then we
> > > > > can decide which way we will go would be good for now. I agree to
> > > > > having 2 functions in pci common code to enable/disable PASID, but
> > > > > we need to have hardcoded PASID cap offset inside these functions
> > > > > as well since PASID capability is not exposed to user. Hence, to be
> > > > > more specific main reason to have hardcoded PASID is,
> > > > > rte_pci_find_ext_capability() function to retrieve the offset
> > > > > returns '0' since PASID is not exposed to user yet.
> > > > >
> > > > > We can see this is vfio_pci_config.c in kernel code where PASID is
> > > > > not exposed to user.  [PCI_EXT_CAP_ID_PASID]  =       0,      /*
> > > > > not yet */
> > > > >
> > > > > So if it is okay to go with hardcoded offset now in these functions
> > > > > I will move the implementation to pci_common file.
> > > >
> > > > I would suggest, add argument option to API whether to probe the
> > > > capability or not? - 0 means probe and- non zero means specific PASID
> > > > cap offset till Linux VFIO is exposing it.
> > >
> > > That doesn't seem particularly useful to me. The calling-API in the
> > > DPDK PMD (assuming it's PMD who use this), is no more likely to know
> > > whether probing will work. Therefore, I think we just hard-code the
> > > offset for now.
> >
> > I think, there are three things here 1) Whether to have common API for
> > dealing with generic function like enabling PASID or not? - I think, we
> > are in agreement to have common public function(Implementation could be
> > hard-coded or probe) 2) Since it is public PCIe API, I thought of adding
> > probing in API as it is just LINUX limitation now. No strong opinion on
> > inclusion of probe in on this as Linux is main EAL which supports PCIe
> > now.  3) Since it is PCIe capability, In my understanding the offset will
> > change based on the number of capabilities available in PCIe config space
> > for a given device. _if so_, an additional argument for the offset needs
> > to be passed from PMD to common PCI API(I.e it can not be hard-coded in
> > common PCI code)
> >
>
> Given these constraints and how late we are in the release cycle, I
> therefore suggest we take the driver-specific bug fix for now. DLB seems
> the only driver affected right now (and I can confirm the issue having
> encountered it myself when running DPDK on Ubuntu 23.04).
>
> I think a general function is a good thing to have, but it seems that such
> a general function should wait until we really can make it generic in the
> future.

+ PCIe maintainers.

I will leave this up to @David Marchand  / @Thomas as this patch has
common code changes and needs to come via main tree.

Also in this case, The comment was given very early(Back in June 7)
for the same.
https://patches.dpdk.org/project/dpdk/patch/20230607210050.107944-1-abdullah.sevincer@intel.com/

>
> Just my 2c. at this point.
>
> /Bruce

RE: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Sevincer, Abdullah]

>++ PCIe maintainers.

>+I will leave this up to @David Marchand  / @Thomas as this patch has common code changes and needs to come via main tree.

>+Also in this case, The comment was given very early(Back in June 7) for the same.
>+https://patches.dpdk.org/project/dpdk/patch/20230607210050.107944-1-abdullah.sevincer@intel.com/

Thanks Jerrin and Bruce for the comments.
I will wait for opinion of PCI maintainers.

Re: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Bruce Richardson]

On Wed, Nov 01, 2023 at 07:05:54PM +0000, Sevincer, Abdullah wrote:
> 
> >++ PCIe maintainers.
> 
> >+I will leave this up to @David Marchand  / @Thomas as this patch has common code changes and needs to come via main tree.
> 
> >+Also in this case, The comment was given very early(Back in June 7) for the same.
> >+https://patches.dpdk.org/project/dpdk/patch/20230607210050.107944-1-abdullah.sevincer@intel.com/
> 
> Thanks Jerrin and Bruce for the comments.
> I will wait for opinion of PCI maintainers.

Thinking on this more, any API for enable/disable pasid would be
internal-only, so therefore would not be subject to ABI/API change rules
AFAIK. This gives us more freedom to change it as more discovery
capabilities become available.
Therefore, an initial version of the function can take the offset as
parameter, and we can update it without API/ABI concerns later. I was
previously worried about trying to get the API correct first time, but for
internal functions, we don't need to.

WDYT, Jerin, Abdullah.

/Bruce

Re: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Thomas Monjalon]

02/11/2023 11:23, Bruce Richardson:
> On Wed, Nov 01, 2023 at 07:05:54PM +0000, Sevincer, Abdullah wrote:
> > 
> > >++ PCIe maintainers.
> > 
> > >+I will leave this up to @David Marchand  / @Thomas as this patch has common code changes and needs to come via main tree.
> > 
> > >+Also in this case, The comment was given very early(Back in June 7) for the same.
> > >+https://patches.dpdk.org/project/dpdk/patch/20230607210050.107944-1-abdullah.sevincer@intel.com/
> > 
> > Thanks Jerrin and Bruce for the comments.
> > I will wait for opinion of PCI maintainers.
> 
> Thinking on this more, any API for enable/disable pasid would be
> internal-only, so therefore would not be subject to ABI/API change rules
> AFAIK. This gives us more freedom to change it as more discovery
> capabilities become available.
> Therefore, an initial version of the function can take the offset as
> parameter, and we can update it without API/ABI concerns later. I was
> previously worried about trying to get the API correct first time, but for
> internal functions, we don't need to.

It looks a good option.
Better to add an internal PCI function than doing it in a PMD.

> WDYT, Jerin, Abdullah.

RE: [PATCH v3] event/dlb2: fix disable PASID for kernel 6.2

[Sevincer, Abdullah]

>+> WDYT, Jerin, Abdullah.

It is a good idea to make it internal, I will work on that and upstream a new one.