From: Harsh Prateek Bora <harshpb@linux.ibm.com>
To: Nicholas Piggin <npiggin@gmail.com>, qemu-ppc@nongnu.org
Cc: qemu-devel@nongnu.org,
Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Subject: Re: [PATCH v3 1/9] target/ppc: Fix width of some 32-bit SPRs
Date: Mon, 15 May 2023 15:44:24 +0530 [thread overview]
Message-ID: <c345f589-7fac-8624-06d4-ead03a2ba005@linux.ibm.com> (raw)
In-Reply-To: <20230515092655.171206-2-npiggin@gmail.com>
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,
next prev parent reply other threads:[~2023-05-15 10:15 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-15 9:26 [PATCH v3 0/9] target/ppc: Assorted ppc target fixes Nicholas Piggin
2023-05-15 9:26 ` [PATCH v3 1/9] target/ppc: Fix width of some 32-bit SPRs Nicholas Piggin
2023-05-15 10:14 ` Harsh Prateek Bora [this message]
2023-05-15 11:14 ` Nicholas Piggin
2023-05-15 11:43 ` Harsh Prateek Bora
2023-05-15 12:03 ` Mark Cave-Ayland
2023-05-15 12:51 ` Nicholas Piggin
2023-05-15 15:19 ` Nicholas Piggin
2023-05-16 7:02 ` Mark Cave-Ayland
2023-05-16 9:39 ` Nicholas Piggin
2023-05-15 9:26 ` [PATCH v3 2/9] target/ppc: Fix PMU MMCR0[PMCjCE] bit in hflags calculation Nicholas Piggin
2023-05-16 9:32 ` Daniel Henrique Barboza
2023-05-16 10:44 ` Nicholas Piggin
2023-05-16 11:07 ` Daniel Henrique Barboza
2023-05-15 9:26 ` [PATCH v3 3/9] target/ppc: Fix instruction loading endianness in alignment interrupt Nicholas Piggin
2023-05-16 15:46 ` Daniel Henrique Barboza
2023-05-15 9:26 ` [PATCH v3 4/9] target/ppc: Alignment faults do not set DSISR in ISA v3.0 onward Nicholas Piggin
2023-05-15 9:26 ` [PATCH v3 5/9] target/ppc: Change partition-scope translate interface Nicholas Piggin
2023-05-15 9:26 ` [PATCH v3 6/9] target/ppc: Add SRR1 prefix indication to interrupt handlers Nicholas Piggin
2023-05-15 9:26 ` [PATCH v3 7/9] target/ppc: Implement HEIR SPR Nicholas Piggin
2023-05-15 9:26 ` [PATCH v3 8/9] target/ppc: Add ISA v3.1 LEV indication in SRR1 for system call interrupts Nicholas Piggin
2023-05-15 9:26 ` [PATCH v3 9/9] target/ppc: Better CTRL SPR implementation Nicholas Piggin
2023-05-27 18:05 ` [PATCH v3 0/9] target/ppc: Assorted ppc target fixes Daniel Henrique Barboza
2023-05-29 1:47 ` Nicholas Piggin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c345f589-7fac-8624-06d4-ead03a2ba005@linux.ibm.com \
--to=harshpb@linux.ibm.com \
--cc=dbarboza@ventanamicro.com \
--cc=npiggin@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).