| Message ID | 20200922151934.899555-1-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] target/i386: always create kvmclock device | expand |
On 22/09/20 17:19, Vitaly Kuznetsov wrote: > QEMU's kvmclock device is only created when KVM PV feature bits for > kvmclock (KVM_FEATURE_CLOCKSOURCE/KVM_FEATURE_CLOCKSOURCE2) are > exposed to the guest. With 'kvm=off' cpu flag the device is not > created and we don't call KVM_GET_CLOCK/KVM_SET_CLOCK upon migration. > It was reported that without these call at least Hyper-V TSC page > clocksouce (which can be enabled independently) gets broken after > migration. > > Switch to creating kvmclock QEMU device unconditionally, it seems > to always make sense to call KVM_GET_CLOCK/KVM_SET_CLOCK on migration. > Use KVM_CAP_ADJUST_CLOCK check instead of CPUID feature bits. > > Reported-by: Antoine Damhet <antoine.damhet@blade-group.com> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > hw/i386/kvm/clock.c | 7 +++++-- > hw/i386/microvm.c | 2 +- > hw/i386/pc.c | 1 + > hw/i386/pc_piix.c | 7 +++++-- > hw/i386/pc_q35.c | 5 ++++- > include/hw/i386/pc.h | 3 +++ > include/hw/kvm/clock.h | 4 ++-- > target/i386/kvm.c | 5 +++++ > target/i386/kvm_i386.h | 1 + > 9 files changed, 27 insertions(+), 8 deletions(-) > > diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c > index 64283358f91d..30cf53393ee1 100644 > --- a/hw/i386/kvm/clock.c > +++ b/hw/i386/kvm/clock.c > @@ -328,11 +328,14 @@ static const TypeInfo kvmclock_info = { > }; > > /* Note: Must be called after VCPU initialization. */ > -void kvmclock_create(void) > +void kvmclock_create(bool create_always) > { > X86CPU *cpu = X86_CPU(first_cpu); > > - if (kvm_enabled() && > + if (!kvm_enabled() || !kvm_has_adjust_clock()) > + return; > + > + if (create_always || > cpu->env.features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) | > (1ULL << KVM_FEATURE_CLOCKSOURCE2))) { > sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL); > diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c > index 81d0888930d1..fd0b84109154 100644 > --- a/hw/i386/microvm.c > +++ b/hw/i386/microvm.c > @@ -119,7 +119,7 @@ static void microvm_devices_init(MicrovmMachineState *mms) > > ioapic_init_gsi(gsi_state, "machine"); > > - kvmclock_create(); > + kvmclock_create(true); > > for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) { > sysbus_create_simple("virtio-mmio", > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index d11daacc23cf..0e036ef9c15f 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1956,6 +1956,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > pcmc->acpi_data_size = 0x20000 + 0x8000; > pcmc->linuxboot_dma_enabled = true; > pcmc->pvh_enabled = true; > + pcmc->kvmclock_create_always = true; > assert(!mc->get_hotplug_handler); > mc->get_hotplug_handler = pc_get_hotplug_handler; > mc->hotplug_allowed = pc_hotplug_allowed; > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 32b1453e6a82..1a68338c737b 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -158,8 +158,8 @@ static void pc_init1(MachineState *machine, > > x86_cpus_init(x86ms, pcmc->default_cpu_version); > > - if (kvm_enabled() && pcmc->kvmclock_enabled) { > - kvmclock_create(); > + if (pcmc->kvmclock_enabled) { > + kvmclock_create(pcmc->kvmclock_create_always); > } > > if (pcmc->pci_enabled) { > @@ -440,11 +440,14 @@ DEFINE_I440FX_MACHINE(v5_2, "pc-i440fx-5.2", NULL, > > static void pc_i440fx_5_1_machine_options(MachineClass *m) > { > + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); > + > pc_i440fx_5_2_machine_options(m); > m->alias = NULL; > m->is_default = false; > compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len); > compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len); > + pcmc->kvmclock_create_always = false; > } > > DEFINE_I440FX_MACHINE(v5_1, "pc-i440fx-5.1", NULL, > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 0cb9c18cd44d..0dd59bd765b1 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -185,7 +185,7 @@ static void pc_q35_init(MachineState *machine) > > x86_cpus_init(x86ms, pcmc->default_cpu_version); > > - kvmclock_create(); > + kvmclock_create(pcmc->kvmclock_create_always); > > /* pci enabled */ > if (pcmc->pci_enabled) { > @@ -366,10 +366,13 @@ DEFINE_Q35_MACHINE(v5_2, "pc-q35-5.2", NULL, > > static void pc_q35_5_1_machine_options(MachineClass *m) > { > + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); > + > pc_q35_5_2_machine_options(m); > m->alias = NULL; > compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len); > compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len); > + pcmc->kvmclock_create_always = false; > } > > DEFINE_Q35_MACHINE(v5_1, "pc-q35-5.1", NULL, > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index fe52e165b27c..6b22671434b9 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -118,6 +118,9 @@ typedef struct PCMachineClass { > > /* use PVH to load kernels that support this feature */ > bool pvh_enabled; > + > + /* create kvmclock device even when KVM PV features are not exposed */ > + bool kvmclock_create_always; > } PCMachineClass; > > #define TYPE_PC_MACHINE "generic-pc-machine" > diff --git a/include/hw/kvm/clock.h b/include/hw/kvm/clock.h > index 81c66b230207..7994071c4fee 100644 > --- a/include/hw/kvm/clock.h > +++ b/include/hw/kvm/clock.h > @@ -15,11 +15,11 @@ > > #ifdef CONFIG_KVM > > -void kvmclock_create(void); > +void kvmclock_create(bool create_always); > > #else /* CONFIG_KVM */ > > -static inline void kvmclock_create(void) > +static inline void kvmclock_create(bool create_always) > { > } > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index 4a8b3a41c1bc..20b31b65307b 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -143,6 +143,11 @@ bool kvm_has_adjust_clock_stable(void) > return (ret == KVM_CLOCK_TSC_STABLE); > } > > +bool kvm_has_adjust_clock(void) > +{ > + return kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK); > +} > + > bool kvm_has_exception_payload(void) > { > return has_exception_payload; > diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h > index 064b8798a26c..0fce4e51d2d6 100644 > --- a/target/i386/kvm_i386.h > +++ b/target/i386/kvm_i386.h > @@ -34,6 +34,7 @@ > > bool kvm_allows_irq0_override(void); > bool kvm_has_smm(void); > +bool kvm_has_adjust_clock(void); > bool kvm_has_adjust_clock_stable(void); > bool kvm_has_exception_payload(void); > void kvm_synchronize_all_tsc(void); > This was added in 2.6.33, I think we can assume it to be there. But feel free to do it on top of this patch. Paolo
Hi, The patch doesn't apply cleanly to master but it works (patched -> patched working, patched -> unpatched: old behavior, unpatched -> patched: old behavior) Thanks to everyone for the swift responses :) On Tue, Sep 22, 2020 at 05:19:34PM +0200, Vitaly Kuznetsov wrote: > QEMU's kvmclock device is only created when KVM PV feature bits for > kvmclock (KVM_FEATURE_CLOCKSOURCE/KVM_FEATURE_CLOCKSOURCE2) are > exposed to the guest. With 'kvm=off' cpu flag the device is not > created and we don't call KVM_GET_CLOCK/KVM_SET_CLOCK upon migration. > It was reported that without these call at least Hyper-V TSC page > clocksouce (which can be enabled independently) gets broken after > migration. > > Switch to creating kvmclock QEMU device unconditionally, it seems > to always make sense to call KVM_GET_CLOCK/KVM_SET_CLOCK on migration. > Use KVM_CAP_ADJUST_CLOCK check instead of CPUID feature bits. > > Reported-by: Antoine Damhet <antoine.damhet@blade-group.com> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Tested-by: Antoine Damhet <antoine.damhet@blade-group.com> > --- > hw/i386/kvm/clock.c | 7 +++++-- > hw/i386/microvm.c | 2 +- > hw/i386/pc.c | 1 + > hw/i386/pc_piix.c | 7 +++++-- > hw/i386/pc_q35.c | 5 ++++- > include/hw/i386/pc.h | 3 +++ > include/hw/kvm/clock.h | 4 ++-- > target/i386/kvm.c | 5 +++++ > target/i386/kvm_i386.h | 1 + > 9 files changed, 27 insertions(+), 8 deletions(-) > > diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c > index 64283358f91d..30cf53393ee1 100644 > --- a/hw/i386/kvm/clock.c > +++ b/hw/i386/kvm/clock.c > @@ -328,11 +328,14 @@ static const TypeInfo kvmclock_info = { > }; > > /* Note: Must be called after VCPU initialization. */ > -void kvmclock_create(void) > +void kvmclock_create(bool create_always) > { > X86CPU *cpu = X86_CPU(first_cpu); > > - if (kvm_enabled() && > + if (!kvm_enabled() || !kvm_has_adjust_clock()) > + return; > + > + if (create_always || > cpu->env.features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) | > (1ULL << KVM_FEATURE_CLOCKSOURCE2))) { > sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL); > diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c > index 81d0888930d1..fd0b84109154 100644 > --- a/hw/i386/microvm.c > +++ b/hw/i386/microvm.c > @@ -119,7 +119,7 @@ static void microvm_devices_init(MicrovmMachineState *mms) > > ioapic_init_gsi(gsi_state, "machine"); > > - kvmclock_create(); > + kvmclock_create(true); > > for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) { > sysbus_create_simple("virtio-mmio", > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index d11daacc23cf..0e036ef9c15f 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1956,6 +1956,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > pcmc->acpi_data_size = 0x20000 + 0x8000; > pcmc->linuxboot_dma_enabled = true; > pcmc->pvh_enabled = true; > + pcmc->kvmclock_create_always = true; > assert(!mc->get_hotplug_handler); > mc->get_hotplug_handler = pc_get_hotplug_handler; > mc->hotplug_allowed = pc_hotplug_allowed; > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 32b1453e6a82..1a68338c737b 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -158,8 +158,8 @@ static void pc_init1(MachineState *machine, > > x86_cpus_init(x86ms, pcmc->default_cpu_version); > > - if (kvm_enabled() && pcmc->kvmclock_enabled) { > - kvmclock_create(); > + if (pcmc->kvmclock_enabled) { > + kvmclock_create(pcmc->kvmclock_create_always); > } > > if (pcmc->pci_enabled) { > @@ -440,11 +440,14 @@ DEFINE_I440FX_MACHINE(v5_2, "pc-i440fx-5.2", NULL, > > static void pc_i440fx_5_1_machine_options(MachineClass *m) > { > + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); > + > pc_i440fx_5_2_machine_options(m); > m->alias = NULL; > m->is_default = false; > compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len); > compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len); > + pcmc->kvmclock_create_always = false; > } > > DEFINE_I440FX_MACHINE(v5_1, "pc-i440fx-5.1", NULL, > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 0cb9c18cd44d..0dd59bd765b1 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -185,7 +185,7 @@ static void pc_q35_init(MachineState *machine) > > x86_cpus_init(x86ms, pcmc->default_cpu_version); > > - kvmclock_create(); > + kvmclock_create(pcmc->kvmclock_create_always); > > /* pci enabled */ > if (pcmc->pci_enabled) { > @@ -366,10 +366,13 @@ DEFINE_Q35_MACHINE(v5_2, "pc-q35-5.2", NULL, > > static void pc_q35_5_1_machine_options(MachineClass *m) > { > + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); > + > pc_q35_5_2_machine_options(m); > m->alias = NULL; > compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len); > compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len); > + pcmc->kvmclock_create_always = false; > } > > DEFINE_Q35_MACHINE(v5_1, "pc-q35-5.1", NULL, > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index fe52e165b27c..6b22671434b9 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -118,6 +118,9 @@ typedef struct PCMachineClass { > > /* use PVH to load kernels that support this feature */ > bool pvh_enabled; > + > + /* create kvmclock device even when KVM PV features are not exposed */ > + bool kvmclock_create_always; > } PCMachineClass; > > #define TYPE_PC_MACHINE "generic-pc-machine" > diff --git a/include/hw/kvm/clock.h b/include/hw/kvm/clock.h > index 81c66b230207..7994071c4fee 100644 > --- a/include/hw/kvm/clock.h > +++ b/include/hw/kvm/clock.h > @@ -15,11 +15,11 @@ > > #ifdef CONFIG_KVM > > -void kvmclock_create(void); > +void kvmclock_create(bool create_always); > > #else /* CONFIG_KVM */ > > -static inline void kvmclock_create(void) > +static inline void kvmclock_create(bool create_always) > { > } > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index 4a8b3a41c1bc..20b31b65307b 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -143,6 +143,11 @@ bool kvm_has_adjust_clock_stable(void) > return (ret == KVM_CLOCK_TSC_STABLE); > } > > +bool kvm_has_adjust_clock(void) > +{ > + return kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK); > +} > + > bool kvm_has_exception_payload(void) > { > return has_exception_payload; > diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h > index 064b8798a26c..0fce4e51d2d6 100644 > --- a/target/i386/kvm_i386.h > +++ b/target/i386/kvm_i386.h > @@ -34,6 +34,7 @@ > > bool kvm_allows_irq0_override(void); > bool kvm_has_smm(void); > +bool kvm_has_adjust_clock(void); > bool kvm_has_adjust_clock_stable(void); > bool kvm_has_exception_payload(void); > void kvm_synchronize_all_tsc(void); > -- > 2.25.4 >
* Vitaly Kuznetsov (vkuznets@redhat.com) wrote: > QEMU's kvmclock device is only created when KVM PV feature bits for > kvmclock (KVM_FEATURE_CLOCKSOURCE/KVM_FEATURE_CLOCKSOURCE2) are > exposed to the guest. With 'kvm=off' cpu flag the device is not > created and we don't call KVM_GET_CLOCK/KVM_SET_CLOCK upon migration. > It was reported that without these call at least Hyper-V TSC page > clocksouce (which can be enabled independently) gets broken after > migration. > > Switch to creating kvmclock QEMU device unconditionally, it seems > to always make sense to call KVM_GET_CLOCK/KVM_SET_CLOCK on migration. > Use KVM_CAP_ADJUST_CLOCK check instead of CPUID feature bits. > > Reported-by: Antoine Damhet <antoine.damhet@blade-group.com> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > hw/i386/kvm/clock.c | 7 +++++-- > hw/i386/microvm.c | 2 +- > hw/i386/pc.c | 1 + > hw/i386/pc_piix.c | 7 +++++-- > hw/i386/pc_q35.c | 5 ++++- > include/hw/i386/pc.h | 3 +++ > include/hw/kvm/clock.h | 4 ++-- > target/i386/kvm.c | 5 +++++ > target/i386/kvm_i386.h | 1 + > 9 files changed, 27 insertions(+), 8 deletions(-) > > diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c > index 64283358f91d..30cf53393ee1 100644 > --- a/hw/i386/kvm/clock.c > +++ b/hw/i386/kvm/clock.c > @@ -328,11 +328,14 @@ static const TypeInfo kvmclock_info = { > }; > > /* Note: Must be called after VCPU initialization. */ > -void kvmclock_create(void) > +void kvmclock_create(bool create_always) > { > X86CPU *cpu = X86_CPU(first_cpu); > > - if (kvm_enabled() && > + if (!kvm_enabled() || !kvm_has_adjust_clock()) > + return; > + > + if (create_always || > cpu->env.features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) | > (1ULL << KVM_FEATURE_CLOCKSOURCE2))) { > sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL); > diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c > index 81d0888930d1..fd0b84109154 100644 > --- a/hw/i386/microvm.c > +++ b/hw/i386/microvm.c > @@ -119,7 +119,7 @@ static void microvm_devices_init(MicrovmMachineState *mms) > > ioapic_init_gsi(gsi_state, "machine"); > > - kvmclock_create(); > + kvmclock_create(true); > > for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) { > sysbus_create_simple("virtio-mmio", > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index d11daacc23cf..0e036ef9c15f 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1956,6 +1956,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > pcmc->acpi_data_size = 0x20000 + 0x8000; > pcmc->linuxboot_dma_enabled = true; > pcmc->pvh_enabled = true; > + pcmc->kvmclock_create_always = true; > assert(!mc->get_hotplug_handler); > mc->get_hotplug_handler = pc_get_hotplug_handler; > mc->hotplug_allowed = pc_hotplug_allowed; > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 32b1453e6a82..1a68338c737b 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -158,8 +158,8 @@ static void pc_init1(MachineState *machine, > > x86_cpus_init(x86ms, pcmc->default_cpu_version); > > - if (kvm_enabled() && pcmc->kvmclock_enabled) { > - kvmclock_create(); > + if (pcmc->kvmclock_enabled) { > + kvmclock_create(pcmc->kvmclock_create_always); > } > > if (pcmc->pci_enabled) { > @@ -440,11 +440,14 @@ DEFINE_I440FX_MACHINE(v5_2, "pc-i440fx-5.2", NULL, > > static void pc_i440fx_5_1_machine_options(MachineClass *m) > { > + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); > + > pc_i440fx_5_2_machine_options(m); > m->alias = NULL; > m->is_default = false; > compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len); > compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len); > + pcmc->kvmclock_create_always = false; Thanks; yes that should keep backwards migration happy. Dave > } > > DEFINE_I440FX_MACHINE(v5_1, "pc-i440fx-5.1", NULL, > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 0cb9c18cd44d..0dd59bd765b1 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -185,7 +185,7 @@ static void pc_q35_init(MachineState *machine) > > x86_cpus_init(x86ms, pcmc->default_cpu_version); > > - kvmclock_create(); > + kvmclock_create(pcmc->kvmclock_create_always); > > /* pci enabled */ > if (pcmc->pci_enabled) { > @@ -366,10 +366,13 @@ DEFINE_Q35_MACHINE(v5_2, "pc-q35-5.2", NULL, > > static void pc_q35_5_1_machine_options(MachineClass *m) > { > + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); > + > pc_q35_5_2_machine_options(m); > m->alias = NULL; > compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len); > compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len); > + pcmc->kvmclock_create_always = false; > } > > DEFINE_Q35_MACHINE(v5_1, "pc-q35-5.1", NULL, > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index fe52e165b27c..6b22671434b9 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -118,6 +118,9 @@ typedef struct PCMachineClass { > > /* use PVH to load kernels that support this feature */ > bool pvh_enabled; > + > + /* create kvmclock device even when KVM PV features are not exposed */ > + bool kvmclock_create_always; > } PCMachineClass; > > #define TYPE_PC_MACHINE "generic-pc-machine" > diff --git a/include/hw/kvm/clock.h b/include/hw/kvm/clock.h > index 81c66b230207..7994071c4fee 100644 > --- a/include/hw/kvm/clock.h > +++ b/include/hw/kvm/clock.h > @@ -15,11 +15,11 @@ > > #ifdef CONFIG_KVM > > -void kvmclock_create(void); > +void kvmclock_create(bool create_always); > > #else /* CONFIG_KVM */ > > -static inline void kvmclock_create(void) > +static inline void kvmclock_create(bool create_always) > { > } > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index 4a8b3a41c1bc..20b31b65307b 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -143,6 +143,11 @@ bool kvm_has_adjust_clock_stable(void) > return (ret == KVM_CLOCK_TSC_STABLE); > } > > +bool kvm_has_adjust_clock(void) > +{ > + return kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK); > +} > + > bool kvm_has_exception_payload(void) > { > return has_exception_payload; > diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h > index 064b8798a26c..0fce4e51d2d6 100644 > --- a/target/i386/kvm_i386.h > +++ b/target/i386/kvm_i386.h > @@ -34,6 +34,7 @@ > > bool kvm_allows_irq0_override(void); > bool kvm_has_smm(void); > +bool kvm_has_adjust_clock(void); > bool kvm_has_adjust_clock_stable(void); > bool kvm_has_exception_payload(void); > void kvm_synchronize_all_tsc(void); > -- > 2.25.4 >
diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c index 64283358f91d..30cf53393ee1 100644 --- a/hw/i386/kvm/clock.c +++ b/hw/i386/kvm/clock.c @@ -328,11 +328,14 @@ static const TypeInfo kvmclock_info = { }; /* Note: Must be called after VCPU initialization. */ -void kvmclock_create(void) +void kvmclock_create(bool create_always) { X86CPU *cpu = X86_CPU(first_cpu); - if (kvm_enabled() && + if (!kvm_enabled() || !kvm_has_adjust_clock()) + return; + + if (create_always || cpu->env.features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) | (1ULL << KVM_FEATURE_CLOCKSOURCE2))) { sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL); diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c index 81d0888930d1..fd0b84109154 100644 --- a/hw/i386/microvm.c +++ b/hw/i386/microvm.c @@ -119,7 +119,7 @@ static void microvm_devices_init(MicrovmMachineState *mms) ioapic_init_gsi(gsi_state, "machine"); - kvmclock_create(); + kvmclock_create(true); for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) { sysbus_create_simple("virtio-mmio", diff --git a/hw/i386/pc.c b/hw/i386/pc.c index d11daacc23cf..0e036ef9c15f 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1956,6 +1956,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) pcmc->acpi_data_size = 0x20000 + 0x8000; pcmc->linuxboot_dma_enabled = true; pcmc->pvh_enabled = true; + pcmc->kvmclock_create_always = true; assert(!mc->get_hotplug_handler); mc->get_hotplug_handler = pc_get_hotplug_handler; mc->hotplug_allowed = pc_hotplug_allowed; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 32b1453e6a82..1a68338c737b 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -158,8 +158,8 @@ static void pc_init1(MachineState *machine, x86_cpus_init(x86ms, pcmc->default_cpu_version); - if (kvm_enabled() && pcmc->kvmclock_enabled) { - kvmclock_create(); + if (pcmc->kvmclock_enabled) { + kvmclock_create(pcmc->kvmclock_create_always); } if (pcmc->pci_enabled) { @@ -440,11 +440,14 @@ DEFINE_I440FX_MACHINE(v5_2, "pc-i440fx-5.2", NULL, static void pc_i440fx_5_1_machine_options(MachineClass *m) { + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); + pc_i440fx_5_2_machine_options(m); m->alias = NULL; m->is_default = false; compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len); compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len); + pcmc->kvmclock_create_always = false; } DEFINE_I440FX_MACHINE(v5_1, "pc-i440fx-5.1", NULL, diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 0cb9c18cd44d..0dd59bd765b1 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -185,7 +185,7 @@ static void pc_q35_init(MachineState *machine) x86_cpus_init(x86ms, pcmc->default_cpu_version); - kvmclock_create(); + kvmclock_create(pcmc->kvmclock_create_always); /* pci enabled */ if (pcmc->pci_enabled) { @@ -366,10 +366,13 @@ DEFINE_Q35_MACHINE(v5_2, "pc-q35-5.2", NULL, static void pc_q35_5_1_machine_options(MachineClass *m) { + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); + pc_q35_5_2_machine_options(m); m->alias = NULL; compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len); compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len); + pcmc->kvmclock_create_always = false; } DEFINE_Q35_MACHINE(v5_1, "pc-q35-5.1", NULL, diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index fe52e165b27c..6b22671434b9 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -118,6 +118,9 @@ typedef struct PCMachineClass { /* use PVH to load kernels that support this feature */ bool pvh_enabled; + + /* create kvmclock device even when KVM PV features are not exposed */ + bool kvmclock_create_always; } PCMachineClass; #define TYPE_PC_MACHINE "generic-pc-machine" diff --git a/include/hw/kvm/clock.h b/include/hw/kvm/clock.h index 81c66b230207..7994071c4fee 100644 --- a/include/hw/kvm/clock.h +++ b/include/hw/kvm/clock.h @@ -15,11 +15,11 @@ #ifdef CONFIG_KVM -void kvmclock_create(void); +void kvmclock_create(bool create_always); #else /* CONFIG_KVM */ -static inline void kvmclock_create(void) +static inline void kvmclock_create(bool create_always) { } diff --git a/target/i386/kvm.c b/target/i386/kvm.c index 4a8b3a41c1bc..20b31b65307b 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -143,6 +143,11 @@ bool kvm_has_adjust_clock_stable(void) return (ret == KVM_CLOCK_TSC_STABLE); } +bool kvm_has_adjust_clock(void) +{ + return kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK); +} + bool kvm_has_exception_payload(void) { return has_exception_payload; diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h index 064b8798a26c..0fce4e51d2d6 100644 --- a/target/i386/kvm_i386.h +++ b/target/i386/kvm_i386.h @@ -34,6 +34,7 @@ bool kvm_allows_irq0_override(void); bool kvm_has_smm(void); +bool kvm_has_adjust_clock(void); bool kvm_has_adjust_clock_stable(void); bool kvm_has_exception_payload(void); void kvm_synchronize_all_tsc(void);
QEMU's kvmclock device is only created when KVM PV feature bits for kvmclock (KVM_FEATURE_CLOCKSOURCE/KVM_FEATURE_CLOCKSOURCE2) are exposed to the guest. With 'kvm=off' cpu flag the device is not created and we don't call KVM_GET_CLOCK/KVM_SET_CLOCK upon migration. It was reported that without these call at least Hyper-V TSC page clocksouce (which can be enabled independently) gets broken after migration. Switch to creating kvmclock QEMU device unconditionally, it seems to always make sense to call KVM_GET_CLOCK/KVM_SET_CLOCK on migration. Use KVM_CAP_ADJUST_CLOCK check instead of CPUID feature bits. Reported-by: Antoine Damhet <antoine.damhet@blade-group.com> Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- hw/i386/kvm/clock.c | 7 +++++-- hw/i386/microvm.c | 2 +- hw/i386/pc.c | 1 + hw/i386/pc_piix.c | 7 +++++-- hw/i386/pc_q35.c | 5 ++++- include/hw/i386/pc.h | 3 +++ include/hw/kvm/clock.h | 4 ++-- target/i386/kvm.c | 5 +++++ target/i386/kvm_i386.h | 1 + 9 files changed, 27 insertions(+), 8 deletions(-)