On 2011-02-07 20:39, Blue Swirl wrote:
> On Mon, Feb 7, 2011 at 1:19 PM, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote:
>> If kvmclock is used, which implies the kernel supports it, register a
>> kvmclock device with the sysbus. Its main purpose is to save and restore
>> the kernel state on migration, but this will also allow to visualize it
>> one day.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>> CC: Glauber Costa <glommer@xxxxxxxxxx>
>> ---
>> Makefile.target | 4 +-
>> hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> hw/kvmclock.h | 14 ++++++
>> hw/pc_piix.c | 31 +++++++++++---
>> 4 files changed, 165 insertions(+), 9 deletions(-)
>> create mode 100644 hw/kvmclock.c
>> create mode 100644 hw/kvmclock.h
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index b0ba95f..30232fa 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU
>> LIBS+=-lm
>> endif
>>
>> -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS)
>> +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS)
>>
>> config-target.h: config-target.h-timestamp
>> config-target.h-timestamp: config-target.mak
>> @@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
>> obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o
>> obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>> obj-i386-y += debugcon.o multiboot.o
>> -obj-i386-y += pc_piix.o
>> +obj-i386-y += pc_piix.o kvmclock.o
>
> Please build kvmclock.o conditionally to CONFIG_something...
>
>> obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>
>> # shared objects
>> diff --git a/hw/kvmclock.c b/hw/kvmclock.c
>> new file mode 100644
>> index 0000000..b6ceddf
>> --- /dev/null
>> +++ b/hw/kvmclock.c
>> @@ -0,0 +1,125 @@
>> +/*
>> + * QEMU KVM support, paravirtual clock device
>> + *
>> + * Copyright (C) 2011 Siemens AG
>> + *
>> + * Authors:
>> + * Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>> + *
>> + * This work is licensed under the terms of the GNU GPL version 2.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qemu-common.h"
>> +#include "sysemu.h"
>> +#include "sysbus.h"
>> +#include "kvm.h"
>> +#include "kvmclock.h"
>> +
>> +#if defined(CONFIG_KVM_PARA) && defined(KVM_CAP_ADJUST_CLOCK)
>> +
>> +#include <linux/kvm.h>
>> +#include <linux/kvm_para.h>
>> +
>> +typedef struct KVMClockState {
>> + SysBusDevice busdev;
>> + uint64_t clock;
>> + bool clock_valid;
>> +} KVMClockState;
>> +
>> +static void kvmclock_pre_save(void *opaque)
>> +{
>> + KVMClockState *s = opaque;
>> + struct kvm_clock_data data;
>> + int ret;
>> +
>> + if (s->clock_valid) {
>> + return;
>> + }
>> + ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data);
>> + if (ret < 0) {
>> + fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret));
>> + data.clock = 0;
>> + }
>> + s->clock = data.clock;
>> + /*
>> + * If the VM is stopped, declare the clock state valid to avoid re-reading
>> + * it on next vmsave (which would return a different value). Will be reset
>> + * when the VM is continued.
>> + */
>> + s->clock_valid = !vm_running;
>> +}
>> +
>> +static int kvmclock_post_load(void *opaque, int version_id)
>> +{
>> + KVMClockState *s = opaque;
>> + struct kvm_clock_data data;
>> +
>> + data.clock = s->clock;
>> + data.flags = 0;
>> + return kvm_vm_ioctl(kvm_state, KVM_SET_CLOCK, &data);
>> +}
>> +
>> +static void kvmclock_vm_state_change(void *opaque, int running, int reason)
>> +{
>> + KVMClockState *s = opaque;
>> +
>> + if (running) {
>> + s->clock_valid = false;
>> + }
>> +}
>> +
>> +static int kvmclock_init(SysBusDevice *dev)
>> +{
>> + KVMClockState *s = FROM_SYSBUS(KVMClockState, dev);
>> +
>> + qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s);
>> + return 0;
>> +}
>> +
>> +static const VMStateDescription kvmclock_vmsd = {
>> + .name = "kvmclock",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .pre_save = kvmclock_pre_save,
>> + .post_load = kvmclock_post_load,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_UINT64(clock, KVMClockState),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static SysBusDeviceInfo kvmclock_info = {
>> + .qdev.name = "kvmclock",
>> + .qdev.size = sizeof(KVMClockState),
>> + .qdev.vmsd = &kvmclock_vmsd,
>> + .qdev.no_user = 1,
>> + .init = kvmclock_init,
>> +};
>> +
>> +/* Note: Must be called after VCPU initialization. */
>> +void kvmclock_create(void)
>> +{
>> + if (kvm_enabled() &&
>> + first_cpu->cpuid_kvm_features & (1ULL << KVM_FEATURE_CLOCKSOURCE)) {
>> + sysbus_create_simple("kvmclock", -1, NULL);
>> + }
>> +}
>
> ... and with this moved to a header as a static inline function, it
> should be possible to use sysbus_try_create() (coming soon) to try to
> create the device. Then it's not fatal if the device can't be created,
> that just means that the capability was not available at build time.
I played with this, and while it is generally a nice thing, it doesn't
help us here. We would just push the logic around, from kvmclock.c to
the header or even to configure (KVM_FEATURE_CLOCKSOURCE is not
unconditionally available).
I rather hope we finally agree on merging the required kvm headers into
qemu so that all this usually broken #ifdef KVM_CAP_* can be removed.
Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
