[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 01/03/18 09:58, Jan Beulich wrote:
>
>>>> @@ -173,11 +175,26 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr,  
>>>> uint64_t *val)
>>>>                 _MSR_MISC_FEATURES_CPUID_FAULTING;
>>>>          break;
>>>>  
>>>> +    case 0x40000000 ... 0x400001ff:
>>> As was already suggested, these want to gain #define-s.
>>>
>>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> with at least the latter taken care of.
>> Just like on the CPUID side, the range of valid MSRs depend on the
>> fallthrough pattern, and which hypervisor(s) we are emulating for.
>>
>> This is clearer by the end of the subsequent patch, but the logic is far
>> easier to follow without these numbers being hidden.
> I disagree (it's simply impossible to make the connection between
> the read side and the right side this way, because the numbers
> could also just happen to be the same, nor is it possible to
> reasonably find all uses of those numbers via e.g. grep), but well,
> I don't want to block the patch over this.

I'm not looking to try and railroad this through.

If you disagree, then what naming would you suggest instead?  I'd be
happy to use any naming which doesn't impede the clarity of the logic,
but I have spent a long time trying to find something suitable, and
failed to do so.  Raw numbers really are clearer to follow than any
naming scheme I tried.  The root of the problem is with the fact that
the MSR spaces overlap, but this information disappears when you try to
put named constants in.  Also notice that the number of CPUID leaves and
the number of MSR leaves have a different stride, which adds more
complexity.

~Andrew

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.