On Thu, 19 Mar 2015 18:08:32 +0800 Chen Fan <[email protected]> wrote:
> > On 03/18/2015 12:29 AM, Igor Mammedov wrote: > > On Tue, 17 Mar 2015 17:08:30 +0800 > > Chen Fan <[email protected]> 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 <[email protected]> > >> --- > > [...] > > > >> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c > >> index 0858b45..020219e 100644 > >> --- a/hw/intc/apic_common.c > >> +++ b/hw/intc/apic_common.c > >> @@ -27,6 +27,8 @@ > >> static int apic_irq_delivered; > >> bool apic_report_tpr_access; > >> > >> +static MemoryRegion *apic_container; > >> + > >> void cpu_set_apic_base(DeviceState *dev, uint64_t val) > >> { > >> trace_cpu_set_apic_base(val); > >> @@ -314,8 +316,11 @@ 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); > >> + memory_region_add_subregion(apic_container, 0, &s->io_memory); > >> + memory_region_add_subregion_overlap(CPU(s->cpu)->as->root, > >> + APIC_DEFAULT_ADDRESS, > >> + apic_container, > >> + 0x1000); > >> mmio_registered = true; > >> } > > Provided per CPU address spaces are used, > > why do you need static apic_container and mmio_registered thing? > > Shouldn't it be possible to register &s->io_memory of each APIC > > in CPU's own AS and avoid using globals? > it seems only need to do > memory_region_add_subregion_overlap(CPU(s->cpu)->as->root, > APIC_DEFAULT_ADDRESS, > &io_memory, > 0x1000); > > at here, right? yep, something along theses lines. I also do not remember if we really need it for every APIC device that inherits from APIC_COMMON.It might be that kvm_iqrchip don't need it at all. You could experiment with -machine kernel_irqchip=on/off options to switch between QEMU emulated APIC and kernel_irqchip. Also this change is independent of dropping icc_bridge, so please split it out into a separate patch. > > Thanks, > Chen > > > >> > >> @@ -431,15 +436,23 @@ static Property apic_properties_common[] = { > >> DEFINE_PROP_END_OF_LIST(), > >> }; > >> > >> +static void apic_common_initfn(Object *obj) > >> +{ > >> + if (!apic_container) { > >> + apic_container = g_malloc(sizeof(*apic_container)); > >> + memory_region_init(apic_container, obj, "apic-container", > >> + APIC_SPACE_SIZE); > >> + } > >> +} > >> + > > > > > > . > > >
