On Fri, 10 Apr 2015 16:18:06 +0800 Chen Fan <chen.fan.f...@cn.fujitsu.com> wrote:
> ICC bus was invented only to provide hotplug capability to > CPU and APIC because at the time being hotplug was available only for > BUS attached devices. > > Now this patch is to drop ICC bus impl, and switch to bus-less > CPU+APIC hotplug, handling them in the same manner as pc-dimm. > and due to APIC is bus-less, so we should reset it as CPU reset. > > Signed-off-by: Chen Fan <chen.fan.f...@cn.fujitsu.com> > --- > default-configs/i386-softmmu.mak | 1 - > default-configs/x86_64-softmmu.mak | 1 - > hw/cpu/Makefile.objs | 2 - > hw/cpu/icc_bus.c | 118 > ------------------------------------- > hw/i386/pc.c | 19 ++---- > hw/i386/pc_piix.c | 9 +-- > hw/i386/pc_q35.c | 9 +-- > hw/intc/apic.c | 6 +- > hw/intc/apic_common.c | 5 +- > include/hw/cpu/icc_bus.h | 82 -------------------------- > include/hw/i386/apic_internal.h | 5 +- > include/hw/i386/pc.h | 2 +- > target-i386/cpu.c | 13 ++-- > 13 files changed, 24 insertions(+), 248 deletions(-) > delete mode 100644 hw/cpu/icc_bus.c > delete mode 100644 include/hw/cpu/icc_bus.h > > diff --git a/default-configs/i386-softmmu.mak > b/default-configs/i386-softmmu.mak > index 6a74e00..3ac6324 100644 > --- a/default-configs/i386-softmmu.mak > +++ b/default-configs/i386-softmmu.mak > @@ -39,7 +39,6 @@ CONFIG_LPC_ICH9=y > CONFIG_PCI_Q35=y > CONFIG_APIC=y > CONFIG_IOAPIC=y > -CONFIG_ICC_BUS=y > CONFIG_PVPANIC=y > CONFIG_MEM_HOTPLUG=y > CONFIG_XIO3130=y > diff --git a/default-configs/x86_64-softmmu.mak > b/default-configs/x86_64-softmmu.mak > index 46b87dd..5c89ea8 100644 > --- a/default-configs/x86_64-softmmu.mak > +++ b/default-configs/x86_64-softmmu.mak > @@ -39,7 +39,6 @@ CONFIG_LPC_ICH9=y > CONFIG_PCI_Q35=y > CONFIG_APIC=y > CONFIG_IOAPIC=y > -CONFIG_ICC_BUS=y > CONFIG_PVPANIC=y > CONFIG_MEM_HOTPLUG=y > CONFIG_XIO3130=y > diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs > index 6381238..193e489 100644 > --- a/hw/cpu/Makefile.objs > +++ b/hw/cpu/Makefile.objs > @@ -2,5 +2,3 @@ obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o > obj-$(CONFIG_REALVIEW) += realview_mpcore.o > obj-$(CONFIG_A9MPCORE) += a9mpcore.o > obj-$(CONFIG_A15MPCORE) += a15mpcore.o > -obj-$(CONFIG_ICC_BUS) += icc_bus.o > - > diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c > deleted file mode 100644 > index 6646ea2..0000000 > --- a/hw/cpu/icc_bus.c > +++ /dev/null > @@ -1,118 +0,0 @@ > -/* icc_bus.c > - * emulate x86 ICC (Interrupt Controller Communications) bus > - * > - * Copyright (c) 2013 Red Hat, Inc > - * > - * Authors: > - * Igor Mammedov <imamm...@redhat.com> > - * > - * 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 "hw/cpu/icc_bus.h" > -#include "hw/sysbus.h" > - > -/* icc-bridge implementation */ > - > -static const TypeInfo icc_bus_info = { > - .name = TYPE_ICC_BUS, > - .parent = TYPE_BUS, > - .instance_size = sizeof(ICCBus), > -}; > - > - > -/* icc-device implementation */ > - > -static void icc_device_realize(DeviceState *dev, Error **errp) > -{ > - ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev); > - > - /* convert to QOM */ > - if (idc->realize) { > - idc->realize(dev, errp); > - } > - > -} > - > -static void icc_device_class_init(ObjectClass *oc, void *data) > -{ > - DeviceClass *dc = DEVICE_CLASS(oc); > - > - dc->realize = icc_device_realize; > - dc->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, > -}; > - > - > -/* icc-bridge implementation */ > - > -typedef struct ICCBridgeState { > - /*< private >*/ > - SysBusDevice parent_obj; > - /*< public >*/ > - > - ICCBus icc_bus; > - MemoryRegion apic_container; > -} ICCBridgeState; > - > -#define ICC_BRIDGE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE) > - > -static void icc_bridge_init(Object *obj) > -{ > - ICCBridgeState *s = ICC_BRIDGE(obj); > - SysBusDevice *sb = SYS_BUS_DEVICE(obj); > - > - qbus_create_inplace(&s->icc_bus, sizeof(s->icc_bus), TYPE_ICC_BUS, > - DEVICE(s), "icc"); > - > - /* Do not change order of registering regions, > - * APIC must be first registered region, board maps it by 0 index > - */ > - memory_region_init(&s->apic_container, obj, "icc-apic-container", > - APIC_SPACE_SIZE); > - sysbus_init_mmio(sb, &s->apic_container); > - s->icc_bus.apic_address_space = &s->apic_container; > -} > - > -static void icc_bridge_class_init(ObjectClass *oc, void *data) > -{ > - DeviceClass *dc = DEVICE_CLASS(oc); > - > - set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > -} > - > -static const TypeInfo icc_bridge_info = { > - .name = TYPE_ICC_BRIDGE, > - .parent = TYPE_SYS_BUS_DEVICE, > - .instance_init = icc_bridge_init, > - .instance_size = sizeof(ICCBridgeState), > - .class_init = icc_bridge_class_init, > -}; > - > - > -static void icc_bus_register_types(void) > -{ > - type_register_static(&icc_bus_info); > - type_register_static(&icc_device_info); > - type_register_static(&icc_bridge_info); > -} > - > -type_init(icc_bus_register_types) > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index a5e2a27..00b3ddd 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -58,7 +58,6 @@ > #include "qemu/config-file.h" > #include "hw/acpi/acpi.h" > #include "hw/acpi/cpu_hotplug.h" > -#include "hw/cpu/icc_bus.h" > #include "hw/boards.h" > #include "hw/pci/pci_host.h" > #include "acpi-build.h" > @@ -990,22 +989,17 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int > level) > } > > static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, > - DeviceState *icc_bridge, Error **errp) > + Error **errp) > { > X86CPU *cpu = NULL; > Error *local_err = NULL; > > - if (icc_bridge == NULL) { > - error_setg(&local_err, "Invalid icc-bridge value"); > - goto out; > - } > - > cpu = cpu_x86_create(cpu_model, &local_err); > if (local_err != NULL) { > goto out; > } > > - qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc")); > + qdev_set_parent_bus(DEVICE(cpu), sysbus_get_default()); > object_unref(OBJECT(cpu)); > > object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err); > @@ -1024,7 +1018,6 @@ static const char *current_cpu_model; > > void pc_hot_add_cpu(const int64_t id, Error **errp) > { > - DeviceState *icc_bridge; > int64_t apic_id = x86_cpu_apic_id_from_index(id); > > if (id < 0) { > @@ -1051,12 +1044,10 @@ void pc_hot_add_cpu(const int64_t id, Error **errp) > return; > } > > - icc_bridge = DEVICE(object_resolve_path_type("icc-bridge", > - TYPE_ICC_BRIDGE, NULL)); > - pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp); > + pc_new_cpu(current_cpu_model, apic_id, errp); > } > > -void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) > +void pc_cpus_init(const char *cpu_model) > { > int i; > X86CPU *cpu = NULL; > @@ -1082,7 +1073,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState > *icc_bridge) > > for (i = 0; i < smp_cpus; i++) { > cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), > - icc_bridge, &error); > + &error); > if (error) { > error_report_err(error); > exit(1); > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 1fe7bfb..6004580 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -39,7 +39,6 @@ > #include "hw/kvm/clock.h" > #include "sysemu/sysemu.h" > #include "hw/sysbus.h" > -#include "hw/cpu/icc_bus.h" > #include "sysemu/arch_init.h" > #include "sysemu/block-backend.h" > #include "hw/i2c/smbus.h" > @@ -98,7 +97,6 @@ static void pc_init1(MachineState *machine, > MemoryRegion *ram_memory; > MemoryRegion *pci_memory; > MemoryRegion *rom_memory; > - DeviceState *icc_bridge; > FWCfgState *fw_cfg = NULL; > PcGuestInfo *guest_info; > ram_addr_t lowmem; > @@ -143,11 +141,7 @@ static void pc_init1(MachineState *machine, > exit(1); > } > > - icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); > - object_property_add_child(qdev_get_machine(), "icc-bridge", > - OBJECT(icc_bridge), NULL); > - > - pc_cpus_init(machine->cpu_model, icc_bridge); > + pc_cpus_init(machine->cpu_model); > > if (kvm_enabled() && kvmclock_enabled) { > kvmclock_create(); > @@ -230,7 +224,6 @@ static void pc_init1(MachineState *machine, > if (pci_enabled) { > ioapic_init_gsi(gsi_state, "i440fx"); > } > - qdev_init_nofail(icc_bridge); > > pc_register_ferr_irq(gsi[13]); > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index dcc17c0..3b89e6a 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -43,7 +43,6 @@ > #include "hw/ide/pci.h" > #include "hw/ide/ahci.h" > #include "hw/usb.h" > -#include "hw/cpu/icc_bus.h" > #include "qemu/error-report.h" > > /* ICH9 AHCI has 6 ports */ > @@ -85,7 +84,6 @@ static void pc_q35_init(MachineState *machine) > int i; > ICH9LPCState *ich9_lpc; > PCIDevice *ahci; > - DeviceState *icc_bridge; > PcGuestInfo *guest_info; > ram_addr_t lowmem; > DriveInfo *hd[MAX_SATA_PORTS]; > @@ -132,11 +130,7 @@ static void pc_q35_init(MachineState *machine) > exit(1); > } > > - icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); > - object_property_add_child(qdev_get_machine(), "icc-bridge", > - OBJECT(icc_bridge), NULL); > - > - pc_cpus_init(machine->cpu_model, icc_bridge); > + pc_cpus_init(machine->cpu_model); > pc_acpi_init("q35-acpi-dsdt.aml"); > > kvmclock_create(); > @@ -240,7 +234,6 @@ static void pc_q35_init(MachineState *machine) > if (pci_enabled) { > ioapic_init_gsi(gsi_state, "q35"); > } > - qdev_init_nofail(icc_bridge); > > pc_register_ferr_irq(gsi[13]); > > diff --git a/hw/intc/apic.c b/hw/intc/apic.c > index 0f97b47..00ae0ec 100644 > --- a/hw/intc/apic.c > +++ b/hw/intc/apic.c > @@ -376,7 +376,7 @@ static void apic_update_irq(APICCommonState *s) > cpu_interrupt(cpu, CPU_INTERRUPT_POLL); > } else if (apic_irq_pending(s) > 0) { > cpu_interrupt(cpu, CPU_INTERRUPT_HARD); > - } else if (!apic_accept_pic_intr(&s->busdev.qdev) || > !pic_get_output(isa_pic)) { > + } else if (!apic_accept_pic_intr(DEVICE(s)) || !pic_get_output(isa_pic)) > { > cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD); > } > } > @@ -549,10 +549,10 @@ static void apic_deliver(DeviceState *dev, uint8_t > dest, uint8_t dest_mode, > > static bool apic_check_pic(APICCommonState *s) > { > - if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) { > + if (!apic_accept_pic_intr(DEVICE(s)) || !pic_get_output(isa_pic)) { > return false; > } > - apic_deliver_pic_intr(&s->busdev.qdev, 1); > + apic_deliver_pic_intr(DEVICE(s), 1); > return true; > } > > diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c > index 004821a..bdc0dc7 100644 > --- a/hw/intc/apic_common.c > +++ b/hw/intc/apic_common.c > @@ -436,13 +436,12 @@ static Property apic_properties_common[] = { > > static void apic_common_class_init(ObjectClass *klass, void *data) > { > - ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass); > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->vmsd = &vmstate_apic_common; > dc->reset = apic_reset_common; > dc->props = apic_properties_common; > - idc->realize = apic_common_realize; > + dc->realize = apic_common_realize; > /* > * Reason: APIC and CPU need to be wired up by > * x86_cpu_apic_create() > @@ -452,7 +451,7 @@ static void apic_common_class_init(ObjectClass *klass, > void *data) > > static const TypeInfo apic_common_type = { > .name = TYPE_APIC_COMMON, > - .parent = TYPE_ICC_DEVICE, > + .parent = TYPE_DEVICE, > .instance_size = sizeof(APICCommonState), > .class_size = sizeof(APICCommonClass), > .class_init = apic_common_class_init, > diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h > deleted file mode 100644 > index 98a979f..0000000 > --- a/include/hw/cpu/icc_bus.h > +++ /dev/null > @@ -1,82 +0,0 @@ > -/* icc_bus.h > - * emulate x86 ICC (Interrupt Controller Communications) bus > - * > - * Copyright (c) 2013 Red Hat, Inc > - * > - * Authors: > - * Igor Mammedov <imamm...@redhat.com> > - * > - * 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 "exec/memory.h" > -#include "hw/qdev-core.h" > - > -#define TYPE_ICC_BUS "icc-bus" > - > -#ifndef CONFIG_USER_ONLY > - > -/** > - * ICCBus: > - * > - * ICC bus > - */ > -typedef struct ICCBus { > - /*< private >*/ > - BusState parent_obj; > - /*< public >*/ > - > - MemoryRegion *apic_address_space; > -} ICCBus; > - > -#define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS) > - > -/** > - * ICCDevice: > - * > - * ICC device > - */ > -typedef struct ICCDevice { > - /*< private >*/ > - DeviceState qdev; > - /*< public >*/ > -} ICCDevice; > - > -/** > - * ICCDeviceClass: > - * @init: Initialization callback for derived classes. > - * > - * ICC device class > - */ > -typedef struct ICCDeviceClass { > - /*< private >*/ > - DeviceClass parent_class; > - /*< public >*/ > - > - DeviceRealize realize; > -} 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) > - > -#define TYPE_ICC_BRIDGE "icc-bridge" > - > -#endif /* CONFIG_USER_ONLY */ > -#endif > diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h > index dc7a89d..6eeceff 100644 > --- a/include/hw/i386/apic_internal.h > +++ b/include/hw/i386/apic_internal.h > @@ -21,7 +21,6 @@ > #define QEMU_APIC_INTERNAL_H > > #include "exec/memory.h" > -#include "hw/cpu/icc_bus.h" > #include "qemu/timer.h" > > /* APIC Local Vector Table */ > @@ -78,7 +77,7 @@ typedef struct APICCommonState APICCommonState; > > typedef struct APICCommonClass > { > - ICCDeviceClass parent_class; > + DeviceClass parent_class; > > DeviceRealize realize; > void (*set_base)(APICCommonState *s, uint64_t val); > @@ -93,7 +92,7 @@ typedef struct APICCommonClass > } APICCommonClass; > > struct APICCommonState { > - ICCDevice busdev; > + DeviceState dev; > > MemoryRegion io_memory; > X86CPU *cpu; > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 1b35168..7c9d044 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -163,7 +163,7 @@ extern int fd_bootchk; > void pc_register_ferr_irq(qemu_irq irq); > void pc_acpi_smi_interrupt(void *opaque, int irq, int level); > > -void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge); > +void pc_cpus_init(const char *cpu_model); > void pc_hot_add_cpu(const int64_t id, Error **errp); > void pc_acpi_init(const char *default_dsdt); > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 03b33cf..d11b219 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -42,7 +42,6 @@ > > #include "sysemu/sysemu.h" > #include "hw/qdev-properties.h" > -#include "hw/cpu/icc_bus.h" > #ifndef CONFIG_USER_ONLY > #include "hw/xen/xen.h" > #include "hw/i386/apic_internal.h" > @@ -2634,6 +2633,14 @@ static void x86_cpu_reset(CPUState *s) > > xcc->parent_reset(s); > > + /* > + * due to apic doesn't attach to sysbus, so when > + * cpu reset, we should reset apic in the meanwhile. s/.../since APIC is a bus-less device, propagate reset to it manually > + */ > + if (cpu->apic_state) { > + device_reset(DEVICE(cpu->apic_state)); > + } > + > memset(env, 0, offsetof(CPUX86State, cpuid_level)); > > tlb_flush(s, 1); > @@ -2757,7 +2764,6 @@ static void mce_init(X86CPU *cpu) > #ifndef CONFIG_USER_ONLY > static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) > { > - DeviceState *dev = DEVICE(cpu); > APICCommonState *apic; > const char *apic_type = "apic"; > > @@ -2767,7 +2773,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error > **errp) > apic_type = "xen-apic"; > } > > - cpu->apic_state = qdev_try_create(qdev_get_parent_bus(dev), apic_type); > + cpu->apic_state = DEVICE(object_new(apic_type)); > if (cpu->apic_state == NULL) { object_new() can't fail, hence drop this NULL check > error_setg(errp, "APIC device '%s' could not be created", apic_type); > return; > @@ -3009,7 +3015,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, > void *data) > > xcc->parent_realize = dc->realize; > dc->realize = x86_cpu_realizefn; > - dc->bus_type = TYPE_ICC_BUS; > dc->props = x86_cpu_properties; > > xcc->parent_reset = cc->reset;
