From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48851) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUsPx-0003vL-9z for qemu-devel@nongnu.org; Thu, 27 Aug 2015 04:20:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUsPs-00030H-6W for qemu-devel@nongnu.org; Thu, 27 Aug 2015 04:20:17 -0400 Received: from [59.151.112.132] (port=36285 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUsPr-0002yz-S2 for qemu-devel@nongnu.org; Thu, 27 Aug 2015 04:20:12 -0400 Message-ID: <55DEC7E4.4080007@cn.fujitsu.com> Date: Thu, 27 Aug 2015 16:18:44 +0800 From: Zhu Guihua MIME-Version: 1.0 References: <27af0665bdcccc032cf477190cad35dd754fc4fb.1439877857.git.zhugh.fnst@cn.fujitsu.com> <20150821225459.GB5816@thinpad.lan.raisama.net> <55DA6B7B.2090408@redhat.com> <37768706.19450373.1440602828235.JavaMail.zimbra@zmail13.collab.prod.int.phx2.redhat.com> <20150826154904.GG4230@thinpad.lan.raisama.net> In-Reply-To: <20150826154904.GG4230@thinpad.lan.raisama.net> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RESEND PATCH v9 1/4] apic: map APIC's MMIO region at each CPU's address space List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , Paolo Bonzini Cc: chen.fan.fnst@cn.fujitsu.com, peter.maydell@linaro.org, qemu-devel@nongnu.org, afaerber@suse.de, imammedo@redhat.com On 08/26/2015 11:49 PM, Eduardo Habkost wrote: > On Wed, Aug 26, 2015 at 11:27:08AM -0400, Paolo Bonzini wrote: > [...] >>>>>> + if (tcg_enabled()) { >>>>>> + memory_region_add_subregion_overlap(cpu->cpu_as_root, >>>>>> + apic->apicbase & >>>>>> + MSR_IA32_APICBASE_BASE, >>>>>> + &apic->io_memory, >>>>>> + 0x1000); >>>>> Why exactly is this necessary? If this is necessary, why don't we need >>>>> to do this for non-TCG accelerators? >>>> At least KVM and qtest do not support per-CPU address spaces. >>> Right, but given this restriction why can't we also do whatever >>> we need to work without the per-CPU address spaces with TCG? >>> >> Because the emulation quality is indeed a bit better with the per-CPU >> address spaces; you could move each APIC's base address independent of >> the others. However, this is not a feature that is actually used by >> anything in practice, so I doubt anyone cares about TCG implementing >> it correctly. > Do we need additional changes in TCG to implement it correctly, or is > this going to work out of the box as soon as we apply this series? > > If it's the latter, the patch makes sense to me (but please add a > comment to the code explaining why). If it's the former, I don't see the > point of making the code more complex before that feature is actually > implemented by TCG. > > Also, we could make the logic simpler if we just check if > cpu->cpu_as_root is set, e.g.: > > /* 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 ?: > get_system_memory(), > apic->apicbase & > MSR_IA32_APICBASE_BASE, > &apic->io_memory, > 0x1000); Yeah, the logic is better. I will take this, thanks. And, comments will be added in next version. Thanks, Zhu