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

[Paolo Bonzini]

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@xxxxxxxxxx>
> 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@xxxxxxxxxxxxxxx
> CC: Ingo Molnar <mingo@xxxxxxxxxx>
> CC: "H. Peter Anvin" <hpa@xxxxxxxxx>
> CC: x86@xxxxxxxxxx
> Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Cc: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxxxx>
> Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> Cc: Arjan van de Ven <arjan@xxxxxxxxxxxxxxx>
> Cc: David Woodhouse <dwmw@xxxxxxxxxxxx>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> 
> ---
> 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@xxxxxxxxxx>

> ---
>  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)
>