kvm clock - xen compatible by accident
Subsystems (4)
cover kvm clock - xen compatible by accident
I think I've misunderstood what you guys wanted to achieve with "xen compatible", but now I get it. It's something that's kvm specific, but happens to be able to communicate with xen guests, provided they do a kvm-aware initialization.
So, here's the two patches for it, using two msrs and non-xen data structures
Userspace is the same, so I'm only sending these ones
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
A message that is not in the archive (sent off-list or not yet received): 1200501067774-git-send-email-gcosta-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
PATCH 1/2 kvmclock - the host part.
This is the host part of kvm clocksource implementation. As it does not include clockevents, it is a fairly simple implementation. We only have to register a per-vcpu area, and start writting to it periodically.
The area is binary compatible with xen, as we use the same shadow_info structure.
--- arch/x86/kvm/x86.c | 98 +++++++++++++++++++++++++++++++++++++++++++- include/asm-x86/kvm_host.h | 6 +++ include/asm-x86/kvm_para.h | 24 +++++++++++ include/linux/kvm.h | 1 + 4 files changed, 128 insertions(+), 1 deletions(-)
arch/x86/kvm/x86.c +97 -1
@@ -19,6 +19,7 @@#include "irq.h" #include "mmu.h" +#include <linux/clocksource.h> #include <linux/kvm.h> #include <linux/fs.h> #include <linux/vmalloc.h>
@@ -412,7 +413,7 @@ static u32 msrs_to_save[] = {#ifdef CONFIG_X86_64 MSR_CSTAR ,MSR_KERNEL_GS_BASE ,MSR_SYSCALL_MASK ,MSR_LSTAR , #endif - MSR_IA32_TIME_STAMP_COUNTER, + MSR_IA32_TIME_STAMP_COUNTER, MSR_KVM_SYSTEM_TIME, };static unsigned num_msrs_to_save;
@@ -467,6 +468,73 @@ static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)return kvm_set_msr (vcpu, index, *data); } +static void kvm_write_wall_clock (struct kvm_vcpu *v,gpa_t wall_clock) +{ +int version =1 ; +struct wall_clock wc; +unsigned long flags; +struct timespec wc_ts; + +local_irq_save (flags); +kvm_get_msr (v,MSR_IA32_TIME_STAMP_COUNTER , + &v->arch.hv_clock.tsc_timestamp); + wc_ts =current_kernel_time (); +local_irq_restore (flags); + +down_write (¤t->mm->mmap_sem); +kvm_write_guest (v->kvm, wall_clock, &version,sizeof (version)); +up_write (¤t->mm->mmap_sem); + +/* With all the info we got, fill in the values */ + wc.wc_sec = wc_ts.tv_sec; + wc.wc_nsec = wc_ts.tv_nsec; + wc.wc_version = ++version; + +down_write (¤t->mm->mmap_sem); +kvm_write_guest (v->kvm, wall_clock, &wc,sizeof (wc)); +up_write (¤t->mm->mmap_sem); +} +static void kvm_write_guest_time (struct kvm_vcpu *v) +{ +struct timespec ts; +unsigned long flags; +struct kvm_vcpu_arch *vcpu = &v->arch; +void *shared_kaddr; + +if ((!vcpu->time_page)) +return ; + +/* Keep irq disabled to prevent changes to the clock */ +local_irq_save (flags); +kvm_get_msr (v,MSR_IA32_TIME_STAMP_COUNTER , + &vcpu->hv_clock.tsc_timestamp); +ktime_get_ts (&ts); +local_irq_restore (flags); + +/* With all the info we got, fill in the values */ + + vcpu->hv_clock.system_time = ts.tv_nsec + + (NSEC_PER_SEC * (u64 )ts.tv_sec); +/* +* The interface expects us to write an even number signaling that the +* update is finished. Since the guest won't see the intermediate states, +* we just write "2" at the end +*/ + vcpu->hv_clock.version =2 ; + +preempt_disable (); + + shared_kaddr =kmap_atomic (vcpu->time_page,KM_USER0 ); + +memcpy (shared_kaddr + vcpu->time_offset, &vcpu->hv_clock, +sizeof (vcpu->hv_clock)); + +kunmap_atomic (shared_kaddr,KM_USER0 ); +preempt_enable (); + +mark_page_dirty (v->kvm, vcpu->time >>PAGE_SHIFT ); +} +int kvm_set_msr_common (struct kvm_vcpu *vcpu,u32 msr,u64 data) {
@@ -494,6 +562,25 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)case MSR_IA32_MISC_ENABLE : vcpu->arch.ia32_misc_enable_msr = data;break ; +case MSR_KVM_WALL_CLOCK : + vcpu->arch.wall_clock = data; +kvm_write_wall_clock (vcpu, data); +break ; +case MSR_KVM_SYSTEM_TIME : { + vcpu->arch.time = data &PAGE_MASK ; + vcpu->arch.time_offset = data & ~PAGE_MASK ; + + vcpu->arch.hv_clock.tsc_to_system_mul = +clocksource_khz2mult (tsc_khz,22 ); + vcpu->arch.hv_clock.tsc_shift =22 ; + +down_write (¤t->mm->mmap_sem); + vcpu->arch.time_page =gfn_to_page (vcpu->kvm, data >>PAGE_SHIFT ); +up_write (¤t->mm->mmap_sem); +if (is_error_page (vcpu->arch.time_page)) + vcpu->arch.time_page =NULL ; +break ; + }default :pr_unimpl (vcpu,"unhandled wrmsr: 0x%x data %llx\n" , msr, data);return 1 ;
@@ -553,6 +640,13 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata) data = vcpu->arch.shadow_efer;break ;#endif +case MSR_KVM_WALL_CLOCK : + data = vcpu->arch.wall_clock; +break ; +case MSR_KVM_SYSTEM_TIME : + data = vcpu->arch.time; +break ; +default :pr_unimpl (vcpu,"unhandled rdmsr: 0x%x\n" , msr);return 1 ;
@@ -680,6 +774,7 @@ int kvm_dev_ioctl_check_extension(long ext)case KVM_CAP_USER_MEMORY :case KVM_CAP_SET_TSS_ADDR :case KVM_CAP_EXT_CPUID : +case KVM_CAP_CLOCKSOURCE : r =1 ;break ;case KVM_CAP_VAPIC :
@@ -737,6 +832,7 @@ out:void kvm_arch_vcpu_load (struct kvm_vcpu *vcpu,int cpu) { kvm_x86_ops->vcpu_load (vcpu, cpu); +kvm_write_guest_time (vcpu); }void kvm_arch_vcpu_put (struct kvm_vcpu *vcpu)
include/asm-x86/kvm_host.h +6 -0
@@ -261,6 +261,12 @@ struct kvm_vcpu_arch {/* emulate context */ struct x86_emulate_ctxt emulate_ctxt; + +gpa_t wall_clock; +gpa_t time; +struct kvm_vcpu_time_info hv_clock; +unsigned int time_offset; +struct page *time_page; };struct kvm_mem_alias {
include/asm-x86/kvm_para.h +24 -0
@@ -10,10 +10,34 @@* paravirtualization, the appropriate feature bit should be checked. */ #define KVM_CPUID_FEATURES 0x40000001 +#define KVM_FEATURE_CLOCKSOURCE 0 + +#define MSR_KVM_WALL_CLOCK 0x11 +#define MSR_KVM_SYSTEM_TIME 0x12#ifdef __KERNEL__#include <asm/processor.h> +/* xen binary-compatible interface. See xen headers for details */ +struct kvm_vcpu_time_info { +uint32_t version; +uint32_t pad0; +uint64_t tsc_timestamp; +uint64_t system_time; +uint32_t tsc_to_system_mul; +int8_t tsc_shift; +};/* 32 bytes */ + +struct wall_clock { +uint32_t wc_version; +uint32_t wc_sec; +uint32_t wc_nsec; +}; + + +extern void kvmclock_init (void ); + +/* This instruction is vmcall. On non-VT architectures, it will generate a * trap that we will then rewrite to the appropriate instruction. */
include/linux/kvm.h +1 -0
@@ -232,6 +232,7 @@ struct kvm_vapic_addr {#define KVM_CAP_SET_TSS_ADDR 4#define KVM_CAP_EXT_CPUID 5#define KVM_CAP_VAPIC 6 +#define KVM_CAP_CLOCKSOURCE 7/* * ioctls for VM fds
Signature · 7 lines
1.5.0.6 ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
A message that is not in the archive (sent off-list or not yet received): 12005010761561-git-send-email-gcosta-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
PATCH 2/2 kvmclock implementation, the guest part.
This is the guest part of kvm clock implementation It does not do tsc-only timing, as tsc can have deltas between cpus, and it did not seem worthy to me to keep adjusting them.
We do use it, however, for fine-grained adjustment.
Other than that, time comes from the host.
--- arch/x86/Kconfig | 10 +++ arch/x86/kernel/Makefile_32 | 1 + arch/x86/kernel/kvmclock.c | 154 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/kernel/setup_32.c | 5 ++ 4 files changed, 170 insertions(+), 0 deletions(-) create mode 100644 arch/x86/kernel/kvmclock.c
arch/x86/Kconfig +10 -0
@@ -350,6 +350,16 @@ config VMI at the moment), by linking the kernel to a GPL-ed ROM module provided by the hypervisor. +config KVM_CLOCK + bool "KVM paravirtualized clock" + select PARAVIRT + help + Turning on this option will allow you to run a paravirtualized clock + when running over the KVM hypervisor. Instead of relying on a PIT + (or probably other) emulation by the underlying device model, the host + provides the guest with timing infrastructure, as time of day, and + timer expiration. + source "arch/x86/lguest/Kconfig" endif
arch/x86/kernel/Makefile_32 +1 -0
@@ -44,6 +44,7 @@ obj-$(CONFIG_K8_NB) += k8.o obj-$(CONFIG_MGEODE_LX) += geode_32.o mfgpt_32.o obj-$(CONFIG_VMI) += vmi_32.o vmiclock_32.o +obj-$(CONFIG_KVM_CLOCK) += kvmclock.o obj-$(CONFIG_PARAVIRT) += paravirt_32.o obj-y += pcspeaker.o
arch/x86/kernel/kvmclock.c new file +154 -0
154 lines not on this page · raw message
arch/x86/kernel/setup_32.c +5 -0
@@ -44,6 +44,7 @@#include <linux/crash_dump.h> #include <linux/dmi.h> #include <linux/pfn.h> +#include <linux/kvm_para.h> #include <video/edid.h>
@@ -614,6 +615,10 @@ void __init setup_arch(char **cmdline_p) max_low_pfn =setup_memory (); +#ifdef CONFIG_KVM_CLOCK +kvmclock_init (); +#endif +#ifdef CONFIG_VMI /* * Must be after max_low_pfn is determined, and before kernel
Signature · 7 lines
1.5.0.6 ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
A message that is not in the archive (sent off-list or not yet received): 1200501082989-git-send-email-gcosta-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Re: [PATCH 2/2] kvmclock implementation, the guest part.
Quoted 1 line
+struct
shared_infoshared_info__attribute__((__aligned__(PAGE_SIZE)));
leftover from old version?
arch/x86/kernel/kvmclock.c line 63
Quoted 8 lines
arch/x86/kernel/kvmclock.c new file +8
-0+unsigned long kvm_get_wallclock (void ) +{ +u32 wc_sec, wc_nsec; +u64 delta, last_tsc; +struct timespec ts; +int version, nsec, cpu =smp_processor_id (); + +native_write_msr (MSR_KVM_WALL_CLOCK ,__pa (&wall_clock));
Huh? Shouldn't that be done once at boot time?
cheers,
Gerd
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 2/2] kvmclock implementation, the guest part.
Quoted 15 lines · Gerd Hoffmann
Quoted 1 line
+struct
shared_infoshared_info__attribute__((__aligned__(PAGE_SIZE)));leftover from old version?
arch/x86/kernel/kvmclock.c line 63
Quoted 8 lines
arch/x86/kernel/kvmclock.c new file +8
-0+unsigned long kvm_get_wallclock (void ) +{ +u32 wc_sec, wc_nsec; +u64 delta, last_tsc; +struct timespec ts; +int version, nsec, cpu =smp_processor_id (); + +native_write_msr (MSR_KVM_WALL_CLOCK ,__pa (&wall_clock));Huh? Shouldn't that be done once at boot time?
Quoted 4 lines
cheers,
Gerd
It can, but I don't think so. The closer to read we do, the more precision we get on unstable tsc systems.
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
Re: [PATCH 1/2] kvmclock - the host part.
In reply to a message not in the archiveQuoted 55 lines · Glauber de Oliveira Costa
This is the host part of kvm clocksource implementation. As it does not include clockevents, it is a fairly simple implementation. We only have to register a per-vcpu area, and start writting to it periodically.
The area is binary compatible with xen, as we use the same
shadow_infostructure.Signed-off-by Glauber de Oliveira Costa <[email protected]>--- arch/x86/kvm/x86.c | 98 +++++++++++++++++++++++++++++++++++++++++++- include/asm-x86/kvm_host.h | 6 +++ include/asm-x86/kvm_para.h | 24 +++++++++++ include/linux/kvm.h | 1 + 4 files changed, 128 insertions(+), 1 deletions(-)arch/x86/kvm/x86.c +18
-1@@ -19,6 +19,7 @@#include "irq.h" #include "mmu.h" +#include <linux/clocksource.h> #include <linux/kvm.h> #include <linux/fs.h> #include <linux/vmalloc.h> @@ -412,7 +413,7 @@ static u32 msrs_to_save[] = {#ifdef CONFIG_X86_64 MSR_CSTAR ,MSR_KERNEL_GS_BASE ,MSR_SYSCALL_MASK ,MSR_LSTAR , #endif - MSR_IA32_TIME_STAMP_COUNTER, + MSR_IA32_TIME_STAMP_COUNTER, MSR_KVM_SYSTEM_TIME, };static unsigned num_msrs_to_save;@@ -467,6 +468,73 @@ static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)return kvm_set_msr (vcpu, index, *data); } +static void kvm_write_wall_clock (struct kvm_vcpu *v,gpa_t wall_clock) +{ +int version =1 ; +struct wall_clock wc; +unsigned long flags; +struct timespec wc_ts; + +local_irq_save (flags); +kvm_get_msr (v,MSR_IA32_TIME_STAMP_COUNTER , + &v->arch.hv_clock.tsc_timestamp); + wc_ts =current_kernel_time (); +local_irq_restore (flags); + +down_write (¤t->mm->mmap_sem); +kvm_write_guest (v->kvm, wall_clock, &version,sizeof (version)); +up_write (¤t->mm->mmap_sem);
arch/x86/kvm/x86.c static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data) · line 496
Quoted 10 lines
arch/x86/kvm/x86.c +9
-0+ +/* With all the info we got, fill in the values */ + wc.wc_sec = wc_ts.tv_sec; + wc.wc_nsec = wc_ts.tv_nsec; + wc.wc_version = ++version; + +down_write (¤t->mm->mmap_sem); +kvm_write_guest (v->kvm, wall_clock, &wc,sizeof (wc)); +up_write (¤t->mm->mmap_sem);
Can we get a comment explaining why we only write the version field and then immediately increment the version and write the whole struct? It's not at all obvious why the first write is needed to me.
Quoted 28 lines
arch/x86/kvm/x86.c +28
-0+} +static void kvm_write_guest_time (struct kvm_vcpu *v) +{ +struct timespec ts; +unsigned long flags; +struct kvm_vcpu_arch *vcpu = &v->arch; +void *shared_kaddr; + +if ((!vcpu->time_page)) +return ; + +/* Keep irq disabled to prevent changes to the clock */ +local_irq_save (flags); +kvm_get_msr (v,MSR_IA32_TIME_STAMP_COUNTER , + &vcpu->hv_clock.tsc_timestamp); +ktime_get_ts (&ts); +local_irq_restore (flags); + +/* With all the info we got, fill in the values */ + + vcpu->hv_clock.system_time = ts.tv_nsec + + (NSEC_PER_SEC * (u64 )ts.tv_sec); +/* +* The interface expects us to write an even number signaling that the +* update is finished. Since the guest won't see the intermediate states, +* we just write "2" at the end +*/ + vcpu->hv_clock.version =2 ;
arch/x86/kvm/x86.c static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data) · line 538
Quoted 10 lines
arch/x86/kvm/x86.c +9
-0+ +preempt_disable (); + + shared_kaddr =kmap_atomic (vcpu->time_page,KM_USER0 ); + +memcpy (shared_kaddr + vcpu->time_offset, &vcpu->hv_clock, +sizeof (vcpu->hv_clock)); + +kunmap_atomic (shared_kaddr,KM_USER0 );
Instead of doing a kmap/memcpy, I think it would be better to store the GPA of the time page and do a kvm_write_guest(). Otherwise, you're pinning this page in memory.
Regards,
Anthony Liguori
Quoted 126 lines
arch/x86/kvm/x86.c +5
-0+preempt_enable (); + +mark_page_dirty (v->kvm, vcpu->time >>PAGE_SHIFT ); +} +int
kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data){arch/x86/kvm/x86.c +28
-0@@ -494,6 +562,25 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)case MSR_IA32_MISC_ENABLE : vcpu->arch.ia32_misc_enable_msr = data;break ; +case MSR_KVM_WALL_CLOCK : + vcpu->arch.wall_clock = data; +kvm_write_wall_clock (vcpu, data); +break ; +case MSR_KVM_SYSTEM_TIME : { + vcpu->arch.time = data &PAGE_MASK ; + vcpu->arch.time_offset = data & ~PAGE_MASK ; + + vcpu->arch.hv_clock.tsc_to_system_mul = +clocksource_khz2mult (tsc_khz,22 ); + vcpu->arch.hv_clock.tsc_shift =22 ; + +down_write (¤t->mm->mmap_sem); + vcpu->arch.time_page =gfn_to_page (vcpu->kvm, data >>PAGE_SHIFT ); +up_write (¤t->mm->mmap_sem); +if (is_error_page (vcpu->arch.time_page)) + vcpu->arch.time_page =NULL ; +break ; + }default :pr_unimpl (vcpu,"unhandled wrmsr: 0x%x data %llx\n" , msr, data);return 1 ;@@ -553,6 +640,13 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata) data = vcpu->arch.shadow_efer;break ;#endif +case MSR_KVM_WALL_CLOCK : + data = vcpu->arch.wall_clock; +break ; +case MSR_KVM_SYSTEM_TIME : + data = vcpu->arch.time; +break ; +default :pr_unimpl (vcpu,"unhandled rdmsr: 0x%x\n" , msr);return 1 ;@@ -680,6 +774,7 @@ int kvm_dev_ioctl_check_extension(long ext)case KVM_CAP_USER_MEMORY :case KVM_CAP_SET_TSS_ADDR :case KVM_CAP_EXT_CPUID : +case KVM_CAP_CLOCKSOURCE : r =1 ;break ;case KVM_CAP_VAPIC :@@ -737,6 +832,7 @@ out:void kvm_arch_vcpu_load (struct kvm_vcpu *vcpu,int cpu) { kvm_x86_ops->vcpu_load (vcpu, cpu); +kvm_write_guest_time (vcpu); }void kvm_arch_vcpu_put (struct kvm_vcpu *vcpu)include/asm-x86/kvm_host.h +6
-0@@ -261,6 +261,12 @@ struct kvm_vcpu_arch {/* emulate context */ struct x86_emulate_ctxt emulate_ctxt; + +gpa_t wall_clock; +gpa_t time; +struct kvm_vcpu_time_info hv_clock; +unsigned int time_offset; +struct page *time_page; };struct kvm_mem_alias {include/asm-x86/kvm_para.h +24
-0@@ -10,10 +10,34 @@* paravirtualization, the appropriate feature bit should be checked. */ #define KVM_CPUID_FEATURES 0x40000001 +#define KVM_FEATURE_CLOCKSOURCE 0 + +#define MSR_KVM_WALL_CLOCK 0x11 +#define MSR_KVM_SYSTEM_TIME 0x12#ifdef __KERNEL__#include <asm/processor.h> +/* xen binary-compatible interface. See xen headers for details */ +struct kvm_vcpu_time_info { +uint32_t version; +uint32_t pad0; +uint64_t tsc_timestamp; +uint64_t system_time; +uint32_t tsc_to_system_mul; +int8_t tsc_shift; +};/* 32 bytes */ + +struct wall_clock { +uint32_t wc_version; +uint32_t wc_sec; +uint32_t wc_nsec; +}; + + +extern void kvmclock_init (void ); + +/* This instruction is vmcall. On non-VT architectures, it will generate a * trap that we will then rewrite to the appropriate instruction. */ include/linux/kvm.h +0
-0
include/linux/kvm.h struct kvm_vapic_addr { · line 238
Quoted 7 lines
include/linux/kvm.h +1
-0@@ -232,6 +232,7 @@ struct kvm_vapic_addr {#define KVM_CAP_SET_TSS_ADDR 4#define KVM_CAP_EXT_CPUID 5#define KVM_CAP_VAPIC 6 +#define KVM_CAP_CLOCKSOURCE 7/* * ioctls for VM fds
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 1/2] kvmclock - the host part.
Quoted 66 lines · Anthony Liguori
Quoted 65 lines · Glauber de Oliveira Costa
This is the host part of kvm clocksource implementation. As it does not include clockevents, it is a fairly simple implementation. We only have to register a per-vcpu area, and start writting to it periodically.
The area is binary compatible with xen, as we use the same
shadow_infostructure.Signed-off-by Glauber de Oliveira Costa <[email protected]>--- arch/x86/kvm/x86.c | 98 +++++++++++++++++++++++++++++++++++++++++++- include/asm-x86/kvm_host.h | 6 +++ include/asm-x86/kvm_para.h | 24 +++++++++++ include/linux/kvm.h | 1 + 4 files changed, 128 insertions(+), 1 deletions(-)arch/x86/kvm/x86.c +24
-1@@ -19,6 +19,7 @@#include "irq.h" #include "mmu.h" +#include <linux/clocksource.h> #include <linux/kvm.h> #include <linux/fs.h> #include <linux/vmalloc.h> @@ -412,7 +413,7 @@ static u32 msrs_to_save[] = {#ifdef CONFIG_X86_64 MSR_CSTAR ,MSR_KERNEL_GS_BASE ,MSR_SYSCALL_MASK ,MSR_LSTAR , #endif - MSR_IA32_TIME_STAMP_COUNTER, + MSR_IA32_TIME_STAMP_COUNTER, MSR_KVM_SYSTEM_TIME, };static unsigned num_msrs_to_save;@@ -467,6 +468,73 @@ static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data) return kvm_set_msr(vcpu, index, *data); } +static void kvm_write_wall_clock (struct kvm_vcpu *v,gpa_t wall_clock) +{ +int version =1 ; +struct wall_clock wc; +unsigned long flags; +struct timespec wc_ts; + +local_irq_save (flags); +kvm_get_msr (v,MSR_IA32_TIME_STAMP_COUNTER , + &v->arch.hv_clock.tsc_timestamp); + wc_ts =current_kernel_time (); +local_irq_restore (flags); + +down_write (¤t->mm->mmap_sem); +kvm_write_guest (v->kvm, wall_clock, &version,sizeof (version)); +up_write (¤t->mm->mmap_sem); + +/* With all the info we got, fill in the values */ + wc.wc_sec = wc_ts.tv_sec; + wc.wc_nsec = wc_ts.tv_nsec; + wc.wc_version = ++version; +
Quoted 8 lines
arch/x86/kvm/x86.c static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data) · line 538
Quoted 4 lines
arch/x86/kvm/x86.c +3
-0+down_write (¤t->mm->mmap_sem); +kvm_write_guest (v->kvm, wall_clock, &wc,sizeof (wc)); +up_write (¤t->mm->mmap_sem);Can we get a comment explaining why we only write the version field and then immediately increment the version and write the whole struct? It's not at all obvious why the first write is needed to me.
If the comment is the only pending thing, can we add the comment in a later commit?
Quoted 36 lines
Quoted 36 lines
arch/x86/kvm/x86.c +24
-0+} +static void kvm_write_guest_time (struct kvm_vcpu *v) +{ +struct timespec ts; +unsigned long flags; +struct kvm_vcpu_arch *vcpu = &v->arch; +void *shared_kaddr; + +if ((!vcpu->time_page)) +return ; + +/* Keep irq disabled to prevent changes to the clock */ +local_irq_save (flags); +kvm_get_msr (v,MSR_IA32_TIME_STAMP_COUNTER , + &vcpu->hv_clock.tsc_timestamp); +ktime_get_ts (&ts); +local_irq_restore (flags); + +/* With all the info we got, fill in the values */ + + vcpu->hv_clock.system_time = ts.tv_nsec + + (NSEC_PER_SEC * (u64 )ts.tv_sec); +/* +* The interface expects us to write an even number signaling that the
+ * update is finished. Since the guest won't see the intermediate
states,arch/x86/kvm/x86.c +9
-0+* we just write "2" at the end +*/ + vcpu->hv_clock.version =2 ; + +preempt_disable (); + + shared_kaddr =kmap_atomic (vcpu->time_page,KM_USER0 ); + + memcpy(shared_kaddr + vcpu->time_offset, &vcpu->hv_clock,
Quoted 8 lines
arch/x86/kvm/x86.c static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data) · line 532
Quoted 4 lines
arch/x86/kvm/x86.c +3
-0+ sizeof(vcpu->hv_clock)); + +kunmap_atomic (shared_kaddr,KM_USER0 );Instead of doing a kmap/memcpy, I think it would be better to store the GPA of the time page and do a
kvm_write_guest(). Otherwise, you're pinning this page in memory.
this functions end up being called from various contexts. Some with the mmap_sem held, some uncontended. kvm_write_guest needs it held, so it would turn the code into a big spaguetti. Using the kmap was avi's suggestion to get around it, which I personally liked: we only grab the semaphore when the msr is registered.
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
Re: [PATCH 1/2] kvmclock - the host part.
In reply to a message not in the archiveQuoted 5 lines · Glauber de Oliveira Costa
This is the host part of kvm clocksource implementation. As it does not include clockevents, it is a fairly simple implementation. We only have to register a per-vcpu area, and start writting to it periodically.
The area is binary compatible with xen, as we use the same
shadow_infostructure.
comment needs an update too ;)
arch/x86/kvm/x86.c static u32 msrs_to_save[] = { · line 416
Quoted 4 lines
arch/x86/kvm/x86.c +1
-1- MSR_IA32_TIME_STAMP_COUNTER, + MSR_IA32_TIME_STAMP_COUNTER, MSR_KVM_SYSTEM_TIME,
+ MSR_KVM_WALL_CLOCK
Looks good otherwise.
cheers,
Gerd
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
Re: [PATCH 1/2] kvmclock - the host part.
In reply to a message not in the archiveQuoted 27 lines · Glauber de Oliveira Costa
This is the host part of kvm clocksource implementation. As it does not include clockevents, it is a fairly simple implementation. We only have to register a per-vcpu area, and start writting to it periodically.
The area is binary compatible with xen, as we use the same
shadow_infostructure.Signed-off-by Glauber de Oliveira Costa <[email protected]>--- arch/x86/kvm/x86.c | 98 +++++++++++++++++++++++++++++++++++++++++++- include/asm-x86/kvm_host.h | 6 +++ include/asm-x86/kvm_para.h | 24 +++++++++++ include/linux/kvm.h | 1 + 4 files changed, 128 insertions(+), 1 deletions(-)arch/x86/kvm/x86.c +1
-0@@ -19,6 +19,7 @@#include "irq.h" #include "mmu.h" +#include <linux/clocksource.h> #include <linux/kvm.h> #include <linux/fs.h> #include <linux/vmalloc.h>
arch/x86/kvm/x86.c static u32 msrs_to_save[] = { · line 417
Quoted 6 lines
arch/x86/kvm/x86.c +1
-1@@ -412,7 +413,7 @@ static u32 msrs_to_save[] = {#ifdef CONFIG_X86_64 MSR_CSTAR ,MSR_KERNEL_GS_BASE ,MSR_SYSCALL_MASK ,MSR_LSTAR , #endif - MSR_IA32_TIME_STAMP_COUNTER, + MSR_IA32_TIME_STAMP_COUNTER, MSR_KVM_SYSTEM_TIME,
Missing second MSR here.
Quoted 18 lines
arch/x86/kvm/x86.c +16
-0+static void kvm_write_wall_clock (struct kvm_vcpu *v,gpa_t wall_clock) +{ +int version =1 ; +struct wall_clock wc; +unsigned long flags; +struct timespec wc_ts; + +local_irq_save (flags); +kvm_get_msr (v,MSR_IA32_TIME_STAMP_COUNTER , + &v->arch.hv_clock.tsc_timestamp); + wc_ts =current_kernel_time (); +local_irq_restore (flags); + +down_write (¤t->mm->mmap_sem); +kvm_write_guest (v->kvm, wall_clock, &version,sizeof (version)); +up_write (¤t->mm->mmap_sem);
arch/x86/kvm/x86.c static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data) · line 496
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+ +/* With all the info we got, fill in the values */ + wc.wc_sec = wc_ts.tv_sec; + wc.wc_nsec = wc_ts.tv_nsec; + wc.wc_version = ++version; + +down_write (¤t->mm->mmap_sem); +kvm_write_guest (v->kvm, wall_clock, &wc,sizeof (wc)); +up_write (¤t->mm->mmap_sem); +}
missing blank line
Quoted 20 lines
arch/x86/kvm/x86.c +19
-0+static void kvm_write_guest_time (struct kvm_vcpu *v) +{ +struct timespec ts; +unsigned long flags; +struct kvm_vcpu_arch *vcpu = &v->arch; +void *shared_kaddr; + +if ((!vcpu->time_page)) +return ; + +/* Keep irq disabled to prevent changes to the clock */ +local_irq_save (flags); +kvm_get_msr (v,MSR_IA32_TIME_STAMP_COUNTER , + &vcpu->hv_clock.tsc_timestamp); +ktime_get_ts (&ts); +local_irq_restore (flags); + +/* With all the info we got, fill in the values */ +
arch/x86/kvm/x86.c static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data) · line 525
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+ vcpu->hv_clock.system_time = ts.tv_nsec + + (NSEC_PER_SEC * (u64 )ts.tv_sec); +/* +* The interface expects us to write an even number signaling that the +* update is finished. Since the guest won't see the intermediate states, +* we just write "2" at the end +*/ + vcpu->hv_clock.version =2 ; + +preempt_disable ();
vcpu_load() is not preemptible anyway (and cannot be).
Quoted 16 lines
arch/x86/kvm/x86.c +12
-0+ + shared_kaddr =kmap_atomic (vcpu->time_page,KM_USER0 ); + +memcpy (shared_kaddr + vcpu->time_offset, &vcpu->hv_clock, +sizeof (vcpu->hv_clock)); + +kunmap_atomic (shared_kaddr,KM_USER0 ); +preempt_enable (); + +mark_page_dirty (v->kvm, vcpu->time >>PAGE_SHIFT ); +} +int kvm_set_msr_common (struct kvm_vcpu *vcpu,u32 msr,u64 data) {
arch/x86/kvm/x86.c int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data) · line 567
Quoted 6 lines
arch/x86/kvm/x86.c +2
-0@@ -494,6 +562,25 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)case MSR_IA32_MISC_ENABLE : vcpu->arch.ia32_misc_enable_msr = data;break ; +case MSR_KVM_WALL_CLOCK : + vcpu->arch.wall_clock = data;
This better be a global msr (in kvm->, not vcpu->). There is precedent; some msrs in hyperthreading-capable processors are global to a core.
We need some notification to update wall clock as a result of time-of-day step changes, due to ntp or the user setting the date. Hopefully such a notifier exists.
Signature · 7 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
Re: [PATCH 1/2] kvmclock - the host part.
In reply to a message not in the archiveinclude/asm-x86/kvm_para.h line 36
Quoted 8 lines · Glauber de Oliveira Costa
include/asm-x86/kvm_para.h +7
-0+ +struct wall_clock { +uint32_t wc_version; +uint32_t wc_sec; +uint32_t wc_nsec; +}; +
kvm_wall_clock?
Signature · 7 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/