Re: [dpdk-dev] [PATCH v6] bus/pci: nvme on Windows requires class id and bus

[Nick Connolly <nick.connolly@mayadata.io>]

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>