Re: [Xen-devel] [PATCH v2 2/5] x86: Handle the Xen MSRs via the new guest_{rd, wr}msr() infrastructure

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.09.18 at 18:01, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 07/09/18 16:47, Jan Beulich wrote:
>>>>>>> On 07.03.18 at 19:58, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> --- a/xen/arch/x86/msr.c
>>>>> +++ b/xen/arch/x86/msr.c
>>>>> @@ -185,6 +185,10 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, 
>>>>> uint64_t *val)
>>>>>          }
>>>>>  
>>>>>          /* Fallthrough. */
>>>>> +    case MSR_XEN_ALT_START ... MSR_XEN_ALT_START + NR_XEN_MSRS - 1:
>>>> To account for what I've said on patch 1, perhaps this better would
>>>> be
>>>>
>>>>     case MSR_HYPERVISOR_START + NR_VIRIDIAN_MSRS - 1 ... MSR_XEN_ALT_START 
>>>> + NR_XEN_MSRS - 1:
>>>>
>>>> to produce consistent results regardless of the value of
>>>> NR_VIRIDIAN_MSRS (which I suppose is somewhere specified)?
>>> This demonstrates perfectly why using names here complicates things.  No
>>> expression like this is going to be obvious to read.
>>>
>>> The raw numbers are the normal way developers think about these ranges,
>>> and its trivial to spot that the ranges are adjacent.
>> That's a personal thing - to me it's sometimes this way, sometimes
>> the other.
>>
>>> Please compare this suggestion to the guest_cpuid().  The CPUID code is
>>> far far clearer to read, and the more I think about this, the more I'm
>>> considering switching back to raw numbers.
>> That depends very much on how easily the reader can associate back
>> the raw numbers. This is more likely to be the case for the rather
>> common CPUID leaves or groups of leaves, and perhaps less likely for
>> some of the less well known MSRs.
> 
> Right, but we are only talking about the course categorisation of the
> Viridian block of MSRs, and the Xen block of MSRs.  All of them are of
> the form 0x40000xxx.
> 
> The end result of this bit of code will look almost identical to the
> CPUID side, with the dispatch function names making the context clear.

Well, okay, let's see how it ends up looking.

Jan



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