| Message ID | 20230530132543.385138-2-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: Assorted ppc target fixes | expand |
On 5/30/23 18:55, Nicholas Piggin wrote: > powerpc ifetch endianness depends on MSR[LE] so it has to byteswap > after cpu_ldl_code(). This corrects DSISR bits in alignment > interrupts when running in little endian mode. > > Reviewed-by: Fabiano Rosas<farosas@suse.de> > Signed-off-by: Nicholas Piggin<npiggin@gmail.com> > --- > target/ppc/excp_helper.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index c13f2afa04..0274617b4a 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -133,6 +133,26 @@ static void dump_hcall(CPUPPCState *env) > env->nip); > } > > +#ifdef CONFIG_TCG > +/* Return true iff byteswap is needed in a scalar memop */ > +static inline bool need_byteswap(CPUArchState *env) > +{ > + /* SOFTMMU builds TARGET_BIG_ENDIAN. Need to swap when MSR[LE] is set */ > + return !!(env->msr & ((target_ulong)1 << MSR_LE)); > +} > + > +static uint32_t ppc_ldl_code(CPUArchState *env, abi_ptr addr) This hunk fails to compile with configure --disable-tcg > FAILED: libqemu-ppc64-softmmu.fa.p/target_ppc_excp_helper.c.o > cc -m64 -mlittle-endian -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. > -Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui > -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 > -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 > -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g > -fstack-protector-strong -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -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 -isystem > /home/Shreya/qemu/linux-headers -isystem linux-headers -iquote . -iquote > /home/Shreya/qemu -iquote /home/Shreya/qemu/include -pthread > -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE > -fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../linux-headers > -isystemlinux-headers -DNEED_CPU_H > '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' > '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ > libqemu-ppc64-softmmu.fa.p/target_ppc_excp_helper.c.o -MF > libqemu-ppc64-softmmu.fa.p/target_ppc_excp_helper.c.o.d -o > libqemu-ppc64-softmmu.fa.p/target_ppc_excp_helper.c.o -c > ../target/ppc/excp_helper.c > ../target/ppc/excp_helper.c:143:49: error: unknown type name ‘abi_ptr’; > did you mean ‘si_ptr’? > 143 | static uint32_t ppc_ldl_code(CPUArchState *env, abi_ptr addr) > | ^~~~~~~ > | si_ptr > ../target/ppc/excp_helper.c: In function ‘powerpc_excp_books’: > ../target/ppc/excp_helper.c:1416:16: error: implicit declaration of > function ‘ppc_ldl_code’ [-Werror=implicit-function-declaration] > 1416 | insn = ppc_ldl_code(env, env->nip); > | ^~~~~~~~~~~~ > ../target/ppc/excp_helper.c:1416:16: error: nested extern declaration of > ‘ppc_ldl_code’ [-Werror=nested-externs] > cc1: all warnings being treated as errors > +{ > + uint32_t insn = cpu_ldl_code(env, addr); > + > + if (need_byteswap(env)) { > + insn = bswap32(insn); > + } > + > + return insn; > +} > +#endif > + > static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp) > { > const char *es; > @@ -3100,7 +3120,7 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, > > /* Restore state and reload the insn we executed, for filling in DSISR. */ > cpu_restore_state(cs, retaddr); > - insn = cpu_ldl_code(env, env->nip); > + insn = ppc_ldl_code(env, env->nip); > > switch (env->mmu_model) { > case POWERPC_MMU_SOFT_4xx:
On Wed Jun 14, 2023 at 3:51 PM AEST, Anushree Mathur wrote: > > On 5/30/23 18:55, Nicholas Piggin wrote: > > powerpc ifetch endianness depends on MSR[LE] so it has to byteswap > > after cpu_ldl_code(). This corrects DSISR bits in alignment > > interrupts when running in little endian mode. > > > > Reviewed-by: Fabiano Rosas<farosas@suse.de> > > Signed-off-by: Nicholas Piggin<npiggin@gmail.com> > > --- > > target/ppc/excp_helper.c | 22 +++++++++++++++++++++- > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > > index c13f2afa04..0274617b4a 100644 > > --- a/target/ppc/excp_helper.c > > +++ b/target/ppc/excp_helper.c > > @@ -133,6 +133,26 @@ static void dump_hcall(CPUPPCState *env) > > env->nip); > > } > > > > +#ifdef CONFIG_TCG > > +/* Return true iff byteswap is needed in a scalar memop */ > > +static inline bool need_byteswap(CPUArchState *env) > > +{ > > + /* SOFTMMU builds TARGET_BIG_ENDIAN. Need to swap when MSR[LE] is set */ > > + return !!(env->msr & ((target_ulong)1 << MSR_LE)); > > +} > > + > > +static uint32_t ppc_ldl_code(CPUArchState *env, abi_ptr addr) > > This hunk fails to compile with configure --disable-tcg I don't see how since it's inside CONFIG_TCG. Seems to work here. You don't have an old version of the patch applied? What configure options exactly? Thanks, Nick
On 6/15/23 08:21, Nicholas Piggin wrote: > On Wed Jun 14, 2023 at 3:51 PM AEST, Anushree Mathur wrote: >> On 5/30/23 18:55, Nicholas Piggin wrote: >>> powerpc ifetch endianness depends on MSR[LE] so it has to byteswap >>> after cpu_ldl_code(). This corrects DSISR bits in alignment >>> interrupts when running in little endian mode. >>> >>> Reviewed-by: Fabiano Rosas<farosas@suse.de> >>> Signed-off-by: Nicholas Piggin<npiggin@gmail.com> >>> --- >>> target/ppc/excp_helper.c | 22 +++++++++++++++++++++- >>> 1 file changed, 21 insertions(+), 1 deletion(-) >>> >>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c >>> index c13f2afa04..0274617b4a 100644 >>> --- a/target/ppc/excp_helper.c >>> +++ b/target/ppc/excp_helper.c >>> @@ -133,6 +133,26 @@ static void dump_hcall(CPUPPCState *env) >>> env->nip); >>> } >>> >>> +#ifdef CONFIG_TCG >>> +/* Return true iff byteswap is needed in a scalar memop */ >>> +static inline bool need_byteswap(CPUArchState *env) >>> +{ >>> + /* SOFTMMU builds TARGET_BIG_ENDIAN. Need to swap when MSR[LE] is set */ >>> + return !!(env->msr & ((target_ulong)1 << MSR_LE)); >>> +} >>> + >>> +static uint32_t ppc_ldl_code(CPUArchState *env, abi_ptr addr) >> This hunk fails to compile with configure --disable-tcg > I don't see how since it's inside CONFIG_TCG. Seems to work here. > You don't have an old version of the patch applied? > > What configure options exactly? > > Thanks, > Nick The configure options i used are: ./configure --target-list=ppc64-softmmu --disable-tcg --prefix=/usr I applied the latest patches but still i was seeing the same issue. Can you check this once! Thanks, Anushree-Mathur
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index c13f2afa04..0274617b4a 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -133,6 +133,26 @@ static void dump_hcall(CPUPPCState *env) env->nip); } +#ifdef CONFIG_TCG +/* Return true iff byteswap is needed in a scalar memop */ +static inline bool need_byteswap(CPUArchState *env) +{ + /* SOFTMMU builds TARGET_BIG_ENDIAN. Need to swap when MSR[LE] is set */ + return !!(env->msr & ((target_ulong)1 << MSR_LE)); +} + +static uint32_t ppc_ldl_code(CPUArchState *env, abi_ptr addr) +{ + uint32_t insn = cpu_ldl_code(env, addr); + + if (need_byteswap(env)) { + insn = bswap32(insn); + } + + return insn; +} +#endif + static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp) { const char *es; @@ -3100,7 +3120,7 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, /* Restore state and reload the insn we executed, for filling in DSISR. */ cpu_restore_state(cs, retaddr); - insn = cpu_ldl_code(env, env->nip); + insn = ppc_ldl_code(env, env->nip); switch (env->mmu_model) { case POWERPC_MMU_SOFT_4xx: