From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33288) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTDpT-0003Le-Ss for qemu-devel@nongnu.org; Thu, 27 Mar 2014 13:11:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTDpO-0006qZ-Bg for qemu-devel@nongnu.org; Thu, 27 Mar 2014 13:10:59 -0400 Message-ID: <53345B85.3090501@gmail.com> Date: Thu, 27 Mar 2014 12:10:29 -0500 From: Tom Musta MIME-Version: 1.0 References: <1395715231-4217-1-git-send-email-anton@samba.org> <871txqspmq.fsf@linaro.org> <53343C80.5060708@suse.de> In-Reply-To: <53343C80.5060708@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 1/6] target-ppc: POWER8 supports the MSR_LE bit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , =?UTF-8?B?QWxleCA=?= =?UTF-8?B?QmVubsOpZQ==?= , Anton Blanchard Cc: clg@fr.ibm.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, gkurz@linux.vnet.ibm.com On 3/27/2014 9:58 AM, Andreas Färber wrote: > Am 25.03.2014 08:29, schrieb Alex Bennée: >> >> Anton Blanchard writes: >> >>> Add MSR_LE to the msr_mask for POWER8. >>> >> >>> - pcc->msr_mask = 0x800000000284FF36ULL; >>> + pcc->msr_mask = 0x800000000284FF37ULL; >> >> >> Should we be adding some #define's for the various bit positions on this >> mask? Looking at the current code it looks like a big ream of magic >> numbers. > > In general I concur that defines would be nice, however for 2.0 that's > too risky for me as temporary maintainer and I'm not sure if these > values are being pieced together by contributors or whether this is > coming directly from the manual? There are defines in target-ppc/cpu.h that identify the bit position. So something like this could/should be done (in 2.1): pcc->msr_mask = (1ULL << MSR_SF) | (1ULL << MSR_TAG) | (1ULL << MSR_ISF) | (1ULL << MSR_DR /* MISSING */ ) | (1ULL << MSR_IR /* MISSING */ ) | (1ULL << MSR_FE1 /* MISSING */ ) | (1ULL << MSR_BE /* MISSING */ ) | (1ULL << MSR_ME /* MISSING */ ) | (1ULL << MSR_SE /* MISSING */ ) | (1ULL << MSR_FE0 /* MISSING */ ) | (1ULL << MSR_FP /* MISSING */ ) | (1ULL << MSR_PR /* MISSING */ ) | (1ULL << MSR_EE /* MISSING */ ); The set of defines is incomplete -- the items marked MISSING are not currently there. They could, of course, easily be added. Unfortunately, this behavior has been repeated over 50 times in the target-ppc/translate_init.c file and some MSR bit positions have multiple meanings based on processor family and era. So a thorough and accurate cleanup would provide a nice challenge :) I'm willing to tackle this after I get done with some decimal floating point work (probably 2 weeks). > The other issue has been that adding a new family, even after the > initial round of cleanups, still requires a chunk of code to be copied, > which seems prone to forgetting little bits on the new one, then maybe > fixing up the original template but not the derived models, etc.