[dpdk-dev] [PATCH] bus/pci: nvme on Windows requires class id and bus
Tal Shnaiderman
talshn at nvidia.com
Wed Jan 27 16:13:35 CET 2021
> 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
More information about the dev
mailing list