DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus
@ 2021-01-25 17:08 Nick Connolly
  2021-01-26 17:45 ` Tal Shnaiderman
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Nick Connolly @ 2021-01-25 17:08 UTC (permalink / raw)
  To: talshn, dmitry.kozliuk, pallavi.kadam, thomas; +Cc: dev, Nick Connolly

Attaching to an NVMe disk on Windows using SPDK requires the
PCI class ID and device.bus fields. Decode the class ID from the PCI
device info strings if it is present and set device.bus.

Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
---
 drivers/bus/pci/windows/pci.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
index f66258452..3d11444c2 100644
--- a/drivers/bus/pci/windows/pci.c
+++ b/drivers/bus/pci/windows/pci.c
@@ -280,17 +280,24 @@ parse_pci_hardware_id(const char *buf, struct rte_pci_id *pci_id)
 {
 	int ids = 0;
 	uint16_t vendor_id, device_id;
-	uint32_t subvendor_id = 0;
+	uint32_t subvendor_id = 0, class_id = 0;
+	const char *cp;
 
 	ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16 "&SUBSYS_%"
 		PRIx32, &vendor_id, &device_id, &subvendor_id);
 	if (ids != 3)
 		return -1;
 
+	/* Try and find PCI class ID */
+	for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
+		if (*cp == '&' && sscanf_s(cp, "&CC_%" PRIx32, &class_id) == 1)
+			break;
+
 	pci_id->vendor_id = vendor_id;
 	pci_id->device_id = device_id;
 	pci_id->subsystem_device_id = subvendor_id >> 16;
 	pci_id->subsystem_vendor_id = subvendor_id & 0xffff;
+	pci_id->class_id = class_id;
 	return 0;
 }
 
@@ -339,6 +346,7 @@ pci_scan_one(HDEVINFO dev_info, PSP_DEVINFO_DATA device_info_data)
 	if (ret != 0)
 		goto end;
 
+	dev->device.bus = &rte_pci_bus.bus;
 	dev->addr = addr;
 	dev->id = pci_id;
 	dev->max_vfs = 0; /* TODO: get max_vfs */
-- 
2.25.1


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

* Re: [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus
  2021-01-25 17:08 [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus Nick Connolly
@ 2021-01-26 17:45 ` Tal Shnaiderman
  2021-01-26 18:18   ` Nick Connolly
  2021-01-28 17:00 ` [dpdk-dev] [PATCH v2] " Nick Connolly
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Tal Shnaiderman @ 2021-01-26 17:45 UTC (permalink / raw)
  To: Nick Connolly, dmitry.kozliuk, pallavi.kadam,
	NBU-Contact-Thomas Monjalon
  Cc: dev

> Subject: [PATCH] bus/pci: nvme on Windows requires class id and bus
> 
> External email: Use caution opening links or attachments
> 
> 
> Attaching to an NVMe disk on Windows using SPDK requires the PCI class ID
> and device.bus fields. Decode the class ID from the PCI device info strings if it
> is present and set device.bus.
> 
> Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
> ---
>  drivers/bus/pci/windows/pci.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
> index f66258452..3d11444c2 100644
> --- a/drivers/bus/pci/windows/pci.c
> +++ b/drivers/bus/pci/windows/pci.c
> @@ -280,17 +280,24 @@ parse_pci_hardware_id(const char *buf, struct
> rte_pci_id *pci_id)  {
>         int ids = 0;
>         uint16_t vendor_id, device_id;
> -       uint32_t subvendor_id = 0;
> +       uint32_t subvendor_id = 0, class_id = 0;
> +       const char *cp;
> 
>         ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16
> "&SUBSYS_%"
>                 PRIx32, &vendor_id, &device_id, &subvendor_id);
>         if (ids != 3)
>                 return -1;
> 
> +       /* Try and find PCI class ID */
> +       for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)

How about 
for (cp = buf; cp[0] || cp[1]; cp++)

> +               if (*cp == '&' && sscanf_s(cp, "&CC_%" PRIx32, &class_id) == 1)

Could there be a case where PCI\\VEN_v(4)&DEV_d(4)&CC_c(2)s(2) exist but PCI\\VEN_v(4)&DEV_d(4)&CC_c(2)s(2)p(2) doesn't? In that case the parsing would be incorrect.

> +                       break;
> +
>         pci_id->vendor_id = vendor_id;
>         pci_id->device_id = device_id;
>         pci_id->subsystem_device_id = subvendor_id >> 16;
>         pci_id->subsystem_vendor_id = subvendor_id & 0xffff;
> +       pci_id->class_id = class_id;
>         return 0;
>  }
> 
> @@ -339,6 +346,7 @@ pci_scan_one(HDEVINFO dev_info,
> PSP_DEVINFO_DATA device_info_data)
>         if (ret != 0)
>                 goto end;
> 
> +       dev->device.bus = &rte_pci_bus.bus;
>         dev->addr = addr;
>         dev->id = pci_id;
>         dev->max_vfs = 0; /* TODO: get max_vfs */
> --
> 2.25.1


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

