From: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
To: Igor Mammedov <imammedo@redhat.com>
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
Subject: Re: [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge
Date: Tue, 15 Sep 2015 08:52:34 +0800 [thread overview]
Message-ID: <55F76BD2.5090508@cn.fujitsu.com> (raw)
In-Reply-To: <20150914151820.31399903@nial.brq.redhat.com>
On 09/14/2015 09:18 PM, Igor Mammedov wrote:
> On Wed, 2 Sep 2015 17:36:21 +0800
> Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
>
>> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>>
>> 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 <chen.fan.fnst@cn.fujitsu.com>
>> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
>> ---
>> 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;
> .
>
next prev parent reply other threads:[~2015-09-15 0:54 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-02 9:36 [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
2015-09-02 9:36 ` [Qemu-devel] [PATCH v11 1/5] apic: move APIC's MMIO region mapping into APIC Zhu Guihua
2015-09-14 13:16 ` Igor Mammedov
2015-09-02 9:36 ` [Qemu-devel] [PATCH v11 2/5] apic: use per CPU AS to map APIC MMIO for TCG Zhu Guihua
2015-09-02 9:36 ` [Qemu-devel] [PATCH v11 3/5] x86: use new method to correct reset sequence Zhu Guihua
2015-09-02 9:36 ` [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge Zhu Guihua
2015-09-14 13:18 ` Igor Mammedov
2015-09-15 0:52 ` Zhu Guihua [this message]
2015-09-15 8:15 ` Igor Mammedov
2015-09-02 9:36 ` [Qemu-devel] [PATCH v11 5/5] icc_bus: drop the unused files Zhu Guihua
2015-09-10 10:21 ` [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55F76BD2.5090508@cn.fujitsu.com \
--to=zhugh.fnst@cn.fujitsu.com \
--cc=afaerber@suse.de \
--cc=chen.fan.fnst@cn.fujitsu.com \
--cc=ehabkost@redhat.com \
--cc=imammedo@redhat.com \
--cc=izumi.taku@jp.fujitsu.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.