Re: [PATCH v3 1/9] target/ppc: Fix width of some 32-bit SPRs

["Nicholas Piggin" <npiggin@gmail.com>]

On Mon May 15, 2023 at 8:14 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 5/15/23 14:56, Nicholas Piggin wrote:
> > Some 32-bit SPRs are incorrectly implemented as 64-bits on 64-bit
> > targets.
> > 
> > This changes VRSAVE, DSISR, HDSISR, DAWRX0, PIDR, LPIDR, DEXCR,
> > HDEXCR, CTRL, TSCR, MMCRH, and PMC[1-6] from to be 32-bit registers.
> > 
> > This only goes by the 32/64 classification in the architecture, it
> > does not try to implement finer details of SPR implementation (e.g.,
> > not all bits implemented as simple read/write storage).
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > Since v2: no change.
> > 
> >   target/ppc/cpu_init.c    | 18 +++++++++---------
> >   target/ppc/helper_regs.c |  2 +-
> >   target/ppc/misc_helper.c |  4 ++--
> >   target/ppc/power8-pmu.c  |  2 +-
> >   target/ppc/translate.c   |  2 +-
> >   5 files changed, 14 insertions(+), 14 deletions(-)
> > 
> > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> > index 0ce2e3c91d..5aa0b3f0f1 100644
> > --- a/target/ppc/cpu_init.c
> > +++ b/target/ppc/cpu_init.c
> > @@ -5085,8 +5085,8 @@ static void register_book3s_altivec_sprs(CPUPPCState *env)
> >       }
> >   
> >       spr_register_kvm(env, SPR_VRSAVE, "VRSAVE",
> > -                     &spr_read_generic, &spr_write_generic,
> > -                     &spr_read_generic, &spr_write_generic,
> > +                     &spr_read_generic, &spr_write_generic32,
> > +                     &spr_read_generic, &spr_write_generic32,
> >                        KVM_REG_PPC_VRSAVE, 0x00000000);
> >   
> >   }
>
> This change broke linux-user build, could you please check once?

Sorry I did notice you reported that already, must have lost it
along the way somewhere. This incremental patch should work?

Thanks,
Nick

---
diff --git a/target/ppc/spr_common.h b/target/ppc/spr_common.h
index 8437eb0340..4c0f2bed77 100644
--- a/target/ppc/spr_common.h
+++ b/target/ppc/spr_common.h
@@ -81,6 +81,7 @@ void _spr_register(CPUPPCState *env, int num, const char *name,
 void spr_noaccess(DisasContext *ctx, int gprn, int sprn);
 void spr_read_generic(DisasContext *ctx, int gprn, int sprn);
 void spr_write_generic(DisasContext *ctx, int sprn, int gprn);
+void spr_write_generic32(DisasContext *ctx, int sprn, int gprn);
 void spr_write_MMCR0(DisasContext *ctx, int sprn, int gprn);
 void spr_write_MMCR1(DisasContext *ctx, int sprn, int gprn);
 void spr_write_PMC(DisasContext *ctx, int sprn, int gprn);
@@ -109,7 +110,6 @@ void spr_write_PMC14_ureg(DisasContext *ctx, int sprn, int gprn);
 void spr_write_PMC56_ureg(DisasContext *ctx, int sprn, int gprn);
 
 #ifndef CONFIG_USER_ONLY
-void spr_write_generic32(DisasContext *ctx, int sprn, int gprn);
 void spr_write_clear(DisasContext *ctx, int sprn, int gprn);
 void spr_access_nop(DisasContext *ctx, int sprn, int gprn);
 void spr_read_decr(DisasContext *ctx, int gprn, int sprn);
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index c03a6bdc9a..f5cf1457db 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -411,6 +411,18 @@ void spr_write_generic(DisasContext *ctx, int sprn, int gprn)
     spr_store_dump_spr(sprn);
 }
 
+void spr_write_generic32(DisasContext *ctx, int sprn, int gprn)
+{
+#ifdef TARGET_PPC64
+    TCGv t0 = tcg_temp_new();
+    tcg_gen_ext32u_tl(t0, cpu_gpr[gprn]);
+    gen_store_spr(sprn, t0);
+    spr_store_dump_spr(sprn);
+#else
+    spr_write_generic(ctx, sprn, gprn);
+#endif
+}
+
 void spr_write_CTRL(DisasContext *ctx, int sprn, int gprn)
 {
     spr_write_generic32(ctx, sprn, gprn);
@@ -424,18 +436,6 @@ void spr_write_CTRL(DisasContext *ctx, int sprn, int gprn)
 }
 
 #if !defined(CONFIG_USER_ONLY)
-void spr_write_generic32(DisasContext *ctx, int sprn, int gprn)
-{
-#ifdef TARGET_PPC64
-    TCGv t0 = tcg_temp_new();
-    tcg_gen_ext32u_tl(t0, cpu_gpr[gprn]);
-    gen_store_spr(sprn, t0);
-    spr_store_dump_spr(sprn);
-#else
-    spr_write_generic(ctx, sprn, gprn);
-#endif
-}
-
 void spr_write_clear(DisasContext *ctx, int sprn, int gprn)
 {
     TCGv t0 = tcg_temp_new();