Re: [PATCH] target-i386: Do not set MCG_SER_P by default

Andreas Färber
2015-11-20T23:11:38Z

Hi,

CC'ing qemu-devel.

Am 21.11.2015 um 00:01 schrieb Borislav Petkov:
> From: Borislav Petkov <bp@suse.de>
>
> Software Error Recovery, i.e. SER, is purely an Intel feature and it
> shouldn't be set by default. Enable it only on Intel.

Is this new in 2.5? Otherwise we would probably need compatibility code
in pc*.[ch] for incoming live migration from older versions.

>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
> target-i386/cpu.c | 7 -------
> target-i386/cpu.h | 9 ++++++++-
> target-i386/kvm.c | 5 +++++
> 3 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 11e5e39a756a..8155ee94fbe1 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2803,13 +2803,6 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
> }
> #endif
>
> -
> -#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
> - (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \
> - (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3)
> -#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \
> - (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \
> - (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3)
> static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> CPUState *cs = CPU(dev);
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index fc4a605d6a29..2605c564239a 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -283,7 +283,7 @@
> #define MCG_CTL_P (1ULL<<8) /* MCG_CAP register available */
> #define MCG_SER_P (1ULL<<24) /* MCA recovery/new status bits */
>
> -#define MCE_CAP_DEF (MCG_CTL_P|MCG_SER_P)
> +#define MCE_CAP_DEF MCG_CTL_P
> #define MCE_BANKS_DEF 10
>
> #define MCG_STATUS_RIPV (1ULL<<0) /* restart ip valid */
> @@ -610,6 +610,13 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_MWAIT_IBE (1U << 1) /* Interrupts can exit capability */
> #define CPUID_MWAIT_EMX (1U << 0) /* enumeration supported */
>
> +#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
> + (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \
> + (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3)
> +#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \
> + (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \
> + (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3)
> +
> #ifndef HYPERV_SPINLOCK_NEVER_RETRY
> #define HYPERV_SPINLOCK_NEVER_RETRY 0xFFFFFFFF
> #endif
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 2a9953b2d4b5..082d38d4838d 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -787,8 +787,13 @@ int kvm_arch_init_vcpu(CPUState *cs)
> if (banks > MCE_BANKS_DEF) {
> banks = MCE_BANKS_DEF;
> }
> +
> mcg_cap &= MCE_CAP_DEF;
> mcg_cap |= banks;
> +
> + if (IS_INTEL_CPU(env))
> + mcg_cap |= MCG_SER_P;

Tabs and missing braces.

> +
> ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, &mcg_cap);
> if (ret < 0) {
> fprintf(stderr, "KVM_X86_SETUP_MCE: %s", strerror(-ret));

Regards,
Andreas

--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)