From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45360) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ws0XA-0000gU-UC for qemu-devel@nongnu.org; Tue, 03 Jun 2014 22:02:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ws0X3-0007H1-Qa for qemu-devel@nongnu.org; Tue, 03 Jun 2014 22:02:32 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:38009) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ws0X3-0007Gh-Hz for qemu-devel@nongnu.org; Tue, 03 Jun 2014 22:02:25 -0400 Received: by mail-pb0-f45.google.com with SMTP id um1so6268930pbc.4 for ; Tue, 03 Jun 2014 19:02:23 -0700 (PDT) Message-ID: <538E7E28.6040003@ozlabs.ru> Date: Wed, 04 Jun 2014 12:02:16 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1401787684-31895-1-git-send-email-aik@ozlabs.ru> <1401787684-31895-15-git-send-email-aik@ozlabs.ru> <538DFDAA.4090007@gmail.com> In-Reply-To: <538DFDAA.4090007@gmail.com> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 14/29] target-ppc: Move POWER7/8 CFAR/DSCR/CTRL/PPR/PCR SPR registration to helpers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tom Musta , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org, Alexander Graf On 06/04/2014 02:54 AM, Tom Musta wrote: > On 6/3/2014 4:27 AM, Alexey Kardashevskiy wrote: >> This moves SCFAR/DSCR/CTRL/PPR/PCR PRs to helpers. Later these helpers >> will be called from generalized init_proc_book3s_64(). >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> target-ppc/translate_init.c | 70 ++++++++++++++++++++++++++------------------- >> 1 file changed, 40 insertions(+), 30 deletions(-) >> >> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c >> index d6557f2..576056c 100644 >> --- a/target-ppc/translate_init.c >> +++ b/target-ppc/translate_init.c >> @@ -7523,6 +7523,42 @@ static void gen_spr_book3s_purr(CPUPPCState *env) >> #endif >> } >> >> +static void gen_spr_power6_dbg(CPUPPCState *env) >> +{ >> +#if !defined(CONFIG_USER_ONLY) >> + spr_register(env, SPR_CFAR, "SPR_CFAR", >> + SPR_NOACCESS, SPR_NOACCESS, >> + &spr_read_cfar, &spr_write_cfar, >> + 0x00000000); >> +#endif >> +} >> + >> +static void gen_spr_power5p_common(CPUPPCState *env) >> +{ >> + spr_register(env, SPR_PPR, "PPR", >> + &spr_read_generic, &spr_write_generic, >> + &spr_read_generic, &spr_write_generic, >> + 0x00000000); >> +} >> + >> +static void gen_spr_power6_common(CPUPPCState *env) >> +{ >> +#if !defined(CONFIG_USER_ONLY) >> + spr_register_kvm(env, SPR_DSCR, "SPR_DSCR", >> + SPR_NOACCESS, SPR_NOACCESS, >> + &spr_read_generic, &spr_write_generic, >> + KVM_REG_PPC_DSCR, 0x00000000); >> +#endif >> + /* >> + * Register PCR to report POWERPC_EXCP_PRIV_REG instead of >> + * POWERPC_EXCP_INVAL_SPR. >> + */ >> + spr_register(env, SPR_PCR, "PCR", >> + SPR_NOACCESS, SPR_NOACCESS, >> + SPR_NOACCESS, SPR_NOACCESS, >> + 0x00000000); >> +} >> + >> static void gen_spr_power8_tce_address_control(CPUPPCState *env) >> { >> spr_register(env, SPR_TAR, "TAR", >> @@ -7745,14 +7781,6 @@ static void init_proc_POWER7 (CPUPPCState *env) >> /* Time base */ >> gen_tbl(env); >> #if !defined(CONFIG_USER_ONLY) >> - spr_register(env, SPR_CFAR, "SPR_CFAR", >> - SPR_NOACCESS, SPR_NOACCESS, >> - &spr_read_cfar, &spr_write_cfar, >> - 0x00000000); >> - spr_register_kvm(env, SPR_DSCR, "SPR_DSCR", >> - SPR_NOACCESS, SPR_NOACCESS, >> - &spr_read_generic, &spr_write_generic, >> - KVM_REG_PPC_DSCR, 0x00000000); >> spr_register_kvm(env, SPR_POWER_MMCRA, "SPR_MMCRA", >> SPR_NOACCESS, SPR_NOACCESS, >> &spr_read_generic, &spr_write_generic, >> @@ -7768,24 +7796,15 @@ static void init_proc_POWER7 (CPUPPCState *env) >> #endif /* !CONFIG_USER_ONLY */ >> gen_spr_book3s_ids(env); >> gen_spr_book3s_purr(env); >> + gen_spr_book3s_common(env); >> + gen_spr_power5p_common(env); >> + gen_spr_power6_common(env); >> + gen_spr_power6_dbg(env); >> gen_spr_amr(env); >> - /* XXX : not implemented */ >> - spr_register(env, SPR_CTRL, "SPR_CTRLT", >> - SPR_NOACCESS, SPR_NOACCESS, >> - SPR_NOACCESS, &spr_write_generic, >> - 0x80800000); >> - spr_register(env, SPR_UCTRL, "SPR_CTRLF", >> - SPR_NOACCESS, SPR_NOACCESS, >> - &spr_read_generic, SPR_NOACCESS, >> - 0x80800000); > > > Note that by switching to using gen_spr_book3s_common, there is an implicit change in the register names > ("SPR_CTRLT" --> "SPR_CTRL" and "SPR_CTLRF -> "SPR_UCTRL"). I am not completely sure of the impact of > this (change in what is seen in the monitor?) .... Well, "info registers"/ppc_cpu_dump_state() does not use these names, so I am not sure if the change will be visible at all. > But I like your new names better than the old ones :) Good :) > > > >> - /* >> - * Register PCR to report POWERPC_EXCP_PRIV_REG instead of >> - * POWERPC_EXCP_INVAL_SPR. >> - */ >> - spr_register(env, SPR_PCR, "PCR", >> - SPR_NOACCESS, SPR_NOACCESS, >> - SPR_NOACCESS, SPR_NOACCESS, >> - 0x00000000); >> } >> > > We probably have quite a few hypervisor SPRs that should also be handled this way ???? This is definitely not today. > Reviewed-by: Tom Musta got it, thanks! -- Alexey