Re: [PATCH 1/4] libx86: Introduce x86_cpuid_lookup_vendor()

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 26/03/2019 11:52, Jan Beulich wrote:
>>>> On 21.03.19 at 13:21, <andrew.cooper3@citrix.com> wrote:
>> Also introduce constants for the vendor strings in CPUID leaf 0.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> albeit I'd appreciate if this was committed together with an actual
> user (other than the testsuite one) of the new function, and
> despite ...
>
>> --- a/xen/include/xen/lib/x86/cpuid.h
>> +++ b/xen/include/xen/lib/x86/cpuid.h
>> @@ -65,6 +65,12 @@ static inline void cpuid_count_leaf(
>>  #undef BX_CON
>>  #undef XCHG
>>  
>> +/**
>> + * Given the vendor id from CPUID leaf 0, look up Xen's internal integer
>> + * vendor ID.  Returns X86_VENDOR_UNKNOWN for any unknown vendor.
>> + */
>> +unsigned int x86_cpuid_lookup_vendor(uint32_t ebx, uint32_t ecx, uint32_t edx);
> ... the undesirable (imo; I think I know you think otherwise) use of
> fixed width types here.

Please, for the benefit of everyone, stop making snide remarks like
this.  It comes across as rude, and is off-putting to contributors.

You are complaining that I didn't write code in way you would have
done.  Just because you dislike-but-don't-object-to how the code look
doesn't make the code wrong, or worthy of comment.


Your judgement of when to use which types is, in my opinion, very
inconsistent.  By my judgement, I am conforming to your expectation of
using fixed width types when the ABI calls for it, which is the case
here - the ABI is that of the CPUID instruction.

If you feel strongly, then please draft a coherent and simple set of
rules for CODING_STYLE.

~Andrew

(Who is clearly very irritated this morning, but this does need saying.)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel