From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37661) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbeVf-0000qx-Bb for qemu-devel@nongnu.org; Mon, 14 Sep 2015 20:54:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZbeVa-0002MF-V1 for qemu-devel@nongnu.org; Mon, 14 Sep 2015 20:54:11 -0400 Received: from [59.151.112.132] (port=6966 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbeVZ-0002Jy-W7 for qemu-devel@nongnu.org; Mon, 14 Sep 2015 20:54:06 -0400 Message-ID: <55F76BD2.5090508@cn.fujitsu.com> Date: Tue, 15 Sep 2015 08:52:34 +0800 From: Zhu Guihua MIME-Version: 1.0 References: <837618167f041a6530635f4337a1ef391311d63c.1441086002.git.zhugh.fnst@cn.fujitsu.com> <20150914151820.31399903@nial.brq.redhat.com> In-Reply-To: <20150914151820.31399903@nial.brq.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: ehabkost@redhat.com, qemu-devel@nongnu.org, izumi.taku@jp.fujitsu.com, chen.fan.fnst@cn.fujitsu.com, pbonzini@redhat.com, afaerber@suse.de On 09/14/2015 09:18 PM, Igor Mammedov wrote: > On Wed, 2 Sep 2015 17:36:21 +0800 > Zhu Guihua wrote: > >> From: Chen Fan >> >> After CPU hotplug has been converted to BUS-less hot-plug infrastructure, >> the only function ICC bus performs is to propagate reset to LAPICs. However >> LAPIC could be reset by registering its reset handler after all device are >> initialized. >> Do so and drop ~200LOC of not needed anymore ICCBus related code. > this patch drops about 30LOCs only ... > > > PS: > Doesn't apply to current master, please rebase. This patch rebased on my another patch '[PATCH v3] i386: keep cpu_model field in MachineState uptodate' https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg03375.html The patch has been reviewed by Eduardo, but not been merged. Should I only rebase on current master? Thanks, Zhu >> Signed-off-by: Chen Fan >> Signed-off-by: Zhu Guihua >> --- >> hw/i386/pc.c | 19 ++++--------------- >> hw/i386/pc_piix.c | 9 +-------- >> hw/i386/pc_q35.c | 9 +-------- >> hw/intc/apic_common.c | 5 ++--- >> include/hw/i386/apic_internal.h | 7 ++++--- >> include/hw/i386/pc.h | 2 +- >> target-i386/cpu.c | 9 +-------- >> 7 files changed, 14 insertions(+), 46 deletions(-) >> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index 4b4a7f3..4c1d68a 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -59,7 +59,6 @@ >> #include "qemu/error-report.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" >> @@ -1052,23 +1051,16 @@ 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")); >> - >> object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err); >> object_property_set_bool(OBJECT(cpu), true, "realized", &local_err); >> >> @@ -1083,7 +1075,6 @@ out: >> >> void pc_hot_add_cpu(const int64_t id, Error **errp) >> { >> - DeviceState *icc_bridge; >> MachineState *machine = MACHINE(qdev_get_machine()); >> X86CPU *cpu; >> int64_t apic_id = x86_cpu_apic_id_from_index(id); >> @@ -1113,9 +1104,7 @@ 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)); >> - cpu = pc_new_cpu(machine->cpu_model, apic_id, icc_bridge, &local_err); >> + cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err); >> if (local_err) { >> error_propagate(errp, local_err); >> return; >> @@ -1123,7 +1112,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp) >> object_unref(OBJECT(cpu)); >> } >> >> -void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge) >> +void pc_cpus_init(PCMachineState *pcms) >> { >> int i; >> X86CPU *cpu = NULL; >> @@ -1149,7 +1138,7 @@ void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge) >> >> for (i = 0; i < smp_cpus; i++) { >> cpu = pc_new_cpu(machine->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 fd6130d..3a97826 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" >> @@ -96,7 +95,6 @@ static void pc_init1(MachineState *machine) >> MemoryRegion *ram_memory; >> MemoryRegion *pci_memory; >> MemoryRegion *rom_memory; >> - DeviceState *icc_bridge; >> PcGuestInfo *guest_info; >> ram_addr_t lowmem; >> >> @@ -141,11 +139,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(pcms, icc_bridge); >> + pc_cpus_init(pcms); >> >> if (kvm_enabled() && kvmclock_enabled) { >> kvmclock_create(); >> @@ -223,7 +217,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 4f76535..414cbbb 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" >> #include "migration/migration.h" >> >> @@ -83,7 +82,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(pcms, icc_bridge); >> + pc_cpus_init(pcms); >> pc_acpi_init("q35-acpi-dsdt.aml"); >> >> kvmclock_create(); >> @@ -237,7 +231,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_common.c b/hw/intc/apic_common.c >> index c0b32eb..ad959c4 100644 >> --- a/hw/intc/apic_common.c >> +++ b/hw/intc/apic_common.c >> @@ -419,13 +419,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() >> @@ -435,7 +434,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/i386/apic_internal.h b/include/hw/i386/apic_internal.h >> index 26632ac..905ada7 100644 >> --- a/include/hw/i386/apic_internal.h >> +++ b/include/hw/i386/apic_internal.h >> @@ -22,7 +22,6 @@ >> >> #include "cpu.h" >> #include "exec/memory.h" >> -#include "hw/cpu/icc_bus.h" >> #include "qemu/timer.h" >> >> /* APIC Local Vector Table */ >> @@ -79,7 +78,7 @@ typedef struct APICCommonState APICCommonState; >> >> typedef struct APICCommonClass >> { >> - ICCDeviceClass parent_class; >> + DeviceClass parent_class; >> >> DeviceRealize realize; >> void (*set_base)(APICCommonState *s, uint64_t val); >> @@ -94,7 +93,9 @@ typedef struct APICCommonClass >> } APICCommonClass; >> >> struct APICCommonState { >> - ICCDevice busdev; >> + /*< private >*/ >> + DeviceState parent_obj; >> + /*< public >*/ >> >> MemoryRegion io_memory; >> X86CPU *cpu; >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h >> index 3732ab5..8eef558 100644 >> --- a/include/hw/i386/pc.h >> +++ b/include/hw/i386/pc.h >> @@ -161,7 +161,7 @@ bool pc_machine_is_smm_enabled(PCMachineState *pcms); >> void pc_register_ferr_irq(qemu_irq irq); >> void pc_acpi_smi_interrupt(void *opaque, int irq, int level); >> >> -void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge); >> +void pc_cpus_init(PCMachineState *pcms); >> 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 0a95162..6686039 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -43,7 +43,6 @@ >> >> #include "sysemu/sysemu.h" >> #include "hw/qdev-properties.h" >> -#include "hw/cpu/icc_bus.h" >> #ifndef CONFIG_USER_ONLY >> #include "exec/address-spaces.h" >> #include "hw/xen/xen.h" >> @@ -2723,7 +2722,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"; >> >> @@ -2733,11 +2731,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); >> - if (cpu->apic_state == NULL) { >> - error_setg(errp, "APIC device '%s' could not be created", apic_type); >> - return; >> - } >> + cpu->apic_state = DEVICE(object_new(apic_type)); >> >> object_property_add_child(OBJECT(cpu), "apic", >> OBJECT(cpu->apic_state), NULL); >> @@ -3162,7 +3156,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; > . >