On 16/06/2016 08:06, Haozhong Zhang wrote:
> + if (!lmce_supported()) {
> + error_setg(&local_err, "KVM unavailable or LMCE not supported");
> + error_propagate(&error_abort, local_err);
> + }
Aborts should never be triggered by user input. The error instead
should propagate from mce_init to its caller with a new errp argument
(i.e. error_setg(errp, "KVM unavailable or LMCE not supported")).
x86_cpu_realizefn can pass &local_err and check the outcome through
local_err != NULL. See the existing call to x86_cpu_apic_create, right
above the call to mce_init.
> @@ -878,7 +891,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
> c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
> if (c) {
> has_msr_feature_control = !!(c->ecx & CPUID_EXT_VMX) ||
> - !!(c->ecx & CPUID_EXT_SMX);
> + !!(c->ecx & CPUID_EXT_SMX) ||
> + !!(env->mcg_cap & MCG_LMCE_P);
This part is wrong; env->mcg_cap is independent from CPUID[1].ECX.
> + }
> +
> + if (has_msr_feature_control && (env->mcg_cap & MCG_LMCE_P)) {
Don't test has_msr_feature_control here, instead set it to true inside
the "if".
> + has_msr_mcg_ext_ctl = true;
> }
>
> c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0);
Which silicon has LMCE? We may want to enable the property for some CPU
models. Apart from this, the patch is pretty much okay.
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
