|
| From: | Daniel Henrique Barboza |
| Subject: | Re: [PATCH 5/6] target/ppc: move msgclrp/msgsndp to decodetree |
| Date: | Thu, 20 Oct 2022 11:25:34 -0300 |
| User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 |
On 10/20/22 09:18, Matheus K. Ferst wrote:
On 19/10/2022 17:59, Daniel Henrique Barboza wrote:Matheus, This patch fails ppc-softmmu emulation: FAILED: libqemu-ppc-softmmu.fa.p/target_ppc_translate.c.occ -m64 -mcx16 -Ilibqemu-ppc-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 -isystem /home/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote /home/danielhb/qemu -iquote /home/danielhb/qemu/include -iquote /home/danielhb/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -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 -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc-softmmu-config-devices.h"' -MD -MQ libqemu-ppc-softmmu.fa.p/target_ppc_translate.c.o -MF libqemu-ppc-softmmu.fa.p/target_ppc_translate.c.o.d -o libqemu-ppc-softmmu.fa.p/target_ppc_translate.c.o -c ../target/ppc/translate.cIn file included from ../target/ppc/translate.c:21: In function ‘trans_MSGCLRP’, inlined from ‘decode_insn32’ at libqemu-ppc-softmmu.fa.p/decode-insn32.c.inc:3250:21, inlined from ‘ppc_tr_translate_insn’ at ../target/ppc/translate.c:7552:15: /home/danielhb/qemu/include/qemu/osdep.h:184:35: error: call to ‘qemu_build_not_reached_always’ declared with attribute error: code path is reachable 184 | #define qemu_build_not_reached() qemu_build_not_reached_always() | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../target/ppc/translate/processor-ctrl-impl.c.inc:79:5: note: in expansion of macro ‘qemu_build_not_reached’ 79 | qemu_build_not_reached(); | ^~~~~~~~~~~~~~~~~~~~~~ The error is down there:Hmm, that's strange. I always build ppc-softmmu on my tests and I'm not seeing this error. I'm using gcc 9.4 though, maybe it's time to update my compiler...On 10/6/22 17:06, Matheus Ferst wrote:Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br> --- target/ppc/insn32.decode | 2 ++ target/ppc/translate.c | 26 ------------------- .../ppc/translate/processor-ctrl-impl.c.inc | 24 +++++++++++++++++ 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode index bba49ded1b..5ba4a6807d 100644 --- a/target/ppc/insn32.decode +++ b/target/ppc/insn32.decode @@ -913,3 +913,5 @@ TLBIEL 011111 ..... - .. . . ..... 0100010010 - @X_tlbie MSGCLR 011111 ----- ----- ..... 0011101110 - @X_rb MSGSND 011111 ----- ----- ..... 0011001110 - @X_rb +MSGCLRP 011111 ----- ----- ..... 0010101110 - @X_rb +MSGSNDP 011111 ----- ----- ..... 0010001110 - @X_rb diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 889cca6325..087ab8e69d 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -6241,28 +6241,6 @@ static void gen_icbt_440(DisasContext *ctx) /* Embedded.Processor Control */ -#if defined(TARGET_PPC64) -static void gen_msgclrp(DisasContext *ctx) -{ -#if defined(CONFIG_USER_ONLY) - GEN_PRIV(ctx); -#else - CHK_SV(ctx); - gen_helper_book3s_msgclrp(cpu_env, cpu_gpr[rB(ctx->opcode)]); -#endif /* defined(CONFIG_USER_ONLY) */ -} - -static void gen_msgsndp(DisasContext *ctx) -{ -#if defined(CONFIG_USER_ONLY) - GEN_PRIV(ctx); -#else - CHK_SV(ctx); - gen_helper_book3s_msgsndp(cpu_env, cpu_gpr[rB(ctx->opcode)]); -#endif /* defined(CONFIG_USER_ONLY) */ -} -#endif - static void gen_msgsync(DisasContext *ctx) { #if defined(CONFIG_USER_ONLY) @@ -6896,10 +6874,6 @@ GEN_HANDLER(vmladduhm, 0x04, 0x11, 0xFF, 0x00000000, PPC_ALTIVEC), GEN_HANDLER_E(maddhd_maddhdu, 0x04, 0x18, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300), GEN_HANDLER_E(maddld, 0x04, 0x19, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300), -GEN_HANDLER2_E(msgsndp, "msgsndp", 0x1F, 0x0E, 0x04, 0x03ff0001, - PPC_NONE, PPC2_ISA207S), -GEN_HANDLER2_E(msgclrp, "msgclrp", 0x1F, 0x0E, 0x05, 0x03ff0001, - PPC_NONE, PPC2_ISA207S), #endif #undef GEN_INT_ARITH_ADD diff --git a/target/ppc/translate/processor-ctrl-impl.c.inc b/target/ppc/translate/processor-ctrl-impl.c.inc index 0192b45c8f..3703001f31 100644 --- a/target/ppc/translate/processor-ctrl-impl.c.inc +++ b/target/ppc/translate/processor-ctrl-impl.c.inc @@ -68,3 +68,27 @@ static bool trans_MSGSND(DisasContext *ctx, arg_X_rb *a) #endif return true; } + +static bool trans_MSGCLRP(DisasContext *ctx, arg_X_rb *a) +{ + REQUIRE_INSNS_FLAGS2(ctx, ISA207S); + REQUIRE_SV(ctx); +#if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64) + gen_helper_book3s_msgclrp(cpu_env, cpu_gpr[a->rb]); +#else + qemu_build_not_reached();^ here. And also+#endif + return true; +} + +static bool trans_MSGSNDP(DisasContext *ctx, arg_X_rb *a) +{ + REQUIRE_INSNS_FLAGS2(ctx, ISA207S); + REQUIRE_SV(ctx); +#if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64) + gen_helper_book3s_msgsndp(cpu_env, cpu_gpr[a->rb]); +#else + qemu_build_not_reached();^ here. It seems like you're filtering for TARGET_PPC64 but you're not guarding for it, and the qemu_build_not_reached() is being hit. I fixed by squashing this in: diff --git a/target/ppc/translate/processor-ctrl-impl.c.inc b/target/ppc/translate/processor-ctrl-impl.c.inc index f253226a75..ff231db1af 100644 --- a/target/ppc/translate/processor-ctrl-impl.c.inc +++ b/target/ppc/translate/processor-ctrl-impl.c.inc @@ -73,6 +73,7 @@ static bool trans_MSGCLRP(DisasContext *ctx, arg_X_rb *a) { REQUIRE_INSNS_FLAGS2(ctx, ISA207S); REQUIRE_SV(ctx); + REQUIRE_64BIT(ctx); #if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64) gen_helper_book3s_msgclrp(cpu_env, cpu_gpr[a->rb]); #else @@ -85,6 +86,7 @@ static bool trans_MSGSNDP(DisasContext *ctx, arg_X_rb *a) { REQUIRE_INSNS_FLAGS2(ctx, ISA207S); REQUIRE_SV(ctx); + REQUIRE_64BIT(ctx); #if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64) gen_helper_book3s_msgsndp(cpu_env, cpu_gpr[a->rb]); #else If you think this fix is acceptable you can consider this patch acked as well.It shouldn't matter since we only want to make the compiler happy, but we should check instruction flags before privilege, so we throw POWERPC_EXCP_INVAL_INVAL instead of POWERPC_EXCP_PRIV_OPC if the CPU doesn't have the instruction: > @@ -73,6 +73,7 @@ static bool trans_MSGCLRP(DisasContext *ctx, arg_X_rb *a) > { > + REQUIRE_64BIT(ctx); > REQUIRE_INSNS_FLAGS2(ctx, ISA207S); > REQUIRE_SV(ctx); > #if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64) > gen_helper_book3s_msgclrp(cpu_env, cpu_gpr[a->rb]); > #else > @@ -85,6 +86,7 @@ static bool trans_MSGSNDP(DisasContext *ctx, arg_X_rb *a) > { > + REQUIRE_64BIT(ctx); > REQUIRE_INSNS_FLAGS2(ctx, ISA207S); > REQUIRE_SV(ctx); > #if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64) > gen_helper_book3s_msgsndp(cpu_env, cpu_gpr[a->rb]); > #else Since all CPUs with ISA207S are 64-bit, it shouldn't make any difference in this context, but someone might use this code as an example, so it's better to have these checks in the correct order. Do you want me to resend with this change?
Nah it's ok. I'll move the REQUIRE_64BIT() call to the start of the function in-tree. Thanks, Daniel
Thanks, Matheus K. Ferst Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/> Analista de Software Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
| [Prev in Thread] | Current Thread | [Next in Thread] |