From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59756) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vsj3P-00017P-KO for qemu-devel@nongnu.org; Mon, 16 Dec 2013 20:02:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vsj3K-0006lP-8d for qemu-devel@nongnu.org; Mon, 16 Dec 2013 20:02:31 -0500 Received: from mail-pb0-x234.google.com ([2607:f8b0:400e:c01::234]:41479) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vsj3K-0006lL-11 for qemu-devel@nongnu.org; Mon, 16 Dec 2013 20:02:26 -0500 Received: by mail-pb0-f52.google.com with SMTP id uo5so6234979pbc.39 for ; Mon, 16 Dec 2013 17:02:25 -0800 (PST) Date: Tue, 17 Dec 2013 11:01:50 +1000 From: "Edgar E. Iglesias" Message-ID: <20131217010149.GH2161@edvb> References: <1387181170-23267-1-git-send-email-edgar.iglesias@gmail.com> <1387181170-23267-9-git-send-email-edgar.iglesias@gmail.com> <52AEEE03.5000801@suse.de> <20131217003424.GE2161@edvb> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v1 08/22] cpu: Add per-cpu address space List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers , Blue Swirl , Anthony Liguori , pcrost@xilinx.com, Paolo Bonzini , Andreas =?iso-8859-1?Q?F=E4rber?= , Aurelien Jarno , Richard Henderson On Tue, Dec 17, 2013 at 12:54:19AM +0000, Peter Maydell wrote: > On 17 December 2013 00:34, Edgar E. Iglesias wrote: > > On Mon, Dec 16, 2013 at 01:11:47PM +0100, Andreas Färber wrote: > >> Why are you adding this field here rather than in CPUState alongside the > >> other field? This being a pointer I can't imagine problems for > >> non-softmmu, and I had posted patches a while ago to move the > >> surrounding fields there. Having it in CPUState would avoid some of the > >> env_ptr accesses above, which were supposed to be an interim solution only. > > > This was discussed when I posted the RFC. My first try had this member in > > CPUState but some of the hot paths for mmio accesses needed to do > > ENV_GET_CPU() to get hold of the CS and AS. ENV_GET_CPU does a runtime type > > check that would slow things down. That's the reason I moved it to env. > > > > I'm not against moving the field back if it doesnt cost too much. Maybe we > > should consider removing the CPU() around ENV_GET_CPU()? > > > > See: > > http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02889.html > > I think that's the wrong way round. We should put the field in the right > place in the code, which is CPUState. To the extent that that means we > discover that our dynamic cast macros are not fast enough for hot > paths we should make the actual error checking be debug builds > only. (There's been pushback on dynamic-casts in fastpaths before > and the preferred approach so far has been "optimise the cast". > I think we should take "...and do it only in debug builds" over "do > the wrong thing because a purely-debug-purposes type check > isn't as fast as we'd like".) Sounds reasonable to me, I'll move the AS back into CPUState for v2. Thanks, Edgar