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 <imamm...@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;
>
