Re: [Xen-devel] [PATCH] x86/MCE: Present MSR_IA32_MCx_MISC(2-6) as invalid on AMD

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.03.13 at 16:32, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
> MSR_IA32_MCx_MISC(4) register on AMD processors is used for error
> thresholding. PV guests may try to set it up for threshold
> interrupts which will fail and result in these warnings in the log:
> 
>   [Firmware Bug]: cpu 0, try to use APIC510 (LVT offset 1) for vector
>   0xf9, but the register is already in use for vector 0x0 on this cpu
> 
> Mark this register as invalid to avoid this. While at it, also present
> other MSR_IA32_MCx_MISC() registers as invalid (except for the first
> GUEST_MC_BANK_NUM which are emulated).

Hmm, I'm not convinced. A PV guest shouldn't, by definition, try to
set up APIC LVTs (or else it is only partially PV).

> --- a/xen/arch/x86/cpu/mcheck/mce.h
> +++ b/xen/arch/x86/cpu/mcheck/mce.h
> @@ -166,6 +166,7 @@ static inline int mce_vendor_bank_msr(const struct vcpu 
> *v, uint32_t msr)
>          case MSR_F10_MC4_MISC1:
>          case MSR_F10_MC4_MISC2:
>          case MSR_F10_MC4_MISC3:
> +        case MSR_IA32_MCx_MISC(GUEST_MC_BANK_NUM)...MSR_IA32_MCx_MISC(6):

And if we take this, then I'd like to see an explanation of the magic
6 here, including rationale why going forward there wouldn't be a
need to bump this to 7, 8, etc.

Plus, even if it happens to work, it's not intended for architectural
MSRs to be dealt with in mce_vendor_bank_msr() (as that code is
expected to match the default cases in bank_mce_{rd,wr}msr()).
In other words, the change as is would create a latent bug.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel