On 1/22/21 6:51 AM, Jan Beulich wrote:
> On 20.01.2021 23:49, Boris Ostrovsky wrote:
>> --- a/xen/arch/x86/msr.c
>> +++ b/xen/arch/x86/msr.c
>> @@ -295,7 +295,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t
>> *val)
>> }
>>
>> /* Fallthrough. */
>> - case 0x40000200 ... 0x400002ff:
>> + case 0x40000200 ... 0x400002fe:
>> ret = guest_rdmsr_xen(v, msr, val);
>> break;
>>
>> @@ -514,7 +514,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t
>> val)
>> }
>>
>> /* Fallthrough. */
>> - case 0x40000200 ... 0x400002ff:
>> + case 0x40000200 ... 0x400002fe:
>> ret = guest_wrmsr_xen(v, msr, val);
>> break;
>
> For both of these, we need some kind of connection to
> MSR_UNHANDLED. Could be a BUILD_BUG_ON() or an explicit
> "case MSR_UNHANDLED:" (preventing someone "correcting" the
> apparent mistake) or yet something else.
I actually meant to add a comment there but forgot.
I'll add an explicit 'case'.
>
>> --- a/xen/include/xen/lib/x86/msr.h
>> +++ b/xen/include/xen/lib/x86/msr.h
>> @@ -2,8 +2,21 @@
>> #ifndef XEN_LIB_X86_MSR_H
>> #define XEN_LIB_X86_MSR_H
>>
>> +/*
>> + * Behavior on accesses to MSRs that are not handled by emulation:
>> + * 0 = return #GP, warning emitted
>> + * 1 = read as 0, writes are dropped, no warning
>> + * 2 = read as 0, writes are dropped, warning emitted
>> + */
>> +#define MSR_UNHANDLED_NEVER 0
>> +#define MSR_UNHANDLED_SILENT 1
>> +#define MSR_UNHANDLED_VERBOSE 2
>> +
>> +/* MSR that is not explicitly processed by emulation */
>> +#define MSR_UNHANDLED 0x400002ff
>
> MSR indexes as well as definitions for MSR contents generally
> live in asm-x86/msr-index.h. I think it would be better for
> the above to also go there.
>
I didn't put it there because I felt that file is for "real" (including
hypervisor range) MSRs. But I can move it to msr-index.h
-boris
