|
From: | Chen Fan |
Subject: | Re: [Qemu-devel] [PATCH v2 1/2] cpu/apic: drop icc bus/bridge/ |
Date: | Mon, 23 Mar 2015 17:38:37 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 |
On 03/23/2015 05:43 PM, Igor Mammedov wrote:
On Mon, 23 Mar 2015 17:07:29 +0800 Chen Fan <address@hidden> wrote:On 03/23/2015 04:23 PM, Igor Mammedov wrote:On Mon, 23 Mar 2015 13:54:23 +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. Signed-off-by: Chen Fan <address@hidden> --- hw/i386/pc.c | 29 +++++++++++------------------ hw/i386/pc_piix.c | 9 +-------- hw/i386/pc_q35.c | 9 +-------- hw/intc/apic.c | 6 +++--- hw/intc/apic_common.c | 11 ++--------- include/hw/i386/apic_internal.h | 5 ++--- include/hw/i386/pc.h | 2 +- target-i386/cpu.c | 2 -- 8 files changed, 21 insertions(+), 52 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 4b46c29..5d15473 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c[...]@@ -1093,8 +1083,11 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) /* 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); + apic = APIC_COMMON(cpu->apic_state); + memory_region_add_subregion_overlap(CPU(cpu)->as->root, + APIC_DEFAULT_ADDRESS, + &apic->io_memory, + 0x1000);Why is it here? Shouldn't it be mapped not once but for each CPU since we are using per CPU address spaces? Split this change out into a separate patch please, with commit message describing what it does.Hi Igor, in your previous mail said, "It might be that kvm_irqchip don't need it at all." I don't know why kvm_irqchip don't need it ?That's why it's MIGHT, I'm not sure since I've not look at that code for a while.because I have test that for kernel_irqchip=on, qemu emulator the kvm-apic object, and sent the MSI to kernel irqchip for pcie devices. it also need map the region.Can we have this test as a patch to qemu/tests? so it would be easier to discuss it.
no problem. Thanks, Chen
Thanks, ChenPS: It should be part of APIC code or at worst case part of CPU's realize. PS2: new cpu tests don't test actual CPU execution, so they can't validate this change. To test it you need to run test in TCG (at least) or TCG + KVM mode, with some guest code that programs and checks APIC of each CPU. PS3: the rest of the patch I'd suggest to merge with 2/2 patch that removes unused icc_bridge code, there isn't point in splitting that from removing icc_bridge from other files. [...]diff --git a/target-i386/cpu.c b/target-i386/cpu.cindex f01690b..2385e6b 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" @@ -2941,7 +2940,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;that isn't the only place in this file that should be changed. See x86_cpu_apic_create(): cpu->apic_state = qdev_try_create(qdev_get_parent_bus(dev), apic_type); probably it's not right to try get parent bus from bus-less device, qdev_try_create() call should be replaced by object_new()/object_unref() pair.dc->props = x86_cpu_properties;xcc->parent_reset = cc->reset;..
[Prev in Thread] | Current Thread | [Next in Thread] |