Re: [PATCH] x86, kvm: Clear paravirt_enabled on KVM guests for espfix32's benefit

[Paolo Bonzini <pbonzini@redhat.com>]

On 06/12/2014 04:03, Andy Lutomirski wrote:
> paravirt_enabled has the following effects:
> 
>  - Disables the F00F bug workaround warning.  There is no F00F bug
>    workaround any more because Linux's standard IDT handling already
>    works around the F00F bug, but the warning still exists.  This
>    is only cosmetic, and, in any event, there is no such thing as
>    KVM on a CPU with the F00F bug.
> 
>  - Disables 32-bit APM BIOS detection.  On a KVM paravirt system,
>    there should be no APM BIOS anyway.
> 
>  - Disables tboot.  I think that the tboot code should check the
>    CPUID hypervisor bit directly if it matters.
> 
>  - paravirt_enabled disables espfix32.  espfix32 should *not* be
>    disabled under KVM paravirt.
> 
> The last point is the purpose of this patch.  It fixes a leak of the
> high 16 bits of the kernel stack address on 32-bit KVM paravirt
> guests.
> 
> While I'm at it, this removes pv_info setup from kvmclock.  That
> code seems to serve no purpose.

kvmclock_init runs before kvm_guest_init, and this is a stable@ patch so
for the sake of extra safety I've left the pv_info.name assignment in.
Applied (locally for now), will be in 3.19.

Paolo

> Cc: stable@vger.kernel.org
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> ---
>  arch/x86/kernel/kvm.c      | 9 ++++++++-
>  arch/x86/kernel/kvmclock.c | 2 --
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index f6945bef2cd1..94f643484300 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -283,7 +283,14 @@ NOKPROBE_SYMBOL(do_async_page_fault);
>  static void __init paravirt_ops_setup(void)
>  {
>  	pv_info.name = "KVM";
> -	pv_info.paravirt_enabled = 1;
> +
> +	/*
> +	 * KVM isn't paravirt in the sense of paravirt_enabled.  A KVM
> +	 * guest kernel works like a bare metal kernel with additional
> +	 * features, and paravirt_enabled is about features that are
> +	 * missing.
> +	 */
> +	pv_info.paravirt_enabled = 0;
>  
>  	if (kvm_para_has_feature(KVM_FEATURE_NOP_IO_DELAY))
>  		pv_cpu_ops.io_delay = kvm_io_delay;
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index d9156ceecdff..d4d9a8ad7893 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -263,8 +263,6 @@ void __init kvmclock_init(void)
>  #endif
>  	kvm_get_preset_lpj();
>  	clocksource_register_hz(&kvm_clock, NSEC_PER_SEC);
> -	pv_info.paravirt_enabled = 1;
> -	pv_info.name = "KVM";
>  
>  	if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT))
>  		pvclock_set_flags(PVCLOCK_TSC_STABLE_BIT);
>