Re: [Xen-devel] [PATCH] x86/msr: Start cleaning up msr-index.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.02.2020 16:59, Andrew Cooper wrote:
> On 21/02/2020 15:41, Jan Beulich wrote:
>> On 21.02.2020 16:19, Andrew Cooper wrote:
>>> --- a/xen/include/asm-x86/msr-index.h
>>> +++ b/xen/include/asm-x86/msr-index.h
>>> @@ -1,7 +1,74 @@
>>>  #ifndef __ASM_MSR_INDEX_H
>>>  #define __ASM_MSR_INDEX_H
>>>  
>>> -/* CPU model specific register (MSR) numbers */
>>> +/*
>>> + * CPU model specific register (MSR) numbers
>>> + *
>>> + * Definitions for an MSR should follow this style:
>>> + *
>>> + * #define MSR_$NAME                        0x$INDEX
>>> + * #define  $NAME_$BIT1                     (_AC(1, ULL) << $POS1)
>>> + * #define  $NAME_$BIT2                     (_AC(1, ULL) << $POS2)
>>> + *
>>> + * Blocks of related constants should be sorted by MSR index.  The constant
>>> + * names should be as concise as possible, and the bit names may have an
>>> + * abbreviated name.
>>> + */
>> Hmm, while "Blocks of related constants" caters for e.g. AMD's
>> MSR_AMD64_DR<n>_ADDRESS_MASK, I think for ease of lookup we
>> may want to be slightly more strict, without requiring strong
>> ordering. E.g. by also stating that multiple such blocks should
>> be ordered relative to one another also numerically (much like
>> we try to do in the insn emulator's huge switch() statement),
>> based on their lowest numbered MSR.
> 
> "Exceptions will be considered on a case-by-case basis" ?

Sounds good.

> It is not as if we'll ever be able to write down rules which will apply
> uniformly to the whole file.
> 
>> (As a nit, the example kind of implies that only single bit
>> fields would ever occur. It might avoid questions if you gave
>> a multi-bit example.)
> 
> Single bit fields are the overwhelmingly common example.  Would
> s/BIT/FIELD/ read any better, perhaps with $X and $Y instead of 1's in
> the _AC() ?

Yes.

Jan

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