* Re: [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus
  2021-01-26 17:45 ` Tal Shnaiderman
@ 2021-01-26 18:18   ` Nick Connolly
  2021-01-26 22:41     ` Thomas Monjalon
  2021-01-27 15:13     ` Tal Shnaiderman
  0 siblings, 2 replies; 18+ messages in thread
From: Nick Connolly @ 2021-01-26 18:18 UTC (permalink / raw)
  To: Tal Shnaiderman, dmitry.kozliuk, pallavi.kadam,
	NBU-Contact-Thomas Monjalon
  Cc: dev

Hi Tal,

Thanks for the comments.

>> +       /* Try and find PCI class ID */
>> +       for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
> How about
> for (cp = buf; cp[0] || cp[1]; cp++)
That would be my preferred idiom, but the DPDK coding style (1.9.1) says 
'do not use ! for tests unless it is a boolean' (but somewhat 
confusingly does so in a section on NULL pointers).  I interpreted it as 
a general prohibition on conditionals without an explicit operator 
(except for booleans).  I'd love to be corrected here!

>> +               if (*cp == '&' && sscanf_s(cp, "&CC_%" PRIx32, &class_id) == 1)
> Could there be a case where PCI\\VEN_v(4)&DEV_d(4)&CC_c(2)s(2) exist but PCI\\VEN_v(4)&DEV_d(4)&CC_c(2)s(2)p(2) doesn't? In that case the parsing would be incorrect.
The MSDN documentation says that the most specific string will be 
returned first, in this case &CC_c(2)s(2)p(2), so if there is a 6-digit 
class ID we will return it.  That left me wondering what to do if we 
only encounter a 4-digit class ID.  If we ignore it, then we won't be 
able to match on the class ID.  We could parse it as 4-digits and supply 
zero for the p(2) field, but that left me wondering why Windows wouldn't 
have already done that for us.  What I see on my own systems is that 
there is always a 6-digit class ID defined, even if the pci-ids 
repository doesn't list any values for p(2) (e.g. 
https://pci-ids.ucw.cz/read/PD/07/80 reports as &CC_078000).

My conclusion was that was probably best just to return the first class 
ID that Windows reports even if it's only 4-digits.  That way the device 
can still be found and will match the CC_ info displayed in device manager.

Thanks,
Nick

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

* Re: [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus
  2021-01-26 18:18   ` Nick Connolly
@ 2021-01-26 22:41     ` Thomas Monjalon
  2021-01-27 14:22       ` Nick Connolly
  2021-01-27 15:13     ` Tal Shnaiderman
  1 sibling, 1 reply; 18+ messages in thread
From: Thomas Monjalon @ 2021-01-26 22:41 UTC (permalink / raw)
  To: Nick Connolly; +Cc: Tal Shnaiderman, dmitry.kozliuk, pallavi.kadam, dev

26/01/2021 19:18, Nick Connolly:
> Hi Tal,
> 
> Thanks for the comments.
> 
> >> +       /* Try and find PCI class ID */
> >> +       for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
> > How about
> > for (cp = buf; cp[0] || cp[1]; cp++)
> That would be my preferred idiom, but the DPDK coding style (1.9.1) says 
> 'do not use ! for tests unless it is a boolean' (but somewhat 
> confusingly does so in a section on NULL pointers).  I interpreted it as 
> a general prohibition on conditionals without an explicit operator 
> (except for booleans).  I'd love to be corrected here!

That's true. Comparisons should be explicit.





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

* Re: [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus
  2021-01-26 22:41     ` Thomas Monjalon
@ 2021-01-27 14:22       ` Nick Connolly
  0 siblings, 0 replies; 18+ messages in thread
From: Nick Connolly @ 2021-01-27 14:22 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Tal Shnaiderman, dmitry.kozliuk, pallavi.kadam, dev

On 26/01/2021 22:41, Thomas Monjalon wrote:
> That's true. Comparisons should be explicit.

Thanks Thomas,
Nick

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

* Re: [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus
  2021-01-26 18:18   ` Nick Connolly
  2021-01-26 22:41     ` Thomas Monjalon
@ 2021-01-27 15:13     ` Tal Shnaiderman
  1 sibling, 0 replies; 18+ messages in thread
From: Tal Shnaiderman @ 2021-01-27 15:13 UTC (permalink / raw)
  To: Nick Connolly, dmitry.kozliuk, pallavi.kadam,
	NBU-Contact-Thomas Monjalon
  Cc: dev

> Subject: Re: [PATCH] bus/pci: nvme on Windows requires class id and bus
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Tal,
> 
> Thanks for the comments.
> 
> >> +       /* Try and find PCI class ID */
> >> +       for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
> > How about
> > for (cp = buf; cp[0] || cp[1]; cp++)
> That would be my preferred idiom, but the DPDK coding style (1.9.1) says 'do
> not use ! for tests unless it is a boolean' (but somewhat confusingly does so
> in a section on NULL pointers).  I interpreted it as a general prohibition on
> conditionals without an explicit operator (except for booleans).  I'd love to be
> corrected here!
> 
> >> +               if (*cp == '&' && sscanf_s(cp, "&CC_%" PRIx32,
> >> + &class_id) == 1)
> > Could there be a case where PCI\\VEN_v(4)&DEV_d(4)&CC_c(2)s(2) exist
> but PCI\\VEN_v(4)&DEV_d(4)&CC_c(2)s(2)p(2) doesn't? In that case the
> parsing would be incorrect.
> The MSDN documentation says that the most specific string will be returned
> first, in this case &CC_c(2)s(2)p(2), so if there is a 6-digit class ID we will
> return it.  That left me wondering what to do if we only encounter a 4-digit
> class ID.  If we ignore it, then we won't be able to match on the class ID.  We
> could parse it as 4-digits and supply zero for the p(2) field, but that left me
> wondering why Windows wouldn't have already done that for us.  What I see
> on my own systems is that there is always a 6-digit class ID defined, even if
> the pci-ids repository doesn't list any values for p(2) (e.g.
> https://pci-ids.ucw.cz/read/PD/07/80 reports as &CC_078000).
> 
> My conclusion was that was probably best just to return the first class ID that
> Windows reports even if it's only 4-digits.  That way the device can still be
> found and will match the CC_ info displayed in device manager.

I'd shift the class_id value to the right in case the returned value is 4 digit, just to verify the result would be "c(2)s(2) 0" and not "0 s(2)p(2)", however when testing your change I couldn't find a case where only 4 digits matched.

Either than that LGTM.
> 
> Thanks,
> Nick

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

* [dpdk-dev] [PATCH v2] bus/pci: nvme on Windows requires class id and bus
  2021-01-25 17:08 [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus Nick Connolly
  2021-01-26 17:45 ` Tal Shnaiderman
@ 2021-01-28 17:00 ` Nick Connolly
  2021-01-28 17:04 ` [dpdk-dev] [PATCH v3] " Nick Connolly
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Nick Connolly @ 2021-01-28 17:00 UTC (permalink / raw)
  To: talshn, dmitry.kozliuk, pallavi.kadam, thomas; +Cc: dev, Nick Connolly

Attaching to an NVMe disk on Windows using SPDK requires the
PCI class ID and device.bus fields. Decode the class ID from the PCI
device info strings if it is present and set device.bus.

Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
---
 drivers/bus/pci/windows/pci.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
index f66258452..d380fc1ae 100644
--- a/drivers/bus/pci/windows/pci.c
+++ b/drivers/bus/pci/windows/pci.c
@@ -280,17 +280,29 @@ parse_pci_hardware_id(const char *buf, struct rte_pci_id *pci_id)
 {
 	int ids = 0;
 	uint16_t vendor_id, device_id;
-	uint32_t subvendor_id = 0;
+	uint32_t subvendor_id = 0, class_id = 0;
+	const char *cp;
 
 	ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16 "&SUBSYS_%"
 		PRIx32, &vendor_id, &device_id, &subvendor_id);
 	if (ids != 3)
 		return -1;
 
+	/* Try and find PCI class ID */
+	for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
+		if (*cp == '&' && sscanf_s(cp,
+				"&CC_%" PRIx32, &class_id) == 1) {
+			/* Convert 4-digit class IDs to 6-digit format */
+			if (strspn(cp + 4, "0123456789abcdefABCDEF") == 4)
+				class_id <<= 8;
+			break;
+		}
+
 	pci_id->vendor_id = vendor_id;
 	pci_id->device_id = device_id;
 	pci_id->subsystem_device_id = subvendor_id >> 16;
 	pci_id->subsystem_vendor_id = subvendor_id & 0xffff;
+	pci_id->class_id = class_id;
 	return 0;
 }
 
@@ -339,6 +351,7 @@ pci_scan_one(HDEVINFO dev_info, PSP_DEVINFO_DATA device_info_data)
 	if (ret != 0)
 		goto end;
 
+	dev->device.bus = &rte_pci_bus.bus;
 	dev->addr = addr;
 	dev->id = pci_id;
 	dev->max_vfs = 0; /* TODO: get max_vfs */
-- 
2.25.1

v2:
* If only a 4-digit class ID is available, convert it to 6-digit format

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

* [dpdk-dev] [PATCH v3] bus/pci: nvme on Windows requires class id and bus
  2021-01-25 17:08 [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus Nick Connolly
  2021-01-26 17:45 ` Tal Shnaiderman
  2021-01-28 17:00 ` [dpdk-dev] [PATCH v2] " Nick Connolly
@ 2021-01-28 17:04 ` Nick Connolly
  2021-01-31 15:56   ` Tal Shnaiderman
  2021-02-02 13:44 ` [dpdk-dev] [PATCH v4] " Nick Connolly
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Nick Connolly @ 2021-01-28 17:04 UTC (permalink / raw)
  To: talshn, dmitry.kozliuk, pallavi.kadam, thomas; +Cc: dev, Nick Connolly

Attaching to an NVMe disk on Windows using SPDK requires the
PCI class ID and device.bus fields. Decode the class ID from the PCI
device info strings if it is present and set device.bus.

Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
---
v3:
* Put version history at top - v2 mistakenly had it after the diffs

v2:
* If only a 4-digit class ID is available, convert it to 6-digit format

 drivers/bus/pci/windows/pci.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
index f66258452..d380fc1ae 100644
--- a/drivers/bus/pci/windows/pci.c
+++ b/drivers/bus/pci/windows/pci.c
@@ -280,17 +280,29 @@ parse_pci_hardware_id(const char *buf, struct rte_pci_id *pci_id)
 {
 	int ids = 0;
 	uint16_t vendor_id, device_id;
-	uint32_t subvendor_id = 0;
+	uint32_t subvendor_id = 0, class_id = 0;
+	const char *cp;
 
 	ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16 "&SUBSYS_%"
 		PRIx32, &vendor_id, &device_id, &subvendor_id);
 	if (ids != 3)
 		return -1;
 
+	/* Try and find PCI class ID */
+	for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
+		if (*cp == '&' && sscanf_s(cp,
+				"&CC_%" PRIx32, &class_id) == 1) {
+			/* Convert 4-digit class IDs to 6-digit format */
+			if (strspn(cp + 4, "0123456789abcdefABCDEF") == 4)
+				class_id <<= 8;
+			break;
+		}
+
 	pci_id->vendor_id = vendor_id;
 	pci_id->device_id = device_id;
 	pci_id->subsystem_device_id = subvendor_id >> 16;
 	pci_id->subsystem_vendor_id = subvendor_id & 0xffff;
+	pci_id->class_id = class_id;
 	return 0;
 }
 
@@ -339,6 +351,7 @@ pci_scan_one(HDEVINFO dev_info, PSP_DEVINFO_DATA device_info_data)
 	if (ret != 0)
 		goto end;
 
+	dev->device.bus = &rte_pci_bus.bus;
 	dev->addr = addr;
 	dev->id = pci_id;
 	dev->max_vfs = 0; /* TODO: get max_vfs */
-- 
2.25.1


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

* Re: [dpdk-dev] [PATCH v3] bus/pci: nvme on Windows requires class id and bus
  2021-01-28 17:04 ` [dpdk-dev] [PATCH v3] " Nick Connolly
@ 2021-01-31 15:56   ` Tal Shnaiderman
  2021-02-02 13:07     ` Nick Connolly
  0 siblings, 1 reply; 18+ messages in thread
From: Tal Shnaiderman @ 2021-01-31 15:56 UTC (permalink / raw)
  To: Nick Connolly, dmitry.kozliuk, pallavi.kadam,
	NBU-Contact-Thomas Monjalon
  Cc: dev

> Subject: [PATCH v3] bus/pci: nvme on Windows requires class id and bus
> 
> External email: Use caution opening links or attachments
> 
> 
> Attaching to an NVMe disk on Windows using SPDK requires the PCI class ID
> and device.bus fields. Decode the class ID from the PCI device info strings if it
> is present and set device.bus.
> 
> Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
> ---
> v3:
> * Put version history at top - v2 mistakenly had it after the diffs
> 
> v2:
> * If only a 4-digit class ID is available, convert it to 6-digit format
> 
>  drivers/bus/pci/windows/pci.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
> index f66258452..d380fc1ae 100644
> --- a/drivers/bus/pci/windows/pci.c
> +++ b/drivers/bus/pci/windows/pci.c
> @@ -280,17 +280,29 @@ parse_pci_hardware_id(const char *buf, struct
> rte_pci_id *pci_id)  {
>         int ids = 0;
>         uint16_t vendor_id, device_id;
> -       uint32_t subvendor_id = 0;
> +       uint32_t subvendor_id = 0, class_id = 0;
> +       const char *cp;
> 
>         ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16
> "&SUBSYS_%"
>                 PRIx32, &vendor_id, &device_id, &subvendor_id);
>         if (ids != 3)
>                 return -1;
> 
> +       /* Try and find PCI class ID */
> +       for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
> +               if (*cp == '&' && sscanf_s(cp,
> +                               "&CC_%" PRIx32, &class_id) == 1) {
> +                       /* Convert 4-digit class IDs to 6-digit format */
> +                       if (strspn(cp + 4, "0123456789abcdefABCDEF") == 4)

Maybe we can move this format string to a define in the header, something like RTE_PCI_DRV_CLASSID_FMT?

> +                               class_id <<= 8;
> +                       break;
> +               }
> +
>         pci_id->vendor_id = vendor_id;
>         pci_id->device_id = device_id;
>         pci_id->subsystem_device_id = subvendor_id >> 16;
>         pci_id->subsystem_vendor_id = subvendor_id & 0xffff;
> +       pci_id->class_id = class_id;
>         return 0;
>  }
> 
> @@ -339,6 +351,7 @@ pci_scan_one(HDEVINFO dev_info,
> PSP_DEVINFO_DATA device_info_data)
>         if (ret != 0)
>                 goto end;
> 
> +       dev->device.bus = &rte_pci_bus.bus;
>         dev->addr = addr;
>         dev->id = pci_id;
>         dev->max_vfs = 0; /* TODO: get max_vfs */
> --
> 2.25.1


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

* Re: [dpdk-dev] [PATCH v3] bus/pci: nvme on Windows requires class id and bus
  2021-01-31 15:56   ` Tal Shnaiderman
@ 2021-02-02 13:07     ` Nick Connolly
  0 siblings, 0 replies; 18+ messages in thread
From: Nick Connolly @ 2021-02-02 13:07 UTC (permalink / raw)
  To: Tal Shnaiderman, dmitry.kozliuk, pallavi.kadam,
	NBU-Contact-Thomas Monjalon
  Cc: dev

Hi Tal,

>> +                       /* Convert 4-digit class IDs to 6-digit format */
>> +                       if (strspn(cp + 4, "0123456789abcdefABCDEF") == 4)
> Maybe we can move this format string to a define in the header, something like RTE_PCI_DRV_CLASSID_FMT?
>

I'll send out a v4 with a definition at the top of pci.c

Regards,
Nick

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

* [dpdk-dev] [PATCH v4] bus/pci: nvme on Windows requires class id and bus
  2021-01-25 17:08 [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus Nick Connolly
                   ` (2 preceding siblings ...)
  2021-01-28 17:04 ` [dpdk-dev] [PATCH v3] " Nick Connolly
@ 2021-02-02 13:44 ` Nick Connolly
  2021-02-02 13:54 ` [dpdk-dev] [PATCH v5] " Nick Connolly
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Nick Connolly @ 2021-02-02 13:44 UTC (permalink / raw)
  To: talshn, dmitry.kozliuk, pallavi.kadam, thomas; +Cc: dev, Nick Connolly

Attaching to an NVMe disk on Windows using SPDK requires the
PCI class ID and device.bus fields. Decode the class ID from the PCI
device info strings if it is present and set device.bus.

Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
---
 drivers/bus/pci/windows/pci.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
index f66258452..dceb0f4b2 100644
--- a/drivers/bus/pci/windows/pci.c
+++ b/drivers/bus/pci/windows/pci.c
@@ -23,6 +23,9 @@ DEFINE_DEVPROPKEY(DEVPKEY_Device_Numa_Node, 0x540b947e, 0x8b40, 0x45bc,
  * the registry hive for PCI devices.
  */
 
+/* Class ID consists of hexadecimal digits */
+#define RTE_PCI_DRV_CLASSID_DIGIT	"0123456789abcdefABCDEF"
+
 /* The functions below are not implemented on Windows,
  * but need to be defined for compilation purposes
  */
@@ -280,17 +283,29 @@ parse_pci_hardware_id(const char *buf, struct rte_pci_id *pci_id)
 {
 	int ids = 0;
 	uint16_t vendor_id, device_id;
-	uint32_t subvendor_id = 0;
+	uint32_t subvendor_id = 0, class_id = 0;
+	const char *cp;
 
 	ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16 "&SUBSYS_%"
 		PRIx32, &vendor_id, &device_id, &subvendor_id);
 	if (ids != 3)
 		return -1;
 
+	/* Try and find PCI class ID */
+	for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
+		if (*cp == '&' && sscanf_s(cp,
+				"&CC_%" PRIx32, &class_id) == 1) {
+			/* Convert 4-digit class IDs to 6-digit format */
+			if (strspn(cp + 4, RTE_PCI_DRV_CLASSID_DIGIT) == 4)
+				class_id <<= 8;
+			break;
+		}
+
 	pci_id->vendor_id = vendor_id;
 	pci_id->device_id = device_id;
 	pci_id->subsystem_device_id = subvendor_id >> 16;
 	pci_id->subsystem_vendor_id = subvendor_id & 0xffff;
+	pci_id->class_id = class_id;
 	return 0;
 }
 
@@ -339,6 +354,7 @@ pci_scan_one(HDEVINFO dev_info, PSP_DEVINFO_DATA device_info_data)
 	if (ret != 0)
 		goto end;
 
+	dev->device.bus = &rte_pci_bus.bus;
 	dev->addr = addr;
 	dev->id = pci_id;
 	dev->max_vfs = 0; /* TODO: get max_vfs */
-- 
2.25.1


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

* [dpdk-dev] [PATCH v5] bus/pci: nvme on Windows requires class id and bus
  2021-01-25 17:08 [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus Nick Connolly
                   ` (3 preceding siblings ...)
  2021-02-02 13:44 ` [dpdk-dev] [PATCH v4] " Nick Connolly
@ 2021-02-02 13:54 ` Nick Connolly
  2021-02-02 14:04   ` Tal Shnaiderman
  2021-02-23 18:18 ` [dpdk-dev] [PATCH v6] " Nick Connolly
  2021-03-01  9:56 ` [dpdk-dev] [PATCH v7] " Nick Connolly
  6 siblings, 1 reply; 18+ messages in thread
From: Nick Connolly @ 2021-02-02 13:54 UTC (permalink / raw)
  To: talshn, dmitry.kozliuk, pallavi.kadam, thomas; +Cc: dev, Nick Connolly

Attaching to an NVMe disk on Windows using SPDK requires the
PCI class ID and device.bus fields. Decode the class ID from the PCI
device info strings if it is present and set device.bus.

Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
---
v5:
* Add missing version history

v4:
* Use #define to determine length of Class ID

v3:
* Put version history at top - v2 mistakenly had it after the diffs

v2:
* If only a 4-digit class ID is available, convert it to 6-digit format

 drivers/bus/pci/windows/pci.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
index f66258452..dceb0f4b2 100644
--- a/drivers/bus/pci/windows/pci.c
+++ b/drivers/bus/pci/windows/pci.c
@@ -23,6 +23,9 @@ DEFINE_DEVPROPKEY(DEVPKEY_Device_Numa_Node, 0x540b947e, 0x8b40, 0x45bc,
  * the registry hive for PCI devices.
  */
 
+/* Class ID consists of hexadecimal digits */
+#define RTE_PCI_DRV_CLASSID_DIGIT	"0123456789abcdefABCDEF"
+
 /* The functions below are not implemented on Windows,
  * but need to be defined for compilation purposes
  */
@@ -280,17 +283,29 @@ parse_pci_hardware_id(const char *buf, struct rte_pci_id *pci_id)
 {
 	int ids = 0;
 	uint16_t vendor_id, device_id;
-	uint32_t subvendor_id = 0;
+	uint32_t subvendor_id = 0, class_id = 0;
+	const char *cp;
 
 	ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16 "&SUBSYS_%"
 		PRIx32, &vendor_id, &device_id, &subvendor_id);
 	if (ids != 3)
 		return -1;
 
+	/* Try and find PCI class ID */
+	for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
+		if (*cp == '&' && sscanf_s(cp,
+				"&CC_%" PRIx32, &class_id) == 1) {
+			/* Convert 4-digit class IDs to 6-digit format */
+			if (strspn(cp + 4, RTE_PCI_DRV_CLASSID_DIGIT) == 4)
+				class_id <<= 8;
+			break;
+		}
+
 	pci_id->vendor_id = vendor_id;
 	pci_id->device_id = device_id;
 	pci_id->subsystem_device_id = subvendor_id >> 16;
 	pci_id->subsystem_vendor_id = subvendor_id & 0xffff;
+	pci_id->class_id = class_id;
 	return 0;
 }
 
@@ -339,6 +354,7 @@ pci_scan_one(HDEVINFO dev_info, PSP_DEVINFO_DATA device_info_data)
 	if (ret != 0)
 		goto end;
 
+	dev->device.bus = &rte_pci_bus.bus;
 	dev->addr = addr;
 	dev->id = pci_id;
 	dev->max_vfs = 0; /* TODO: get max_vfs */
-- 
2.25.1


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

* Re: [dpdk-dev] [PATCH v5] bus/pci: nvme on Windows requires class id and bus
  2021-02-02 13:54 ` [dpdk-dev] [PATCH v5] " Nick Connolly
@ 2021-02-02 14:04   ` Tal Shnaiderman
  0 siblings, 0 replies; 18+ messages in thread
From: Tal Shnaiderman @ 2021-02-02 14:04 UTC (permalink / raw)
  To: Nick Connolly, dmitry.kozliuk, pallavi.kadam,
	NBU-Contact-Thomas Monjalon
  Cc: dev

> Subject: [PATCH v5] bus/pci: nvme on Windows requires class id and bus
> 
> External email: Use caution opening links or attachments
> 
> 
> Attaching to an NVMe disk on Windows using SPDK requires the PCI class ID
> and device.bus fields. Decode the class ID from the PCI device info strings if it
> is present and set device.bus.
> 
> Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
> ---
> v5:
> * Add missing version history
> 
> v4:
> * Use #define to determine length of Class ID
> 
> v3:
> * Put version history at top - v2 mistakenly had it after the diffs
> 
> v2:
> * If only a 4-digit class ID is available, convert it to 6-digit format
> 
>  drivers/bus/pci/windows/pci.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
> index f66258452..dceb0f4b2 100644
> --- a/drivers/bus/pci/windows/pci.c
> +++ b/drivers/bus/pci/windows/pci.c
> @@ -23,6 +23,9 @@ DEFINE_DEVPROPKEY(DEVPKEY_Device_Numa_Node,
> 0x540b947e, 0x8b40, 0x45bc,
>   * the registry hive for PCI devices.
>   */
> 
> +/* Class ID consists of hexadecimal digits */
> +#define RTE_PCI_DRV_CLASSID_DIGIT      "0123456789abcdefABCDEF"
> +
>  /* The functions below are not implemented on Windows,
>   * but need to be defined for compilation purposes
>   */
> @@ -280,17 +283,29 @@ parse_pci_hardware_id(const char *buf, struct
> rte_pci_id *pci_id)  {
>         int ids = 0;
>         uint16_t vendor_id, device_id;
> -       uint32_t subvendor_id = 0;
> +       uint32_t subvendor_id = 0, class_id = 0;
> +       const char *cp;
> 
>         ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16
> "&SUBSYS_%"
>                 PRIx32, &vendor_id, &device_id, &subvendor_id);
>         if (ids != 3)
>                 return -1;
> 
> +       /* Try and find PCI class ID */
> +       for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
> +               if (*cp == '&' && sscanf_s(cp,
> +                               "&CC_%" PRIx32, &class_id) == 1) {
> +                       /* Convert 4-digit class IDs to 6-digit format */
> +                       if (strspn(cp + 4, RTE_PCI_DRV_CLASSID_DIGIT) == 4)
> +                               class_id <<= 8;
> +                       break;
> +               }
> +
>         pci_id->vendor_id = vendor_id;
>         pci_id->device_id = device_id;
>         pci_id->subsystem_device_id = subvendor_id >> 16;
>         pci_id->subsystem_vendor_id = subvendor_id & 0xffff;
> +       pci_id->class_id = class_id;
>         return 0;
>  }
> 
> @@ -339,6 +354,7 @@ pci_scan_one(HDEVINFO dev_info,
> PSP_DEVINFO_DATA device_info_data)
>         if (ret != 0)
>                 goto end;
> 
> +       dev->device.bus = &rte_pci_bus.bus;
>         dev->addr = addr;
>         dev->id = pci_id;
>         dev->max_vfs = 0; /* TODO: get max_vfs */
> --
> 2.25.1

Acked-by: Tal Shnaiderman <talshn@nvidia.com>

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

* [dpdk-dev] [PATCH v6] bus/pci: nvme on Windows requires class id and bus
  2021-01-25 17:08 [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus Nick Connolly
                   ` (4 preceding siblings ...)
  2021-02-02 13:54 ` [dpdk-dev] [PATCH v5] " Nick Connolly
@ 2021-02-23 18:18 ` Nick Connolly
  2021-02-28 14:38   ` Dmitry Kozlyuk
  2021-03-01  9:56 ` [dpdk-dev] [PATCH v7] " Nick Connolly
  6 siblings, 1 reply; 18+ messages in thread
From: Nick Connolly @ 2021-02-23 18:18 UTC (permalink / raw)
  To: talshn, dmitry.kozliuk, pallavi.kadam, thomas; +Cc: dev, Nick Connolly

Attaching to an NVMe disk on Windows using SPDK requires the
PCI class ID and device.bus fields. Decode the class ID from the PCI
device info strings if it is present and set device.bus.

Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
Acked-by: Tal Shnaiderman <talshn@nvidia.com>
---
v6:
* no changes - resending to resolve spurious iol-testing failure

v5:
* Add missing version history

v4:
* Use #define to determine length of Class ID

v3:
* Put version history at top - v2 mistakenly had it after the diffs

v2:
* If only a 4-digit class ID is available, convert it to 6-digit format

 drivers/bus/pci/windows/pci.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
index f66258452..dceb0f4b2 100644
--- a/drivers/bus/pci/windows/pci.c
+++ b/drivers/bus/pci/windows/pci.c
@@ -23,6 +23,9 @@ DEFINE_DEVPROPKEY(DEVPKEY_Device_Numa_Node, 0x540b947e, 0x8b40, 0x45bc,
  * the registry hive for PCI devices.
  */
 
+/* Class ID consists of hexadecimal digits */
+#define RTE_PCI_DRV_CLASSID_DIGIT	"0123456789abcdefABCDEF"
+
 /* The functions below are not implemented on Windows,
  * but need to be defined for compilation purposes
  */
@@ -280,17 +283,29 @@ parse_pci_hardware_id(const char *buf, struct rte_pci_id *pci_id)
 {
 	int ids = 0;
 	uint16_t vendor_id, device_id;
-	uint32_t subvendor_id = 0;
+	uint32_t subvendor_id = 0, class_id = 0;
+	const char *cp;
 
 	ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16 "&SUBSYS_%"
 		PRIx32, &vendor_id, &device_id, &subvendor_id);
 	if (ids != 3)
 		return -1;
 
+	/* Try and find PCI class ID */
+	for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
+		if (*cp == '&' && sscanf_s(cp,
+				"&CC_%" PRIx32, &class_id) == 1) {
+			/* Convert 4-digit class IDs to 6-digit format */
+			if (strspn(cp + 4, RTE_PCI_DRV_CLASSID_DIGIT) == 4)
+				class_id <<= 8;
+			break;
+		}
+
 	pci_id->vendor_id = vendor_id;
 	pci_id->device_id = device_id;
 	pci_id->subsystem_device_id = subvendor_id >> 16;
 	pci_id->subsystem_vendor_id = subvendor_id & 0xffff;
+	pci_id->class_id = class_id;
 	return 0;
 }
 
@@ -339,6 +354,7 @@ pci_scan_one(HDEVINFO dev_info, PSP_DEVINFO_DATA device_info_data)
 	if (ret != 0)
 		goto end;
 
+	dev->device.bus = &rte_pci_bus.bus;
 	dev->addr = addr;
 	dev->id = pci_id;
 	dev->max_vfs = 0; /* TODO: get max_vfs */
-- 
2.25.1


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

* Re: [dpdk-dev] [PATCH v6] bus/pci: nvme on Windows requires class id and bus
  2021-02-23 18:18 ` [dpdk-dev] [PATCH v6] " Nick Connolly
@ 2021-02-28 14:38   ` Dmitry Kozlyuk
  2021-03-01  9:24     ` Nick Connolly
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Kozlyuk @ 2021-02-28 14:38 UTC (permalink / raw)
  To: Nick Connolly; +Cc: talshn, pallavi.kadam, thomas, dev

2021-02-23 18:18, Nick Connolly:
> Attaching to an NVMe disk on Windows using SPDK requires the
> PCI class ID and device.bus fields. Decode the class ID from the PCI
> device info strings if it is present and set device.bus.
> 
> Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
> Acked-by: Tal Shnaiderman <talshn@nvidia.com>
> ---
> v6:
> * no changes - resending to resolve spurious iol-testing failure
> 
> v5:
> * Add missing version history
> 
> v4:
> * Use #define to determine length of Class ID
> 
> v3:
> * Put version history at top - v2 mistakenly had it after the diffs
> 
> v2:
> * If only a 4-digit class ID is available, convert it to 6-digit format
> 
>  drivers/bus/pci/windows/pci.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
> index f66258452..dceb0f4b2 100644
> --- a/drivers/bus/pci/windows/pci.c
> +++ b/drivers/bus/pci/windows/pci.c
> @@ -23,6 +23,9 @@ DEFINE_DEVPROPKEY(DEVPKEY_Device_Numa_Node, 0x540b947e, 0x8b40, 0x45bc,
>   * the registry hive for PCI devices.
>   */
>  
> +/* Class ID consists of hexadecimal digits */
> +#define RTE_PCI_DRV_CLASSID_DIGIT	"0123456789abcdefABCDEF"
> +
>  /* The functions below are not implemented on Windows,
>   * but need to be defined for compilation purposes
>   */
> @@ -280,17 +283,29 @@ parse_pci_hardware_id(const char *buf, struct rte_pci_id *pci_id)
>  {
>  	int ids = 0;
>  	uint16_t vendor_id, device_id;
> -	uint32_t subvendor_id = 0;
> +	uint32_t subvendor_id = 0, class_id = 0;
> +	const char *cp;
>  
>  	ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16 "&SUBSYS_%"
>  		PRIx32, &vendor_id, &device_id, &subvendor_id);
>  	if (ids != 3)
>  		return -1;
>  
> +	/* Try and find PCI class ID */
> +	for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
> +		if (*cp == '&' && sscanf_s(cp,
> +				"&CC_%" PRIx32, &class_id) == 1) {
> +			/* Convert 4-digit class IDs to 6-digit format */
> +			if (strspn(cp + 4, RTE_PCI_DRV_CLASSID_DIGIT) == 4)
> +				class_id <<= 8;
> +			break;
> +		}
> +

Is "4/6-digit format" used commonly for class ID, subclass ID, and optional
programming interface code? If not, I suggest sticking to official
terminology, something like "Assume zero programming interface code if
unspecified".

In general, a link to format reference would be useful in commit message or
function comment, for readers to understand what's being parsed:

https://docs.microsoft.com/en-us/windows-hardware/drivers/install/identifiers-for-pci-devices

With above nits,
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

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

* Re: [dpdk-dev] [PATCH v6] bus/pci: nvme on Windows requires class id and bus
  2021-02-28 14:38   ` Dmitry Kozlyuk
@ 2021-03-01  9:24     ` Nick Connolly
  0 siblings, 0 replies; 18+ messages in thread
From: Nick Connolly @ 2021-03-01  9:24 UTC (permalink / raw)
  To: Dmitry Kozlyuk; +Cc: talshn, pallavi.kadam, thomas, dev

Thanks Dmitry - will address and send v7.

On 28/02/2021 14:38, Dmitry Kozlyuk wrote:
> 2021-02-23 18:18, Nick Connolly:
>> Attaching to an NVMe disk on Windows using SPDK requires the
>> PCI class ID and device.bus fields. Decode the class ID from the PCI
>> device info strings if it is present and set device.bus.
>>
>> Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
>> Acked-by: Tal Shnaiderman <talshn@nvidia.com>
>> ---
>> v6:
>> * no changes - resending to resolve spurious iol-testing failure
>>
>> v5:
>> * Add missing version history
>>
>> v4:
>> * Use #define to determine length of Class ID
>>
>> v3:
>> * Put version history at top - v2 mistakenly had it after the diffs
>>
>> v2:
>> * If only a 4-digit class ID is available, convert it to 6-digit format
>>
>>   drivers/bus/pci/windows/pci.c | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
>> index f66258452..dceb0f4b2 100644
>> --- a/drivers/bus/pci/windows/pci.c
>> +++ b/drivers/bus/pci/windows/pci.c
>> @@ -23,6 +23,9 @@ DEFINE_DEVPROPKEY(DEVPKEY_Device_Numa_Node, 0x540b947e, 0x8b40, 0x45bc,
>>    * the registry hive for PCI devices.
>>    */
>>   
>> +/* Class ID consists of hexadecimal digits */
>> +#define RTE_PCI_DRV_CLASSID_DIGIT	"0123456789abcdefABCDEF"
>> +
>>   /* The functions below are not implemented on Windows,
>>    * but need to be defined for compilation purposes
>>    */
>> @@ -280,17 +283,29 @@ parse_pci_hardware_id(const char *buf, struct rte_pci_id *pci_id)
>>   {
>>   	int ids = 0;
>>   	uint16_t vendor_id, device_id;
>> -	uint32_t subvendor_id = 0;
>> +	uint32_t subvendor_id = 0, class_id = 0;
>> +	const char *cp;
>>   
>>   	ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16 "&SUBSYS_%"
>>   		PRIx32, &vendor_id, &device_id, &subvendor_id);
>>   	if (ids != 3)
>>   		return -1;
>>   
>> +	/* Try and find PCI class ID */
>> +	for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
>> +		if (*cp == '&' && sscanf_s(cp,
>> +				"&CC_%" PRIx32, &class_id) == 1) {
>> +			/* Convert 4-digit class IDs to 6-digit format */
>> +			if (strspn(cp + 4, RTE_PCI_DRV_CLASSID_DIGIT) == 4)
>> +				class_id <<= 8;
>> +			break;
>> +		}
>> +
> Is "4/6-digit format" used commonly for class ID, subclass ID, and optional
> programming interface code? If not, I suggest sticking to official
> terminology, something like "Assume zero programming interface code if
> unspecified".
>
> In general, a link to format reference would be useful in commit message or
> function comment, for readers to understand what's being parsed:
>
> https://docs.microsoft.com/en-us/windows-hardware/drivers/install/identifiers-for-pci-devices
>
> With above nits,
> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>


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

* [dpdk-dev] [PATCH v7] bus/pci: nvme on Windows requires class id and bus
  2021-01-25 17:08 [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus Nick Connolly
                   ` (5 preceding siblings ...)
  2021-02-23 18:18 ` [dpdk-dev] [PATCH v6] " Nick Connolly
@ 2021-03-01  9:56 ` Nick Connolly
  2021-03-16 13:45   ` Thomas Monjalon
  6 siblings, 1 reply; 18+ messages in thread
From: Nick Connolly @ 2021-03-01  9:56 UTC (permalink / raw)
  To: talshn, dmitry.kozliuk, pallavi.kadam, thomas; +Cc: dev, Nick Connolly

Attaching to an NVMe disk on Windows using SPDK requires the
PCI class ID and device.bus fields. Decode the class ID from the PCI
device info strings if it is present and set device.bus.

Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
Acked-by: Tal Shnaiderman <talshn@nvidia.com>
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
v7:
* Improve comments as per review by Dmitry Kozlyuk

v6:
* no changes - resending to resolve spurious iol-testing failure

v5:
* Add missing version history

v4:
* Use #define to determine length of Class ID

v3:
* Put version history at top - v2 mistakenly had it after the diffs

v2:
* If only a 4-digit class ID is available, convert it to 6-digit format

 drivers/bus/pci/windows/pci.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
index f66258452..bc173af2d 100644
--- a/drivers/bus/pci/windows/pci.c
+++ b/drivers/bus/pci/windows/pci.c
@@ -23,6 +23,9 @@ DEFINE_DEVPROPKEY(DEVPKEY_Device_Numa_Node, 0x540b947e, 0x8b40, 0x45bc,
  * the registry hive for PCI devices.
  */
 
+/* Class ID consists of hexadecimal digits */
+#define RTE_PCI_DRV_CLASSID_DIGIT	"0123456789abcdefABCDEF"
+
 /* The functions below are not implemented on Windows,
  * but need to be defined for compilation purposes
  */
@@ -274,23 +277,41 @@ get_pci_hardware_id(HDEVINFO dev_info, PSP_DEVINFO_DATA device_info_data,
 
 /*
  * parse the SPDRP_HARDWAREID output and assign to rte_pci_id
+ *
+ * A list of the device identification string formats can be found at:
+ * https://docs.microsoft.com/en-us/windows-hardware/drivers/install/identifiers-for-pci-devices
  */
 static int
 parse_pci_hardware_id(const char *buf, struct rte_pci_id *pci_id)
 {
 	int ids = 0;
 	uint16_t vendor_id, device_id;
-	uint32_t subvendor_id = 0;
+	uint32_t subvendor_id = 0, class_id = 0;
+	const char *cp;
 
 	ids = sscanf_s(buf, "PCI\\VEN_%" PRIx16 "&DEV_%" PRIx16 "&SUBSYS_%"
 		PRIx32, &vendor_id, &device_id, &subvendor_id);
 	if (ids != 3)
 		return -1;
 
+	/* Try and find PCI class ID */
+	for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++)
+		if (*cp == '&' && sscanf_s(cp,
+				"&CC_%" PRIx32, &class_id) == 1) {
+			/*
+			 * If the Programming Interface code is not specified,
+			 * assume that it is zero.
+			 */
+			if (strspn(cp + 4, RTE_PCI_DRV_CLASSID_DIGIT) == 4)
+				class_id <<= 8;
+			break;
+		}
+
 	pci_id->vendor_id = vendor_id;
 	pci_id->device_id = device_id;
 	pci_id->subsystem_device_id = subvendor_id >> 16;
 	pci_id->subsystem_vendor_id = subvendor_id & 0xffff;
+	pci_id->class_id = class_id;
 	return 0;
 }
 
@@ -339,6 +360,7 @@ pci_scan_one(HDEVINFO dev_info, PSP_DEVINFO_DATA device_info_data)
 	if (ret != 0)
 		goto end;
 
+	dev->device.bus = &rte_pci_bus.bus;
 	dev->addr = addr;
 	dev->id = pci_id;
 	dev->max_vfs = 0; /* TODO: get max_vfs */
-- 
2.25.1


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

* Re: [dpdk-dev] [PATCH v7] bus/pci: nvme on Windows requires class id and bus
  2021-03-01  9:56 ` [dpdk-dev] [PATCH v7] " Nick Connolly
@ 2021-03-16 13:45   ` Thomas Monjalon
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Monjalon @ 2021-03-16 13:45 UTC (permalink / raw)
  To: Nick Connolly; +Cc: talshn, dmitry.kozliuk, pallavi.kadam, dev

01/03/2021 10:56, Nick Connolly:
> Attaching to an NVMe disk on Windows using SPDK requires the
> PCI class ID and device.bus fields. Decode the class ID from the PCI
> device info strings if it is present and set device.bus.
> 
> Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
> Acked-by: Tal Shnaiderman <talshn@nvidia.com>
> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Applied with the title "bus/pci: set Windows device class and bus",
thanks



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

end of thread, other threads:[~2021-03-16 13:45 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-25 17:08 [dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus Nick Connolly
2021-01-26 17:45 ` Tal Shnaiderman
2021-01-26 18:18   ` Nick Connolly
2021-01-26 22:41     ` Thomas Monjalon
2021-01-27 14:22       ` Nick Connolly
2021-01-27 15:13     ` Tal Shnaiderman
2021-01-28 17:00 ` [dpdk-dev] [PATCH v2] " Nick Connolly
2021-01-28 17:04 ` [dpdk-dev] [PATCH v3] " Nick Connolly
2021-01-31 15:56   ` Tal Shnaiderman
2021-02-02 13:07     ` Nick Connolly
2021-02-02 13:44 ` [dpdk-dev] [PATCH v4] " Nick Connolly
2021-02-02 13:54 ` [dpdk-dev] [PATCH v5] " Nick Connolly
2021-02-02 14:04   ` Tal Shnaiderman
2021-02-23 18:18 ` [dpdk-dev] [PATCH v6] " Nick Connolly
2021-02-28 14:38   ` Dmitry Kozlyuk
2021-03-01  9:24     ` Nick Connolly
2021-03-01  9:56 ` [dpdk-dev] [PATCH v7] " Nick Connolly
2021-03-16 13:45   ` Thomas Monjalon

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