Re: [PATCH] KVM: X86: Allow userspace to define the microcode version

Paolo Bonzini
2018-02-27T08:33:21Z

On 26/02/2018 22:30, Konrad Rzeszutek Wilk wrote:
> On Mon, Feb 26, 2018 at 03:51:30PM -0500, Konrad Rzeszutek Wilk wrote:
>> On Mon, Feb 26, 2018 at 08:37:11PM +0100, Borislav Petkov wrote:
>>> On Mon, Feb 26, 2018 at 09:39:12AM -0500, Konrad Rzeszutek Wilk wrote:
>>>> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
>>>> index d19e903214b4..87d044ce837f 100644
>>>> --- a/arch/x86/kernel/cpu/intel.c
>>>> +++ b/arch/x86/kernel/cpu/intel.c
>>>> @@ -144,6 +144,13 @@ static bool bad_spectre_microcode(struct cpuinfo_x86 *c)
>>>> {
>>>> int i;
>>>>
>>>> + /*
>>>> + * We know that the hypervisor lie to us on the microcode version so
>>>> + * we may as well trust that it is running the correct version.
>>>> + */
>>>> + if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
>>>
>>> I guess
>>>
>>> cpu_has(c, X86_FEATURE_HYPERVISOR)
>>>
>>> since we're passing a ptr to the current CPU.
>>
>> Ah yes. Let me fix it up and repost.
>
> I've posted it (but I can't seem to find it on LKML). Here it is in this
> thread. Also adding ingo + tglrx
>
> From 6abac2ccf105d57d60c094950e32139e435cbefe Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Mon, 26 Feb 2018 09:35:01 -0500
> Subject: [PATCH v2] x86/spectre_v2: Don't check bad microcode versions when
> running under hypervisors.
>
> As:
> 1) We know they lie about the env anyhow (host mismatch)
> 2) Even if the hypervisor (Xen, KVM, VMWare, etc) provided
> a valid "correct" value, it all gets to be very murky
> when migration happens (do you provide the "new"
> microcode of the machine?).
>
> And in reality the cloud vendors are the ones that should make
> sure that the microcode that is running is correct and we should
> just sing lalalala and trust them.
>
> CC: stable@vger.kernel.org
> CC: Ingo Molnar <mingo@redhat.com>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: x86@kernel.org
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Arjan van de Ven <arjan@linux.intel.com>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> ---
> v2: Change comments to be more in line with the state of the world.
> v3: Use cpu_has instead of boot_cpu_has per Borislav's review.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> ---
> arch/x86/kernel/cpu/intel.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index d19e903214b4..4aa9fd379390 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -144,6 +144,13 @@ static bool bad_spectre_microcode(struct cpuinfo_x86 *c)
> {
> int i;
>
> + /*
> + * We know that the hypervisor lie to us on the microcode version so
> + * we may as well hope that it is running the correct version.
> + */
> + if (cpu_has(c, X86_FEATURE_HYPERVISOR))
> + return false;
> +
> for (i = 0; i < ARRAY_SIZE(spectre_bad_microcodes); i++) {
> if (c->x86_model == spectre_bad_microcodes[i].model &&
> c->x86_stepping == spectre_bad_microcodes[i].stepping)
>