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
> 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
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
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.
On 26/01/2021 22:41, Thomas Monjalon wrote:
> That's true. Comparisons should be explicit.
Thanks Thomas,
Nick
> 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
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
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
> 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
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
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
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
> 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>
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
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>
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>
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
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