qemu-ppc
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Harsh Prateek Bora
Subject: Re: [PATCH v3 1/9] target/ppc: Fix width of some 32-bit SPRs
Date: Mon, 15 May 2023 15:44:24 +0530
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 


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?
[1776/2718] Compiling C object libqemu-ppc64le-linux-user.fa.p/target_ppc_cpu_init.c.o 
FAILED: libqemu-ppc64le-linux-user.fa.p/target_ppc_cpu_init.c.o
gcc -m64 -mcx16 -Ilibqemu-ppc64le-linux-user.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../common-user/host/x86_64 -I../linux-user/include/host/x86_64 -I../linux-user/include -Ilinux-user -I../linux-user -Ilinux-user/ppc -I../linux-user/ppc -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /home/travis/build/Harsh-Prateek-Bora/qemu/linux-headers -isystem linux-headers -iquote . -iquote /home/travis/build/Harsh-Prateek-Bora/qemu -iquote /home/travis/build/Harsh-Prateek-Bora/qemu/include -iquote /home/travis/build/Harsh-Prateek-Bora/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc64le-linux-user-config-target.h"' '-DCONFIG_DEVICES="ppc64le-linux-user-config-devices.h"' -MD -MQ libqemu-ppc64le-linux-user.fa.p/target_ppc_cpu_init.c.o -MF libqemu-ppc64le-linux-user.fa.p/target_ppc_cpu_init.c.o.d -o libqemu-ppc64le-linux-user.fa.p/target_ppc_cpu_init.c.o -c ../target/ppc/cpu_init.c 
In file included from ../target/ppc/cpu_init.c:46:
../target/ppc/cpu_init.c: In function ‘register_book3s_altivec_sprs’:
../target/ppc/cpu_init.c:5089:42: error: ‘spr_write_generic32’ undeclared (first use in this function); did you mean ‘spr_write_generic’? 
 5089 |                      &spr_read_generic, &spr_write_generic32,
      |                                          ^~~~~~~~~~~~~~~~~~~
../target/ppc/spr_common.h:25:24: note: in definition of macro ‘USR_ARG’
   25 | # define USR_ARG(X)    X,
      |                        ^
