* [Qemu-devel] [PATCH v10 1/4] apic: map APIC's MMIO region at each CPU's address space
2015-08-31 9:47 [Qemu-devel] [PATCH v10 0/4] remove icc bus/bridge Zhu Guihua
@ 2015-08-31 9:47 ` Zhu Guihua
2015-08-31 13:21 ` Igor Mammedov
2015-08-31 9:47 ` [Qemu-devel] [PATCH v10 2/4] x86: use new method to correct reset sequence Zhu Guihua
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Zhu Guihua @ 2015-08-31 9:47 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
Cc: chen.fan.fnst, izumi.taku, Zhu Guihua
From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
After ICC bus/bridge have been removed, APIC MMIO area could
not be mapped into sysbus MMIO any more.
So replace mapping APIC at global system address space with
mapping it at per-CPU address spaces.
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 | 7 -------
hw/intc/apic_common.c | 6 ------
target-i386/cpu.c | 16 ++++++++++++++++
3 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b1c96a8..e15971c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1157,13 +1157,6 @@ void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
object_unref(OBJECT(cpu));
}
- /* 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 0032b97..c0b32eb 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -296,7 +296,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.",
@@ -307,11 +306,6 @@ 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;
- }
/* Note: We need at least 1M to map the VAPIC option ROM */
if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cfb8aa7..171cdc0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2745,6 +2745,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
/* TODO: convert to link<> */
apic = APIC_COMMON(cpu->apic_state);
apic->cpu = cpu;
+ apic->apicbase = APIC_DEFAULT_ADDRESS | MSR_IA32_APICBASE_ENABLE;
}
static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
@@ -2789,6 +2790,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
X86CPU *cpu = X86_CPU(dev);
X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
CPUX86State *env = &cpu->env;
+ APICCommonState *apic;
Error *local_err = NULL;
static bool ht_warned;
@@ -2877,6 +2879,20 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
if (local_err != NULL) {
goto out;
}
+
+ apic = APIC_COMMON(cpu->apic_state);
+ /* Map APIC MMIO area, use per-CPU address space if available (TCG
+ * supports it, KVM doesn't). This allows the APIC base address of
+ * each CPU to be moved independently.
+ */
+ memory_region_add_subregion_overlap(cpu->cpu_as_root ?
+ cpu->cpu_as_root :
+ get_system_memory(),
+ apic->apicbase &
+ MSR_IA32_APICBASE_BASE,
+ &apic->io_memory,
+ 0x1000);
+
cpu_reset(cs);
xcc->parent_realize(dev, &local_err);
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH v10 1/4] apic: map APIC's MMIO region at each CPU's address space
2015-08-31 9:47 ` [Qemu-devel] [PATCH v10 1/4] apic: map APIC's MMIO region at each CPU's address space Zhu Guihua
@ 2015-08-31 13:21 ` Igor Mammedov
2015-09-01 1:05 ` Zhu Guihua
0 siblings, 1 reply; 8+ messages in thread
From: Igor Mammedov @ 2015-08-31 13:21 UTC (permalink / raw)
To: Zhu Guihua
Cc: ehabkost, qemu-devel, izumi.taku, chen.fan.fnst, pbonzini,
afaerber
On Mon, 31 Aug 2015 17:47:44 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>
> After ICC bus/bridge have been removed, APIC MMIO area could
> not be mapped into sysbus MMIO any more.
> So replace mapping APIC at global system address space with
> mapping it at per-CPU address spaces.
When ICC bus/bridge is removed, APIC MMIO will be left
unmapped since it was mapped into system's address space
indirectly by ICC bridge.
Fix it by moving mapping into APIC code, so it would be
possible to remove ICC bus/bridge code later.
>
> 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 | 7 -------
> hw/intc/apic_common.c | 6 ------
> target-i386/cpu.c | 16 ++++++++++++++++
> 3 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index b1c96a8..e15971c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1157,13 +1157,6 @@ void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
> object_unref(OBJECT(cpu));
> }
>
> - /* 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 0032b97..c0b32eb 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -296,7 +296,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.",
> @@ -307,11 +306,6 @@ 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;
> - }
>
> /* Note: We need at least 1M to map the VAPIC option ROM */
> if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index cfb8aa7..171cdc0 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2745,6 +2745,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
> /* TODO: convert to link<> */
> apic = APIC_COMMON(cpu->apic_state);
> apic->cpu = cpu;
> + apic->apicbase = APIC_DEFAULT_ADDRESS | MSR_IA32_APICBASE_ENABLE;
> }
>
> static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
> @@ -2789,6 +2790,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> X86CPU *cpu = X86_CPU(dev);
> X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
> CPUX86State *env = &cpu->env;
> + APICCommonState *apic;
> Error *local_err = NULL;
> static bool ht_warned;
>
> @@ -2877,6 +2879,20 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> if (local_err != NULL) {
> goto out;
> }
> +
> + apic = APIC_COMMON(cpu->apic_state);
> + /* Map APIC MMIO area, use per-CPU address space if available (TCG
> + * supports it, KVM doesn't). This allows the APIC base address of
> + * each CPU to be moved independently.
> + */
> + memory_region_add_subregion_overlap(cpu->cpu_as_root ?
> + cpu->cpu_as_root :
> + get_system_memory(),
> + apic->apicbase &
> + MSR_IA32_APICBASE_BASE,
> + &apic->io_memory,
> + 0x1000);
You have lost apic_mmio_map_once from previous version,
as result in KVM case io_memory will be mapped many times
over itself, pls fix it.
Pls, also split per CPU AS change into a separate patch
as Eduardo asked you for, with following order:
1. move region mapping into APIC in this patch
2. add per CPU AS in an additional path
> cpu_reset(cs);
>
> xcc->parent_realize(dev, &local_err);
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH v10 1/4] apic: map APIC's MMIO region at each CPU's address space
2015-08-31 13:21 ` Igor Mammedov
@ 2015-09-01 1:05 ` Zhu Guihua
0 siblings, 0 replies; 8+ messages in thread
From: Zhu Guihua @ 2015-09-01 1:05 UTC (permalink / raw)
To: Igor Mammedov
Cc: ehabkost, qemu-devel, izumi.taku, chen.fan.fnst, pbonzini,
afaerber
On 08/31/2015 09:21 PM, Igor Mammedov wrote:
> On Mon, 31 Aug 2015 17:47:44 +0800
> Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
>
>> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>>
>> After ICC bus/bridge have been removed, APIC MMIO area could
>> not be mapped into sysbus MMIO any more.
>> So replace mapping APIC at global system address space with
>> mapping it at per-CPU address spaces.
> When ICC bus/bridge is removed, APIC MMIO will be left
> unmapped since it was mapped into system's address space
> indirectly by ICC bridge.
> Fix it by moving mapping into APIC code, so it would be
> possible to remove ICC bus/bridge code later.
>
>> 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 | 7 -------
>> hw/intc/apic_common.c | 6 ------
>> target-i386/cpu.c | 16 ++++++++++++++++
>> 3 files changed, 16 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index b1c96a8..e15971c 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1157,13 +1157,6 @@ void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
>> object_unref(OBJECT(cpu));
>> }
>>
>> - /* 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 0032b97..c0b32eb 100644
>> --- a/hw/intc/apic_common.c
>> +++ b/hw/intc/apic_common.c
>> @@ -296,7 +296,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.",
>> @@ -307,11 +306,6 @@ 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;
>> - }
>>
>> /* Note: We need at least 1M to map the VAPIC option ROM */
>> if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index cfb8aa7..171cdc0 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -2745,6 +2745,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>> /* TODO: convert to link<> */
>> apic = APIC_COMMON(cpu->apic_state);
>> apic->cpu = cpu;
>> + apic->apicbase = APIC_DEFAULT_ADDRESS | MSR_IA32_APICBASE_ENABLE;
>> }
>>
>> static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
>> @@ -2789,6 +2790,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>> X86CPU *cpu = X86_CPU(dev);
>> X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
>> CPUX86State *env = &cpu->env;
>> + APICCommonState *apic;
>> Error *local_err = NULL;
>> static bool ht_warned;
>>
>> @@ -2877,6 +2879,20 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>> if (local_err != NULL) {
>> goto out;
>> }
>> +
>> + apic = APIC_COMMON(cpu->apic_state);
>> + /* Map APIC MMIO area, use per-CPU address space if available (TCG
>> + * supports it, KVM doesn't). This allows the APIC base address of
>> + * each CPU to be moved independently.
>> + */
>> + memory_region_add_subregion_overlap(cpu->cpu_as_root ?
>> + cpu->cpu_as_root :
>> + get_system_memory(),
>> + apic->apicbase &
>> + MSR_IA32_APICBASE_BASE,
>> + &apic->io_memory,
>> + 0x1000);
> You have lost apic_mmio_map_once from previous version,
> as result in KVM case io_memory will be mapped many times
> over itself, pls fix it.
I'm sorry to forget apic_mmio_map_once, so the logic must be as previous
version. I will fix it.
Thanks,
Zhu
> Pls, also split per CPU AS change into a separate patch
> as Eduardo asked you for, with following order:
> 1. move region mapping into APIC in this patch
> 2. add per CPU AS in an additional path
>
>> cpu_reset(cs);
>>
>> xcc->parent_realize(dev, &local_err);
> .
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v10 2/4] x86: use new method to correct reset sequence
2015-08-31 9:47 [Qemu-devel] [PATCH v10 0/4] remove icc bus/bridge Zhu Guihua
2015-08-31 9:47 ` [Qemu-devel] [PATCH v10 1/4] apic: map APIC's MMIO region at each CPU's address space Zhu Guihua
@ 2015-08-31 9:47 ` Zhu Guihua
2015-08-31 13:40 ` Igor Mammedov
2015-08-31 9:47 ` [Qemu-devel] [PATCH v10 3/4] cpu/apic: drop icc bus/bridge Zhu Guihua
2015-08-31 9:47 ` [Qemu-devel] [PATCH v10 4/4] icc_bus: drop the unused files Zhu Guihua
3 siblings, 1 reply; 8+ messages in thread
From: Zhu Guihua @ 2015-08-31 9:47 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
Cc: chen.fan.fnst, izumi.taku, Zhu Guihua
Something must be occur during reset of the X86 platform in a specific
order. For example, some devices (such as hpet, rtc) reset will send
irq to apic, this will update the apic register. In order to ensure
the apic register could be set to default values, apic reset must be
after those devices reset.
This patch uses the new QEMUMachine reset method to solve the above
problem, ensuring the various reset happen in the correct order.
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
hw/i386/pc.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e15971c..875ada8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1928,6 +1928,27 @@ static void pc_machine_initfn(Object *obj)
NULL, &error_abort);
}
+static void pc_machine_reset(void)
+{
+ CPUState *cs;
+ X86CPU *cpu;
+
+ qemu_devices_reset();
+
+ /* When some devices (such as hpet, rtc) do their reset, they will
+ * send irq to APIC. This will modify the value of the APIC register.
+ * In order to ensure the APIC register can be set to default value,
+ * APIC reset must be after those devices reset.
+ */
+ CPU_FOREACH(cs) {
+ cpu = X86_CPU(cs);
+
+ if (cpu->apic_state) {
+ device_reset(cpu->apic_state);
+ }
+ }
+}
+
static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
{
unsigned pkg_id, core_id, smt_id;
@@ -1948,6 +1969,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
mc->default_boot_order = "cad";
mc->hot_add_cpu = pc_hot_add_cpu;
mc->max_cpus = 255;
+ mc->reset = pc_machine_reset;
hc->plug = pc_machine_device_plug_cb;
hc->unplug_request = pc_machine_device_unplug_request_cb;
hc->unplug = pc_machine_device_unplug_cb;
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH v10 2/4] x86: use new method to correct reset sequence
2015-08-31 9:47 ` [Qemu-devel] [PATCH v10 2/4] x86: use new method to correct reset sequence Zhu Guihua
@ 2015-08-31 13:40 ` Igor Mammedov
0 siblings, 0 replies; 8+ messages in thread
From: Igor Mammedov @ 2015-08-31 13:40 UTC (permalink / raw)
To: Zhu Guihua
Cc: ehabkost, qemu-devel, izumi.taku, chen.fan.fnst, pbonzini,
afaerber
On Mon, 31 Aug 2015 17:47:45 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
> Something must be occur during reset of the X86 platform in a specific
> order. For example, some devices (such as hpet, rtc) reset will send
> irq to apic, this will update the apic register. In order to ensure
> the apic register could be set to default values, apic reset must be
> after those devices reset.
>
May be:
During reset some devices (such as hpet, rtc) might send IRQ to APIC
which changes APIC's state from default one it's supposed to have
at machine startup time.
Fix this by resetting APIC after devices have been reset to cancel
any changes that qemu_devices_reset() might have done to its state.
> This patch uses the new QEMUMachine reset method to solve the above
> problem, ensuring the various reset happen in the correct order.
>
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
> hw/i386/pc.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index e15971c..875ada8 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1928,6 +1928,27 @@ static void pc_machine_initfn(Object *obj)
> NULL, &error_abort);
> }
>
> +static void pc_machine_reset(void)
> +{
> + CPUState *cs;
> + X86CPU *cpu;
> +
> + qemu_devices_reset();
> +
> + /* When some devices (such as hpet, rtc) do their reset, they will
> + * send irq to APIC. This will modify the value of the APIC register.
> + * In order to ensure the APIC register can be set to default value,
> + * APIC reset must be after those devices reset.
> + */
S/^^^/
Reset APIC after devices have been reset to cancel
any changes that qemu_devices_reset() might have done.
> + CPU_FOREACH(cs) {
> + cpu = X86_CPU(cs);
> +
> + if (cpu->apic_state) {
> + device_reset(cpu->apic_state);
> + }
> + }
> +}
> +
> static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
> {
> unsigned pkg_id, core_id, smt_id;
> @@ -1948,6 +1969,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
> mc->default_boot_order = "cad";
> mc->hot_add_cpu = pc_hot_add_cpu;
> mc->max_cpus = 255;
> + mc->reset = pc_machine_reset;
> hc->plug = pc_machine_device_plug_cb;
> hc->unplug_request = pc_machine_device_unplug_request_cb;
> hc->unplug = pc_machine_device_unplug_cb;
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v10 3/4] cpu/apic: drop icc bus/bridge
2015-08-31 9:47 [Qemu-devel] [PATCH v10 0/4] remove icc bus/bridge Zhu Guihua
2015-08-31 9:47 ` [Qemu-devel] [PATCH v10 1/4] apic: map APIC's MMIO region at each CPU's address space Zhu Guihua
2015-08-31 9:47 ` [Qemu-devel] [PATCH v10 2/4] x86: use new method to correct reset sequence Zhu Guihua
@ 2015-08-31 9:47 ` Zhu Guihua
2015-08-31 9:47 ` [Qemu-devel] [PATCH v10 4/4] icc_bus: drop the unused files Zhu Guihua
3 siblings, 0 replies; 8+ messages in thread
From: Zhu Guihua @ 2015-08-31 9:47 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
Cc: chen.fan.fnst, izumi.taku, Zhu Guihua
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.
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 875ada8..3f89e6c 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 171cdc0..683a89b 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);
@@ -3154,7 +3148,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] 8+ messages in thread* [Qemu-devel] [PATCH v10 4/4] icc_bus: drop the unused files
2015-08-31 9:47 [Qemu-devel] [PATCH v10 0/4] remove icc bus/bridge Zhu Guihua
` (2 preceding siblings ...)
2015-08-31 9:47 ` [Qemu-devel] [PATCH v10 3/4] cpu/apic: drop icc bus/bridge Zhu Guihua
@ 2015-08-31 9:47 ` Zhu Guihua
3 siblings, 0 replies; 8+ messages in thread
From: Zhu Guihua @ 2015-08-31 9:47 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
Cc: chen.fan.fnst, izumi.taku, Zhu Guihua
ICC bus impl has been droped, so all icc related files are not useful
any more; delete them.
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
default-configs/i386-softmmu.mak | 1 -
default-configs/x86_64-softmmu.mak | 1 -
hw/cpu/Makefile.objs | 1 -
hw/cpu/icc_bus.c | 118 -------------------------------------
include/hw/cpu/icc_bus.h | 82 --------------------------
5 files changed, 203 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 5eaafa1..70391ed 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -43,7 +43,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 28e2099..dfb8095 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -44,7 +44,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..0954a18 100644
--- a/hw/cpu/Makefile.objs
+++ b/hw/cpu/Makefile.objs
@@ -2,5 +2,4 @@ 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/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
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread