| Message ID | 20160512214509.GA5248@aepfle.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 12 May 2016, Olaf Hering wrote: > On Thu, May 12, Olaf Hering wrote: > > > One thing to fix it in staging-4.5 is to introduce a dummy device which > > handles a section named "kvm-tpr-opt". I already have a hack which does > > that, and the migration proceeds. I will propose a patch to deal with > > this part of the bug. > > Something like shown below. Thanks for looking into this. I don't think that adding a dummy device in QEMU is acceptable. This kind of problems is usually solved with versioning the PC machine in QEMU, see all the pc_machine_* in hw/i386/pc_piix.c. One specific version of the machine is supposed to remain identical over multiple QEMU releases. In this case xenfv (or the pc machine you are using, if you are not using xenfv) has to be always identical. That's why I think we need to add kvmapic back to it for compatibility. I know it sucks. But we can choose a different PC machine with accel=xen for new VMs. > > Unfortunately later the VM appears to be alive, but nothing happens in > > it. I assume it gets no further interrupts. Guess I need help with this > > part of the bug. > > > > > --- > hw/i386/Makefile.objs | 1 > hw/i386/xen44_kvmvapic.c | 147 +++++++++++++++++++++++++++++++++++++++++++++++ > xen-hvm.c | 3 > 3 files changed, 151 insertions(+) > > --- a/hw/i386/Makefile.objs > +++ b/hw/i386/Makefile.objs > @@ -5,6 +5,7 @@ obj-y += pc_sysfw.o > obj-$(CONFIG_XEN) += ../xenpv/ xen/ > > obj-y += kvmvapic.o > +obj-y += xen44_kvmvapic.o > obj-y += acpi-build.o > obj-y += bios-linker-loader.o > hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \ > --- /dev/null > +++ b/hw/i386/xen44_kvmvapic.c > @@ -0,0 +1,147 @@ > +/* > + * Copy of kvmvapic to allow migration from xen-4.4 qemu-xen with "kvm-tpr-opt" > + * > + * Copyright (C) 2007-2008 Qumranet Technologies > + * Copyright (C) 2012 Jan Kiszka, Siemens AG > + * > + * This work is licensed under the terms of the GNU GPL version 2, or > + * (at your option) any later version. See the COPYING file in the > + * top-level directory. > + */ > +#include "sysemu/sysemu.h" > +#include "sysemu/cpus.h" > +#include "sysemu/kvm.h" > +#include "hw/i386/apic_internal.h" > +#include "hw/sysbus.h" > +#include "hw/xen/xen.h" > + > +typedef struct VAPICHandlers { > + uint32_t set_tpr; > + uint32_t set_tpr_eax; > + uint32_t get_tpr[8]; > + uint32_t get_tpr_stack; > +} QEMU_PACKED VAPICHandlers; > + > +typedef struct GuestROMState { > + char signature[8]; > + uint32_t vaddr; > + uint32_t fixup_start; > + uint32_t fixup_end; > + uint32_t vapic_vaddr; > + uint32_t vapic_size; > + uint32_t vcpu_shift; > + uint32_t real_tpr_addr; > + VAPICHandlers up; > + VAPICHandlers mp; > +} QEMU_PACKED GuestROMState; > + > +typedef struct VAPICROMState { > + SysBusDevice busdev; > + MemoryRegion io; > + MemoryRegion rom; > + uint32_t state; > + uint32_t rom_state_paddr; > + uint32_t rom_state_vaddr; > + uint32_t vapic_paddr; > + uint32_t real_tpr_addr; > + GuestROMState rom_state; > + size_t rom_size; > + bool rom_mapped_writable; > +} VAPICROMState; > + > +#define TYPE_XEN_KVMVAPIC "xen_kvmvapic" /* copy of "kvmvapic" */ > + > +static void xen44_vapic_realize(DeviceState *dev, Error **errp) > +{ > + fprintf(stderr, "%s(%u) dev %p\n", __func__, __LINE__, dev); > +} > + > +static int xen44_vapic_post_load(void *opaque, int version_id) > +{ > + if (xen_enabled()) { > + int i; > + fprintf(stderr, "%s(%u) %p 0x%x\n", __func__, __LINE__, opaque, version_id); > + for (i = 12; i > 0; i--) { > + sleep(1); > + fprintf(stderr, "%s(%u) sleep %d %ld\n", __func__, __LINE__, i, (long)getpid()); > + } > + } > + return 0; > +} > + > +static const VMStateDescription vmstate_handlers = { > + .name = "kvmvapic-handlers", > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(set_tpr, VAPICHandlers), > + VMSTATE_UINT32(set_tpr_eax, VAPICHandlers), > + VMSTATE_UINT32_ARRAY(get_tpr, VAPICHandlers, 8), > + VMSTATE_UINT32(get_tpr_stack, VAPICHandlers), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_guest_rom = { > + .name = "kvmvapic-guest-rom", > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UNUSED(8), /* signature */ > + VMSTATE_UINT32(vaddr, GuestROMState), > + VMSTATE_UINT32(fixup_start, GuestROMState), > + VMSTATE_UINT32(fixup_end, GuestROMState), > + VMSTATE_UINT32(vapic_vaddr, GuestROMState), > + VMSTATE_UINT32(vapic_size, GuestROMState), > + VMSTATE_UINT32(vcpu_shift, GuestROMState), > + VMSTATE_UINT32(real_tpr_addr, GuestROMState), > + VMSTATE_STRUCT(up, GuestROMState, 0, vmstate_handlers, VAPICHandlers), > + VMSTATE_STRUCT(mp, GuestROMState, 0, vmstate_handlers, VAPICHandlers), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_xen44_vapic = { > + .name = "kvm-tpr-opt", /* compatible with qemu-kvm VAPIC */ > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, > + .post_load = xen44_vapic_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_STRUCT(rom_state, VAPICROMState, 0, vmstate_guest_rom, > + GuestROMState), > + VMSTATE_UINT32(state, VAPICROMState), > + VMSTATE_UINT32(real_tpr_addr, VAPICROMState), > + VMSTATE_UINT32(rom_state_vaddr, VAPICROMState), > + VMSTATE_UINT32(vapic_paddr, VAPICROMState), > + VMSTATE_UINT32(rom_state_paddr, VAPICROMState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static void xen44_vapic_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + fprintf(stderr, "%s(%u) klass %p data %p\n", __func__, __LINE__, klass, data); > + dc->vmsd = &vmstate_xen44_vapic; > + dc->realize = xen44_vapic_realize; > +} > + > +static const TypeInfo xen44_vapic_type = { > + .name = TYPE_XEN_KVMVAPIC, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(VAPICROMState), > + .class_init = xen44_vapic_class_init, > +}; > + > +static void xen44_vapic_register(void) > +{ > + fprintf(stderr, "%s(%u)\n", __func__, __LINE__); > + type_register_static(&xen44_vapic_type); > +} > + > +type_init(xen44_vapic_register); > + > --- a/xen-hvm.c > +++ b/xen-hvm.c > @@ -15,6 +15,7 @@ > #include "hw/i386/apic-msidef.h" > #include "hw/xen/xen_common.h" > #include "hw/xen/xen_backend.h" > +#include "hw/sysbus.h" > #include "qmp-commands.h" > > #include "sysemu/char.h" > @@ -161,6 +162,8 @@ static void xen_set_irq(void *opaque, in > > qemu_irq *xen_interrupt_controller_init(void) > { > + DeviceState *vapic = sysbus_create_simple("kvmvapic", -1, NULL); > + > return qemu_allocate_irqs(xen_set_irq, NULL, 16); > } > >
As a followup to the issue below, and the one which "just" popped in in qemu-2.6+: Why is the machine description for xen not fixed? Shouldnt the be some sort of verification of old and new 'xenfv' when a new qemu rc1 is done? Is there a way to dump the machine description in text form? Olaf On Fri, May 13, Stefano Stabellini wrote: > On Thu, 12 May 2016, Olaf Hering wrote: > > On Thu, May 12, Olaf Hering wrote: > > > > > One thing to fix it in staging-4.5 is to introduce a dummy device which > > > handles a section named "kvm-tpr-opt". I already have a hack which does > > > that, and the migration proceeds. I will propose a patch to deal with > > > this part of the bug. > > > > Something like shown below. > > Thanks for looking into this. I don't think that adding a dummy device > in QEMU is acceptable. This kind of problems is usually solved with > versioning the PC machine in QEMU, see all the pc_machine_* in > hw/i386/pc_piix.c. One specific version of the machine is supposed to > remain identical over multiple QEMU releases. In this case xenfv (or the > pc machine you are using, if you are not using xenfv) has to be always > identical. That's why I think we need to add kvmapic back to it for > compatibility. I know it sucks. But we can choose a different PC machine > with accel=xen for new VMs.
On Tue, 2 Aug 2016, Olaf Hering wrote: > As a followup to the issue below, and the one which "just" popped in in > qemu-2.6+: > > Why is the machine description for xen not fixed? xenfv comes from a time when QEMU didn't have machine description versioning. To have versioning, it is possible to use a regular PC machine plus accel=xen. I tried to change the libxl default from xenfv to a versioned pc machine with accel=xen a couple of years back, but unfortunately it created ABI incompatibilities, see: http://marc.info/?i=alpine.DEB.2.02.1406121512360.13771%40kaball.uk.xensource.com > Shouldnt the be some sort of verification of old and new 'xenfv' when a > new qemu rc1 is done? It would be great to have > Is there a way to dump the machine description in text form? I don't know, but people at qemu-devel might. > On Fri, May 13, Stefano Stabellini wrote: > > > On Thu, 12 May 2016, Olaf Hering wrote: > > > On Thu, May 12, Olaf Hering wrote: > > > > > > > One thing to fix it in staging-4.5 is to introduce a dummy device which > > > > handles a section named "kvm-tpr-opt". I already have a hack which does > > > > that, and the migration proceeds. I will propose a patch to deal with > > > > this part of the bug. > > > > > > Something like shown below. > > > > Thanks for looking into this. I don't think that adding a dummy device > > in QEMU is acceptable. This kind of problems is usually solved with > > versioning the PC machine in QEMU, see all the pc_machine_* in > > hw/i386/pc_piix.c. One specific version of the machine is supposed to > > remain identical over multiple QEMU releases. In this case xenfv (or the > > pc machine you are using, if you are not using xenfv) has to be always > > identical. That's why I think we need to add kvmapic back to it for > > compatibility. I know it sucks. But we can choose a different PC machine > > with accel=xen for new VMs.
--- a/hw/i386/Makefile.objs +++ b/hw/i386/Makefile.objs @@ -5,6 +5,7 @@ obj-y += pc_sysfw.o obj-$(CONFIG_XEN) += ../xenpv/ xen/ obj-y += kvmvapic.o +obj-y += xen44_kvmvapic.o obj-y += acpi-build.o obj-y += bios-linker-loader.o hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \ --- /dev/null +++ b/hw/i386/xen44_kvmvapic.c @@ -0,0 +1,147 @@ +/* + * Copy of kvmvapic to allow migration from xen-4.4 qemu-xen with "kvm-tpr-opt" + * + * Copyright (C) 2007-2008 Qumranet Technologies + * Copyright (C) 2012 Jan Kiszka, Siemens AG + * + * This work is licensed under the terms of the GNU GPL version 2, or + * (at your option) any later version. See the COPYING file in the + * top-level directory. + */ +#include "sysemu/sysemu.h" +#include "sysemu/cpus.h" +#include "sysemu/kvm.h" +#include "hw/i386/apic_internal.h" +#include "hw/sysbus.h" +#include "hw/xen/xen.h" + +typedef struct VAPICHandlers { + uint32_t set_tpr; + uint32_t set_tpr_eax; + uint32_t get_tpr[8]; + uint32_t get_tpr_stack; +} QEMU_PACKED VAPICHandlers; + +typedef struct GuestROMState { + char signature[8]; + uint32_t vaddr; + uint32_t fixup_start; + uint32_t fixup_end; + uint32_t vapic_vaddr; + uint32_t vapic_size; + uint32_t vcpu_shift; + uint32_t real_tpr_addr; + VAPICHandlers up; + VAPICHandlers mp; +} QEMU_PACKED GuestROMState; + +typedef struct VAPICROMState { + SysBusDevice busdev; + MemoryRegion io; + MemoryRegion rom; + uint32_t state; + uint32_t rom_state_paddr; + uint32_t rom_state_vaddr; + uint32_t vapic_paddr; + uint32_t real_tpr_addr; + GuestROMState rom_state; + size_t rom_size; + bool rom_mapped_writable; +} VAPICROMState; + +#define TYPE_XEN_KVMVAPIC "xen_kvmvapic" /* copy of "kvmvapic" */ + +static void xen44_vapic_realize(DeviceState *dev, Error **errp) +{ + fprintf(stderr, "%s(%u) dev %p\n", __func__, __LINE__, dev); +} + +static int xen44_vapic_post_load(void *opaque, int version_id) +{ + if (xen_enabled()) { + int i; + fprintf(stderr, "%s(%u) %p 0x%x\n", __func__, __LINE__, opaque, version_id); + for (i = 12; i > 0; i--) { + sleep(1); + fprintf(stderr, "%s(%u) sleep %d %ld\n", __func__, __LINE__, i, (long)getpid()); + } + } + return 0; +} + +static const VMStateDescription vmstate_handlers = { + .name = "kvmvapic-handlers", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(set_tpr, VAPICHandlers), + VMSTATE_UINT32(set_tpr_eax, VAPICHandlers), + VMSTATE_UINT32_ARRAY(get_tpr, VAPICHandlers, 8), + VMSTATE_UINT32(get_tpr_stack, VAPICHandlers), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription vmstate_guest_rom = { + .name = "kvmvapic-guest-rom", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField[]) { + VMSTATE_UNUSED(8), /* signature */ + VMSTATE_UINT32(vaddr, GuestROMState), + VMSTATE_UINT32(fixup_start, GuestROMState), + VMSTATE_UINT32(fixup_end, GuestROMState), + VMSTATE_UINT32(vapic_vaddr, GuestROMState), + VMSTATE_UINT32(vapic_size, GuestROMState), + VMSTATE_UINT32(vcpu_shift, GuestROMState), + VMSTATE_UINT32(real_tpr_addr, GuestROMState), + VMSTATE_STRUCT(up, GuestROMState, 0, vmstate_handlers, VAPICHandlers), + VMSTATE_STRUCT(mp, GuestROMState, 0, vmstate_handlers, VAPICHandlers), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription vmstate_xen44_vapic = { + .name = "kvm-tpr-opt", /* compatible with qemu-kvm VAPIC */ + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .post_load = xen44_vapic_post_load, + .fields = (VMStateField[]) { + VMSTATE_STRUCT(rom_state, VAPICROMState, 0, vmstate_guest_rom, + GuestROMState), + VMSTATE_UINT32(state, VAPICROMState), + VMSTATE_UINT32(real_tpr_addr, VAPICROMState), + VMSTATE_UINT32(rom_state_vaddr, VAPICROMState), + VMSTATE_UINT32(vapic_paddr, VAPICROMState), + VMSTATE_UINT32(rom_state_paddr, VAPICROMState), + VMSTATE_END_OF_LIST() + } +}; + +static void xen44_vapic_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + fprintf(stderr, "%s(%u) klass %p data %p\n", __func__, __LINE__, klass, data); + dc->vmsd = &vmstate_xen44_vapic; + dc->realize = xen44_vapic_realize; +} + +static const TypeInfo xen44_vapic_type = { + .name = TYPE_XEN_KVMVAPIC, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(VAPICROMState), + .class_init = xen44_vapic_class_init, +}; + +static void xen44_vapic_register(void) +{ + fprintf(stderr, "%s(%u)\n", __func__, __LINE__); + type_register_static(&xen44_vapic_type); +} + +type_init(xen44_vapic_register); + --- a/xen-hvm.c +++ b/xen-hvm.c @@ -15,6 +15,7 @@ #include "hw/i386/apic-msidef.h" #include "hw/xen/xen_common.h" #include "hw/xen/xen_backend.h" +#include "hw/sysbus.h" #include "qmp-commands.h" #include "sysemu/char.h" @@ -161,6 +162,8 @@ static void xen_set_irq(void *opaque, in qemu_irq *xen_interrupt_controller_init(void) { + DeviceState *vapic = sysbus_create_simple("kvmvapic", -1, NULL); + return qemu_allocate_irqs(xen_set_irq, NULL, 16); }