../target/ppc/spr_common.h:66:5: note: in expansion of macro ‘spr_register_kvm_hv’ 66 | spr_register_kvm_hv(env, num, name, uea_read, uea_write, oea_read, \ 
      |     ^~~~~~~~~~~~~~~~~~~
../target/ppc/cpu_init.c:5088:5: note: in expansion of macro ‘spr_register_kvm’ 
 5088 |     spr_register_kvm(env, SPR_VRSAVE, "VRSAVE",
      |     ^~~~~~~~~~~~~~~~
../target/ppc/cpu_init.c:5089:42: note: each undeclared identifier is reported only once for each function it appears in 
 5089 |                      &spr_read_generic, &spr_write_generic32,
      |                                          ^~~~~~~~~~~~~~~~~~~
../target/ppc/spr_common.h:25:24: note: in definition of macro ‘USR_ARG’
   25 | # define USR_ARG(X)    X,
      |                        ^
../target/ppc/spr_common.h:66:5: note: in expansion of macro ‘spr_register_kvm_hv’ 66 | spr_register_kvm_hv(env, num, name, uea_read, uea_write, oea_read, \ 
      |     ^~~~~~~~~~~~~~~~~~~
../target/ppc/cpu_init.c:5088:5: note: in expansion of macro ‘spr_register_kvm’ 
 5088 |     spr_register_kvm(env, SPR_VRSAVE, "VRSAVE",
      |     ^~~~~~~~~~~~~~~~
[1777/2718] Compiling C object libqemu-ppc64le-linux-user.fa.p/target_ppc_cpu-models.c.o [1778/2718] Compiling C object libqemu-ppc64le-linux-user.fa.p/target_ppc_fpu_helper.c.o 
ninja: build stopped: subcommand failed.
make: *** [Makefile:165: run-ninja] Error 1

regards,
Harsh


@@ -5120,7 +5120,7 @@ static void register_book3s_207_dbg_sprs(CPUPPCState *env)
      spr_register_kvm_hv(env, SPR_DAWRX0, "DAWRX0",
                          SPR_NOACCESS, SPR_NOACCESS,
                          SPR_NOACCESS, SPR_NOACCESS,
-                        &spr_read_generic, &spr_write_generic,
+                        &spr_read_generic, &spr_write_generic32,
                          KVM_REG_PPC_DAWRX, 0x00000000);
      spr_register_kvm_hv(env, SPR_CIABR, "CIABR",
                          SPR_NOACCESS, SPR_NOACCESS,
@@ -5376,7 +5376,7 @@ static void register_book3s_ids_sprs(CPUPPCState *env)
      spr_register_hv(env, SPR_TSCR, "TSCR",
                   SPR_NOACCESS, SPR_NOACCESS,
                   SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
+                 &spr_read_generic, &spr_write_generic32,
                   0x00000000);
      spr_register_hv(env, SPR_HMER, "HMER",
                   SPR_NOACCESS, SPR_NOACCESS,
@@ -5406,7 +5406,7 @@ static void register_book3s_ids_sprs(CPUPPCState *env)
      spr_register_hv(env, SPR_MMCRC, "MMCRC",
                   SPR_NOACCESS, SPR_NOACCESS,
                   SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
+                 &spr_read_generic, &spr_write_generic32,
                   0x00000000);
      spr_register_hv(env, SPR_MMCRH, "MMCRH",
                   SPR_NOACCESS, SPR_NOACCESS,
@@ -5441,7 +5441,7 @@ static void register_book3s_ids_sprs(CPUPPCState *env)
      spr_register_hv(env, SPR_HDSISR, "HDSISR",
                   SPR_NOACCESS, SPR_NOACCESS,
                   SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
+                 &spr_read_generic, &spr_write_generic32,
                   0x00000000);
      spr_register_hv(env, SPR_HRMOR, "HRMOR",
                   SPR_NOACCESS, SPR_NOACCESS,
@@ -5665,7 +5665,7 @@ static void register_power7_book4_sprs(CPUPPCState *env)
                       KVM_REG_PPC_ACOP, 0);
      spr_register_kvm(env, SPR_BOOKS_PID, "PID",
                       SPR_NOACCESS, SPR_NOACCESS,
-                     &spr_read_generic, &spr_write_generic,
+                     &spr_read_generic, &spr_write_generic32,
                       KVM_REG_PPC_PID, 0);
  #endif
  }
@@ -5730,7 +5730,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env)
  {
      spr_register(env, SPR_DEXCR, "DEXCR",
              SPR_NOACCESS, SPR_NOACCESS,
-            &spr_read_generic, &spr_write_generic,
+            &spr_read_generic, &spr_write_generic32,
              0);
spr_register(env, SPR_UDEXCR, "DEXCR", 
@@ -5741,7 +5741,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env)
      spr_register_hv(env, SPR_HDEXCR, "HDEXCR",
              SPR_NOACCESS, SPR_NOACCESS,
              SPR_NOACCESS, SPR_NOACCESS,
-            &spr_read_generic, &spr_write_generic,
+            &spr_read_generic, &spr_write_generic32,
              0);
spr_register(env, SPR_UHDEXCR, "HDEXCR", 
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 779e7db513..fb351c303f 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -448,7 +448,7 @@ void register_non_embedded_sprs(CPUPPCState *env)
      /* Exception processing */
      spr_register_kvm(env, SPR_DSISR, "DSISR",
                       SPR_NOACCESS, SPR_NOACCESS,
-                     &spr_read_generic, &spr_write_generic,
+                     &spr_read_generic, &spr_write_generic32,
                       KVM_REG_PPC_DSISR, 0x00000000);
      spr_register_kvm(env, SPR_DAR, "DAR",
                       SPR_NOACCESS, SPR_NOACCESS,
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index a9bc1522e2..40ddc5c08c 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -190,13 +190,13 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong 
val)
void helper_store_pidr(CPUPPCState *env, target_ulong val) 
  {
-    env->spr[SPR_BOOKS_PID] = val;
+    env->spr[SPR_BOOKS_PID] = (uint32_t)val;
      tlb_flush(env_cpu(env));
  }
void helper_store_lpidr(CPUPPCState *env, target_ulong val) 
  {
-    env->spr[SPR_LPIDR] = val;
+    env->spr[SPR_LPIDR] = (uint32_t)val;
/* 
       * We need to flush the TLB on LPID changes as we only tag HV vs
diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
index 1381072b9e..64a64865d7 100644
--- a/target/ppc/power8-pmu.c
+++ b/target/ppc/power8-pmu.c
@@ -272,7 +272,7 @@ void helper_store_pmc(CPUPPCState *env, uint32_t sprn, 
uint64_t value)
  {
      pmu_update_cycles(env);
- env->spr[sprn] = value; 
+    env->spr[sprn] = (uint32_t)value;
pmc_update_overflow_timer(env, sprn); 
  }
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index f603f1a939..c03a6bdc9a 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -413,7 +413,7 @@ void spr_write_generic(DisasContext *ctx, int sprn, int 
gprn)
void spr_write_CTRL(DisasContext *ctx, int sprn, int gprn) 
  {
-    spr_write_generic(ctx, sprn, gprn);
+    spr_write_generic32(ctx, sprn, gprn);
/* 
       * SPR_CTRL writes must force a new translation block,
reply via email to
[Prev in Thread] Current Thread [Next in Thread]