KVM Paravirt clock - take2
Subsystems (4)
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Folks,
Here's an updated version of the kvm paravirt clock infrastructure. Some of the issues you all raised were addressed, some were not. This work basically focus on general issues / clocksource, so don't expect to see much new things here in the clock events area.
For this, I intend to start implementing avi's idea of using a dedicated
irq chip other than the apic, as nobody seem to have voiced any strong
opposition.
As for the HPET vs clockevents, my solution is as follows: I will implement the clockevents, as it is already on its way, and whoever wants, can do an hpet implementation ;-)
Thinking about this, this patch now makes clocksource and clockevents really decoupled from a functionality pov. You can now turn one of them off by a kernel parameter.
There were some bugs in my previous clocksource being masked by a userspace call to settimeofday() in my distribution image initialization, so the code here fixes it.
Also, With all methods I've tried to keep the wallclock of guest and host n'sync, I experienced a little difference in all situations. The one you'll see in this patch is not the most beautiful thing in the world, but worked quite well, even in situations in which the nsec remainder is quite small.
I'll keep up a git tree for it atgit://git.kernel.org/pub/scm/linux/kernel/git/glommer/kvmclock.git, and for this version, there's also a one big patch at
http://glommer.net/kvm-clock-one-big-patch-v2.patch
It's also below, for your appreciation.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Remi - http://enigmail.mozdev.org
iD8DBQFHHn9TjYI8LaFUWXMRAl2DAKDEDCR+SkGe7dUXWEFQB0ACxT26NwCgicos +sxVhnM2KOBYFqnVQzuJ+WM=
=sOTl
-----END PGP SIGNATURE-----
arch/i386/Kconfig +9 -0
@@ -248,6 +248,15 @@ config VMI (it could be used by other hypervisors in theory too, but is not at the moment), by linking the kernel to a GPL-ed ROM module provided by the hypervisor. +config KVM_CLOCK + bool "KVM paravirtualized clock" + depends on PARAVIRT && GENERIC_CLOCKEVENTS + 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. config ACPI_SRAT bool
arch/x86/kernel/Makefile_32 +1 -0
@@ -43,6 +43,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 +274 -0
274 lines not on this page · raw message
arch/x86/kernel/paravirt_32.c +9 -1
@@ -20,6 +20,7 @@#include <linux/efi.h> #include <linux/bcd.h> #include <linux/highmem.h> +#include <linux/kvm_para.h> #include <asm/bug.h> #include <asm/paravirt.h>
@@ -50,6 +51,13 @@ char *memory_setup(void)return pv_init_ops.memory_setup (); } +static void paravirt_init_special_features (void ) +{ +#ifdef CONFIG_KVM_CLOCK +kvmclock_init (); +#endif +} +/* Simple instruction patching code. */ #define DEF_NATIVE (ops, name, code) \ extern const char start_##ops##_##name[], end_##ops##_##name[]; \
@@ -334,7 +342,7 @@ struct pv_info pv_info = {struct pv_init_ops pv_init_ops = { .patch = native_patch, .banner = default_banner, - .arch_setup = paravirt_nop, + .arch_setup = paravirt_init_special_features, .memory_setup = machine_specific_memory_setup, };
arch/x86/kernel/setup_32.c +1 -2
@@ -546,6 +546,7 @@ void __init setup_arch(char **cmdline_p)unsigned long max_low_pfn;memcpy (&boot_cpu_data, &new_cpu_data,sizeof (new_cpu_data)); +parse_early_param ();pre_setup_arch_hook ();early_cpu_init ();
@@ -605,8 +606,6 @@ void __init setup_arch(char **cmdline_p) bss_resource.start =virt_to_phys (&__bss_start); bss_resource.end =virt_to_phys (&__bss_stop)-1 ; -parse_early_param (); -if (user_defined_memmap) {printk (KERN_INFO "user-defined physical RAM map:\n" );print_memory_map ("user" );
drivers/kvm/irq.c +11 -1
@@ -33,6 +33,8 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v) {struct kvm_pic *s; +if (v->timer_vector !=-1 ) +return 1 ;if (kvm_apic_has_interrupt (v) ==-1 ) {/* LAPIC */ if (kvm_apic_accept_pic_intr (v)) { s =pic_irqchip (v->kvm);/* PIC */
@@ -44,6 +46,12 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v) }EXPORT_SYMBOL_GPL (kvm_cpu_has_interrupt); +static int kvm_get_pvclock_interrupt (struct kvm_vcpu *v) +{ +int ret = v->timer_vector; + v->timer_vector =-1 ; +return ret; +}/* * Read pending interrupt vector and intack. */
@@ -52,7 +60,9 @@ int kvm_cpu_get_interrupt(struct kvm_vcpu *v)struct kvm_pic *s;int vector; - vector =kvm_get_apic_interrupt (v);/* APIC */ + vector =kvm_get_pvclock_interrupt (v); +if (vector ==-1 ) + vector =kvm_get_apic_interrupt (v);/* APIC */ if (vector ==-1 ) {if (kvm_apic_accept_pic_intr (v)) { s =pic_irqchip (v->kvm);
drivers/kvm/kvm.h +4 -0
@@ -362,6 +362,10 @@ struct kvm {struct kvm_mem_alias aliases[KVM_ALIAS_SLOTS ];int nmemslots;struct kvm_memory_slot memslots[KVM_MEMORY_SLOTS ]; + +struct page *clock_page; +u64 clock_gpa; +int time_needs_update;/* * Hash table of struct kvm_mmu_page. */
drivers/kvm/kvm_main.c +85 -0
140 lines not on this page · raw message
drivers/kvm/x86.h +3 -0
@@ -83,6 +83,9 @@ struct kvm_vcpu {/* emulate context */ struct x86_emulate_ctxt emulate_ctxt; +struct hrtimer oneshooter; +int timer_vector; + }; static inline int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva,
include/asm-x86/kvm_para.h +23 -0
@@ -10,9 +10,12 @@* paravirtualization, the appropriate feature bit should be checked. */ #define KVM_CPUID_FEATURES 0x40000001 +#define KVM_FEATURE_CLOCKEVENTS 0 +#define KVM_FEATURE_CLOCKSOURCE 1#ifdef __KERNEL__#include <asm/processor.h> +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.
@@ -29,6 +32,26 @@* noted by the particular hypercall. */ +#define KVM_HCALL_REGISTER_CLOCK 0 +#define KVM_HCALL_SET_ALARM 1 +#define KVM_HCALL_STOP_ONESHOT 2 + +struct kvm_hv_clock { +u64 tsc_mult; +u64 now_ns; +/* That's the wall clock, not the water closet */ +u64 wc_sec; +u64 wc_nsec; +u64 last_tsc; +/* +* At first, we could use the tsc value as a marker, but Jeremy +* well noted that it will cause us locking problems in 32-bit sys, +* so we have a special version field +*/ +u32 version; +u32 stable_tsc;/* use raw tsc for clock_read */ +}; +static inline long kvm_hypercall0 (unsigned int nr) {long ret;
include/linux/kvm_para.h +4 -0
@@ -9,6 +9,10 @@* - kvm_para_available */ +#define KVM_CPUID_FEATURES 0x40000001 +#define KVM_FEATURE_CLOCKEVENTS 0 +#define KVM_FEATURE_CLOCKSOURCE 1 +/* Return values for hypercalls */ #define KVM_ENOSYS 1000
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 11 lines · Glauber de Oliveira Costa
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1Hi Folks,
Here's an updated version of the kvm paravirt clock infrastructure. Some of the issues you all raised were addressed, some were not. This work basically focus on general issues / clocksource, so don't expect to see much new things here in the clock events area.
Me like.
Please separate the guest and host parts; this allows backporting the guest part to older kernels.
arch/x86/kernel/kvmclock.c line 181
Quoted 10 lines
arch/x86/kernel/kvmclock.c new file +9
-0+ +/* This is our clockevents structure. We only support one shot operation */ +static struct clock_event_device kvm_clockevent = { + .name ="kvm-timer" , + .features =CLOCK_EVT_FEAT_ONESHOT , + .shift =0 , + .mult =1 , + .max_delta_ns =0xffffffff , + .min_delta_ns =1000000 ,
Why this particular number? Doesn't it depend on the host's clocksource?
Quoted 5 lines
drivers/kvm/kvm_main.c +1
-0@@ -1564,6 +1566,36 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu) }EXPORT_SYMBOL_GPL (kvm_emulate_halt); +static struct kvm_hv_clock hv_clock;
drivers/kvm/kvm_main.c int kvm_emulate_halt(struct kvm_vcpu *vcpu) · line 1579
Quoted 10 lines
drivers/kvm/kvm_main.c +9
-0+ +static void kvm_write_guest_time (struct kvm_vcpu *vcpu) +{ +struct kvm *kvm = vcpu->kvm; +struct timespec ts; +void *clock_addr; + +if ((!kvm->clock_page) || !(kvm->clock_gpa)) +return ;
Why two checks?
drivers/kvm/kvm_main.c int kvm_emulate_halt(struct kvm_vcpu *vcpu)
Quoted 3 lines
drivers/kvm/kvm_main.c +2
-0+ + clock_addr =kmap (kvm->clock_page);
kmap_atomic() is much faster and more akpm-resistant.
drivers/kvm/kvm_main.c int kvm_emulate_halt(struct kvm_vcpu *vcpu)
Quoted 11 lines
drivers/kvm/kvm_main.c +10
-0+ +/* Updates version to the next odd number, indicating we're writing */ + hv_clock.version++; +/* Updating the tsc count is the first thing we do */ +kvm_get_msr (vcpu,MSR_IA32_TIME_STAMP_COUNTER , &hv_clock.last_tsc); +ktime_get_ts (&ts); + hv_clock.now_ns = ts.tv_nsec +NSEC_PER_SEC * ts.tv_sec; + hv_clock.wc_sec =get_seconds (); + hv_clock.version++; +WARN_ON (hv_clock.version &1 );
smp_wmb()s are missing here.
The guest can trigger the WARN_ON() by being naughty with .version.
Quoted 12 lines
drivers/kvm/kvm_main.c +8
-0+ +memcpy (clock_addr, &hv_clock,sizeof (hv_clock)); +mark_page_dirty (kvm, kvm->clock_gpa >>PAGE_SHIFT ); +kunmap (kvm->clock_page); + + kvm->time_needs_update =0 ; +} +int
kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
{drivers/kvm/kvm_main.c +0
-0unsigned long nr, a0, a1, a2, a3, ret;
drivers/kvm/kvm_main.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) · line 1624
Quoted 9 lines
drivers/kvm/kvm_main.c +4
-0@@ -1584,7 +1616,37 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) a3 &=0xFFFFFFFF ; } + ret =0 ;switch (nr) { +case KVM_HCALL_REGISTER_CLOCK : { +if (!irqchip_in_kernel (vcpu->kvm)) { + ret =-1 ;
Return a proper KVM error.
drivers/kvm/kvm_main.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
Quoted 5 lines
drivers/kvm/kvm_main.c +4
-0+break ; + } + + vcpu->kvm->clock_gpa = a0;
i386 gpas are 64-bit. Better define it as a gfn (that absolves you of checking alignment too).
drivers/kvm/kvm_main.c int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
Quoted 3 lines
drivers/kvm/kvm_main.c +2
-0+ vcpu->kvm->clock_page =gfn_to_page (vcpu->kvm, a0 >>PAGE_SHIFT ); +
You're touching shared state here. Can probably cause problems if the guest turns nasty.
Quoted 19 lines
drivers/kvm/x86.h +0
-0static inline int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva,include/asm-x86/kvm_para.h +3
-0@@ -10,9 +10,12 @@* paravirtualization, the appropriate feature bit should be checked. */ #define KVM_CPUID_FEATURES 0x40000001 +#define KVM_FEATURE_CLOCKEVENTS 0 +#define KVM_FEATURE_CLOCKSOURCE 1#ifdef __KERNEL__#include <asm/processor.h> +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/asm-x86/kvm_para.h line 36
Quoted 5 lines
include/asm-x86/kvm_para.h +1
-0@@ -29,6 +32,26 @@* noted by the particular hypercall. */ +#define KVM_HCALL_REGISTER_CLOCK 0
Let's start hypercall numbers at 1 to trap some cases on uninitialized data coming that way.
Quoted 6 lines
include/asm-x86/kvm_para.h +0
-0long ret;include/linux/kvm_para.h +0
-0
include/linux/kvm_para.h line 18
Quoted 10 lines
include/linux/kvm_para.h +4
-0@@ -9,6 +9,10 @@* - kvm_para_available */ +#define KVM_CPUID_FEATURES 0x40000001 +#define KVM_FEATURE_CLOCKEVENTS 0 +#define KVM_FEATURE_CLOCKSOURCE 1 +/* Return values for hypercalls */ #define KVM_ENOSYS 1000
Need to expose the capability to userspace as well via KVM_CHECK_EXTENSION so userspace will know to expose it to the guest.
Finally, how about making this per-vcpu? That solves problems with tscs running differently on different cpus, you don't need the wmb()s on the host side, is faster for the guest due to less cacheline bouncing, I could go on forever.
Signature · 8 lines
error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/