On 28.04.2022 11:13, Roger Pau Monne wrote:
> --- a/xen/arch/x86/cpu/mcheck/mce.h
> +++ b/xen/arch/x86/cpu/mcheck/mce.h
> @@ -169,6 +169,11 @@ static inline int mce_vendor_bank_msr(const struct vcpu
> *v, uint32_t msr)
> if (msr >= MSR_IA32_MC0_CTL2 &&
> msr < MSR_IA32_MCx_CTL2(v->arch.vmce.mcg_cap & MCG_CAP_COUNT) )
> return 1;
> +
> + case X86_VENDOR_CENTAUR:
> + case X86_VENDOR_SHANGHAI:
> + if (msr == MSR_P5_MC_ADDR || msr == MSR_P5_MC_TYPE)
> + return 1;
> break;
You want to have some fall-through annotation there, perhaps preferably
the pseudo-keyword one.
> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c
> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
> @@ -1008,8 +1008,24 @@ int vmce_intel_wrmsr(struct vcpu *v, uint32_t msr,
> uint64_t val)
>
> int vmce_intel_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
> {
> + const struct cpuid_policy *cp = v->domain->arch.cpuid;
> unsigned int bank = msr - MSR_IA32_MC0_CTL2;
>
> + switch ( msr )
> + {
> + case MSR_P5_MC_ADDR:
> + /* Bank 0 is used for the 'bank 0 quirk' on older processors. */
> + *val = v->arch.vmce.bank[1].mci_addr;
> + return 1;
> +
> + case MSR_P5_MC_TYPE:
> + *val = v->arch.vmce.bank[1].mci_status;
> + return 1;
> + }
Could I ask you to add a reference to vcpu_fill_mc_msrs() in the comment?
> + if ( cp->x86_vendor & (X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
> + return 0;
I think this better would be !(cp->x86_vendor & X86_VENDOR_INTEL).
Jan
