* [Qemu-devel] [PATCH v3 0/2] remove icc bus/bridge
@ 2015-04-10 8:18 Chen Fan
2015-04-10 8:18 ` [Qemu-devel] [PATCH v3 1/2] apic: move apic mmio register to each apic realizefn Chen Fan
2015-04-10 8:18 ` [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/ Chen Fan
0 siblings, 2 replies; 7+ messages in thread
From: Chen Fan @ 2015-04-10 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: guz.fnst, imammedo, zhugh.fnst, afaerber, isimatu.yasuaki
ICC Bus was used for providing a hotpluggable bus for APIC and CPU,
but new we use HotplugHandler to make hotplug. so ICC Bus is
unnecessary.
this codes has passed the new pc-cpu-test.
and I have tested with kvm along with kernel_irqchip=on/off,
It works fine.
v2-v3:
1. replace init apic by object_new()
2. add reset apic at the time of CPU reset
Chen Fan (2):
apic: move apic mmio register to each apic realizefn
cpu/apic: drop icc bus/bridge/
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 | 26 ++------
hw/i386/pc_piix.c | 9 +--
hw/i386/pc_q35.c | 9 +--
hw/intc/apic.c | 6 +-
hw/intc/apic_common.c | 16 +++--
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, 29 insertions(+), 261 deletions(-)
delete mode 100644 hw/cpu/icc_bus.c
delete mode 100644 include/hw/cpu/icc_bus.h
--
1.9.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v3 1/2] apic: move apic mmio register to each apic realizefn
2015-04-10 8:18 [Qemu-devel] [PATCH v3 0/2] remove icc bus/bridge Chen Fan
@ 2015-04-10 8:18 ` Chen Fan
2015-04-14 12:41 ` Igor Mammedov
2015-04-10 8:18 ` [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/ Chen Fan
1 sibling, 1 reply; 7+ messages in thread
From: Chen Fan @ 2015-04-10 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: guz.fnst, imammedo, zhugh.fnst, afaerber, isimatu.yasuaki
Due to local apic address is in view of CPU's address space,
so able to move apic mapping to each apic realizefn.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
hw/i386/pc.c | 7 -------
hw/intc/apic_common.c | 11 +++++------
2 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a8e6be1..a5e2a27 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1089,13 +1089,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
}
}
- /* map APIC MMIO area if CPU has APIC */
- if (cpu && cpu->apic_state) {
- /* XXX: what if the base changes? */
- sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
- APIC_DEFAULT_ADDRESS, 0x1000);
- }
-
/* tell smbios about cpuid version and features */
smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
}
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 042e960..004821a 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -306,7 +306,6 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
APICCommonClass *info;
static DeviceState *vapic;
static int apic_no;
- static bool mmio_registered;
if (apic_no >= MAX_APICS) {
error_setg(errp, "%s initialization failed.",
@@ -317,11 +316,11 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
info = APIC_COMMON_GET_CLASS(s);
info->realize(dev, errp);
- if (!mmio_registered) {
- ICCBus *b = ICC_BUS(qdev_get_parent_bus(dev));
- memory_region_add_subregion(b->apic_address_space, 0, &s->io_memory);
- mmio_registered = true;
- }
+
+ memory_region_add_subregion_overlap(CPU(s->cpu)->as->root,
+ APIC_DEFAULT_ADDRESS,
+ &s->io_memory,
+ 0x1000);
/* Note: We need at least 1M to map the VAPIC option ROM */
if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
--
1.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/
2015-04-10 8:18 [Qemu-devel] [PATCH v3 0/2] remove icc bus/bridge Chen Fan
2015-04-10 8:18 ` [Qemu-devel] [PATCH v3 1/2] apic: move apic mmio register to each apic realizefn Chen Fan
@ 2015-04-10 8:18 ` Chen Fan
2015-04-14 13:27 ` Igor Mammedov
1 sibling, 1 reply; 7+ messages in thread
From: Chen Fan @ 2015-04-10 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: guz.fnst, imammedo, zhugh.fnst, afaerber, isimatu.yasuaki
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.fnst@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 <imammedo@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 <imammedo@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.
+ */
+ 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) {
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;
--
1.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] apic: move apic mmio register to each apic realizefn
2015-04-10 8:18 ` [Qemu-devel] [PATCH v3 1/2] apic: move apic mmio register to each apic realizefn Chen Fan
@ 2015-04-14 12:41 ` Igor Mammedov
2015-04-14 12:47 ` Andreas Färber
0 siblings, 1 reply; 7+ messages in thread
From: Igor Mammedov @ 2015-04-14 12:41 UTC (permalink / raw)
To: Chen Fan; +Cc: zhugh.fnst, qemu-devel, isimatu.yasuaki, guz.fnst, Paolo,
afaerber
On Fri, 10 Apr 2015 16:18:05 +0800
Chen Fan <chen.fan.fnst@cn.fujitsu.com> wrote:
> Due to local apic address is in view of CPU's address space,
> so able to move apic mapping to each apic realizefn.
>
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
> hw/i386/pc.c | 7 -------
> hw/intc/apic_common.c | 11 +++++------
> 2 files changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a8e6be1..a5e2a27 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1089,13 +1089,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
> }
> }
>
> - /* map APIC MMIO area if CPU has APIC */
> - if (cpu && cpu->apic_state) {
> - /* XXX: what if the base changes? */
> - sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
> - APIC_DEFAULT_ADDRESS, 0x1000);
> - }
> -
> /* tell smbios about cpuid version and features */
> smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
> }
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 042e960..004821a 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -306,7 +306,6 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
> APICCommonClass *info;
> static DeviceState *vapic;
> static int apic_no;
> - static bool mmio_registered;
>
> if (apic_no >= MAX_APICS) {
> error_setg(errp, "%s initialization failed.",
> @@ -317,11 +316,11 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>
> info = APIC_COMMON_GET_CLASS(s);
> info->realize(dev, errp);
> - if (!mmio_registered) {
> - ICCBus *b = ICC_BUS(qdev_get_parent_bus(dev));
> - memory_region_add_subregion(b->apic_address_space, 0, &s->io_memory);
> - mmio_registered = true;
> - }
> +
> + memory_region_add_subregion_overlap(CPU(s->cpu)->as->root,
if you look at AddressSpace declaration, root field is marked as private
so you can't access it.
Perhaps you need to add wrapper to get root, something like this:
address_space_root_memory_region()
> + APIC_DEFAULT_ADDRESS,
use s->apicbase here and set its default value from CPU using cpu_set_apic_base()
for example in: x86_cpu_apic_create()
PS:
we also probably need to teach kvm_apic_set_base()/apic_set_base() to
remap APIC in case base changes but it's out of scope of this series.
> + &s->io_memory,
> + 0x1000);
>
> /* Note: We need at least 1M to map the VAPIC option ROM */
> if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] apic: move apic mmio register to each apic realizefn
2015-04-14 12:41 ` Igor Mammedov
@ 2015-04-14 12:47 ` Andreas Färber
0 siblings, 0 replies; 7+ messages in thread
From: Andreas Färber @ 2015-04-14 12:47 UTC (permalink / raw)
To: Chen Fan
Cc: zhugh.fnst, qemu-devel, isimatu.yasuaki, Paolo, guz.fnst,
Igor Mammedov
Am 14.04.2015 um 14:41 schrieb Igor Mammedov:
> On Fri, 10 Apr 2015 16:18:05 +0800
> Chen Fan <chen.fan.fnst@cn.fujitsu.com> wrote:
>
>> Due to local apic address is in view of CPU's address space,
>> so able to move apic mapping to each apic realizefn.
>>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> ---
>> hw/i386/pc.c | 7 -------
>> hw/intc/apic_common.c | 11 +++++------
>> 2 files changed, 5 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index a8e6be1..a5e2a27 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1089,13 +1089,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>> }
>> }
>>
>> - /* map APIC MMIO area if CPU has APIC */
>> - if (cpu && cpu->apic_state) {
>> - /* XXX: what if the base changes? */
>> - sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
>> - APIC_DEFAULT_ADDRESS, 0x1000);
>> - }
>> -
>> /* tell smbios about cpuid version and features */
>> smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
>> }
>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>> index 042e960..004821a 100644
>> --- a/hw/intc/apic_common.c
>> +++ b/hw/intc/apic_common.c
>> @@ -306,7 +306,6 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>> APICCommonClass *info;
>> static DeviceState *vapic;
>> static int apic_no;
>> - static bool mmio_registered;
>>
>> if (apic_no >= MAX_APICS) {
>> error_setg(errp, "%s initialization failed.",
>> @@ -317,11 +316,11 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>>
>> info = APIC_COMMON_GET_CLASS(s);
>> info->realize(dev, errp);
>> - if (!mmio_registered) {
>> - ICCBus *b = ICC_BUS(qdev_get_parent_bus(dev));
>> - memory_region_add_subregion(b->apic_address_space, 0, &s->io_memory);
>> - mmio_registered = true;
>> - }
>> +
>> + memory_region_add_subregion_overlap(CPU(s->cpu)->as->root,
> if you look at AddressSpace declaration, root field is marked as private
> so you can't access it.
> Perhaps you need to add wrapper to get root, something like this:
> address_space_root_memory_region()
Also, by convention please use a local CPUState *cpu or *cs variable
rather than calling CPU() inline.
Regards,
Andreas
>> + APIC_DEFAULT_ADDRESS,
> use s->apicbase here and set its default value from CPU using cpu_set_apic_base()
> for example in: x86_cpu_apic_create()
>
> PS:
> we also probably need to teach kvm_apic_set_base()/apic_set_base() to
> remap APIC in case base changes but it's out of scope of this series.
>
>> + &s->io_memory,
>> + 0x1000);
>>
>> /* Note: We need at least 1M to map the VAPIC option ROM */
>> if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/
2015-04-10 8:18 ` [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/ Chen Fan
@ 2015-04-14 13:27 ` Igor Mammedov
2015-04-14 13:41 ` Andreas Färber
0 siblings, 1 reply; 7+ messages in thread
From: Igor Mammedov @ 2015-04-14 13:27 UTC (permalink / raw)
To: Chen Fan; +Cc: guz.fnst, zhugh.fnst, qemu-devel, isimatu.yasuaki, afaerber
On Fri, 10 Apr 2015 16:18:06 +0800
Chen Fan <chen.fan.fnst@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.fnst@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 <imammedo@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 <imammedo@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;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/
2015-04-14 13:27 ` Igor Mammedov
@ 2015-04-14 13:41 ` Andreas Färber
0 siblings, 0 replies; 7+ messages in thread
From: Andreas Färber @ 2015-04-14 13:41 UTC (permalink / raw)
To: Igor Mammedov, Chen Fan
Cc: guz.fnst, Paolo Bonzini, zhugh.fnst, qemu-devel, isimatu.yasuaki
Am 14.04.2015 um 15:27 schrieb Igor Mammedov:
> On Fri, 10 Apr 2015 16:18:06 +0800
> Chen Fan <chen.fan.fnst@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.fnst@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 <imammedo@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)
I think it may help to split this patch into two - one doing the APIC
changes and once unused dropping the large files.
>> 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;
>> }
>>
Can you put these cast fixes in a preparatory patch please?
>> 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 <imammedo@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;
Please name parent_obj. While at it, can you please add private/public
markers around the two?
>>
>> 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
Hm, depending on why this was a "try", there's multiple options for
keeping the current error handling:
* check whether the type exits before object_new(), cf. CPU
* use g_try_malloc0()+object_initialize() if it's a size issue
That said, I like where this is going. It should help with cleaning up
the recently discussed reset back-and-forth with BSP.
Regards,
Andreas
>> 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;
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-04-14 13:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-10 8:18 [Qemu-devel] [PATCH v3 0/2] remove icc bus/bridge Chen Fan
2015-04-10 8:18 ` [Qemu-devel] [PATCH v3 1/2] apic: move apic mmio register to each apic realizefn Chen Fan
2015-04-14 12:41 ` Igor Mammedov
2015-04-14 12:47 ` Andreas Färber
2015-04-10 8:18 ` [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/ Chen Fan
2015-04-14 13:27 ` Igor Mammedov
2015-04-14 13:41 ` Andreas Färber
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).