From: Glauber Costa <glommer@redhat.com> Date: Tue, 26 Jan 2010 15:29:51 -0500 Subject: [kvm] pvclock on i386 suffers from double registering Message-id: <1264519791-14206-1-git-send-email-glommer@redhat.com> Patchwork-id: 22904 O-Subject: [PATCH] RHEL5.5 BZ557095 - kvm pvclock on i386 suffers from double registering Bugzilla: 557095 RH-Acked-by: Juan Quintela <quintela@redhat.com> RH-Acked-by: Don Dutile <ddutile@redhat.com> RH-Acked-by: Marcelo Tosatti <mtosatti@redhat.com> By looking at the dmesg on a i386 kvm guest with pvclock enabled, I noticed that we're registering the boot clock after primary and secondary clock are already registered, as part of the cpu identification code. This causes kvm to write data to a random memory area during guest's life. The root cause is that init_hypervisor() is called multiple times during a cpu boot process (at least twice). Since this is needed by vmware, I guess removing some of the calls is risky at best. The solution is to disallow the boot clock to be registered twice (any other clock should be safe). We do that by removing kvm code from init_hypervisor(), and calling kvmclock_init() explicitly only when we need it. Signed-off-by: Glauber Costa <glommer@redhat.com> RH-Upstream-status: N/A RH-Bugzilla: BZ557095 diff --git a/arch/i386/kernel/cpu/hypervisor.c b/arch/i386/kernel/cpu/hypervisor.c index f60d069..6f988ac 100644 --- a/arch/i386/kernel/cpu/hypervisor.c +++ b/arch/i386/kernel/cpu/hypervisor.c @@ -66,8 +66,6 @@ hypervisor_set_feature_bits(struct cpuinfo_x86 *c) vmware_set_feature_bits(c); return; } - if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_KVM) - kvmclock_init(); } void __cpuinit init_hypervisor(struct cpuinfo_x86 *c) diff --git a/arch/i386/kernel/setup.c b/arch/i386/kernel/setup.c index 38ac780..2b3a5f6 100644 --- a/arch/i386/kernel/setup.c +++ b/arch/i386/kernel/setup.c @@ -64,6 +64,7 @@ #include <bios_ebda.h> #include <asm/generic-hypervisor.h> +#include <asm/kvm_para.h> /* Forward Declaration. */ void __init find_max_pfn(void); @@ -1692,6 +1693,14 @@ void __init setup_arch(char **cmdline_p) */ init_hypervisor(&boot_cpu_data); + /* + * init_hypervisor gets called more than one time throughout + * the life of the cpu, but that hurts kvm. We only need it + * once, so we do it explicitly here + */ + if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_KVM) + kvmclock_init(); + #ifdef CONFIG_X86_GENERICARCH generic_apic_probe(*cmdline_p); #endif diff --git a/arch/x86_64/kernel/setup.c b/arch/x86_64/kernel/setup.c index 2333350..dacb23a 100644 --- a/arch/x86_64/kernel/setup.c +++ b/arch/x86_64/kernel/setup.c @@ -68,6 +68,7 @@ #include <asm/dmi.h> #include <asm/generic-hypervisor.h> #include <asm/pci-direct.h> +#include <asm/kvm_para.h> /* * Machine setup.. @@ -581,6 +582,14 @@ void __init setup_arch(char **cmdline_p) */ init_hypervisor(&boot_cpu_data); + /* + * init_hypervisor gets called more than one time throughout + * the life of the cpu, but that hurts kvm. We only need it + * once, so we do it explicitly here + */ + if (boot_cpu_data.x86_hyper_vendor == X86_HYPER_VENDOR_KVM) + kvmclock_init(); + #ifdef CONFIG_ACPI /* * Initialize the ACPI boot-time table parser (gets the RSDP and SDT).