From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=32905 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Poc9i-0006LX-4P for qemu-devel@nongnu.org; Sun, 13 Feb 2011 08:38:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Poc9g-0008WI-8i for qemu-devel@nongnu.org; Sun, 13 Feb 2011 08:38:26 -0500 Received: from ozlabs.org ([203.10.76.45]:58582) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Poc9f-0008Vq-Pv for qemu-devel@nongnu.org; Sun, 13 Feb 2011 08:38:24 -0500 Date: Mon, 14 Feb 2011 00:38:14 +1100 From: David Gibson Subject: Re: [Qemu-devel] Re: [PATCH 12/15] Support 1T segments on ppc Message-ID: <20110213133814.GI18294@yookeroo> References: <1297522467-5975-1-git-send-email-david@gibson.dropbear.id.au> <1297522467-5975-13-git-send-email-david@gibson.dropbear.id.au> <4A986836-CE8C-4577-B565-FD88F1BDBF11@suse.de> <20110213093440.GB17099@yookeroo> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: paulus@samba.org, qemu-devel@nongnu.org, anton@samba.org On Sun, Feb 13, 2011 at 01:37:12PM +0100, Alexander Graf wrote: > On 13.02.2011, at 10:34, David Gibson wrote: > > On Sat, Feb 12, 2011 at 04:57:39PM +0100, Alexander Graf wrote: > >> On 12.02.2011, at 15:54, David Gibson wrote: > > [snip] > >>> + if (rb & (0x1000 - env->slb_nr)) > >> > >> Braces... > > > > Oops, yeah. These later patches in the series I haven't really > > audited for coding style adequately yet. I'll fix these before the > > next version. > > > > [snip] > >>> + return -1; /* 1T segment on MMU that doesn't support it */ > >>> + > >>> + /* We stuff a copy of the B field into slb->esid to simplify > >>> + * lookup later */ > >>> + slb->esid = (rb & (SLB_ESID_ESID | SLB_ESID_V)) | > >>> + (rs >> SLB_VSID_SSIZE_SHIFT); > >> > >> Wouldn't it be easier to add another field? > > > > Easier for what? The reason I put these bits in here is that the rest > > of the things slb_lookup() needs to scan for are all in the esid > > field, so putting B in there means slb_lookup() needs only one > > comparison per-slot, per segment size. > > Hrm - but it also needs random & ~3 masking in other code which is > very unpretty. Comparing two numbers really shouldn't hurt > performance too much, but makes the code better maintainable. Well, it's only one place. But fair enough, I'll avoid this hack in the next version. > struct slb_entry { > uint64_t esid; > uint64_t vsid; > int b; > } > > or so :). Actually, we don't even need that. The B field is already in slb->vsid. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson