[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/
|
From: |
Igor Mammedov |
|
Subject: |
Re: [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/ |
|
Date: |
Tue, 14 Apr 2015 15:27:59 +0200 |
On Fri, 10 Apr 2015 16:18:06 +0800
Chen Fan <address@hidden> 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 <address@hidden>
> ---
> 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 <address@hidden>
> - *
> - * 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 <address@hidden>
> - *
> - * 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;