| Message ID | 1363876125-8264-8-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
Il 21/03/2013 15:28, Igor Mammedov ha scritto: > Introduce hot-pluggable ICC_BUS and convert APIC devices from SysBusDevice > to ICCDevice wich has ICC_BUS as default one. > > * attach APIC and kvmvapic to ICC_BUS during creation > * use memory API directly instead of using SysBus proxy functions > * introduce get_icc_bus() for getting access to system wide ICC_BUS > * make ICC_BUS default one for X86CPU class, so that device_add would > set it for CPU instead of SysBus > * attach cold-pluged CPUs to icc_bus in softmmu mode, so they could be > uplugged in future > * if kvmvapic init() fails return -1 instead of aborting. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/apic_common.c | 14 ++++++--- > hw/apic_internal.h | 6 ++-- > hw/i386/Makefile.objs | 2 +- > hw/i386/kvmvapic.c | 15 +++++---- > hw/icc_bus.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++ > hw/icc_bus.h | 47 ++++++++++++++++++++++++++++++ > stubs/Makefile.objs | 1 + > stubs/get_icc_bus.c | 6 ++++ > target-i386/cpu.c | 16 ++++++++-- > 9 files changed, 162 insertions(+), 20 deletions(-) > create mode 100644 hw/icc_bus.c > create mode 100644 hw/icc_bus.h > create mode 100644 stubs/get_icc_bus.c > > diff --git a/hw/apic_common.c b/hw/apic_common.c > index d0c2616..61599d4 100644 > --- a/hw/apic_common.c > +++ b/hw/apic_common.c > @@ -21,6 +21,7 @@ > #include "hw/apic_internal.h" > #include "trace.h" > #include "sysemu/kvm.h" > +#include "qdev.h" > > static int apic_irq_delivered; > bool apic_report_tpr_access; > @@ -282,9 +283,10 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id) > return 0; > } > > -static int apic_init_common(SysBusDevice *dev) > +static int apic_init_common(ICCDevice *dev) > { > APICCommonState *s = APIC_COMMON(dev); > + DeviceState *d = DEVICE(dev); > APICCommonClass *info; > static DeviceState *vapic; > static int apic_no; > @@ -297,12 +299,14 @@ static int apic_init_common(SysBusDevice *dev) > info = APIC_COMMON_GET_CLASS(s); > info->init(s); > > - sysbus_init_mmio(dev, &s->io_memory); > > /* Note: We need at least 1M to map the VAPIC option ROM */ > if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK && > ram_size >= 1024 * 1024) { > - vapic = sysbus_create_simple("kvmvapic", -1, NULL); > + vapic = qdev_try_create(d->parent_bus, "kvmvapic"); > + if ((vapic == NULL) || (qdev_init(vapic) != 0)) { > + return -1; > + } > } > s->vapic = vapic; > if (apic_report_tpr_access && info->enable_tpr_reporting) { > @@ -375,7 +379,7 @@ static Property apic_properties_common[] = { > > static void apic_common_class_init(ObjectClass *klass, void *data) > { > - SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass); > + ICCDeviceClass *sc = ICC_DEVICE_CLASS(klass); > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->vmsd = &vmstate_apic_common; > @@ -387,7 +391,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data) > > static const TypeInfo apic_common_type = { > .name = TYPE_APIC_COMMON, > - .parent = TYPE_SYS_BUS_DEVICE, > + .parent = TYPE_ICC_DEVICE, > .instance_size = sizeof(APICCommonState), > .class_size = sizeof(APICCommonClass), > .class_init = apic_common_class_init, > diff --git a/hw/apic_internal.h b/hw/apic_internal.h > index 578241f..e7b378e 100644 > --- a/hw/apic_internal.h > +++ b/hw/apic_internal.h > @@ -21,7 +21,7 @@ > #define QEMU_APIC_INTERNAL_H > > #include "exec/memory.h" > -#include "hw/sysbus.h" > +#include "hw/icc_bus.h" > #include "qemu/timer.h" > > /* APIC Local Vector Table */ > @@ -80,7 +80,7 @@ typedef struct APICCommonState APICCommonState; > > typedef struct APICCommonClass > { > - SysBusDeviceClass parent_class; > + ICCDeviceClass parent_class; > > void (*init)(APICCommonState *s); > void (*set_base)(APICCommonState *s, uint64_t val); > @@ -94,7 +94,7 @@ typedef struct APICCommonClass > } APICCommonClass; > > struct APICCommonState { > - SysBusDevice busdev; > + ICCDevice busdev; > > MemoryRegion io_memory; > X86CPU *cpu; > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > index a78c0b2..316f999 100644 > --- a/hw/i386/Makefile.objs > +++ b/hw/i386/Makefile.objs > @@ -1,5 +1,5 @@ > obj-y += mc146818rtc.o > -obj-y += apic_common.o apic.o > +obj-y += apic_common.o apic.o icc_bus.o > obj-y += sga.o ioapic_common.o ioapic.o piix_pci.o > obj-y += vmport.o > obj-y += pci/pci-hotplug.o wdt_ib700.o > diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c > index 21551a5..7de75db 100644 > --- a/hw/i386/kvmvapic.c > +++ b/hw/i386/kvmvapic.c > @@ -12,6 +12,8 @@ > #include "sysemu/cpus.h" > #include "sysemu/kvm.h" > #include "hw/apic_internal.h" > +#include "migration/vmstate.h" > +#include "exec/address-spaces.h" > > #define APIC_DEFAULT_ADDRESS 0xfee00000 > > @@ -49,7 +51,7 @@ typedef struct GuestROMState { > } QEMU_PACKED GuestROMState; > > typedef struct VAPICROMState { > - SysBusDevice busdev; > + ICCDevice busdev; > MemoryRegion io; > MemoryRegion rom; > uint32_t state; > @@ -581,7 +583,7 @@ static void vapic_map_rom_writable(VAPICROMState *s) > size_t rom_size; > uint8_t *ram; > > - as = sysbus_address_space(&s->busdev); > + as = get_system_memory(); > > if (s->rom_mapped_writable) { > memory_region_del_subregion(as, &s->rom); > @@ -693,13 +695,12 @@ static const MemoryRegionOps vapic_ops = { > .endianness = DEVICE_NATIVE_ENDIAN, > }; > > -static int vapic_init(SysBusDevice *dev) > +static int vapic_init(ICCDevice *dev) > { > VAPICROMState *s = VAPIC_DEVICE(dev); > > memory_region_init_io(&s->io, &vapic_ops, s, "kvmvapic", 2); > - sysbus_add_io(dev, VAPIC_IO_PORT, &s->io); > - sysbus_init_ioports(dev, VAPIC_IO_PORT, 2); > + memory_region_add_subregion(get_system_io(), VAPIC_IO_PORT, &s->io); > > option_rom[nb_option_roms].name = "kvmvapic.bin"; > option_rom[nb_option_roms].bootindex = -1; > @@ -801,7 +802,7 @@ static const VMStateDescription vmstate_vapic = { > > static void vapic_class_init(ObjectClass *klass, void *data) > { > - SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass); > + ICCDeviceClass *sc = ICC_DEVICE_CLASS(klass); > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->no_user = 1; > @@ -812,7 +813,7 @@ static void vapic_class_init(ObjectClass *klass, void *data) > > static const TypeInfo vapic_type = { > .name = TYPE_VAPIC_DEVICE, > - .parent = TYPE_SYS_BUS_DEVICE, > + .parent = TYPE_ICC_DEVICE, > .instance_size = sizeof(VAPICROMState), > .class_init = vapic_class_init, > }; > diff --git a/hw/icc_bus.c b/hw/icc_bus.c > new file mode 100644 > index 0000000..b170648 > --- /dev/null > +++ b/hw/icc_bus.c > @@ -0,0 +1,75 @@ > +/* icc_bus.c > + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus > + * > + * Copyright (c) 2013 Red Hat, Inc > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see <http://www.gnu.org/licenses/> > + */ > +#include "icc_bus.h" > +#include "qemu/module.h" > + > +static const TypeInfo icc_bus_info = { > + .name = TYPE_ICC_BUS, > + .parent = TYPE_BUS, > + .instance_size = sizeof(ICCBus), > +}; > + > +static int icc_device_init(DeviceState *dev) > +{ > + ICCDevice *id = ICC_DEVICE(dev); > + ICCDeviceClass *k = ICC_DEVICE_GET_CLASS(id); > + > + return k->init(id); > +} > + > +static void icc_device_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *k = DEVICE_CLASS(klass); > + > + k->init = icc_device_init; > + k->bus_type = TYPE_ICC_BUS; > +} > + > +static const TypeInfo icc_device_info = { > + .name = TYPE_ICC_DEVICE, > + .parent = TYPE_DEVICE, > + .abstract = true, > + .instance_size = sizeof(ICCDevice), > + .class_size = sizeof(ICCDeviceClass), > + .class_init = icc_device_class_init, > +}; > + > +static BusState *icc_bus; > + > +BusState *get_icc_bus(void) > +{ > + if (icc_bus == NULL) { > + icc_bus = g_malloc0(icc_bus_info.instance_size); > + qbus_create_inplace(icc_bus, TYPE_ICC_BUS, NULL, "icc-bus"); > + icc_bus->allow_hotplug = 1; > + OBJECT(icc_bus)->free = g_free; You can just use qbus_create. > + object_property_add_child(container_get(qdev_get_machine(), > + "/unattached"), Please put it straight under /machine, not /unattached. But actually, you lack a device that instantiates the bus. That device would be a simple container device similar hw/a15mpcore.c, and would also take care of registering the MSI memory region. Create that device in the boards, and you won't need a special object_property_add_child. get_icc_bus() would then become simply object_resolve_path_type("icc-bus", TYPE_ICC_BUS, NULL) or something like that. Paolo > + "icc-bus", OBJECT(icc_bus), NULL); > + } > + return BUS(icc_bus); > +} > + > +static void icc_bus_register_types(void) > +{ > + type_register_static(&icc_bus_info); > + type_register_static(&icc_device_info); > +} > + > +type_init(icc_bus_register_types) > diff --git a/hw/icc_bus.h b/hw/icc_bus.h > new file mode 100644 > index 0000000..a00fef5 > --- /dev/null > +++ b/hw/icc_bus.h > @@ -0,0 +1,47 @@ > +/* icc_bus.h > + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus > + * > + * Copyright (c) 2013 Red Hat, Inc > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see <http://www.gnu.org/licenses/> > + */ > +#ifndef ICC_BUS_H > +#define ICC_BUS_H > + > +#include "hw/qdev-core.h" > + > +typedef struct ICCBus { > + BusState qbus; > +} ICCBus; > +#define TYPE_ICC_BUS "ICC" > +#define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS) > + > +typedef struct ICCDevice { > + DeviceState qdev; > +} ICCDevice; > + > +typedef struct ICCDeviceClass { > + DeviceClass parent_class; > + int (*init)(ICCDevice *dev); > +} ICCDeviceClass; > +#define TYPE_ICC_DEVICE "icc-device" > +#define ICC_DEVICE(obj) OBJECT_CHECK(ICCDevice, (obj), TYPE_ICC_DEVICE) > +#define ICC_DEVICE_CLASS(klass) \ > + OBJECT_CLASS_CHECK(ICCDeviceClass, (klass), TYPE_ICC_DEVICE) > +#define ICC_DEVICE_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(ICCDeviceClass, (obj), TYPE_ICC_DEVICE) > + > +BusState *get_icc_bus(void); > + > +#endif > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs > index 28fb4f8..9741e16 100644 > --- a/stubs/Makefile.objs > +++ b/stubs/Makefile.objs > @@ -24,3 +24,4 @@ stub-obj-y += vm-stop.o > stub-obj-y += vmstate.o > stub-obj-$(CONFIG_WIN32) += fd-register.o > stub-obj-y += resume_vcpu.o > +stub-obj-y += get_icc_bus.o > diff --git a/stubs/get_icc_bus.c b/stubs/get_icc_bus.c > new file mode 100644 > index 0000000..43db181 > --- /dev/null > +++ b/stubs/get_icc_bus.c > @@ -0,0 +1,6 @@ > +#include "hw/icc_bus.h" > + > +BusState *get_icc_bus(void) > +{ > + return NULL; > +} > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 631bcd8..ae46f81 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -42,10 +42,11 @@ > > #include "sysemu/sysemu.h" > #include "hw/qdev-properties.h" > +#include "hw/icc_bus.h" > #ifndef CONFIG_USER_ONLY > #include "hw/xen.h" > -#include "hw/sysbus.h" > #include "hw/apic_internal.h" > +#include "exec/address-spaces.h" > #endif > > static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, > @@ -1640,6 +1641,7 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp) > CPUX86State *env; > gchar **model_pieces; > char *name, *features; > + BusState *icc_bus = get_icc_bus(); > > model_pieces = g_strsplit(cpu_model, ",", 2); > if (!model_pieces[0]) { > @@ -1650,6 +1652,10 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp) > features = model_pieces[1]; > > cpu = X86_CPU(object_new(TYPE_X86_CPU)); > + if (icc_bus) { > + qdev_set_parent_bus(DEVICE(cpu), icc_bus); > + object_unref(OBJECT(cpu)); > + } > env = &cpu->env; > env->cpu_model_str = cpu_model; > > @@ -2145,7 +2151,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) > apic_type = "xen-apic"; > } > > - env->apic_state = qdev_try_create(NULL, apic_type); > + env->apic_state = qdev_try_create(get_icc_bus(), apic_type); > if (env->apic_state == NULL) { > error_setg(errp, "APIC device '%s' could not be created", apic_type); > return; > @@ -2176,11 +2182,12 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) > > /* XXX: mapping more APICs at the same memory location */ > if (apic_mapped == 0) { > + APICCommonState *apic = APIC_COMMON(env->apic_state); > /* NOTE: the APIC is directly connected to the CPU - it is not > on the global memory bus. */ > /* XXX: what if the base changes? */ > - sysbus_mmio_map_overlap(SYS_BUS_DEVICE(env->apic_state), 0, > - MSI_ADDR_BASE, 0x1000); > + memory_region_add_subregion_overlap(get_system_memory(), MSI_ADDR_BASE, > + &apic->io_memory, 0x1000); > apic_mapped = 1; > } > } > @@ -2356,6 +2363,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data) > xcc->parent_realize = dc->realize; > dc->props = cpu_x86_properties; > dc->realize = x86_cpu_realizefn; > + dc->bus_type = TYPE_ICC_BUS; > > xcc->parent_reset = cc->reset; > cc->reset = x86_cpu_reset; >
On Wed, 27 Mar 2013 11:57:53 +0100 Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 21/03/2013 15:28, Igor Mammedov ha scritto: > > Introduce hot-pluggable ICC_BUS and convert APIC devices from SysBusDevice > > to ICCDevice wich has ICC_BUS as default one. > > > > * attach APIC and kvmvapic to ICC_BUS during creation > > * use memory API directly instead of using SysBus proxy functions > > * introduce get_icc_bus() for getting access to system wide ICC_BUS > > * make ICC_BUS default one for X86CPU class, so that device_add would > > set it for CPU instead of SysBus > > * attach cold-pluged CPUs to icc_bus in softmmu mode, so they could be > > uplugged in future > > * if kvmvapic init() fails return -1 instead of aborting. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > hw/apic_common.c | 14 ++++++--- > > hw/apic_internal.h | 6 ++-- > > hw/i386/Makefile.objs | 2 +- > > hw/i386/kvmvapic.c | 15 +++++---- > > hw/icc_bus.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++ > > hw/icc_bus.h | 47 ++++++++++++++++++++++++++++++ > > stubs/Makefile.objs | 1 + > > stubs/get_icc_bus.c | 6 ++++ > > target-i386/cpu.c | 16 ++++++++-- > > 9 files changed, 162 insertions(+), 20 deletions(-) > > create mode 100644 hw/icc_bus.c > > create mode 100644 hw/icc_bus.h > > create mode 100644 stubs/get_icc_bus.c > > > > diff --git a/hw/apic_common.c b/hw/apic_common.c > > index d0c2616..61599d4 100644 > > --- a/hw/apic_common.c > > +++ b/hw/apic_common.c > > @@ -21,6 +21,7 @@ > > #include "hw/apic_internal.h" > > #include "trace.h" > > #include "sysemu/kvm.h" > > +#include "qdev.h" > > > > static int apic_irq_delivered; > > bool apic_report_tpr_access; > > @@ -282,9 +283,10 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id) > > return 0; > > } > > > > -static int apic_init_common(SysBusDevice *dev) > > +static int apic_init_common(ICCDevice *dev) > > { > > APICCommonState *s = APIC_COMMON(dev); > > + DeviceState *d = DEVICE(dev); > > APICCommonClass *info; > > static DeviceState *vapic; > > static int apic_no; > > @@ -297,12 +299,14 @@ static int apic_init_common(SysBusDevice *dev) > > info = APIC_COMMON_GET_CLASS(s); > > info->init(s); > > > > - sysbus_init_mmio(dev, &s->io_memory); > > > > /* Note: We need at least 1M to map the VAPIC option ROM */ > > if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK && > > ram_size >= 1024 * 1024) { > > - vapic = sysbus_create_simple("kvmvapic", -1, NULL); > > + vapic = qdev_try_create(d->parent_bus, "kvmvapic"); > > + if ((vapic == NULL) || (qdev_init(vapic) != 0)) { > > + return -1; > > + } > > } > > s->vapic = vapic; > > if (apic_report_tpr_access && info->enable_tpr_reporting) { > > @@ -375,7 +379,7 @@ static Property apic_properties_common[] = { > > > > static void apic_common_class_init(ObjectClass *klass, void *data) > > { > > - SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass); > > + ICCDeviceClass *sc = ICC_DEVICE_CLASS(klass); > > DeviceClass *dc = DEVICE_CLASS(klass); > > > > dc->vmsd = &vmstate_apic_common; > > @@ -387,7 +391,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data) > > > > static const TypeInfo apic_common_type = { > > .name = TYPE_APIC_COMMON, > > - .parent = TYPE_SYS_BUS_DEVICE, > > + .parent = TYPE_ICC_DEVICE, > > .instance_size = sizeof(APICCommonState), > > .class_size = sizeof(APICCommonClass), > > .class_init = apic_common_class_init, > > diff --git a/hw/apic_internal.h b/hw/apic_internal.h > > index 578241f..e7b378e 100644 > > --- a/hw/apic_internal.h > > +++ b/hw/apic_internal.h > > @@ -21,7 +21,7 @@ > > #define QEMU_APIC_INTERNAL_H > > > > #include "exec/memory.h" > > -#include "hw/sysbus.h" > > +#include "hw/icc_bus.h" > > #include "qemu/timer.h" > > > > /* APIC Local Vector Table */ > > @@ -80,7 +80,7 @@ typedef struct APICCommonState APICCommonState; > > > > typedef struct APICCommonClass > > { > > - SysBusDeviceClass parent_class; > > + ICCDeviceClass parent_class; > > > > void (*init)(APICCommonState *s); > > void (*set_base)(APICCommonState *s, uint64_t val); > > @@ -94,7 +94,7 @@ typedef struct APICCommonClass > > } APICCommonClass; > > > > struct APICCommonState { > > - SysBusDevice busdev; > > + ICCDevice busdev; > > > > MemoryRegion io_memory; > > X86CPU *cpu; > > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > > index a78c0b2..316f999 100644 > > --- a/hw/i386/Makefile.objs > > +++ b/hw/i386/Makefile.objs > > @@ -1,5 +1,5 @@ > > obj-y += mc146818rtc.o > > -obj-y += apic_common.o apic.o > > +obj-y += apic_common.o apic.o icc_bus.o > > obj-y += sga.o ioapic_common.o ioapic.o piix_pci.o > > obj-y += vmport.o > > obj-y += pci/pci-hotplug.o wdt_ib700.o > > diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c > > index 21551a5..7de75db 100644 > > --- a/hw/i386/kvmvapic.c > > +++ b/hw/i386/kvmvapic.c > > @@ -12,6 +12,8 @@ > > #include "sysemu/cpus.h" > > #include "sysemu/kvm.h" > > #include "hw/apic_internal.h" > > +#include "migration/vmstate.h" > > +#include "exec/address-spaces.h" > > > > #define APIC_DEFAULT_ADDRESS 0xfee00000 > > > > @@ -49,7 +51,7 @@ typedef struct GuestROMState { > > } QEMU_PACKED GuestROMState; > > > > typedef struct VAPICROMState { > > - SysBusDevice busdev; > > + ICCDevice busdev; > > MemoryRegion io; > > MemoryRegion rom; > > uint32_t state; > > @@ -581,7 +583,7 @@ static void vapic_map_rom_writable(VAPICROMState *s) > > size_t rom_size; > > uint8_t *ram; > > > > - as = sysbus_address_space(&s->busdev); > > + as = get_system_memory(); > > > > if (s->rom_mapped_writable) { > > memory_region_del_subregion(as, &s->rom); > > @@ -693,13 +695,12 @@ static const MemoryRegionOps vapic_ops = { > > .endianness = DEVICE_NATIVE_ENDIAN, > > }; > > > > -static int vapic_init(SysBusDevice *dev) > > +static int vapic_init(ICCDevice *dev) > > { > > VAPICROMState *s = VAPIC_DEVICE(dev); > > > > memory_region_init_io(&s->io, &vapic_ops, s, "kvmvapic", 2); > > - sysbus_add_io(dev, VAPIC_IO_PORT, &s->io); > > - sysbus_init_ioports(dev, VAPIC_IO_PORT, 2); > > + memory_region_add_subregion(get_system_io(), VAPIC_IO_PORT, &s->io); > > > > option_rom[nb_option_roms].name = "kvmvapic.bin"; > > option_rom[nb_option_roms].bootindex = -1; > > @@ -801,7 +802,7 @@ static const VMStateDescription vmstate_vapic = { > > > > static void vapic_class_init(ObjectClass *klass, void *data) > > { > > - SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass); > > + ICCDeviceClass *sc = ICC_DEVICE_CLASS(klass); > > DeviceClass *dc = DEVICE_CLASS(klass); > > > > dc->no_user = 1; > > @@ -812,7 +813,7 @@ static void vapic_class_init(ObjectClass *klass, void *data) > > > > static const TypeInfo vapic_type = { > > .name = TYPE_VAPIC_DEVICE, > > - .parent = TYPE_SYS_BUS_DEVICE, > > + .parent = TYPE_ICC_DEVICE, > > .instance_size = sizeof(VAPICROMState), > > .class_init = vapic_class_init, > > }; > > diff --git a/hw/icc_bus.c b/hw/icc_bus.c > > new file mode 100644 > > index 0000000..b170648 > > --- /dev/null > > +++ b/hw/icc_bus.c > > @@ -0,0 +1,75 @@ > > +/* icc_bus.c > > + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus > > + * > > + * Copyright (c) 2013 Red Hat, Inc > > + * > > + * This library is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU Lesser General Public > > + * License as published by the Free Software Foundation; either > > + * version 2 of the License, or (at your option) any later version. > > + * > > + * This library is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + * Lesser General Public License for more details. > > + * > > + * You should have received a copy of the GNU Lesser General Public > > + * License along with this library; if not, see <http://www.gnu.org/licenses/> > > + */ > > +#include "icc_bus.h" > > +#include "qemu/module.h" > > + > > +static const TypeInfo icc_bus_info = { > > + .name = TYPE_ICC_BUS, > > + .parent = TYPE_BUS, > > + .instance_size = sizeof(ICCBus), > > +}; > > + > > +static int icc_device_init(DeviceState *dev) > > +{ > > + ICCDevice *id = ICC_DEVICE(dev); > > + ICCDeviceClass *k = ICC_DEVICE_GET_CLASS(id); > > + > > + return k->init(id); > > +} > > + > > +static void icc_device_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *k = DEVICE_CLASS(klass); > > + > > + k->init = icc_device_init; > > + k->bus_type = TYPE_ICC_BUS; > > +} > > + > > +static const TypeInfo icc_device_info = { > > + .name = TYPE_ICC_DEVICE, > > + .parent = TYPE_DEVICE, > > + .abstract = true, > > + .instance_size = sizeof(ICCDevice), > > + .class_size = sizeof(ICCDeviceClass), > > + .class_init = icc_device_class_init, > > +}; > > + > > +static BusState *icc_bus; > > + > > +BusState *get_icc_bus(void) > > +{ > > + if (icc_bus == NULL) { > > + icc_bus = g_malloc0(icc_bus_info.instance_size); > > + qbus_create_inplace(icc_bus, TYPE_ICC_BUS, NULL, "icc-bus"); > > + icc_bus->allow_hotplug = 1; > > + OBJECT(icc_bus)->free = g_free; > > You can just use qbus_create. > > > + object_property_add_child(container_get(qdev_get_machine(), > > + "/unattached"), > > Please put it straight under /machine, not /unattached. sure > > But actually, you lack a device that instantiates the bus. That device > would be a simple container device similar hw/a15mpcore.c, and would About a year ago something like that device was proposed "cpu_sockets", but there was suggestion to drop it: http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02157.html So I've opted in favor of parent-less bus, but I could create it explicitly at board level as you suggest. However having parent sysbus device for icc-bus will allow to see it via monitor info qtree and reset on SysBus could be propagated through it as well, it may be good to add it later. > also take care of registering the MSI memory region. Create that device it probably impossible to do, because [kvm]apic sets its private callbacks to it when initializing memory region, and CPU just maps it at specified address, or this region and APIC might not even exists at all if qemu started with -apic feature flag. > in the boards, and you won't need a special object_property_add_child. > > get_icc_bus() would then become simply > object_resolve_path_type("icc-bus", TYPE_ICC_BUS, NULL) or something > like that. > > Paolo > > > + "icc-bus", OBJECT(icc_bus), NULL); > > + } > > + return BUS(icc_bus); > > +} > > + > > +static void icc_bus_register_types(void) > > +{ > > + type_register_static(&icc_bus_info); > > + type_register_static(&icc_device_info); > > +} > > + > > +type_init(icc_bus_register_types) > > diff --git a/hw/icc_bus.h b/hw/icc_bus.h > > new file mode 100644 > > index 0000000..a00fef5 > > --- /dev/null > > +++ b/hw/icc_bus.h > > @@ -0,0 +1,47 @@ > > +/* icc_bus.h > > + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus > > + * > > + * Copyright (c) 2013 Red Hat, Inc > > + * > > + * This library is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU Lesser General Public > > + * License as published by the Free Software Foundation; either > > + * version 2 of the License, or (at your option) any later version. > > + * > > + * This library is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + * Lesser General Public License for more details. > > + * > > + * You should have received a copy of the GNU Lesser General Public > > + * License along with this library; if not, see <http://www.gnu.org/licenses/> > > + */ > > +#ifndef ICC_BUS_H > > +#define ICC_BUS_H > > + > > +#include "hw/qdev-core.h" > > + > > +typedef struct ICCBus { > > + BusState qbus; > > +} ICCBus; > > +#define TYPE_ICC_BUS "ICC" > > +#define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS) > > + > > +typedef struct ICCDevice { > > + DeviceState qdev; > > +} ICCDevice; > > + > > +typedef struct ICCDeviceClass { > > + DeviceClass parent_class; > > + int (*init)(ICCDevice *dev); > > +} ICCDeviceClass; > > +#define TYPE_ICC_DEVICE "icc-device" > > +#define ICC_DEVICE(obj) OBJECT_CHECK(ICCDevice, (obj), TYPE_ICC_DEVICE) > > +#define ICC_DEVICE_CLASS(klass) \ > > + OBJECT_CLASS_CHECK(ICCDeviceClass, (klass), TYPE_ICC_DEVICE) > > +#define ICC_DEVICE_GET_CLASS(obj) \ > > + OBJECT_GET_CLASS(ICCDeviceClass, (obj), TYPE_ICC_DEVICE) > > + > > +BusState *get_icc_bus(void); > > + > > +#endif > > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs > > index 28fb4f8..9741e16 100644 > > --- a/stubs/Makefile.objs > > +++ b/stubs/Makefile.objs > > @@ -24,3 +24,4 @@ stub-obj-y += vm-stop.o > > stub-obj-y += vmstate.o > > stub-obj-$(CONFIG_WIN32) += fd-register.o > > stub-obj-y += resume_vcpu.o > > +stub-obj-y += get_icc_bus.o > > diff --git a/stubs/get_icc_bus.c b/stubs/get_icc_bus.c > > new file mode 100644 > > index 0000000..43db181 > > --- /dev/null > > +++ b/stubs/get_icc_bus.c > > @@ -0,0 +1,6 @@ > > +#include "hw/icc_bus.h" > > + > > +BusState *get_icc_bus(void) > > +{ > > + return NULL; > > +} > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 631bcd8..ae46f81 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -42,10 +42,11 @@ > > > > #include "sysemu/sysemu.h" > > #include "hw/qdev-properties.h" > > +#include "hw/icc_bus.h" > > #ifndef CONFIG_USER_ONLY > > #include "hw/xen.h" > > -#include "hw/sysbus.h" > > #include "hw/apic_internal.h" > > +#include "exec/address-spaces.h" > > #endif > > > > static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, > > @@ -1640,6 +1641,7 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp) > > CPUX86State *env; > > gchar **model_pieces; > > char *name, *features; > > + BusState *icc_bus = get_icc_bus(); > > > > model_pieces = g_strsplit(cpu_model, ",", 2); > > if (!model_pieces[0]) { > > @@ -1650,6 +1652,10 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp) > > features = model_pieces[1]; > > > > cpu = X86_CPU(object_new(TYPE_X86_CPU)); > > + if (icc_bus) { > > + qdev_set_parent_bus(DEVICE(cpu), icc_bus); > > + object_unref(OBJECT(cpu)); > > + } > > env = &cpu->env; > > env->cpu_model_str = cpu_model; > > > > @@ -2145,7 +2151,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) > > apic_type = "xen-apic"; > > } > > > > - env->apic_state = qdev_try_create(NULL, apic_type); > > + env->apic_state = qdev_try_create(get_icc_bus(), apic_type); > > if (env->apic_state == NULL) { > > error_setg(errp, "APIC device '%s' could not be created", apic_type); > > return; > > @@ -2176,11 +2182,12 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) > > > > /* XXX: mapping more APICs at the same memory location */ > > if (apic_mapped == 0) { > > + APICCommonState *apic = APIC_COMMON(env->apic_state); > > /* NOTE: the APIC is directly connected to the CPU - it is not > > on the global memory bus. */ > > /* XXX: what if the base changes? */ > > - sysbus_mmio_map_overlap(SYS_BUS_DEVICE(env->apic_state), 0, > > - MSI_ADDR_BASE, 0x1000); > > + memory_region_add_subregion_overlap(get_system_memory(), MSI_ADDR_BASE, > > + &apic->io_memory, 0x1000); > > apic_mapped = 1; > > } > > } > > @@ -2356,6 +2363,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data) > > xcc->parent_realize = dc->realize; > > dc->props = cpu_x86_properties; > > dc->realize = x86_cpu_realizefn; > > + dc->bus_type = TYPE_ICC_BUS; > > > > xcc->parent_reset = cc->reset; > > cc->reset = x86_cpu_reset; > > >
在 2013-03-28四的 11:55 +0100，Igor Mammedov写道： > On Wed, 27 Mar 2013 11:57:53 +0100 > Paolo Bonzini <pbonzini@redhat.com> wrote: > > > Il 21/03/2013 15:28, Igor Mammedov ha scritto: > > > Introduce hot-pluggable ICC_BUS and convert APIC devices from SysBusDevice > > > to ICCDevice wich has ICC_BUS as default one. > > > > > > * attach APIC and kvmvapic to ICC_BUS during creation > > > * use memory API directly instead of using SysBus proxy functions > > > * introduce get_icc_bus() for getting access to system wide ICC_BUS > > > * make ICC_BUS default one for X86CPU class, so that device_add would > > > set it for CPU instead of SysBus > > > * attach cold-pluged CPUs to icc_bus in softmmu mode, so they could be > > > uplugged in future > > > * if kvmvapic init() fails return -1 instead of aborting. [...] > > > +}; > > > + > > > +static BusState *icc_bus; > > > + > > > +BusState *get_icc_bus(void) > > > +{ > > > + if (icc_bus == NULL) { > > > + icc_bus = g_malloc0(icc_bus_info.instance_size); > > > + qbus_create_inplace(icc_bus, TYPE_ICC_BUS, NULL, "icc-bus"); > > > + icc_bus->allow_hotplug = 1; > > > + OBJECT(icc_bus)->free = g_free; > > > > You can just use qbus_create. > > > > > + object_property_add_child(container_get(qdev_get_machine(), > > > + "/unattached"), > > > > Please put it straight under /machine, not /unattached. > > sure > > > > > But actually, you lack a device that instantiates the bus. That device > > would be a simple container device similar hw/a15mpcore.c, and would > > About a year ago something like that device was proposed "cpu_sockets", but what about link<Socket> implementation which was mentioned by Andreas few days ago at a thread discussion for acpi memory hotplug? (http://list-archives.org/2012/12/18/qemu-devel-nongnu-org/rfc-patch-v4-00-30-acpi-memory-hotplug/f/4115902514) do consider to implement a link<> property between CPU and cpu_socket? > there was suggestion to drop it: > http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02157.html > So I've opted in favor of parent-less bus, but I could create it > explicitly at board level as you suggest. > > However having parent sysbus device for icc-bus will allow to see it via > monitor info qtree and reset on SysBus could be propagated through it as well, > it may be good to add it later. > > > also take care of registering the MSI memory region. Create that device > > it probably impossible to do, because [kvm]apic sets its private callbacks > to it when initializing memory region, and CPU just maps it at specified > address, or this region and APIC might not even exists at all if qemu started > with -apic feature flag. > > > in the boards, and you won't need a special object_property_add_child. > > > > get_icc_bus() would then become simply > > object_resolve_path_type("icc-bus", TYPE_ICC_BUS, NULL) or something > > like that. > > > > Paolo > > > > > + "icc-bus", OBJECT(icc_bus), NULL); > > > + } > > > + return BUS(icc_bus); > > > +} > > > +
On Fri, 29 Mar 2013 15:22:47 +0800 li guang <lig.fnst@cn.fujitsu.com> wrote: > 在 2013-03-28四的 11:55 +0100，Igor Mammedov写道： > > On Wed, 27 Mar 2013 11:57:53 +0100 > > Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > > Il 21/03/2013 15:28, Igor Mammedov ha scritto: > > > > Introduce hot-pluggable ICC_BUS and convert APIC devices from SysBusDevice > > > > to ICCDevice wich has ICC_BUS as default one. > > > > > > > > * attach APIC and kvmvapic to ICC_BUS during creation > > > > * use memory API directly instead of using SysBus proxy functions > > > > * introduce get_icc_bus() for getting access to system wide ICC_BUS > > > > * make ICC_BUS default one for X86CPU class, so that device_add would > > > > set it for CPU instead of SysBus > > > > * attach cold-pluged CPUs to icc_bus in softmmu mode, so they could be > > > > uplugged in future > > > > * if kvmvapic init() fails return -1 instead of aborting. > [...] > > > > +}; > > > > + > > > > +static BusState *icc_bus; > > > > + > > > > +BusState *get_icc_bus(void) > > > > +{ > > > > + if (icc_bus == NULL) { > > > > + icc_bus = g_malloc0(icc_bus_info.instance_size); > > > > + qbus_create_inplace(icc_bus, TYPE_ICC_BUS, NULL, "icc-bus"); > > > > + icc_bus->allow_hotplug = 1; > > > > + OBJECT(icc_bus)->free = g_free; > > > > > > You can just use qbus_create. > > > > > > > + object_property_add_child(container_get(qdev_get_machine(), > > > > + "/unattached"), > > > > > > Please put it straight under /machine, not /unattached. > > > > sure > > > > > > > > But actually, you lack a device that instantiates the bus. That device > > > would be a simple container device similar hw/a15mpcore.c, and would > > > > About a year ago something like that device was proposed "cpu_sockets", but > > what about link<Socket> implementation which was mentioned by Andreas > few days ago at a thread discussion for acpi memory hotplug? > (http://list-archives.org/2012/12/18/qemu-devel-nongnu-org/rfc-patch-v4-00-30-acpi-memory-hotplug/f/4115902514) > do consider to implement a link<> property between CPU and cpu_socket? since CPU slowly moving towards being able to be added via device_add, there is a limited usefulness in link<> property. It still could be used to expose possible APIC IDs to user so that user won't have calculate them by itself. But it requires fixing device_realize first so I haven't included it in this series. > > > there was suggestion to drop it: > > http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02157.html > > So I've opted in favor of parent-less bus, but I could create it > > explicitly at board level as you suggest. > > > > However having parent sysbus device for icc-bus will allow to see it via > > monitor info qtree and reset on SysBus could be propagated through it as well, > > it may be good to add it later. > > > > > also take care of registering the MSI memory region. Create that device > > > > it probably impossible to do, because [kvm]apic sets its private callbacks > > to it when initializing memory region, and CPU just maps it at specified > > address, or this region and APIC might not even exists at all if qemu started > > with -apic feature flag. > > > > > in the boards, and you won't need a special object_property_add_child. > > > > > > get_icc_bus() would then become simply > > > object_resolve_path_type("icc-bus", TYPE_ICC_BUS, NULL) or something > > > like that. > > > > > > Paolo > > > > > > > + "icc-bus", OBJECT(icc_bus), NULL); > > > > + } > > > > + return BUS(icc_bus); > > > > +} > > > > + > > >
Am 28.03.2013 11:55, schrieb Igor Mammedov: > On Wed, 27 Mar 2013 11:57:53 +0100 > Paolo Bonzini <pbonzini@redhat.com> wrote: > >> Il 21/03/2013 15:28, Igor Mammedov ha scritto: >>> +static BusState *icc_bus; >>> + >>> +BusState *get_icc_bus(void) >>> +{ >>> + if (icc_bus == NULL) { >>> + icc_bus = g_malloc0(icc_bus_info.instance_size); >>> + qbus_create_inplace(icc_bus, TYPE_ICC_BUS, NULL, "icc-bus"); >>> + icc_bus->allow_hotplug = 1; >>> + OBJECT(icc_bus)->free = g_free; >> >> You can just use qbus_create. >> >>> + object_property_add_child(container_get(qdev_get_machine(), >>> + "/unattached"), >> >> Please put it straight under /machine, not /unattached. > > sure > >> >> But actually, you lack a device that instantiates the bus. That device >> would be a simple container device similar hw/a15mpcore.c, and would > > About a year ago something like that device was proposed "cpu_sockets", but > there was suggestion to drop it: > http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02157.html > So I've opted in favor of parent-less bus, but I could create it > explicitly at board level as you suggest. > > However having parent sysbus device for icc-bus will allow to see it via > monitor info qtree As I said on IRC, info qtree should not influence our design decisions. All that's missing is a convenient script that pretty-prints the output from the QOM graph using qom-list and qom-get in a non-interactive way. Whether CPUs or busses are shown in info qtree is totally irrelevant IMO by now. > and reset on SysBus could be propagated through it as well, > it may be good to add it later. What exactly is this ICC bus needed for? Is it just a workaround for qdev_init() or something making assumptions about busses and falling back to SysBus otherwise? Or about avoiding SysBus? And where would it sit if be hosted you think of something like Qseven modules - on the board or on the module? I don't see pc.c being touched here at all... Andreas
On Thu, 04 Apr 2013 13:10:54 +0200 Andreas Färber <afaerber@suse.de> wrote: > Am 28.03.2013 11:55, schrieb Igor Mammedov: > > On Wed, 27 Mar 2013 11:57:53 +0100 > > Paolo Bonzini <pbonzini@redhat.com> wrote: > > > >> Il 21/03/2013 15:28, Igor Mammedov ha scritto: > >>> +static BusState *icc_bus; > >>> + > >>> +BusState *get_icc_bus(void) > >>> +{ > >>> + if (icc_bus == NULL) { > >>> + icc_bus = g_malloc0(icc_bus_info.instance_size); > >>> + qbus_create_inplace(icc_bus, TYPE_ICC_BUS, NULL, "icc-bus"); > >>> + icc_bus->allow_hotplug = 1; > >>> + OBJECT(icc_bus)->free = g_free; > >> > >> You can just use qbus_create. > >> > >>> + object_property_add_child(container_get(qdev_get_machine(), > >>> + "/unattached"), > >> > >> Please put it straight under /machine, not /unattached. > > > > sure > > > >> > >> But actually, you lack a device that instantiates the bus. That device > >> would be a simple container device similar hw/a15mpcore.c, and would > > > > About a year ago something like that device was proposed "cpu_sockets", but > > there was suggestion to drop it: > > http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02157.html > > So I've opted in favor of parent-less bus, but I could create it > > explicitly at board level as you suggest. > > > > However having parent sysbus device for icc-bus will allow to see it via > > monitor info qtree > > As I said on IRC, info qtree should not influence our design decisions. > All that's missing is a convenient script that pretty-prints the output > from the QOM graph using qom-list and qom-get in a non-interactive way. > Whether CPUs or busses are shown in info qtree is totally irrelevant IMO > by now. > > > and reset on SysBus could be propagated through it as well, > > it may be good to add it later. > > What exactly is this ICC bus needed for? Is it just a workaround for > qdev_init() or something making assumptions about busses and falling > back to SysBus otherwise? Or about avoiding SysBus? SysBus doesn't allow hotplug on it, I've asked Anthony on IRC and short answer was do not touch what doesn't exists. Hence hotplug-able bus that exists on real hw. x86 hw had ICC bus some time ago through which APICs, IOAPICs and CPUs communicated. So here it is. More close to the code: - device_add assigns SysBus to any bus-less device, therefore CPU hits assertion because hotplug is not allowed on bus. - when x86 CPU is realizing, it creates APIC and possible kvmvapic, which are SysBus devices presently, it triggers the same assert as above. > > And where would it sit if be hosted you think of something like Qseven > modules - on the board or on the module? I don't see pc.c being touched > here at all... I've reworked it quite a bit to Paolo's suggestions. now board creates following QOM tree: /machine/icc-bridge<SYS_BUS_DEVICE>/--->child<icc-bus>/->link<apic[..]> | | | ->link<kvmvapic[..]> | | | ->link<cpu[..] | | | ->link<ioapic> ->child<ioapic> where icc-bridge is created by board and serves as a parent, which creates icc-bus, ioapic and takes care about mapping memory regions registered by *apic* devices. > > > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
diff --git a/hw/apic_common.c b/hw/apic_common.c index d0c2616..61599d4 100644 --- a/hw/apic_common.c +++ b/hw/apic_common.c @@ -21,6 +21,7 @@ #include "hw/apic_internal.h" #include "trace.h" #include "sysemu/kvm.h" +#include "qdev.h" static int apic_irq_delivered; bool apic_report_tpr_access; @@ -282,9 +283,10 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id) return 0; } -static int apic_init_common(SysBusDevice *dev) +static int apic_init_common(ICCDevice *dev) { APICCommonState *s = APIC_COMMON(dev); + DeviceState *d = DEVICE(dev); APICCommonClass *info; static DeviceState *vapic; static int apic_no; @@ -297,12 +299,14 @@ static int apic_init_common(SysBusDevice *dev) info = APIC_COMMON_GET_CLASS(s); info->init(s); - sysbus_init_mmio(dev, &s->io_memory); /* Note: We need at least 1M to map the VAPIC option ROM */ if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK && ram_size >= 1024 * 1024) { - vapic = sysbus_create_simple("kvmvapic", -1, NULL); + vapic = qdev_try_create(d->parent_bus, "kvmvapic"); + if ((vapic == NULL) || (qdev_init(vapic) != 0)) { + return -1; + } } s->vapic = vapic; if (apic_report_tpr_access && info->enable_tpr_reporting) { @@ -375,7 +379,7 @@ static Property apic_properties_common[] = { static void apic_common_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass); + ICCDeviceClass *sc = ICC_DEVICE_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); dc->vmsd = &vmstate_apic_common; @@ -387,7 +391,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data) static const TypeInfo apic_common_type = { .name = TYPE_APIC_COMMON, - .parent = TYPE_SYS_BUS_DEVICE, + .parent = TYPE_ICC_DEVICE, .instance_size = sizeof(APICCommonState), .class_size = sizeof(APICCommonClass), .class_init = apic_common_class_init, diff --git a/hw/apic_internal.h b/hw/apic_internal.h index 578241f..e7b378e 100644 --- a/hw/apic_internal.h +++ b/hw/apic_internal.h @@ -21,7 +21,7 @@ #define QEMU_APIC_INTERNAL_H #include "exec/memory.h" -#include "hw/sysbus.h" +#include "hw/icc_bus.h" #include "qemu/timer.h" /* APIC Local Vector Table */ @@ -80,7 +80,7 @@ typedef struct APICCommonState APICCommonState; typedef struct APICCommonClass { - SysBusDeviceClass parent_class; + ICCDeviceClass parent_class; void (*init)(APICCommonState *s); void (*set_base)(APICCommonState *s, uint64_t val); @@ -94,7 +94,7 @@ typedef struct APICCommonClass } APICCommonClass; struct APICCommonState { - SysBusDevice busdev; + ICCDevice busdev; MemoryRegion io_memory; X86CPU *cpu; diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs index a78c0b2..316f999 100644 --- a/hw/i386/Makefile.objs +++ b/hw/i386/Makefile.objs @@ -1,5 +1,5 @@ obj-y += mc146818rtc.o -obj-y += apic_common.o apic.o +obj-y += apic_common.o apic.o icc_bus.o obj-y += sga.o ioapic_common.o ioapic.o piix_pci.o obj-y += vmport.o obj-y += pci/pci-hotplug.o wdt_ib700.o diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c index 21551a5..7de75db 100644 --- a/hw/i386/kvmvapic.c +++ b/hw/i386/kvmvapic.c @@ -12,6 +12,8 @@ #include "sysemu/cpus.h" #include "sysemu/kvm.h" #include "hw/apic_internal.h" +#include "migration/vmstate.h" +#include "exec/address-spaces.h" #define APIC_DEFAULT_ADDRESS 0xfee00000 @@ -49,7 +51,7 @@ typedef struct GuestROMState { } QEMU_PACKED GuestROMState; typedef struct VAPICROMState { - SysBusDevice busdev; + ICCDevice busdev; MemoryRegion io; MemoryRegion rom; uint32_t state; @@ -581,7 +583,7 @@ static void vapic_map_rom_writable(VAPICROMState *s) size_t rom_size; uint8_t *ram; - as = sysbus_address_space(&s->busdev); + as = get_system_memory(); if (s->rom_mapped_writable) { memory_region_del_subregion(as, &s->rom); @@ -693,13 +695,12 @@ static const MemoryRegionOps vapic_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static int vapic_init(SysBusDevice *dev) +static int vapic_init(ICCDevice *dev) { VAPICROMState *s = VAPIC_DEVICE(dev); memory_region_init_io(&s->io, &vapic_ops, s, "kvmvapic", 2); - sysbus_add_io(dev, VAPIC_IO_PORT, &s->io); - sysbus_init_ioports(dev, VAPIC_IO_PORT, 2); + memory_region_add_subregion(get_system_io(), VAPIC_IO_PORT, &s->io); option_rom[nb_option_roms].name = "kvmvapic.bin"; option_rom[nb_option_roms].bootindex = -1; @@ -801,7 +802,7 @@ static const VMStateDescription vmstate_vapic = { static void vapic_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass); + ICCDeviceClass *sc = ICC_DEVICE_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); dc->no_user = 1; @@ -812,7 +813,7 @@ static void vapic_class_init(ObjectClass *klass, void *data) static const TypeInfo vapic_type = { .name = TYPE_VAPIC_DEVICE, - .parent = TYPE_SYS_BUS_DEVICE, + .parent = TYPE_ICC_DEVICE, .instance_size = sizeof(VAPICROMState), .class_init = vapic_class_init, }; diff --git a/hw/icc_bus.c b/hw/icc_bus.c new file mode 100644 index 0000000..b170648 --- /dev/null +++ b/hw/icc_bus.c @@ -0,0 +1,75 @@ +/* icc_bus.c + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus + * + * Copyright (c) 2013 Red Hat, Inc + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see <http://www.gnu.org/licenses/> + */ +#include "icc_bus.h" +#include "qemu/module.h" + +static const TypeInfo icc_bus_info = { + .name = TYPE_ICC_BUS, + .parent = TYPE_BUS, + .instance_size = sizeof(ICCBus), +}; + +static int icc_device_init(DeviceState *dev) +{ + ICCDevice *id = ICC_DEVICE(dev); + ICCDeviceClass *k = ICC_DEVICE_GET_CLASS(id); + + return k->init(id); +} + +static void icc_device_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *k = DEVICE_CLASS(klass); + + k->init = icc_device_init; + k->bus_type = TYPE_ICC_BUS; +} + +static const TypeInfo icc_device_info = { + .name = TYPE_ICC_DEVICE, + .parent = TYPE_DEVICE, + .abstract = true, + .instance_size = sizeof(ICCDevice), + .class_size = sizeof(ICCDeviceClass), + .class_init = icc_device_class_init, +}; + +static BusState *icc_bus; + +BusState *get_icc_bus(void) +{ + if (icc_bus == NULL) { + icc_bus = g_malloc0(icc_bus_info.instance_size); + qbus_create_inplace(icc_bus, TYPE_ICC_BUS, NULL, "icc-bus"); + icc_bus->allow_hotplug = 1; + OBJECT(icc_bus)->free = g_free; + object_property_add_child(container_get(qdev_get_machine(), + "/unattached"), + "icc-bus", OBJECT(icc_bus), NULL); + } + return BUS(icc_bus); +} + +static void icc_bus_register_types(void) +{ + type_register_static(&icc_bus_info); + type_register_static(&icc_device_info); +} + +type_init(icc_bus_register_types) diff --git a/hw/icc_bus.h b/hw/icc_bus.h new file mode 100644 index 0000000..a00fef5 --- /dev/null +++ b/hw/icc_bus.h @@ -0,0 +1,47 @@ +/* icc_bus.h + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus + * + * Copyright (c) 2013 Red Hat, Inc + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see <http://www.gnu.org/licenses/> + */ +#ifndef ICC_BUS_H +#define ICC_BUS_H + +#include "hw/qdev-core.h" + +typedef struct ICCBus { + BusState qbus; +} ICCBus; +#define TYPE_ICC_BUS "ICC" +#define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS) + +typedef struct ICCDevice { + DeviceState qdev; +} ICCDevice; + +typedef struct ICCDeviceClass { + DeviceClass parent_class; + int (*init)(ICCDevice *dev); +} ICCDeviceClass; +#define TYPE_ICC_DEVICE "icc-device" +#define ICC_DEVICE(obj) OBJECT_CHECK(ICCDevice, (obj), TYPE_ICC_DEVICE) +#define ICC_DEVICE_CLASS(klass) \ + OBJECT_CLASS_CHECK(ICCDeviceClass, (klass), TYPE_ICC_DEVICE) +#define ICC_DEVICE_GET_CLASS(obj) \ + OBJECT_GET_CLASS(ICCDeviceClass, (obj), TYPE_ICC_DEVICE) + +BusState *get_icc_bus(void); + +#endif diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 28fb4f8..9741e16 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -24,3 +24,4 @@ stub-obj-y += vm-stop.o stub-obj-y += vmstate.o stub-obj-$(CONFIG_WIN32) += fd-register.o stub-obj-y += resume_vcpu.o +stub-obj-y += get_icc_bus.o diff --git a/stubs/get_icc_bus.c b/stubs/get_icc_bus.c new file mode 100644 index 0000000..43db181 --- /dev/null +++ b/stubs/get_icc_bus.c @@ -0,0 +1,6 @@ +#include "hw/icc_bus.h" + +BusState *get_icc_bus(void) +{ + return NULL; +} diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 631bcd8..ae46f81 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -42,10 +42,11 @@ #include "sysemu/sysemu.h" #include "hw/qdev-properties.h" +#include "hw/icc_bus.h" #ifndef CONFIG_USER_ONLY #include "hw/xen.h" -#include "hw/sysbus.h" #include "hw/apic_internal.h" +#include "exec/address-spaces.h" #endif static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, @@ -1640,6 +1641,7 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp) CPUX86State *env; gchar **model_pieces; char *name, *features; + BusState *icc_bus = get_icc_bus(); model_pieces = g_strsplit(cpu_model, ",", 2); if (!model_pieces[0]) { @@ -1650,6 +1652,10 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp) features = model_pieces[1]; cpu = X86_CPU(object_new(TYPE_X86_CPU)); + if (icc_bus) { + qdev_set_parent_bus(DEVICE(cpu), icc_bus); + object_unref(OBJECT(cpu)); + } env = &cpu->env; env->cpu_model_str = cpu_model; @@ -2145,7 +2151,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) apic_type = "xen-apic"; } - env->apic_state = qdev_try_create(NULL, apic_type); + env->apic_state = qdev_try_create(get_icc_bus(), apic_type); if (env->apic_state == NULL) { error_setg(errp, "APIC device '%s' could not be created", apic_type); return; @@ -2176,11 +2182,12 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) /* XXX: mapping more APICs at the same memory location */ if (apic_mapped == 0) { + APICCommonState *apic = APIC_COMMON(env->apic_state); /* NOTE: the APIC is directly connected to the CPU - it is not on the global memory bus. */ /* XXX: what if the base changes? */ - sysbus_mmio_map_overlap(SYS_BUS_DEVICE(env->apic_state), 0, - MSI_ADDR_BASE, 0x1000); + memory_region_add_subregion_overlap(get_system_memory(), MSI_ADDR_BASE, + &apic->io_memory, 0x1000); apic_mapped = 1; } } @@ -2356,6 +2363,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data) xcc->parent_realize = dc->realize; dc->props = cpu_x86_properties; dc->realize = x86_cpu_realizefn; + dc->bus_type = TYPE_ICC_BUS; xcc->parent_reset = cc->reset; cc->reset = x86_cpu_reset;
Introduce hot-pluggable ICC_BUS and convert APIC devices from SysBusDevice to ICCDevice wich has ICC_BUS as default one. * attach APIC and kvmvapic to ICC_BUS during creation * use memory API directly instead of using SysBus proxy functions * introduce get_icc_bus() for getting access to system wide ICC_BUS * make ICC_BUS default one for X86CPU class, so that device_add would set it for CPU instead of SysBus * attach cold-pluged CPUs to icc_bus in softmmu mode, so they could be uplugged in future * if kvmvapic init() fails return -1 instead of aborting. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/apic_common.c | 14 ++++++--- hw/apic_internal.h | 6 ++-- hw/i386/Makefile.objs | 2 +- hw/i386/kvmvapic.c | 15 +++++---- hw/icc_bus.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++ hw/icc_bus.h | 47 ++++++++++++++++++++++++++++++ stubs/Makefile.objs | 1 + stubs/get_icc_bus.c | 6 ++++ target-i386/cpu.c | 16 ++++++++-- 9 files changed, 162 insertions(+), 20 deletions(-) create mode 100644 hw/icc_bus.c create mode 100644 hw/icc_bus.h create mode 100644 stubs/get_icc_bus.c