* [PATCH 0/6] Enable doorbell instruction for POWER8 CPUs
@ 2022-10-06 20:06 Matheus Ferst
2022-10-06 20:06 ` [PATCH 1/6] target/ppc: fix msgclr/msgsnd insns flags Matheus Ferst
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Matheus Ferst @ 2022-10-06 20:06 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, farosas, Matheus Ferst
Reviewing the interrupt rework patch series, Fabiano noted[1] that the
POWER8 User's Manual lists the Directed Hypervisor Doorbell interrupt,
even without implementing the "Embedded.Processor Control" category. The
manual also lists the msgclr and msgsnd instructions, but the decode
legacy code currently gates them with the PPC2_PRCNTL flag, which is not
enabled for this CPU.
Reading the Power ISA v2.07, we noticed that the title of the
Processor Control chapter does not include the category information like
in Power ISA v2.06, and the instruction listing in the appendices says
that their category is "S\nE.PC". The document is not clear about this
notation, but since implementations should support only one environment
(embedded or server), I'd interpret this change as "these instructions
are now available if the processor implements the server environment or
E.PC category."
While reviewing the flag for those instructions, we also saw that the
msgsync, introduced in PowerISA v3.0, was available on all processors
with the PPC2_PRCNTL flag, which includes older CPUs like e500mc and
e6500.
This patch series fixes the instruction flags for these three
instructions. We then take this opportunity to move processor control
instruction to decodetree, fixing an embarrassing error in the
definition of the REQUIRE_HV macro along the way.
[1] https://lists.gnu.org/archive/html/qemu-ppc/2022-09/msg00586.html
Matheus Ferst (6):
target/ppc: fix msgclr/msgsnd insns flags
target/ppc: fix msgsync insns flags
target/ppc: fix REQUIRE_HV macro definition
target/ppc: move msgclr/msgsnd to decodetree
target/ppc: move msgclrp/msgsndp to decodetree
target/ppc: move msgsync to decodetree
target/ppc/insn32.decode | 8 ++
target/ppc/translate.c | 86 ++-------------
.../ppc/translate/processor-ctrl-impl.c.inc | 103 ++++++++++++++++++
3 files changed, 119 insertions(+), 78 deletions(-)
create mode 100644 target/ppc/translate/processor-ctrl-impl.c.inc
--
2.25.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/6] target/ppc: fix msgclr/msgsnd insns flags
2022-10-06 20:06 [PATCH 0/6] Enable doorbell instruction for POWER8 CPUs Matheus Ferst
@ 2022-10-06 20:06 ` Matheus Ferst
2022-10-07 19:07 ` Fabiano Rosas
2022-10-06 20:06 ` [PATCH 2/6] target/ppc: fix msgsync " Matheus Ferst
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Matheus Ferst @ 2022-10-06 20:06 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, farosas, Matheus Ferst
On Power ISA v2.07, the category for these instructions became
"Embedded.Processor Control" or "Book S".
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index e810842925..37d7018d18 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6902,9 +6902,9 @@ GEN_HANDLER2_E(tlbivax_booke206, "tlbivax", 0x1F, 0x12, 0x18, 0x00000001,
GEN_HANDLER2_E(tlbilx_booke206, "tlbilx", 0x1F, 0x12, 0x00, 0x03800001,
PPC_NONE, PPC2_BOOKE206),
GEN_HANDLER2_E(msgsnd, "msgsnd", 0x1F, 0x0E, 0x06, 0x03ff0001,
- PPC_NONE, PPC2_PRCNTL),
+ PPC_NONE, (PPC2_PRCNTL | PPC2_ISA207S)),
GEN_HANDLER2_E(msgclr, "msgclr", 0x1F, 0x0E, 0x07, 0x03ff0001,
- PPC_NONE, PPC2_PRCNTL),
+ PPC_NONE, (PPC2_PRCNTL | PPC2_ISA207S)),
GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
PPC_NONE, PPC2_PRCNTL),
GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/6] target/ppc: fix msgsync insns flags
2022-10-06 20:06 [PATCH 0/6] Enable doorbell instruction for POWER8 CPUs Matheus Ferst
2022-10-06 20:06 ` [PATCH 1/6] target/ppc: fix msgclr/msgsnd insns flags Matheus Ferst
@ 2022-10-06 20:06 ` Matheus Ferst
2022-10-07 19:10 ` Fabiano Rosas
2022-10-06 20:06 ` [PATCH 3/6] target/ppc: fix REQUIRE_HV macro definition Matheus Ferst
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Matheus Ferst @ 2022-10-06 20:06 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, farosas, Matheus Ferst
This instruction was added by Power ISA 3.0, using PPC2_PRCNTL makes it
available for older processors, like de e5500 and e6500.
Fixes: 7af1e7b02264 ("target/ppc: add support for hypervisor doorbells on book3s CPUs")
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 37d7018d18..eaac8670b1 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6906,7 +6906,7 @@ GEN_HANDLER2_E(msgsnd, "msgsnd", 0x1F, 0x0E, 0x06, 0x03ff0001,
GEN_HANDLER2_E(msgclr, "msgclr", 0x1F, 0x0E, 0x07, 0x03ff0001,
PPC_NONE, (PPC2_PRCNTL | PPC2_ISA207S)),
GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
- PPC_NONE, PPC2_PRCNTL),
+ PPC_NONE, PPC2_ISA300),
GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/6] target/ppc: fix REQUIRE_HV macro definition
2022-10-06 20:06 [PATCH 0/6] Enable doorbell instruction for POWER8 CPUs Matheus Ferst
2022-10-06 20:06 ` [PATCH 1/6] target/ppc: fix msgclr/msgsnd insns flags Matheus Ferst
2022-10-06 20:06 ` [PATCH 2/6] target/ppc: fix msgsync " Matheus Ferst
@ 2022-10-06 20:06 ` Matheus Ferst
2022-10-07 19:07 ` Fabiano Rosas
2022-10-06 20:06 ` [PATCH 4/6] target/ppc: move msgclr/msgsnd to decodetree Matheus Ferst
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Matheus Ferst @ 2022-10-06 20:06 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, farosas, Matheus Ferst
The macro is missing a '{' after the if condition. Any use of REQUIRE_HV
would cause a compilation error.
Fixes: fc34e81acd51 ("target/ppc: add macros to check privilege level")
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/translate.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index eaac8670b1..435066c4a3 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6545,12 +6545,12 @@ static int64_t dw_compose_ea(DisasContext *ctx, int x)
} \
} while (0)
-#define REQUIRE_HV(CTX) \
- do { \
- if (unlikely((CTX)->pr || !(CTX)->hv)) \
- gen_priv_opc(CTX); \
- return true; \
- } \
+#define REQUIRE_HV(CTX) \
+ do { \
+ if (unlikely((CTX)->pr || !(CTX)->hv)) { \
+ gen_priv_opc(CTX); \
+ return true; \
+ } \
} while (0)
#else
#define REQUIRE_SV(CTX) do { gen_priv_opc(CTX); return true; } while (0)
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/6] target/ppc: move msgclr/msgsnd to decodetree
2022-10-06 20:06 [PATCH 0/6] Enable doorbell instruction for POWER8 CPUs Matheus Ferst
` (2 preceding siblings ...)
2022-10-06 20:06 ` [PATCH 3/6] target/ppc: fix REQUIRE_HV macro definition Matheus Ferst
@ 2022-10-06 20:06 ` Matheus Ferst
2022-10-19 20:59 ` Daniel Henrique Barboza
2022-10-06 20:06 ` [PATCH 5/6] target/ppc: move msgclrp/msgsndp " Matheus Ferst
2022-10-06 20:06 ` [PATCH 6/6] target/ppc: move msgsync " Matheus Ferst
5 siblings, 1 reply; 15+ messages in thread
From: Matheus Ferst @ 2022-10-06 20:06 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, farosas, Matheus Ferst
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/insn32.decode | 5 ++
target/ppc/translate.c | 34 +--------
.../ppc/translate/processor-ctrl-impl.c.inc | 70 +++++++++++++++++++
3 files changed, 77 insertions(+), 32 deletions(-)
create mode 100644 target/ppc/translate/processor-ctrl-impl.c.inc
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index a5249ee32c..bba49ded1b 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -908,3 +908,8 @@ SLBSYNC 011111 ----- ----- ----- 0101010010 -
TLBIE 011111 ..... - .. . . ..... 0100110010 - @X_tlbie
TLBIEL 011111 ..... - .. . . ..... 0100010010 - @X_tlbie
+
+# Processor Control Instructions
+
+MSGCLR 011111 ----- ----- ..... 0011101110 - @X_rb
+MSGSND 011111 ----- ----- ..... 0011001110 - @X_rb
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 435066c4a3..889cca6325 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6241,34 +6241,6 @@ static void gen_icbt_440(DisasContext *ctx)
/* Embedded.Processor Control */
-static void gen_msgclr(DisasContext *ctx)
-{
-#if defined(CONFIG_USER_ONLY)
- GEN_PRIV(ctx);
-#else
- CHK_HV(ctx);
- if (is_book3s_arch2x(ctx)) {
- gen_helper_book3s_msgclr(cpu_env, cpu_gpr[rB(ctx->opcode)]);
- } else {
- gen_helper_msgclr(cpu_env, cpu_gpr[rB(ctx->opcode)]);
- }
-#endif /* defined(CONFIG_USER_ONLY) */
-}
-
-static void gen_msgsnd(DisasContext *ctx)
-{
-#if defined(CONFIG_USER_ONLY)
- GEN_PRIV(ctx);
-#else
- CHK_HV(ctx);
- if (is_book3s_arch2x(ctx)) {
- gen_helper_book3s_msgsnd(cpu_gpr[rB(ctx->opcode)]);
- } else {
- gen_helper_msgsnd(cpu_gpr[rB(ctx->opcode)]);
- }
-#endif /* defined(CONFIG_USER_ONLY) */
-}
-
#if defined(TARGET_PPC64)
static void gen_msgclrp(DisasContext *ctx)
{
@@ -6628,6 +6600,8 @@ static bool resolve_PLS_D(DisasContext *ctx, arg_D *d, arg_PLS_D *a)
#include "translate/branch-impl.c.inc"
+#include "translate/processor-ctrl-impl.c.inc"
+
#include "translate/storage-ctrl-impl.c.inc"
/* Handles lfdp */
@@ -6901,10 +6875,6 @@ GEN_HANDLER2_E(tlbivax_booke206, "tlbivax", 0x1F, 0x12, 0x18, 0x00000001,
PPC_NONE, PPC2_BOOKE206),
GEN_HANDLER2_E(tlbilx_booke206, "tlbilx", 0x1F, 0x12, 0x00, 0x03800001,
PPC_NONE, PPC2_BOOKE206),
-GEN_HANDLER2_E(msgsnd, "msgsnd", 0x1F, 0x0E, 0x06, 0x03ff0001,
- PPC_NONE, (PPC2_PRCNTL | PPC2_ISA207S)),
-GEN_HANDLER2_E(msgclr, "msgclr", 0x1F, 0x0E, 0x07, 0x03ff0001,
- PPC_NONE, (PPC2_PRCNTL | PPC2_ISA207S)),
GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
PPC_NONE, PPC2_ISA300),
GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
diff --git a/target/ppc/translate/processor-ctrl-impl.c.inc b/target/ppc/translate/processor-ctrl-impl.c.inc
new file mode 100644
index 0000000000..0192b45c8f
--- /dev/null
+++ b/target/ppc/translate/processor-ctrl-impl.c.inc
@@ -0,0 +1,70 @@
+/*
+ * Power ISA decode for Storage Control instructions
+ *
+ * Copyright (c) 2022 Instituto de Pesquisas Eldorado (eldorado.org.br)
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Processor Control Instructions
+ */
+
+static bool trans_MSGCLR(DisasContext *ctx, arg_X_rb *a)
+{
+ if (!(ctx->insns_flags2 & PPC2_ISA207S)) {
+ /*
+ * Before Power ISA 2.07, processor control instructions were only
+ * implemented in the "Embedded.Processor Control" category.
+ */
+ REQUIRE_INSNS_FLAGS2(ctx, PRCNTL);
+ }
+
+ REQUIRE_HV(ctx);
+
+#if !defined(CONFIG_USER_ONLY)
+ if (is_book3s_arch2x(ctx)) {
+ gen_helper_book3s_msgclr(cpu_env, cpu_gpr[a->rb]);
+ } else {
+ gen_helper_msgclr(cpu_env, cpu_gpr[a->rb]);
+ }
+#else
+ qemu_build_not_reached();
+#endif
+ return true;
+}
+
+static bool trans_MSGSND(DisasContext *ctx, arg_X_rb *a)
+{
+ if (!(ctx->insns_flags2 & PPC2_ISA207S)) {
+ /*
+ * Before Power ISA 2.07, processor control instructions were only
+ * implemented in the "Embedded.Processor Control" category.
+ */
+ REQUIRE_INSNS_FLAGS2(ctx, PRCNTL);
+ }
+
+ REQUIRE_HV(ctx);
+
+#if !defined(CONFIG_USER_ONLY)
+ if (is_book3s_arch2x(ctx)) {
+ gen_helper_book3s_msgsnd(cpu_gpr[a->rb]);
+ } else {
+ gen_helper_msgsnd(cpu_gpr[a->rb]);
+ }
+#else
+ qemu_build_not_reached();
+#endif
+ return true;
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/6] target/ppc: move msgclrp/msgsndp to decodetree
2022-10-06 20:06 [PATCH 0/6] Enable doorbell instruction for POWER8 CPUs Matheus Ferst
` (3 preceding siblings ...)
2022-10-06 20:06 ` [PATCH 4/6] target/ppc: move msgclr/msgsnd to decodetree Matheus Ferst
@ 2022-10-06 20:06 ` Matheus Ferst
2022-10-19 20:59 ` Daniel Henrique Barboza
2022-10-06 20:06 ` [PATCH 6/6] target/ppc: move msgsync " Matheus Ferst
5 siblings, 1 reply; 15+ messages in thread
From: Matheus Ferst @ 2022-10-06 20:06 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, farosas, Matheus Ferst
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();
+#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();
+#endif
+ return true;
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/6] target/ppc: move msgsync to decodetree
2022-10-06 20:06 [PATCH 0/6] Enable doorbell instruction for POWER8 CPUs Matheus Ferst
` (4 preceding siblings ...)
2022-10-06 20:06 ` [PATCH 5/6] target/ppc: move msgclrp/msgsndp " Matheus Ferst
@ 2022-10-06 20:06 ` Matheus Ferst
2022-10-19 20:59 ` Daniel Henrique Barboza
5 siblings, 1 reply; 15+ messages in thread
From: Matheus Ferst @ 2022-10-06 20:06 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, farosas, Matheus Ferst
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/insn32.decode | 1 +
target/ppc/translate.c | 14 --------------
target/ppc/translate/processor-ctrl-impl.c.inc | 9 +++++++++
3 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 5ba4a6807d..70a3b4de5e 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -915,3 +915,4 @@ MSGCLR 011111 ----- ----- ..... 0011101110 - @X_rb
MSGSND 011111 ----- ----- ..... 0011001110 - @X_rb
MSGCLRP 011111 ----- ----- ..... 0010101110 - @X_rb
MSGSNDP 011111 ----- ----- ..... 0010001110 - @X_rb
+MSGSYNC 011111 ----- ----- ----- 1101110110 -
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 087ab8e69d..f092bbeb8b 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6239,18 +6239,6 @@ static void gen_icbt_440(DisasContext *ctx)
*/
}
-/* Embedded.Processor Control */
-
-static void gen_msgsync(DisasContext *ctx)
-{
-#if defined(CONFIG_USER_ONLY)
- GEN_PRIV(ctx);
-#else
- CHK_HV(ctx);
-#endif /* defined(CONFIG_USER_ONLY) */
- /* interpreted as no-op */
-}
-
#if defined(TARGET_PPC64)
static void gen_maddld(DisasContext *ctx)
{
@@ -6853,8 +6841,6 @@ GEN_HANDLER2_E(tlbivax_booke206, "tlbivax", 0x1F, 0x12, 0x18, 0x00000001,
PPC_NONE, PPC2_BOOKE206),
GEN_HANDLER2_E(tlbilx_booke206, "tlbilx", 0x1F, 0x12, 0x00, 0x03800001,
PPC_NONE, PPC2_BOOKE206),
-GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
- PPC_NONE, PPC2_ISA300),
GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
diff --git a/target/ppc/translate/processor-ctrl-impl.c.inc b/target/ppc/translate/processor-ctrl-impl.c.inc
index 3703001f31..021e365a57 100644
--- a/target/ppc/translate/processor-ctrl-impl.c.inc
+++ b/target/ppc/translate/processor-ctrl-impl.c.inc
@@ -92,3 +92,12 @@ static bool trans_MSGSNDP(DisasContext *ctx, arg_X_rb *a)
#endif
return true;
}
+
+static bool trans_MSGSYNC(DisasContext *ctx, arg_MSGSYNC *a)
+{
+ REQUIRE_INSNS_FLAGS2(ctx, ISA300);
+ REQUIRE_HV(ctx);
+
+ /* interpreted as no-op */
+ return true;
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] target/ppc: fix REQUIRE_HV macro definition
2022-10-06 20:06 ` [PATCH 3/6] target/ppc: fix REQUIRE_HV macro definition Matheus Ferst
@ 2022-10-07 19:07 ` Fabiano Rosas
0 siblings, 0 replies; 15+ messages in thread
From: Fabiano Rosas @ 2022-10-07 19:07 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, Matheus Ferst
Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
> The macro is missing a '{' after the if condition. Any use of REQUIRE_HV
> would cause a compilation error.
>
> Fixes: fc34e81acd51 ("target/ppc: add macros to check privilege level")
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> target/ppc/translate.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index eaac8670b1..435066c4a3 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6545,12 +6545,12 @@ static int64_t dw_compose_ea(DisasContext *ctx, int x)
> } \
> } while (0)
>
> -#define REQUIRE_HV(CTX) \
> - do { \
> - if (unlikely((CTX)->pr || !(CTX)->hv)) \
> - gen_priv_opc(CTX); \
> - return true; \
> - } \
> +#define REQUIRE_HV(CTX) \
> + do { \
> + if (unlikely((CTX)->pr || !(CTX)->hv)) { \
> + gen_priv_opc(CTX); \
> + return true; \
> + } \
> } while (0)
> #else
> #define REQUIRE_SV(CTX) do { gen_priv_opc(CTX); return true; } while (0)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] target/ppc: fix msgclr/msgsnd insns flags
2022-10-06 20:06 ` [PATCH 1/6] target/ppc: fix msgclr/msgsnd insns flags Matheus Ferst
@ 2022-10-07 19:07 ` Fabiano Rosas
0 siblings, 0 replies; 15+ messages in thread
From: Fabiano Rosas @ 2022-10-07 19:07 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, Matheus Ferst
Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
> On Power ISA v2.07, the category for these instructions became
> "Embedded.Processor Control" or "Book S".
>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> target/ppc/translate.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index e810842925..37d7018d18 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6902,9 +6902,9 @@ GEN_HANDLER2_E(tlbivax_booke206, "tlbivax", 0x1F, 0x12, 0x18, 0x00000001,
> GEN_HANDLER2_E(tlbilx_booke206, "tlbilx", 0x1F, 0x12, 0x00, 0x03800001,
> PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER2_E(msgsnd, "msgsnd", 0x1F, 0x0E, 0x06, 0x03ff0001,
> - PPC_NONE, PPC2_PRCNTL),
> + PPC_NONE, (PPC2_PRCNTL | PPC2_ISA207S)),
> GEN_HANDLER2_E(msgclr, "msgclr", 0x1F, 0x0E, 0x07, 0x03ff0001,
> - PPC_NONE, PPC2_PRCNTL),
> + PPC_NONE, (PPC2_PRCNTL | PPC2_ISA207S)),
> GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
> PPC_NONE, PPC2_PRCNTL),
> GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] target/ppc: fix msgsync insns flags
2022-10-06 20:06 ` [PATCH 2/6] target/ppc: fix msgsync " Matheus Ferst
@ 2022-10-07 19:10 ` Fabiano Rosas
0 siblings, 0 replies; 15+ messages in thread
From: Fabiano Rosas @ 2022-10-07 19:10 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc
Cc: clg, danielhb413, david, groug, Matheus Ferst
Matheus Ferst <matheus.ferst@eldorado.org.br> writes:
> This instruction was added by Power ISA 3.0, using PPC2_PRCNTL makes it
> available for older processors, like de e5500 and e6500.
>
> Fixes: 7af1e7b02264 ("target/ppc: add support for hypervisor doorbells on book3s CPUs")
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> target/ppc/translate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 37d7018d18..eaac8670b1 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6906,7 +6906,7 @@ GEN_HANDLER2_E(msgsnd, "msgsnd", 0x1F, 0x0E, 0x06, 0x03ff0001,
> GEN_HANDLER2_E(msgclr, "msgclr", 0x1F, 0x0E, 0x07, 0x03ff0001,
> PPC_NONE, (PPC2_PRCNTL | PPC2_ISA207S)),
> GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
> - PPC_NONE, PPC2_PRCNTL),
> + PPC_NONE, PPC2_ISA300),
> GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
> GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
> GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/6] target/ppc: move msgclrp/msgsndp to decodetree
2022-10-06 20:06 ` [PATCH 5/6] target/ppc: move msgclrp/msgsndp " Matheus Ferst
@ 2022-10-19 20:59 ` Daniel Henrique Barboza
2022-10-20 12:18 ` Matheus K. Ferst
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Henrique Barboza @ 2022-10-19 20:59 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc; +Cc: clg, david, groug, farosas
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:
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.
Thanks,
Daniel
> +#endif
> + return true;
> +}
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/6] target/ppc: move msgclr/msgsnd to decodetree
2022-10-06 20:06 ` [PATCH 4/6] target/ppc: move msgclr/msgsnd to decodetree Matheus Ferst
@ 2022-10-19 20:59 ` Daniel Henrique Barboza
0 siblings, 0 replies; 15+ messages in thread
From: Daniel Henrique Barboza @ 2022-10-19 20:59 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc; +Cc: clg, david, groug, farosas
On 10/6/22 17:06, Matheus Ferst wrote:
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> target/ppc/insn32.decode | 5 ++
> target/ppc/translate.c | 34 +--------
> .../ppc/translate/processor-ctrl-impl.c.inc | 70 +++++++++++++++++++
> 3 files changed, 77 insertions(+), 32 deletions(-)
> create mode 100644 target/ppc/translate/processor-ctrl-impl.c.inc
>
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index a5249ee32c..bba49ded1b 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -908,3 +908,8 @@ SLBSYNC 011111 ----- ----- ----- 0101010010 -
>
> TLBIE 011111 ..... - .. . . ..... 0100110010 - @X_tlbie
> TLBIEL 011111 ..... - .. . . ..... 0100010010 - @X_tlbie
> +
> +# Processor Control Instructions
> +
> +MSGCLR 011111 ----- ----- ..... 0011101110 - @X_rb
> +MSGSND 011111 ----- ----- ..... 0011001110 - @X_rb
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 435066c4a3..889cca6325 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6241,34 +6241,6 @@ static void gen_icbt_440(DisasContext *ctx)
>
> /* Embedded.Processor Control */
>
> -static void gen_msgclr(DisasContext *ctx)
> -{
> -#if defined(CONFIG_USER_ONLY)
> - GEN_PRIV(ctx);
> -#else
> - CHK_HV(ctx);
> - if (is_book3s_arch2x(ctx)) {
> - gen_helper_book3s_msgclr(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> - } else {
> - gen_helper_msgclr(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> - }
> -#endif /* defined(CONFIG_USER_ONLY) */
> -}
> -
> -static void gen_msgsnd(DisasContext *ctx)
> -{
> -#if defined(CONFIG_USER_ONLY)
> - GEN_PRIV(ctx);
> -#else
> - CHK_HV(ctx);
> - if (is_book3s_arch2x(ctx)) {
> - gen_helper_book3s_msgsnd(cpu_gpr[rB(ctx->opcode)]);
> - } else {
> - gen_helper_msgsnd(cpu_gpr[rB(ctx->opcode)]);
> - }
> -#endif /* defined(CONFIG_USER_ONLY) */
> -}
> -
> #if defined(TARGET_PPC64)
> static void gen_msgclrp(DisasContext *ctx)
> {
> @@ -6628,6 +6600,8 @@ static bool resolve_PLS_D(DisasContext *ctx, arg_D *d, arg_PLS_D *a)
>
> #include "translate/branch-impl.c.inc"
>
> +#include "translate/processor-ctrl-impl.c.inc"
> +
> #include "translate/storage-ctrl-impl.c.inc"
>
> /* Handles lfdp */
> @@ -6901,10 +6875,6 @@ GEN_HANDLER2_E(tlbivax_booke206, "tlbivax", 0x1F, 0x12, 0x18, 0x00000001,
> PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER2_E(tlbilx_booke206, "tlbilx", 0x1F, 0x12, 0x00, 0x03800001,
> PPC_NONE, PPC2_BOOKE206),
> -GEN_HANDLER2_E(msgsnd, "msgsnd", 0x1F, 0x0E, 0x06, 0x03ff0001,
> - PPC_NONE, (PPC2_PRCNTL | PPC2_ISA207S)),
> -GEN_HANDLER2_E(msgclr, "msgclr", 0x1F, 0x0E, 0x07, 0x03ff0001,
> - PPC_NONE, (PPC2_PRCNTL | PPC2_ISA207S)),
> GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
> PPC_NONE, PPC2_ISA300),
> GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
> diff --git a/target/ppc/translate/processor-ctrl-impl.c.inc b/target/ppc/translate/processor-ctrl-impl.c.inc
> new file mode 100644
> index 0000000000..0192b45c8f
> --- /dev/null
> +++ b/target/ppc/translate/processor-ctrl-impl.c.inc
> @@ -0,0 +1,70 @@
> +/*
> + * Power ISA decode for Storage Control instructions
> + *
> + * Copyright (c) 2022 Instituto de Pesquisas Eldorado (eldorado.org.br)
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/*
> + * Processor Control Instructions
> + */
> +
> +static bool trans_MSGCLR(DisasContext *ctx, arg_X_rb *a)
> +{
> + if (!(ctx->insns_flags2 & PPC2_ISA207S)) {
> + /*
> + * Before Power ISA 2.07, processor control instructions were only
> + * implemented in the "Embedded.Processor Control" category.
> + */
> + REQUIRE_INSNS_FLAGS2(ctx, PRCNTL);
> + }
> +
> + REQUIRE_HV(ctx);
> +
> +#if !defined(CONFIG_USER_ONLY)
> + if (is_book3s_arch2x(ctx)) {
> + gen_helper_book3s_msgclr(cpu_env, cpu_gpr[a->rb]);
> + } else {
> + gen_helper_msgclr(cpu_env, cpu_gpr[a->rb]);
> + }
> +#else
> + qemu_build_not_reached();
> +#endif
> + return true;
> +}
> +
> +static bool trans_MSGSND(DisasContext *ctx, arg_X_rb *a)
> +{
> + if (!(ctx->insns_flags2 & PPC2_ISA207S)) {
> + /*
> + * Before Power ISA 2.07, processor control instructions were only
> + * implemented in the "Embedded.Processor Control" category.
> + */
> + REQUIRE_INSNS_FLAGS2(ctx, PRCNTL);
> + }
> +
> + REQUIRE_HV(ctx);
> +
> +#if !defined(CONFIG_USER_ONLY)
> + if (is_book3s_arch2x(ctx)) {
> + gen_helper_book3s_msgsnd(cpu_gpr[a->rb]);
> + } else {
> + gen_helper_msgsnd(cpu_gpr[a->rb]);
> + }
> +#else
> + qemu_build_not_reached();
> +#endif
> + return true;
> +}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 6/6] target/ppc: move msgsync to decodetree
2022-10-06 20:06 ` [PATCH 6/6] target/ppc: move msgsync " Matheus Ferst
@ 2022-10-19 20:59 ` Daniel Henrique Barboza
0 siblings, 0 replies; 15+ messages in thread
From: Daniel Henrique Barboza @ 2022-10-19 20:59 UTC (permalink / raw)
To: Matheus Ferst, qemu-devel, qemu-ppc; +Cc: clg, david, groug, farosas
On 10/6/22 17:06, Matheus Ferst wrote:
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> target/ppc/insn32.decode | 1 +
> target/ppc/translate.c | 14 --------------
> target/ppc/translate/processor-ctrl-impl.c.inc | 9 +++++++++
> 3 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index 5ba4a6807d..70a3b4de5e 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -915,3 +915,4 @@ MSGCLR 011111 ----- ----- ..... 0011101110 - @X_rb
> MSGSND 011111 ----- ----- ..... 0011001110 - @X_rb
> MSGCLRP 011111 ----- ----- ..... 0010101110 - @X_rb
> MSGSNDP 011111 ----- ----- ..... 0010001110 - @X_rb
> +MSGSYNC 011111 ----- ----- ----- 1101110110 -
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 087ab8e69d..f092bbeb8b 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6239,18 +6239,6 @@ static void gen_icbt_440(DisasContext *ctx)
> */
> }
>
> -/* Embedded.Processor Control */
> -
> -static void gen_msgsync(DisasContext *ctx)
> -{
> -#if defined(CONFIG_USER_ONLY)
> - GEN_PRIV(ctx);
> -#else
> - CHK_HV(ctx);
> -#endif /* defined(CONFIG_USER_ONLY) */
> - /* interpreted as no-op */
> -}
> -
> #if defined(TARGET_PPC64)
> static void gen_maddld(DisasContext *ctx)
> {
> @@ -6853,8 +6841,6 @@ GEN_HANDLER2_E(tlbivax_booke206, "tlbivax", 0x1F, 0x12, 0x18, 0x00000001,
> PPC_NONE, PPC2_BOOKE206),
> GEN_HANDLER2_E(tlbilx_booke206, "tlbilx", 0x1F, 0x12, 0x00, 0x03800001,
> PPC_NONE, PPC2_BOOKE206),
> -GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
> - PPC_NONE, PPC2_ISA300),
> GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
> GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
> GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
> diff --git a/target/ppc/translate/processor-ctrl-impl.c.inc b/target/ppc/translate/processor-ctrl-impl.c.inc
> index 3703001f31..021e365a57 100644
> --- a/target/ppc/translate/processor-ctrl-impl.c.inc
> +++ b/target/ppc/translate/processor-ctrl-impl.c.inc
> @@ -92,3 +92,12 @@ static bool trans_MSGSNDP(DisasContext *ctx, arg_X_rb *a)
> #endif
> return true;
> }
> +
> +static bool trans_MSGSYNC(DisasContext *ctx, arg_MSGSYNC *a)
> +{
> + REQUIRE_INSNS_FLAGS2(ctx, ISA300);
> + REQUIRE_HV(ctx);
> +
> + /* interpreted as no-op */
> + return true;
> +}
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/6] target/ppc: move msgclrp/msgsndp to decodetree
2022-10-19 20:59 ` Daniel Henrique Barboza
@ 2022-10-20 12:18 ` Matheus K. Ferst
2022-10-20 14:25 ` Daniel Henrique Barboza
0 siblings, 1 reply; 15+ messages in thread
From: Matheus K. Ferst @ 2022-10-20 12:18 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel, qemu-ppc; +Cc: clg, david, groug, farosas
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.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?
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>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/6] target/ppc: move msgclrp/msgsndp to decodetree
2022-10-20 12:18 ` Matheus K. Ferst
@ 2022-10-20 14:25 ` Daniel Henrique Barboza
0 siblings, 0 replies; 15+ messages in thread
From: Daniel Henrique Barboza @ 2022-10-20 14:25 UTC (permalink / raw)
To: Matheus K. Ferst, qemu-devel, qemu-ppc; +Cc: clg, david, groug, farosas
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.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.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>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-10-20 16:56 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-06 20:06 [PATCH 0/6] Enable doorbell instruction for POWER8 CPUs Matheus Ferst
2022-10-06 20:06 ` [PATCH 1/6] target/ppc: fix msgclr/msgsnd insns flags Matheus Ferst
2022-10-07 19:07 ` Fabiano Rosas
2022-10-06 20:06 ` [PATCH 2/6] target/ppc: fix msgsync " Matheus Ferst
2022-10-07 19:10 ` Fabiano Rosas
2022-10-06 20:06 ` [PATCH 3/6] target/ppc: fix REQUIRE_HV macro definition Matheus Ferst
2022-10-07 19:07 ` Fabiano Rosas
2022-10-06 20:06 ` [PATCH 4/6] target/ppc: move msgclr/msgsnd to decodetree Matheus Ferst
2022-10-19 20:59 ` Daniel Henrique Barboza
2022-10-06 20:06 ` [PATCH 5/6] target/ppc: move msgclrp/msgsndp " Matheus Ferst
2022-10-19 20:59 ` Daniel Henrique Barboza
2022-10-20 12:18 ` Matheus K. Ferst
2022-10-20 14:25 ` Daniel Henrique Barboza
2022-10-06 20:06 ` [PATCH 6/6] target/ppc: move msgsync " Matheus Ferst
2022-10-19 20:59 ` Daniel Henrique Barboza
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).