* [Qemu-devel] [PATCH V3] Introduce a new bus "ICC" to connect APIC
@ 2011-11-03 9:30 pingfank
2011-11-06 18:06 ` Jan Kiszka
0 siblings, 1 reply; 4+ messages in thread
From: pingfank @ 2011-11-03 9:30 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, aliguori, ryanh, jan.kiszka, Liu Ping Fan
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Introduce a new structure CPUS as the controller of ICC (INTERRUPT
CONTROLLER COMMUNICATIONS), and new bus "ICC" to hold APIC,instead
of sysbus. So we can support APIC hot-plug feature.
Signed-off-by: liu ping fan <pingfank@linux.vnet.ibm.com>
---
Makefile.target | 1 +
hw/apic.c | 24 +++++++++----
hw/apic.h | 1 +
hw/icc_bus.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++
hw/icc_bus.h | 61 +++++++++++++++++++++++++++++++++
hw/pc.c | 9 +++--
hw/pc_piix.c | 14 +++++++-
target-i386/cpu.h | 1 +
target-i386/cpuid.c | 16 +++++++++
9 files changed, 207 insertions(+), 12 deletions(-)
create mode 100644 hw/icc_bus.c
create mode 100644 hw/icc_bus.h
diff --git a/Makefile.target b/Makefile.target
index 9011f28..5607c6d 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
obj-i386-y += testdev.o
obj-i386-y += acpi.o acpi_piix4.o
+obj-i386-y += icc_bus.o
obj-i386-y += pcspk.o i8254.o
obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
diff --git a/hw/apic.c b/hw/apic.c
index 69d6ac5..34fa1dd 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -21,9 +21,10 @@
#include "ioapic.h"
#include "qemu-timer.h"
#include "host-utils.h"
-#include "sysbus.h"
+#include "icc_bus.h"
#include "trace.h"
#include "kvm.h"
+#include "exec-memory.h"
/* APIC Local Vector Table */
#define APIC_LVT_TIMER 0
@@ -80,7 +81,7 @@
typedef struct APICState APICState;
struct APICState {
- SysBusDevice busdev;
+ ICCBusDevice busdev;
MemoryRegion io_memory;
void *cpu_env;
uint32_t apicbase;
@@ -1104,9 +1105,19 @@ static const MemoryRegionOps apic_io_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static int apic_init1(SysBusDevice *dev)
+int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
{
- APICState *s = FROM_SYSBUS(APICState, dev);
+ APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
+
+ memory_region_add_subregion(get_system_memory(),
+ base,
+ &s->io_memory);
+ return 0;
+}
+
+static int apic_init1(ICCBusDevice *dev)
+{
+ APICState *s = DO_UPCAST(APICState, busdev, dev);
static int last_apic_idx;
if (last_apic_idx >= MAX_APICS) {
@@ -1114,7 +1125,6 @@ static int apic_init1(SysBusDevice *dev)
}
memory_region_init_io(&s->io_memory, &apic_io_ops, s, "apic",
MSI_ADDR_SIZE);
- sysbus_init_mmio_region(dev, &s->io_memory);
s->timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
s->idx = last_apic_idx++;
@@ -1122,7 +1132,7 @@ static int apic_init1(SysBusDevice *dev)
return 0;
}
-static SysBusDeviceInfo apic_info = {
+static ICCBusDeviceInfo apic_info = {
.init = apic_init1,
.qdev.name = "apic",
.qdev.size = sizeof(APICState),
@@ -1138,7 +1148,7 @@ static SysBusDeviceInfo apic_info = {
static void apic_register_devices(void)
{
- sysbus_register_withprop(&apic_info);
+ iccbus_register_devinfo(&apic_info);
}
device_init(apic_register_devices)
diff --git a/hw/apic.h b/hw/apic.h
index c857d52..e2c0af5 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
uint8_t cpu_get_apic_tpr(DeviceState *s);
void apic_init_reset(DeviceState *s);
void apic_sipi(DeviceState *s);
+int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
/* pc.c */
int cpu_is_bsp(CPUState *env);
diff --git a/hw/icc_bus.c b/hw/icc_bus.c
new file mode 100644
index 0000000..ac88f2e
--- /dev/null
+++ b/hw/icc_bus.c
@@ -0,0 +1,92 @@
+/* icc_bus.c
+ * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * 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"
+
+static CPUSockets *cpu_sockets;
+
+static ICCBusInfo icc_bus_info = {
+ .qinfo.name = "icc",
+ .qinfo.size = sizeof(ICCBus),
+ .qinfo.props = (Property[]) {
+ DEFINE_PROP_END_OF_LIST(),
+ }
+};
+
+static int iccbus_device_init(DeviceState *dev, DeviceInfo *base)
+{
+ ICCBusDeviceInfo *info = container_of(base, ICCBusDeviceInfo, qdev);
+ ICCBusDevice *idev = DO_UPCAST(ICCBusDevice, qdev, dev);
+
+ return info->init(idev);
+}
+
+void iccbus_register_devinfo(ICCBusDeviceInfo *info)
+{
+ info->qdev.init = iccbus_device_init;
+ info->qdev.bus_info = &icc_bus_info.qinfo;
+ assert(info->qdev.size >= sizeof(ICCBusDevice));
+ qdev_register(&info->qdev);
+}
+
+BusState *get_icc_bus(void)
+{
+ return &cpu_sockets->icc_bus->qbus;
+}
+
+/*Must be called before vcpu's creation*/
+static int cpusockets_init(SysBusDevice *dev)
+{
+ CPUSockets *cpus = DO_UPCAST(CPUSockets, sysdev, dev);
+ BusState *b = qbus_create(&icc_bus_info.qinfo,
+ &cpu_sockets->sysdev.qdev,
+ "icc");
+ if (b == NULL) {
+ return -1;
+ }
+ b->allow_hotplug = 1; /* Yes, we can */
+ cpus->icc_bus = DO_UPCAST(ICCBus, qbus, b);
+ cpu_sockets = cpus;
+ return 0;
+
+}
+
+static const VMStateDescription vmstate_cpusockets = {
+ .name = "cpusockets",
+ .version_id = 1,
+ .minimum_version_id = 0,
+ .fields = (VMStateField[]) {
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static SysBusDeviceInfo cpusockets_info = {
+ .init = cpusockets_init,
+ .qdev.name = "cpusockets",
+ .qdev.size = sizeof(CPUSockets),
+ .qdev.vmsd = &vmstate_cpusockets,
+ .qdev.reset = NULL,
+ .qdev.no_user = 1,
+};
+
+static void cpusockets_register_devices(void)
+{
+ sysbus_register_withprop(&cpusockets_info);
+}
+
+device_init(cpusockets_register_devices)
diff --git a/hw/icc_bus.h b/hw/icc_bus.h
new file mode 100644
index 0000000..9f10e1e
--- /dev/null
+++ b/hw/icc_bus.h
@@ -0,0 +1,61 @@
+/* ICCBus.h
+ * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * 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 QEMU_ICC_H
+#define QEMU_ICC_H
+
+#include "qdev.h"
+#include "sysbus.h"
+
+typedef struct CPUSockets CPUSockets;
+typedef struct ICCBus ICCBus;
+typedef struct ICCBusInfo ICCBusInfo;
+typedef struct ICCBusDevice ICCBusDevice;
+typedef struct ICCBusDeviceInfo ICCBusDeviceInfo;
+
+struct CPUSockets {
+ SysBusDevice sysdev;
+ ICCBus *icc_bus;
+};
+
+struct CPUSInfo {
+ DeviceInfo info;
+};
+
+struct ICCBus {
+ BusState qbus;
+};
+
+struct ICCBusInfo {
+ BusInfo qinfo;
+};
+struct ICCBusDevice {
+ DeviceState qdev;
+};
+
+typedef int (*iccbus_initfn)(ICCBusDevice *dev);
+
+struct ICCBusDeviceInfo {
+ DeviceInfo qdev;
+ iccbus_initfn init;
+};
+
+void iccbus_register_devinfo(ICCBusDeviceInfo *info);
+BusState *get_icc_bus(void);
+
+#endif
diff --git a/hw/pc.c b/hw/pc.c
index 6b3662e..ffdca64 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -24,6 +24,7 @@
#include "hw.h"
#include "pc.h"
#include "apic.h"
+#include "icc_bus.h"
#include "fdc.h"
#include "ide.h"
#include "pci.h"
@@ -875,21 +876,21 @@ DeviceState *cpu_get_current_apic(void)
static DeviceState *apic_init(void *env, uint8_t apic_id)
{
DeviceState *dev;
- SysBusDevice *d;
+ BusState *b;
static int apic_mapped;
- dev = qdev_create(NULL, "apic");
+ b = get_icc_bus();
+ dev = qdev_create(b, "apic");
qdev_prop_set_uint8(dev, "id", apic_id);
qdev_prop_set_ptr(dev, "cpu_env", env);
qdev_init_nofail(dev);
- d = sysbus_from_qdev(dev);
/* XXX: mapping more APICs at the same memory location */
if (apic_mapped == 0) {
/* 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(d, 0, MSI_ADDR_BASE);
+ apic_mmio_map(dev, MSI_ADDR_BASE);
apic_mapped = 1;
}
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 7055591..09aece9 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -41,7 +41,6 @@
#include "blockdev.h"
#include "smbus.h"
#include "xen.h"
-#include "memory.h"
#include "exec-memory.h"
#ifdef CONFIG_XEN
# include <xen/hvm/hvm_info_table.h>
@@ -102,8 +101,21 @@ static void pc_init1(MemoryRegion *system_memory,
MemoryRegion *ram_memory;
MemoryRegion *pci_memory;
+ if (cpu_model == NULL) {
+#ifdef TARGET_X86_64
+ cpu_model = "qemu64";
+#else
+ cpu_model = "qemu32";
+#endif
+ }
+
global_cpu_model = cpu_model;
+ if (cpu_has_apic_feature(cpu_model) || smp_cpus > 1) {
+ DeviceState *d = qdev_create(NULL, "cpusockets");
+ qdev_init_nofail(d);
+ }
+
pc_cpus_init(cpu_model);
if (kvmclock_enabled) {
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 2a071f2..abdeb40 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -884,6 +884,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx);
int cpu_x86_register (CPUX86State *env, const char *cpu_model);
+int cpu_has_apic_feature(const char *cpu_model);
void cpu_clear_apic_feature(CPUX86State *env);
void host_cpuid(uint32_t function, uint32_t count,
uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index f179999..beaef5b 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -850,6 +850,22 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
}
}
+int cpu_has_apic_feature(const char *cpu_model)
+{
+ x86_def_t def1, *def = &def1;
+
+ memset(def, 0, sizeof(*def));
+
+ if (cpu_x86_find_by_name(def, cpu_model) < 0) {
+ return 0;
+ }
+ if (def->features & CPUID_APIC) {
+ return 1;
+ } else {
+ return 0;
+ }
+}
+
int cpu_x86_register (CPUX86State *env, const char *cpu_model)
{
x86_def_t def1, *def = &def1;
--
1.7.4.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH V3] Introduce a new bus "ICC" to connect APIC
2011-11-03 9:30 [Qemu-devel] [PATCH V3] Introduce a new bus "ICC" to connect APIC pingfank
@ 2011-11-06 18:06 ` Jan Kiszka
2011-11-08 1:44 ` liu ping fan
0 siblings, 1 reply; 4+ messages in thread
From: Jan Kiszka @ 2011-11-06 18:06 UTC (permalink / raw)
To: pingfank; +Cc: blauwirbel, aliguori, ryanh, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 12807 bytes --]
On 2011-11-03 10:30, pingfank@linux.vnet.ibm.com wrote:
> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>
> Introduce a new structure CPUS as the controller of ICC (INTERRUPT
> CONTROLLER COMMUNICATIONS), and new bus "ICC" to hold APIC,instead
> of sysbus. So we can support APIC hot-plug feature.
>
> Signed-off-by: liu ping fan <pingfank@linux.vnet.ibm.com>
> ---
> Makefile.target | 1 +
> hw/apic.c | 24 +++++++++----
> hw/apic.h | 1 +
> hw/icc_bus.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/icc_bus.h | 61 +++++++++++++++++++++++++++++++++
> hw/pc.c | 9 +++--
> hw/pc_piix.c | 14 +++++++-
> target-i386/cpu.h | 1 +
> target-i386/cpuid.c | 16 +++++++++
> 9 files changed, 207 insertions(+), 12 deletions(-)
> create mode 100644 hw/icc_bus.c
> create mode 100644 hw/icc_bus.h
>
> diff --git a/Makefile.target b/Makefile.target
> index 9011f28..5607c6d 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
> obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
> obj-i386-y += testdev.o
> obj-i386-y += acpi.o acpi_piix4.o
> +obj-i386-y += icc_bus.o
>
> obj-i386-y += pcspk.o i8254.o
> obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
> diff --git a/hw/apic.c b/hw/apic.c
> index 69d6ac5..34fa1dd 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -21,9 +21,10 @@
> #include "ioapic.h"
> #include "qemu-timer.h"
> #include "host-utils.h"
> -#include "sysbus.h"
> +#include "icc_bus.h"
> #include "trace.h"
> #include "kvm.h"
> +#include "exec-memory.h"
>
> /* APIC Local Vector Table */
> #define APIC_LVT_TIMER 0
> @@ -80,7 +81,7 @@
> typedef struct APICState APICState;
>
> struct APICState {
> - SysBusDevice busdev;
> + ICCBusDevice busdev;
> MemoryRegion io_memory;
> void *cpu_env;
> uint32_t apicbase;
> @@ -1104,9 +1105,19 @@ static const MemoryRegionOps apic_io_ops = {
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> -static int apic_init1(SysBusDevice *dev)
> +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
> {
> - APICState *s = FROM_SYSBUS(APICState, dev);
> + APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
> +
> + memory_region_add_subregion(get_system_memory(),
> + base,
> + &s->io_memory);
> + return 0;
> +}
> +
> +static int apic_init1(ICCBusDevice *dev)
> +{
> + APICState *s = DO_UPCAST(APICState, busdev, dev);
> static int last_apic_idx;
>
> if (last_apic_idx >= MAX_APICS) {
> @@ -1114,7 +1125,6 @@ static int apic_init1(SysBusDevice *dev)
> }
> memory_region_init_io(&s->io_memory, &apic_io_ops, s, "apic",
> MSI_ADDR_SIZE);
> - sysbus_init_mmio_region(dev, &s->io_memory);
>
> s->timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
> s->idx = last_apic_idx++;
> @@ -1122,7 +1132,7 @@ static int apic_init1(SysBusDevice *dev)
> return 0;
> }
>
> -static SysBusDeviceInfo apic_info = {
> +static ICCBusDeviceInfo apic_info = {
> .init = apic_init1,
> .qdev.name = "apic",
> .qdev.size = sizeof(APICState),
> @@ -1138,7 +1148,7 @@ static SysBusDeviceInfo apic_info = {
>
> static void apic_register_devices(void)
> {
> - sysbus_register_withprop(&apic_info);
> + iccbus_register_devinfo(&apic_info);
> }
>
> device_init(apic_register_devices)
> diff --git a/hw/apic.h b/hw/apic.h
> index c857d52..e2c0af5 100644
> --- a/hw/apic.h
> +++ b/hw/apic.h
> @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
> uint8_t cpu_get_apic_tpr(DeviceState *s);
> void apic_init_reset(DeviceState *s);
> void apic_sipi(DeviceState *s);
> +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
>
> /* pc.c */
> int cpu_is_bsp(CPUState *env);
> diff --git a/hw/icc_bus.c b/hw/icc_bus.c
> new file mode 100644
> index 0000000..ac88f2e
> --- /dev/null
> +++ b/hw/icc_bus.c
> @@ -0,0 +1,92 @@
> +/* icc_bus.c
> + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * 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"
> +
> +static CPUSockets *cpu_sockets;
> +
> +static ICCBusInfo icc_bus_info = {
> + .qinfo.name = "icc",
> + .qinfo.size = sizeof(ICCBus),
> + .qinfo.props = (Property[]) {
> + DEFINE_PROP_END_OF_LIST(),
> + }
> +};
> +
> +static int iccbus_device_init(DeviceState *dev, DeviceInfo *base)
> +{
> + ICCBusDeviceInfo *info = container_of(base, ICCBusDeviceInfo, qdev);
> + ICCBusDevice *idev = DO_UPCAST(ICCBusDevice, qdev, dev);
> +
> + return info->init(idev);
> +}
> +
> +void iccbus_register_devinfo(ICCBusDeviceInfo *info)
> +{
> + info->qdev.init = iccbus_device_init;
> + info->qdev.bus_info = &icc_bus_info.qinfo;
> + assert(info->qdev.size >= sizeof(ICCBusDevice));
> + qdev_register(&info->qdev);
> +}
> +
> +BusState *get_icc_bus(void)
> +{
> + return &cpu_sockets->icc_bus->qbus;
> +}
> +
> +/*Must be called before vcpu's creation*/
> +static int cpusockets_init(SysBusDevice *dev)
> +{
> + CPUSockets *cpus = DO_UPCAST(CPUSockets, sysdev, dev);
> + BusState *b = qbus_create(&icc_bus_info.qinfo,
> + &cpu_sockets->sysdev.qdev,
> + "icc");
> + if (b == NULL) {
> + return -1;
> + }
> + b->allow_hotplug = 1; /* Yes, we can */
> + cpus->icc_bus = DO_UPCAST(ICCBus, qbus, b);
> + cpu_sockets = cpus;
> + return 0;
> +
> +}
> +
> +static const VMStateDescription vmstate_cpusockets = {
> + .name = "cpusockets",
> + .version_id = 1,
> + .minimum_version_id = 0,
> + .fields = (VMStateField[]) {
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static SysBusDeviceInfo cpusockets_info = {
> + .init = cpusockets_init,
> + .qdev.name = "cpusockets",
> + .qdev.size = sizeof(CPUSockets),
> + .qdev.vmsd = &vmstate_cpusockets,
> + .qdev.reset = NULL,
> + .qdev.no_user = 1,
> +};
> +
> +static void cpusockets_register_devices(void)
> +{
> + sysbus_register_withprop(&cpusockets_info);
> +}
> +
> +device_init(cpusockets_register_devices)
> diff --git a/hw/icc_bus.h b/hw/icc_bus.h
> new file mode 100644
> index 0000000..9f10e1e
> --- /dev/null
> +++ b/hw/icc_bus.h
> @@ -0,0 +1,61 @@
> +/* ICCBus.h
> + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * 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 QEMU_ICC_H
> +#define QEMU_ICC_H
> +
> +#include "qdev.h"
> +#include "sysbus.h"
> +
> +typedef struct CPUSockets CPUSockets;
> +typedef struct ICCBus ICCBus;
> +typedef struct ICCBusInfo ICCBusInfo;
> +typedef struct ICCBusDevice ICCBusDevice;
> +typedef struct ICCBusDeviceInfo ICCBusDeviceInfo;
> +
> +struct CPUSockets {
> + SysBusDevice sysdev;
> + ICCBus *icc_bus;
> +};
> +
> +struct CPUSInfo {
> + DeviceInfo info;
> +};
> +
> +struct ICCBus {
> + BusState qbus;
> +};
> +
> +struct ICCBusInfo {
> + BusInfo qinfo;
> +};
> +struct ICCBusDevice {
> + DeviceState qdev;
> +};
> +
> +typedef int (*iccbus_initfn)(ICCBusDevice *dev);
> +
> +struct ICCBusDeviceInfo {
> + DeviceInfo qdev;
> + iccbus_initfn init;
> +};
> +
> +void iccbus_register_devinfo(ICCBusDeviceInfo *info);
> +BusState *get_icc_bus(void);
> +
> +#endif
> diff --git a/hw/pc.c b/hw/pc.c
> index 6b3662e..ffdca64 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -24,6 +24,7 @@
> #include "hw.h"
> #include "pc.h"
> #include "apic.h"
> +#include "icc_bus.h"
> #include "fdc.h"
> #include "ide.h"
> #include "pci.h"
> @@ -875,21 +876,21 @@ DeviceState *cpu_get_current_apic(void)
> static DeviceState *apic_init(void *env, uint8_t apic_id)
> {
> DeviceState *dev;
> - SysBusDevice *d;
> + BusState *b;
> static int apic_mapped;
>
> - dev = qdev_create(NULL, "apic");
> + b = get_icc_bus();
> + dev = qdev_create(b, "apic");
> qdev_prop_set_uint8(dev, "id", apic_id);
> qdev_prop_set_ptr(dev, "cpu_env", env);
> qdev_init_nofail(dev);
> - d = sysbus_from_qdev(dev);
>
> /* XXX: mapping more APICs at the same memory location */
> if (apic_mapped == 0) {
> /* 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(d, 0, MSI_ADDR_BASE);
> + apic_mmio_map(dev, MSI_ADDR_BASE);
> apic_mapped = 1;
> }
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 7055591..09aece9 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -41,7 +41,6 @@
> #include "blockdev.h"
> #include "smbus.h"
> #include "xen.h"
> -#include "memory.h"
That change looks stray.
> #include "exec-memory.h"
> #ifdef CONFIG_XEN
> # include <xen/hvm/hvm_info_table.h>
> @@ -102,8 +101,21 @@ static void pc_init1(MemoryRegion *system_memory,
> MemoryRegion *ram_memory;
> MemoryRegion *pci_memory;
>
> + if (cpu_model == NULL) {
> +#ifdef TARGET_X86_64
> + cpu_model = "qemu64";
> +#else
> + cpu_model = "qemu32";
> +#endif
> + }
> +
> global_cpu_model = cpu_model;
>
> + if (cpu_has_apic_feature(cpu_model) || smp_cpus > 1) {
> + DeviceState *d = qdev_create(NULL, "cpusockets");
> + qdev_init_nofail(d);
Indention is still off.
While at it, why not move all this into pc_cpus_init? That way other PC
chipsets could once make use of it as well.
> + }
> +
> pc_cpus_init(cpu_model);
>
> if (kvmclock_enabled) {
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 2a071f2..abdeb40 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -884,6 +884,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> uint32_t *eax, uint32_t *ebx,
> uint32_t *ecx, uint32_t *edx);
> int cpu_x86_register (CPUX86State *env, const char *cpu_model);
> +int cpu_has_apic_feature(const char *cpu_model);
> void cpu_clear_apic_feature(CPUX86State *env);
> void host_cpuid(uint32_t function, uint32_t count,
> uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> index f179999..beaef5b 100644
> --- a/target-i386/cpuid.c
> +++ b/target-i386/cpuid.c
> @@ -850,6 +850,22 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
> }
> }
>
> +int cpu_has_apic_feature(const char *cpu_model)
> +{
> + x86_def_t def1, *def = &def1;
> +
> + memset(def, 0, sizeof(*def));
> +
> + if (cpu_x86_find_by_name(def, cpu_model) < 0) {
> + return 0;
> + }
> + if (def->features & CPUID_APIC) {
> + return 1;
> + } else {
> + return 0;
> + }
> +}
> +
> int cpu_x86_register (CPUX86State *env, const char *cpu_model)
> {
> x86_def_t def1, *def = &def1;
Ack from my side if you address that one thing.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH V3] Introduce a new bus "ICC" to connect APIC
2011-11-06 18:06 ` Jan Kiszka
@ 2011-11-08 1:44 ` liu ping fan
2011-11-08 10:57 ` Jan Kiszka
0 siblings, 1 reply; 4+ messages in thread
From: liu ping fan @ 2011-11-08 1:44 UTC (permalink / raw)
To: Jan Kiszka; +Cc: blauwirbel, aliguori, ryanh, qemu-devel
On Sun, Nov 06, 2011 at 07:06:29PM +0100, Jan Kiszka wrote:
> On 2011-11-03 10:30, pingfank@linux.vnet.ibm.com wrote:
> > From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> >
> > Introduce a new structure CPUS as the controller of ICC (INTERRUPT
> > CONTROLLER COMMUNICATIONS), and new bus "ICC" to hold APIC,instead
> > of sysbus. So we can support APIC hot-plug feature.
> >
> > Signed-off-by: liu ping fan <pingfank@linux.vnet.ibm.com>
> > ---
> > Makefile.target | 1 +
> > hw/apic.c | 24 +++++++++----
> > hw/apic.h | 1 +
> > hw/icc_bus.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > hw/icc_bus.h | 61 +++++++++++++++++++++++++++++++++
> > hw/pc.c | 9 +++--
> > hw/pc_piix.c | 14 +++++++-
> > target-i386/cpu.h | 1 +
> > target-i386/cpuid.c | 16 +++++++++
> > 9 files changed, 207 insertions(+), 12 deletions(-)
> > create mode 100644 hw/icc_bus.c
> > create mode 100644 hw/icc_bus.h
> >
> > diff --git a/Makefile.target b/Makefile.target
> > index 9011f28..5607c6d 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
> > obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
> > obj-i386-y += testdev.o
> > obj-i386-y += acpi.o acpi_piix4.o
> > +obj-i386-y += icc_bus.o
> >
> > obj-i386-y += pcspk.o i8254.o
> > obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
> > diff --git a/hw/apic.c b/hw/apic.c
> > index 69d6ac5..34fa1dd 100644
> > --- a/hw/apic.c
> > +++ b/hw/apic.c
> > @@ -21,9 +21,10 @@
> > #include "ioapic.h"
> > #include "qemu-timer.h"
> > #include "host-utils.h"
> > -#include "sysbus.h"
> > +#include "icc_bus.h"
> > #include "trace.h"
> > #include "kvm.h"
> > +#include "exec-memory.h"
> >
> > /* APIC Local Vector Table */
> > #define APIC_LVT_TIMER 0
> > @@ -80,7 +81,7 @@
> > typedef struct APICState APICState;
> >
> > struct APICState {
> > - SysBusDevice busdev;
> > + ICCBusDevice busdev;
> > MemoryRegion io_memory;
> > void *cpu_env;
> > uint32_t apicbase;
> > @@ -1104,9 +1105,19 @@ static const MemoryRegionOps apic_io_ops = {
> > .endianness = DEVICE_NATIVE_ENDIAN,
> > };
> >
> > -static int apic_init1(SysBusDevice *dev)
> > +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
> > {
> > - APICState *s = FROM_SYSBUS(APICState, dev);
> > + APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
> > +
> > + memory_region_add_subregion(get_system_memory(),
> > + base,
> > + &s->io_memory);
> > + return 0;
> > +}
> > +
> > +static int apic_init1(ICCBusDevice *dev)
> > +{
> > + APICState *s = DO_UPCAST(APICState, busdev, dev);
> > static int last_apic_idx;
> >
> > if (last_apic_idx >= MAX_APICS) {
> > @@ -1114,7 +1125,6 @@ static int apic_init1(SysBusDevice *dev)
> > }
> > memory_region_init_io(&s->io_memory, &apic_io_ops, s, "apic",
> > MSI_ADDR_SIZE);
> > - sysbus_init_mmio_region(dev, &s->io_memory);
> >
> > s->timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
> > s->idx = last_apic_idx++;
> > @@ -1122,7 +1132,7 @@ static int apic_init1(SysBusDevice *dev)
> > return 0;
> > }
> >
> > -static SysBusDeviceInfo apic_info = {
> > +static ICCBusDeviceInfo apic_info = {
> > .init = apic_init1,
> > .qdev.name = "apic",
> > .qdev.size = sizeof(APICState),
> > @@ -1138,7 +1148,7 @@ static SysBusDeviceInfo apic_info = {
> >
> > static void apic_register_devices(void)
> > {
> > - sysbus_register_withprop(&apic_info);
> > + iccbus_register_devinfo(&apic_info);
> > }
> >
> > device_init(apic_register_devices)
> > diff --git a/hw/apic.h b/hw/apic.h
> > index c857d52..e2c0af5 100644
> > --- a/hw/apic.h
> > +++ b/hw/apic.h
> > @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
> > uint8_t cpu_get_apic_tpr(DeviceState *s);
> > void apic_init_reset(DeviceState *s);
> > void apic_sipi(DeviceState *s);
> > +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
> >
> > /* pc.c */
> > int cpu_is_bsp(CPUState *env);
> > diff --git a/hw/icc_bus.c b/hw/icc_bus.c
> > new file mode 100644
> > index 0000000..ac88f2e
> > --- /dev/null
> > +++ b/hw/icc_bus.c
> > @@ -0,0 +1,92 @@
> > +/* icc_bus.c
> > + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
> > + *
> > + * Copyright IBM, Corp. 2011
> > + *
> > + * 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"
> > +
> > +static CPUSockets *cpu_sockets;
> > +
> > +static ICCBusInfo icc_bus_info = {
> > + .qinfo.name = "icc",
> > + .qinfo.size = sizeof(ICCBus),
> > + .qinfo.props = (Property[]) {
> > + DEFINE_PROP_END_OF_LIST(),
> > + }
> > +};
> > +
> > +static int iccbus_device_init(DeviceState *dev, DeviceInfo *base)
> > +{
> > + ICCBusDeviceInfo *info = container_of(base, ICCBusDeviceInfo, qdev);
> > + ICCBusDevice *idev = DO_UPCAST(ICCBusDevice, qdev, dev);
> > +
> > + return info->init(idev);
> > +}
> > +
> > +void iccbus_register_devinfo(ICCBusDeviceInfo *info)
> > +{
> > + info->qdev.init = iccbus_device_init;
> > + info->qdev.bus_info = &icc_bus_info.qinfo;
> > + assert(info->qdev.size >= sizeof(ICCBusDevice));
> > + qdev_register(&info->qdev);
> > +}
> > +
> > +BusState *get_icc_bus(void)
> > +{
> > + return &cpu_sockets->icc_bus->qbus;
> > +}
> > +
> > +/*Must be called before vcpu's creation*/
> > +static int cpusockets_init(SysBusDevice *dev)
> > +{
> > + CPUSockets *cpus = DO_UPCAST(CPUSockets, sysdev, dev);
> > + BusState *b = qbus_create(&icc_bus_info.qinfo,
> > + &cpu_sockets->sysdev.qdev,
> > + "icc");
> > + if (b == NULL) {
> > + return -1;
> > + }
> > + b->allow_hotplug = 1; /* Yes, we can */
> > + cpus->icc_bus = DO_UPCAST(ICCBus, qbus, b);
> > + cpu_sockets = cpus;
> > + return 0;
> > +
> > +}
> > +
> > +static const VMStateDescription vmstate_cpusockets = {
> > + .name = "cpusockets",
> > + .version_id = 1,
> > + .minimum_version_id = 0,
> > + .fields = (VMStateField[]) {
> > + VMSTATE_END_OF_LIST()
> > + }
> > +};
> > +
> > +static SysBusDeviceInfo cpusockets_info = {
> > + .init = cpusockets_init,
> > + .qdev.name = "cpusockets",
> > + .qdev.size = sizeof(CPUSockets),
> > + .qdev.vmsd = &vmstate_cpusockets,
> > + .qdev.reset = NULL,
> > + .qdev.no_user = 1,
> > +};
> > +
> > +static void cpusockets_register_devices(void)
> > +{
> > + sysbus_register_withprop(&cpusockets_info);
> > +}
> > +
> > +device_init(cpusockets_register_devices)
> > diff --git a/hw/icc_bus.h b/hw/icc_bus.h
> > new file mode 100644
> > index 0000000..9f10e1e
> > --- /dev/null
> > +++ b/hw/icc_bus.h
> > @@ -0,0 +1,61 @@
> > +/* ICCBus.h
> > + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
> > + *
> > + * Copyright IBM, Corp. 2011
> > + *
> > + * 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 QEMU_ICC_H
> > +#define QEMU_ICC_H
> > +
> > +#include "qdev.h"
> > +#include "sysbus.h"
> > +
> > +typedef struct CPUSockets CPUSockets;
> > +typedef struct ICCBus ICCBus;
> > +typedef struct ICCBusInfo ICCBusInfo;
> > +typedef struct ICCBusDevice ICCBusDevice;
> > +typedef struct ICCBusDeviceInfo ICCBusDeviceInfo;
> > +
> > +struct CPUSockets {
> > + SysBusDevice sysdev;
> > + ICCBus *icc_bus;
> > +};
> > +
> > +struct CPUSInfo {
> > + DeviceInfo info;
> > +};
> > +
> > +struct ICCBus {
> > + BusState qbus;
> > +};
> > +
> > +struct ICCBusInfo {
> > + BusInfo qinfo;
> > +};
> > +struct ICCBusDevice {
> > + DeviceState qdev;
> > +};
> > +
> > +typedef int (*iccbus_initfn)(ICCBusDevice *dev);
> > +
> > +struct ICCBusDeviceInfo {
> > + DeviceInfo qdev;
> > + iccbus_initfn init;
> > +};
> > +
> > +void iccbus_register_devinfo(ICCBusDeviceInfo *info);
> > +BusState *get_icc_bus(void);
> > +
> > +#endif
> > diff --git a/hw/pc.c b/hw/pc.c
> > index 6b3662e..ffdca64 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -24,6 +24,7 @@
> > #include "hw.h"
> > #include "pc.h"
> > #include "apic.h"
> > +#include "icc_bus.h"
> > #include "fdc.h"
> > #include "ide.h"
> > #include "pci.h"
> > @@ -875,21 +876,21 @@ DeviceState *cpu_get_current_apic(void)
> > static DeviceState *apic_init(void *env, uint8_t apic_id)
> > {
> > DeviceState *dev;
> > - SysBusDevice *d;
> > + BusState *b;
> > static int apic_mapped;
> >
> > - dev = qdev_create(NULL, "apic");
> > + b = get_icc_bus();
> > + dev = qdev_create(b, "apic");
> > qdev_prop_set_uint8(dev, "id", apic_id);
> > qdev_prop_set_ptr(dev, "cpu_env", env);
> > qdev_init_nofail(dev);
> > - d = sysbus_from_qdev(dev);
> >
> > /* XXX: mapping more APICs at the same memory location */
> > if (apic_mapped == 0) {
> > /* 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(d, 0, MSI_ADDR_BASE);
> > + apic_mmio_map(dev, MSI_ADDR_BASE);
> > apic_mapped = 1;
> > }
> >
> > diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> > index 7055591..09aece9 100644
> > --- a/hw/pc_piix.c
> > +++ b/hw/pc_piix.c
> > @@ -41,7 +41,6 @@
> > #include "blockdev.h"
> > #include "smbus.h"
> > #include "xen.h"
> > -#include "memory.h"
>
> That change looks stray.
>
Adopted,
> > #include "exec-memory.h"
> > #ifdef CONFIG_XEN
> > # include <xen/hvm/hvm_info_table.h>
> > @@ -102,8 +101,21 @@ static void pc_init1(MemoryRegion *system_memory,
> > MemoryRegion *ram_memory;
> > MemoryRegion *pci_memory;
> >
> > + if (cpu_model == NULL) {
> > +#ifdef TARGET_X86_64
> > + cpu_model = "qemu64";
> > +#else
> > + cpu_model = "qemu32";
> > +#endif
> > + }
> > +
> > global_cpu_model = cpu_model;
> >
> > + if (cpu_has_apic_feature(cpu_model) || smp_cpus > 1) {
> > + DeviceState *d = qdev_create(NULL, "cpusockets");
> > + qdev_init_nofail(d);
>
> Indention is still off.
>
Adopted, BTW, could you tell me how do you find this(checkpatch.pl seems not to work on it), so I can avoid such mistake in future :), thanks.
> While at it, why not move all this into pc_cpus_init? That way other PC
> chipsets could once make use of it as well.
>
As we have discussed, ICC is a board level facility, so I think it is better to put it in pc_init1 than in pc_cpus_init, where I consider it as the initialization of CPUs.
Thanks and regards,
ping fan
> > + }
> > +
> > pc_cpus_init(cpu_model);
> >
> > if (kvmclock_enabled) {
> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > index 2a071f2..abdeb40 100644
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -884,6 +884,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> > uint32_t *eax, uint32_t *ebx,
> > uint32_t *ecx, uint32_t *edx);
> > int cpu_x86_register (CPUX86State *env, const char *cpu_model);
> > +int cpu_has_apic_feature(const char *cpu_model);
> > void cpu_clear_apic_feature(CPUX86State *env);
> > void host_cpuid(uint32_t function, uint32_t count,
> > uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> > diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> > index f179999..beaef5b 100644
> > --- a/target-i386/cpuid.c
> > +++ b/target-i386/cpuid.c
> > @@ -850,6 +850,22 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
> > }
> > }
> >
> > +int cpu_has_apic_feature(const char *cpu_model)
> > +{
> > + x86_def_t def1, *def = &def1;
> > +
> > + memset(def, 0, sizeof(*def));
> > +
> > + if (cpu_x86_find_by_name(def, cpu_model) < 0) {
> > + return 0;
> > + }
> > + if (def->features & CPUID_APIC) {
> > + return 1;
> > + } else {
> > + return 0;
> > + }
> > +}
> > +
> > int cpu_x86_register (CPUX86State *env, const char *cpu_model)
> > {
> > x86_def_t def1, *def = &def1;
>
> Ack from my side if you address that one thing.
>
> Jan
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH V3] Introduce a new bus "ICC" to connect APIC
2011-11-08 1:44 ` liu ping fan
@ 2011-11-08 10:57 ` Jan Kiszka
0 siblings, 0 replies; 4+ messages in thread
From: Jan Kiszka @ 2011-11-08 10:57 UTC (permalink / raw)
To: liu ping fan; +Cc: blauwirbel, aliguori, ryanh, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 12678 bytes --]
On 2011-11-08 02:44, liu ping fan wrote:
> On Sun, Nov 06, 2011 at 07:06:29PM +0100, Jan Kiszka wrote:
>> On 2011-11-03 10:30, pingfank@linux.vnet.ibm.com wrote:
>>> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>>>
>>> Introduce a new structure CPUS as the controller of ICC (INTERRUPT
>>> CONTROLLER COMMUNICATIONS), and new bus "ICC" to hold APIC,instead
>>> of sysbus. So we can support APIC hot-plug feature.
>>>
>>> Signed-off-by: liu ping fan <pingfank@linux.vnet.ibm.com>
>>> ---
>>> Makefile.target | 1 +
>>> hw/apic.c | 24 +++++++++----
>>> hw/apic.h | 1 +
>>> hw/icc_bus.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>> hw/icc_bus.h | 61 +++++++++++++++++++++++++++++++++
>>> hw/pc.c | 9 +++--
>>> hw/pc_piix.c | 14 +++++++-
>>> target-i386/cpu.h | 1 +
>>> target-i386/cpuid.c | 16 +++++++++
>>> 9 files changed, 207 insertions(+), 12 deletions(-)
>>> create mode 100644 hw/icc_bus.c
>>> create mode 100644 hw/icc_bus.h
>>>
>>> diff --git a/Makefile.target b/Makefile.target
>>> index 9011f28..5607c6d 100644
>>> --- a/Makefile.target
>>> +++ b/Makefile.target
>>> @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
>>> obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>> obj-i386-y += testdev.o
>>> obj-i386-y += acpi.o acpi_piix4.o
>>> +obj-i386-y += icc_bus.o
>>>
>>> obj-i386-y += pcspk.o i8254.o
>>> obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
>>> diff --git a/hw/apic.c b/hw/apic.c
>>> index 69d6ac5..34fa1dd 100644
>>> --- a/hw/apic.c
>>> +++ b/hw/apic.c
>>> @@ -21,9 +21,10 @@
>>> #include "ioapic.h"
>>> #include "qemu-timer.h"
>>> #include "host-utils.h"
>>> -#include "sysbus.h"
>>> +#include "icc_bus.h"
>>> #include "trace.h"
>>> #include "kvm.h"
>>> +#include "exec-memory.h"
>>>
>>> /* APIC Local Vector Table */
>>> #define APIC_LVT_TIMER 0
>>> @@ -80,7 +81,7 @@
>>> typedef struct APICState APICState;
>>>
>>> struct APICState {
>>> - SysBusDevice busdev;
>>> + ICCBusDevice busdev;
>>> MemoryRegion io_memory;
>>> void *cpu_env;
>>> uint32_t apicbase;
>>> @@ -1104,9 +1105,19 @@ static const MemoryRegionOps apic_io_ops = {
>>> .endianness = DEVICE_NATIVE_ENDIAN,
>>> };
>>>
>>> -static int apic_init1(SysBusDevice *dev)
>>> +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
>>> {
>>> - APICState *s = FROM_SYSBUS(APICState, dev);
>>> + APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
>>> +
>>> + memory_region_add_subregion(get_system_memory(),
>>> + base,
>>> + &s->io_memory);
>>> + return 0;
>>> +}
>>> +
>>> +static int apic_init1(ICCBusDevice *dev)
>>> +{
>>> + APICState *s = DO_UPCAST(APICState, busdev, dev);
>>> static int last_apic_idx;
>>>
>>> if (last_apic_idx >= MAX_APICS) {
>>> @@ -1114,7 +1125,6 @@ static int apic_init1(SysBusDevice *dev)
>>> }
>>> memory_region_init_io(&s->io_memory, &apic_io_ops, s, "apic",
>>> MSI_ADDR_SIZE);
>>> - sysbus_init_mmio_region(dev, &s->io_memory);
>>>
>>> s->timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
>>> s->idx = last_apic_idx++;
>>> @@ -1122,7 +1132,7 @@ static int apic_init1(SysBusDevice *dev)
>>> return 0;
>>> }
>>>
>>> -static SysBusDeviceInfo apic_info = {
>>> +static ICCBusDeviceInfo apic_info = {
>>> .init = apic_init1,
>>> .qdev.name = "apic",
>>> .qdev.size = sizeof(APICState),
>>> @@ -1138,7 +1148,7 @@ static SysBusDeviceInfo apic_info = {
>>>
>>> static void apic_register_devices(void)
>>> {
>>> - sysbus_register_withprop(&apic_info);
>>> + iccbus_register_devinfo(&apic_info);
>>> }
>>>
>>> device_init(apic_register_devices)
>>> diff --git a/hw/apic.h b/hw/apic.h
>>> index c857d52..e2c0af5 100644
>>> --- a/hw/apic.h
>>> +++ b/hw/apic.h
>>> @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
>>> uint8_t cpu_get_apic_tpr(DeviceState *s);
>>> void apic_init_reset(DeviceState *s);
>>> void apic_sipi(DeviceState *s);
>>> +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
>>>
>>> /* pc.c */
>>> int cpu_is_bsp(CPUState *env);
>>> diff --git a/hw/icc_bus.c b/hw/icc_bus.c
>>> new file mode 100644
>>> index 0000000..ac88f2e
>>> --- /dev/null
>>> +++ b/hw/icc_bus.c
>>> @@ -0,0 +1,92 @@
>>> +/* icc_bus.c
>>> + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
>>> + *
>>> + * Copyright IBM, Corp. 2011
>>> + *
>>> + * 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"
>>> +
>>> +static CPUSockets *cpu_sockets;
>>> +
>>> +static ICCBusInfo icc_bus_info = {
>>> + .qinfo.name = "icc",
>>> + .qinfo.size = sizeof(ICCBus),
>>> + .qinfo.props = (Property[]) {
>>> + DEFINE_PROP_END_OF_LIST(),
>>> + }
>>> +};
>>> +
>>> +static int iccbus_device_init(DeviceState *dev, DeviceInfo *base)
>>> +{
>>> + ICCBusDeviceInfo *info = container_of(base, ICCBusDeviceInfo, qdev);
>>> + ICCBusDevice *idev = DO_UPCAST(ICCBusDevice, qdev, dev);
>>> +
>>> + return info->init(idev);
>>> +}
>>> +
>>> +void iccbus_register_devinfo(ICCBusDeviceInfo *info)
>>> +{
>>> + info->qdev.init = iccbus_device_init;
>>> + info->qdev.bus_info = &icc_bus_info.qinfo;
>>> + assert(info->qdev.size >= sizeof(ICCBusDevice));
>>> + qdev_register(&info->qdev);
>>> +}
>>> +
>>> +BusState *get_icc_bus(void)
>>> +{
>>> + return &cpu_sockets->icc_bus->qbus;
>>> +}
>>> +
>>> +/*Must be called before vcpu's creation*/
>>> +static int cpusockets_init(SysBusDevice *dev)
>>> +{
>>> + CPUSockets *cpus = DO_UPCAST(CPUSockets, sysdev, dev);
>>> + BusState *b = qbus_create(&icc_bus_info.qinfo,
>>> + &cpu_sockets->sysdev.qdev,
>>> + "icc");
>>> + if (b == NULL) {
>>> + return -1;
>>> + }
>>> + b->allow_hotplug = 1; /* Yes, we can */
>>> + cpus->icc_bus = DO_UPCAST(ICCBus, qbus, b);
>>> + cpu_sockets = cpus;
>>> + return 0;
>>> +
>>> +}
>>> +
>>> +static const VMStateDescription vmstate_cpusockets = {
>>> + .name = "cpusockets",
>>> + .version_id = 1,
>>> + .minimum_version_id = 0,
>>> + .fields = (VMStateField[]) {
>>> + VMSTATE_END_OF_LIST()
>>> + }
>>> +};
>>> +
>>> +static SysBusDeviceInfo cpusockets_info = {
>>> + .init = cpusockets_init,
>>> + .qdev.name = "cpusockets",
>>> + .qdev.size = sizeof(CPUSockets),
>>> + .qdev.vmsd = &vmstate_cpusockets,
>>> + .qdev.reset = NULL,
>>> + .qdev.no_user = 1,
>>> +};
>>> +
>>> +static void cpusockets_register_devices(void)
>>> +{
>>> + sysbus_register_withprop(&cpusockets_info);
>>> +}
>>> +
>>> +device_init(cpusockets_register_devices)
>>> diff --git a/hw/icc_bus.h b/hw/icc_bus.h
>>> new file mode 100644
>>> index 0000000..9f10e1e
>>> --- /dev/null
>>> +++ b/hw/icc_bus.h
>>> @@ -0,0 +1,61 @@
>>> +/* ICCBus.h
>>> + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
>>> + *
>>> + * Copyright IBM, Corp. 2011
>>> + *
>>> + * 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 QEMU_ICC_H
>>> +#define QEMU_ICC_H
>>> +
>>> +#include "qdev.h"
>>> +#include "sysbus.h"
>>> +
>>> +typedef struct CPUSockets CPUSockets;
>>> +typedef struct ICCBus ICCBus;
>>> +typedef struct ICCBusInfo ICCBusInfo;
>>> +typedef struct ICCBusDevice ICCBusDevice;
>>> +typedef struct ICCBusDeviceInfo ICCBusDeviceInfo;
>>> +
>>> +struct CPUSockets {
>>> + SysBusDevice sysdev;
>>> + ICCBus *icc_bus;
>>> +};
>>> +
>>> +struct CPUSInfo {
>>> + DeviceInfo info;
>>> +};
>>> +
>>> +struct ICCBus {
>>> + BusState qbus;
>>> +};
>>> +
>>> +struct ICCBusInfo {
>>> + BusInfo qinfo;
>>> +};
>>> +struct ICCBusDevice {
>>> + DeviceState qdev;
>>> +};
>>> +
>>> +typedef int (*iccbus_initfn)(ICCBusDevice *dev);
>>> +
>>> +struct ICCBusDeviceInfo {
>>> + DeviceInfo qdev;
>>> + iccbus_initfn init;
>>> +};
>>> +
>>> +void iccbus_register_devinfo(ICCBusDeviceInfo *info);
>>> +BusState *get_icc_bus(void);
>>> +
>>> +#endif
>>> diff --git a/hw/pc.c b/hw/pc.c
>>> index 6b3662e..ffdca64 100644
>>> --- a/hw/pc.c
>>> +++ b/hw/pc.c
>>> @@ -24,6 +24,7 @@
>>> #include "hw.h"
>>> #include "pc.h"
>>> #include "apic.h"
>>> +#include "icc_bus.h"
>>> #include "fdc.h"
>>> #include "ide.h"
>>> #include "pci.h"
>>> @@ -875,21 +876,21 @@ DeviceState *cpu_get_current_apic(void)
>>> static DeviceState *apic_init(void *env, uint8_t apic_id)
>>> {
>>> DeviceState *dev;
>>> - SysBusDevice *d;
>>> + BusState *b;
>>> static int apic_mapped;
>>>
>>> - dev = qdev_create(NULL, "apic");
>>> + b = get_icc_bus();
>>> + dev = qdev_create(b, "apic");
>>> qdev_prop_set_uint8(dev, "id", apic_id);
>>> qdev_prop_set_ptr(dev, "cpu_env", env);
>>> qdev_init_nofail(dev);
>>> - d = sysbus_from_qdev(dev);
>>>
>>> /* XXX: mapping more APICs at the same memory location */
>>> if (apic_mapped == 0) {
>>> /* 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(d, 0, MSI_ADDR_BASE);
>>> + apic_mmio_map(dev, MSI_ADDR_BASE);
>>> apic_mapped = 1;
>>> }
>>>
>>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>>> index 7055591..09aece9 100644
>>> --- a/hw/pc_piix.c
>>> +++ b/hw/pc_piix.c
>>> @@ -41,7 +41,6 @@
>>> #include "blockdev.h"
>>> #include "smbus.h"
>>> #include "xen.h"
>>> -#include "memory.h"
>>
>> That change looks stray.
>>
> Adopted,
>>> #include "exec-memory.h"
>>> #ifdef CONFIG_XEN
>>> # include <xen/hvm/hvm_info_table.h>
>>> @@ -102,8 +101,21 @@ static void pc_init1(MemoryRegion *system_memory,
>>> MemoryRegion *ram_memory;
>>> MemoryRegion *pci_memory;
>>>
>>> + if (cpu_model == NULL) {
>>> +#ifdef TARGET_X86_64
>>> + cpu_model = "qemu64";
>>> +#else
>>> + cpu_model = "qemu32";
>>> +#endif
>>> + }
>>> +
>>> global_cpu_model = cpu_model;
>>>
>>> + if (cpu_has_apic_feature(cpu_model) || smp_cpus > 1) {
>>> + DeviceState *d = qdev_create(NULL, "cpusockets");
>>> + qdev_init_nofail(d);
>>
>> Indention is still off.
>>
> Adopted, BTW, could you tell me how do you find this(checkpatch.pl seems not to work on it), so I can avoid such mistake in future :), thanks.
I guess it's a checkpatch deficit.
>
>> While at it, why not move all this into pc_cpus_init? That way other PC
>> chipsets could once make use of it as well.
>>
> As we have discussed, ICC is a board level facility, so I think it is better to put it in pc_init1 than in pc_cpus_init, where I consider it as the initialization of CPUs.
The board can control the ICC creation by preventing APICs (via CPU type
and QEMUMachine.max_cpus). But the instantiation is a generic feature
again. So pc_cpus_init makes more sense.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-11-08 10:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-03 9:30 [Qemu-devel] [PATCH V3] Introduce a new bus "ICC" to connect APIC pingfank
2011-11-06 18:06 ` Jan Kiszka
2011-11-08 1:44 ` liu ping fan
2011-11-08 10:57 ` Jan Kiszka
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).