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.o
cc -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.c
In 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.fe...@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?
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>
