From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51299) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYXUF-0007B1-8p for qemu-devel@nongnu.org; Thu, 19 Mar 2015 06:15:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YYXUB-0004iG-5g for qemu-devel@nongnu.org; Thu, 19 Mar 2015 06:15:35 -0400 Received: from [59.151.112.132] (port=46922 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYXUA-0004f1-N2 for qemu-devel@nongnu.org; Thu, 19 Mar 2015 06:15:31 -0400 Message-ID: <550AA020.7050209@cn.fujitsu.com> Date: Thu, 19 Mar 2015 18:08:32 +0800 From: Chen Fan MIME-Version: 1.0 References: <1426583311-29455-1-git-send-email-chen.fan.fnst@cn.fujitsu.com> <20150317172951.7825eace@nial.brq.redhat.com> In-Reply-To: <20150317172951.7825eace@nial.brq.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] cpu/apic: drop icc bus/bridge/ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: zhugh.fnst@cn.fujitsu.com, qemu-devel@nongnu.org, tangchen@cn.fujitsu.com, isimatu.yasuaki@jp.fujitsu.com, guz.fnst@cn.fujitsu.com, afaerber@suse.de On 03/18/2015 12:29 AM, Igor Mammedov wrote: > On Tue, 17 Mar 2015 17:08:30 +0800 > Chen Fan 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 >> --- > [...] > >> 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? 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); >> + } >> +} >> + > > > . >