From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34886) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WrrIs-0003Xl-Vh for qemu-devel@nongnu.org; Tue, 03 Jun 2014 12:11:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WrrIm-0004xg-Mk for qemu-devel@nongnu.org; Tue, 03 Jun 2014 12:11:10 -0400 Message-ID: <538DF396.5010709@suse.de> Date: Tue, 03 Jun 2014 18:11:02 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1401787684-31895-1-git-send-email-aik@ozlabs.ru> <1401787684-31895-3-git-send-email-aik@ozlabs.ru> <20140603174057.38316acb@bahia.local> In-Reply-To: <20140603174057.38316acb@bahia.local> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 02/29] target-ppc: Merge 970FX and 970MP into a single 970 class List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: Alexey Kardashevskiy , Tom Musta , qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 06/03/2014 05:40 PM, Greg Kurz wrote: > On Tue, 3 Jun 2014 19:27:37 +1000 > Alexey Kardashevskiy wrote: > >> The differences between classes were: >> 1. SLB size, was 32 for 970 and 64 for others, should be 64 for all; >> 2. check_pow() callback, HID0 format is the same so should be the same >> 0x01C00000 which means "deep nap", "doze" and "nap" bits set; >> 3. LPCR - 970 does not have it but 970MP had one (by mistake). >> >> This fixes wrong differences and makes one 970 class. >> >> This fixes wrong registration of LPCR which is not present on 970. >> >> This does not copy MSR_SHV (Hypervisor State, HV) bit from 970FX to >> 970 class as we do not emulate hypervisor in QEMU anyway. >> >> This does not remove check_pow_970FX now as it is still used by POWER5+ >> class, this will be addressed later. >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> target-ppc/cpu-models.c | 14 +-- >> target-ppc/translate_init.c | 222 ++++---------------------------------------- >> 2 files changed, 23 insertions(+), 213 deletions(-) >> >> diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c >> index 9a66c03..97a81d8 100644 >> --- a/target-ppc/cpu-models.c >> +++ b/target-ppc/cpu-models.c >> @@ -1142,19 +1142,19 @@ >> "POWER8 v1.0") >> POWERPC_DEF("970", CPU_POWERPC_970, 970, >> "PowerPC 970") >> - POWERPC_DEF("970fx_v1.0", CPU_POWERPC_970FX_v10, 970FX, >> + POWERPC_DEF("970fx_v1.0", CPU_POWERPC_970FX_v10, 970, >> "PowerPC 970FX v1.0 (G5)") >> - POWERPC_DEF("970fx_v2.0", CPU_POWERPC_970FX_v20, 970FX, >> + POWERPC_DEF("970fx_v2.0", CPU_POWERPC_970FX_v20, 970, >> "PowerPC 970FX v2.0 (G5)") >> - POWERPC_DEF("970fx_v2.1", CPU_POWERPC_970FX_v21, 970FX, >> + POWERPC_DEF("970fx_v2.1", CPU_POWERPC_970FX_v21, 970, >> "PowerPC 970FX v2.1 (G5)") >> - POWERPC_DEF("970fx_v3.0", CPU_POWERPC_970FX_v30, 970FX, >> + POWERPC_DEF("970fx_v3.0", CPU_POWERPC_970FX_v30, 970, >> "PowerPC 970FX v3.0 (G5)") >> - POWERPC_DEF("970fx_v3.1", CPU_POWERPC_970FX_v31, 970FX, >> + POWERPC_DEF("970fx_v3.1", CPU_POWERPC_970FX_v31, 970, >> "PowerPC 970FX v3.1 (G5)") >> - POWERPC_DEF("970mp_v1.0", CPU_POWERPC_970MP_v10, 970MP, >> + POWERPC_DEF("970mp_v1.0", CPU_POWERPC_970MP_v10, 970, >> "PowerPC 970MP v1.0") >> - POWERPC_DEF("970mp_v1.1", CPU_POWERPC_970MP_v11, 970MP, >> + POWERPC_DEF("970mp_v1.1", CPU_POWERPC_970MP_v11, 970, >> "PowerPC 970MP v1.1") >> #if defined(TODO) >> POWERPC_DEF("Cell", CPU_POWERPC_CELL, 970, >> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c >> index fa137af..2f40d0d 100644 >> --- a/target-ppc/translate_init.c >> +++ b/target-ppc/translate_init.c >> @@ -7268,8 +7268,9 @@ POWERPC_FAMILY(e600)(ObjectClass *oc, void *data) >> >> static int check_pow_970 (CPUPPCState *env) >> { >> - if (env->spr[SPR_HID0] & 0x00600000) >> + if (env->spr[SPR_HID0] & 0x01C00000) { > What about killing magic numbers with something like: > > #define HID0_DEEPNAP (1<<24) > #define HID0_DOZE (1<<23) > #define HID0_NAP (1<<22) I like the idea. But IMHO this can easily come as a follow-up patch if that's the only nit on this patch set :). Alex