All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v3 0/5] Port PPC64/PowerNV MMU tests to QEMU
@ 2022-04-18 19:10 Leandro Lupori
  2022-04-18 19:10 ` [RFC PATCH v3 1/5] ppc64: Add semihosting support Leandro Lupori
                   ` (4 more replies)
  0 siblings, 5 replies; 35+ messages in thread
From: Leandro Lupori @ 2022-04-18 19:10 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: Leandro Lupori, danielhb413, richard.henderson, groug, clg,
	pbonzini, alex.bennee, david

Changes from v2:
- Added semihosting support for ppc64
- Use semihosting calls to exit tests, instead of using Processor
Attention instruction
- Use semihosting calls for console output, instead of programming
emulated serial hardware

Leandro Lupori (5):
  ppc64: Add semihosting support
  ppc64: Fix semihosting on ppc64le
  tests/tcg/ppc64: Add basic softmmu test support
  tests/tcg/ppc64: Add MMU test sources
  tests/tcg/ppc64: Build PowerNV and LE tests

 MAINTAINERS                               |   2 +
 configs/devices/ppc64-softmmu/default.mak |   3 +
 include/exec/softmmu-semi.h               |  23 +-
 qemu-options.hx                           |  18 +-
 semihosting/arm-compat-semi.c             |  33 +
 target/ppc/cpu.h                          |   3 +-
 target/ppc/excp_helper.c                  |   9 +
 target/ppc/translate.c                    |  14 +
 tests/tcg/ppc64/Makefile.softmmu-rules    |  34 +
 tests/tcg/ppc64/Makefile.softmmu-target   | 125 ++++
 tests/tcg/ppc64/system/include/asm.h      |  68 ++
 tests/tcg/ppc64/system/lib/boot.S         |  84 +++
 tests/tcg/ppc64/system/lib/powerpc.lds    |  27 +
 tests/tcg/ppc64/system/mmu-head.S         | 142 ++++
 tests/tcg/ppc64/system/mmu.c              | 764 ++++++++++++++++++++++
 tests/tcg/ppc64/system/mmu.h              |   9 +
 16 files changed, 1346 insertions(+), 12 deletions(-)
 create mode 100644 tests/tcg/ppc64/Makefile.softmmu-rules
 create mode 100644 tests/tcg/ppc64/Makefile.softmmu-target
 create mode 100644 tests/tcg/ppc64/system/include/asm.h
 create mode 100644 tests/tcg/ppc64/system/lib/boot.S
 create mode 100644 tests/tcg/ppc64/system/lib/powerpc.lds
 create mode 100644 tests/tcg/ppc64/system/mmu-head.S
 create mode 100644 tests/tcg/ppc64/system/mmu.c
 create mode 100644 tests/tcg/ppc64/system/mmu.h

-- 
2.25.1



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-18 19:10 [RFC PATCH v3 0/5] Port PPC64/PowerNV MMU tests to QEMU Leandro Lupori
@ 2022-04-18 19:10 ` Leandro Lupori
  2022-04-18 20:22   ` Cédric Le Goater
                     ` (3 more replies)
  2022-04-18 19:10 ` [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le Leandro Lupori
                   ` (3 subsequent siblings)
  4 siblings, 4 replies; 35+ messages in thread
From: Leandro Lupori @ 2022-04-18 19:10 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: Leandro Lupori, danielhb413, richard.henderson, groug, clg,
	pbonzini, alex.bennee, david

Add semihosting support for PPC64. This implementation is
based on the standard for ARM semihosting version 2.0, as
implemented by QEMU and documented in

    https://github.com/ARM-software/abi-aa/releases

The PPC64 specific differences are the following:

Semihosting Trap Instruction: sc 7
Operation Number Register: r3
Parameter Register: r4
Return Register: r3
Data block field size: 64 bits

Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
---
 configs/devices/ppc64-softmmu/default.mak |  3 +++
 qemu-options.hx                           | 18 ++++++++-----
 semihosting/arm-compat-semi.c             | 33 +++++++++++++++++++++++
 target/ppc/cpu.h                          |  3 ++-
 target/ppc/excp_helper.c                  |  9 +++++++
 target/ppc/translate.c                    | 14 ++++++++++
 6 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/configs/devices/ppc64-softmmu/default.mak b/configs/devices/ppc64-softmmu/default.mak
index b90e5bf455..43b618fa32 100644
--- a/configs/devices/ppc64-softmmu/default.mak
+++ b/configs/devices/ppc64-softmmu/default.mak
@@ -8,3 +8,6 @@ CONFIG_POWERNV=y
 
 # For pSeries
 CONFIG_PSERIES=y
+
+CONFIG_SEMIHOSTING=y
+CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/qemu-options.hx b/qemu-options.hx
index 34e9b32a5c..6e76f7de96 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4527,11 +4527,12 @@ SRST
 ERST
 DEF("semihosting", 0, QEMU_OPTION_semihosting,
     "-semihosting    semihosting mode\n",
-    QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA |
-    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
+    QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_MIPS |
+    QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV | QEMU_ARCH_PPC)
 SRST
 ``-semihosting``
-    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V only).
+    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V and
+    PPC only).
 
     Note that this allows guest direct access to the host filesystem, so
     should only be used with a trusted guest OS.
@@ -4542,12 +4543,12 @@ ERST
 DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
     "-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]\n" \
     "                semihosting configuration\n",
-QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA |
-QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
+QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_MIPS |
+QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV | QEMU_ARCH_PPC)
 SRST
 ``-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]``
-    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V
-    only).
+    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II,
+    RISC-V, PPC only).
 
     Note that this allows guest direct access to the host filesystem, so
     should only be used with a trusted guest OS.
@@ -4563,6 +4564,9 @@ SRST
 
     On RISC-V this implements the standard semihosting API, version 0.2.
 
+    On PPC this implements the standard Arm semihosting API, version 2.0,
+    with only the trap instruction and register definitions changed.
+
     ``target=native|gdb|auto``
         Defines where the semihosting calls will be addressed, to QEMU
         (``native``) or to GDB (``gdb``). The default is ``auto``, which
diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index 7a51fd0737..e1279c316c 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -268,6 +268,31 @@ common_semi_sys_exit_extended(CPUState *cs, int nr)
 
 #endif
 
+#ifdef TARGET_PPC64
+static inline target_ulong
+common_semi_arg(CPUState *cs, int argno)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    return env->gpr[3 + argno];
+}
+
+static inline void
+common_semi_set_ret(CPUState *cs, target_ulong ret)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    env->gpr[3] = ret;
+}
+
+static inline bool
+common_semi_sys_exit_extended(CPUState *cs, int nr)
+{
+    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
+}
+
+#endif
+
 /*
  * Allocate a new guest file descriptor and return it; if we
  * couldn't allocate a new fd then return -1.
@@ -450,6 +475,12 @@ static target_ulong common_semi_flen_buf(CPUState *cs)
 
     sp = env->gpr[xSP];
 #endif
+#ifdef TARGET_PPC64
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+
+    sp = env->gpr[1];
+#endif
 
     return sp - 64;
 }
@@ -780,6 +811,8 @@ static inline bool is_64bit_semihosting(CPUArchState *env)
     return is_a64(env);
 #elif defined(TARGET_RISCV)
     return riscv_cpu_mxl(env) != MXL_RV32;
+#elif defined(TARGET_PPC64)
+    return true;
 #else
 #error un-handled architecture
 #endif
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 047b24ba50..349104ad79 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -129,8 +129,9 @@ enum {
     POWERPC_EXCP_SYSCALL_VECTORED = 102, /* scv exception                     */
     POWERPC_EXCP_PERFM_EBB = 103,    /* Performance Monitor EBB Exception    */
     POWERPC_EXCP_EXTERNAL_EBB = 104, /* External EBB Exception               */
+    POWERPC_EXCP_SEMIHOST = 105,     /* Semihosting Exception                */
     /* EOL                                                                   */
-    POWERPC_EXCP_NB       = 105,
+    POWERPC_EXCP_NB       = 106,
     /* QEMU exceptions: special cases we want to stop translation            */
     POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only      */
 };
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index d3e2cfcd71..af34a57082 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -25,6 +25,7 @@
 #include "helper_regs.h"
 
 #include "trace.h"
+#include "semihosting/common-semi.h"
 
 #ifdef CONFIG_TCG
 #include "exec/helper-proto.h"
@@ -100,6 +101,7 @@ static const char *powerpc_excp_name(int excp)
     case POWERPC_EXCP_SDOOR_HV: return "SDOOR_HV";
     case POWERPC_EXCP_HVIRT:    return "HVIRT";
     case POWERPC_EXCP_SYSCALL_VECTORED: return "SYSCALL_VECTORED";
+    case POWERPC_EXCP_SEMIHOST: return "SEMIHOST";
     default:
         g_assert_not_reached();
     }
@@ -1327,6 +1329,13 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
     target_ulong msr, new_msr, vector;
     int srr0, srr1, lev = -1;
 
+    /* Handle semihost exceptions first */
+    if (excp == POWERPC_EXCP_SEMIHOST) {
+        env->gpr[3] = do_common_semihosting(cs);
+        env->nip += 4;
+        return;
+    }
+
     /* new srr1 value excluding must-be-zero bits */
     msr = env->msr & ~0x783f0000ULL;
 
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 408ae26173..c013889a84 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4520,6 +4520,20 @@ static void gen_sc(DisasContext *ctx)
     uint32_t lev;
 
     lev = (ctx->opcode >> 5) & 0x7F;
+
+#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+    /*
+     * PowerPC semihosting uses the following
+     * instruction to flag a semihosting call:
+     *
+     *      sc 7            0x440000e2
+     */
+    if (lev == 7) {
+        gen_exception(ctx, POWERPC_EXCP_SEMIHOST);
+        return;
+    }
+#endif
+
     gen_exception_err(ctx, POWERPC_SYSCALL, lev);
 }
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le
  2022-04-18 19:10 [RFC PATCH v3 0/5] Port PPC64/PowerNV MMU tests to QEMU Leandro Lupori
  2022-04-18 19:10 ` [RFC PATCH v3 1/5] ppc64: Add semihosting support Leandro Lupori
@ 2022-04-18 19:10 ` Leandro Lupori
  2022-04-18 23:36   ` Richard Henderson
  2022-04-20 19:42   ` Peter Maydell
  2022-04-18 19:10 ` [RFC PATCH v3 3/5] tests/tcg/ppc64: Add basic softmmu test support Leandro Lupori
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 35+ messages in thread
From: Leandro Lupori @ 2022-04-18 19:10 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: Leandro Lupori, danielhb413, richard.henderson, groug, clg,
	pbonzini, alex.bennee, david

PPC64 CPUs can change its endian dynamically, so semihosting code
must check its MSR at run time to determine if byte swapping is
needed.

Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
---
 include/exec/softmmu-semi.h | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/include/exec/softmmu-semi.h b/include/exec/softmmu-semi.h
index fbcae88f4b..723aa2f58a 100644
--- a/include/exec/softmmu-semi.h
+++ b/include/exec/softmmu-semi.h
@@ -12,12 +12,27 @@
 
 #include "cpu.h"
 
+#ifdef TARGET_PPC64
+static inline uint64_t sh_swap64(CPUArchState *env, uint64_t val)
+{
+    return msr_le ? val : tswap64(val);
+}
+
+static inline uint32_t sh_swap32(CPUArchState *env, uint32_t val)
+{
+    return msr_le ? val : tswap32(val);
+}
+#else
+#define sh_swap64(env, val)     tswap64(val)
+#define sh_swap32(env, val)     tswap32(val)
+#endif
+
 static inline uint64_t softmmu_tget64(CPUArchState *env, target_ulong addr)
 {
     uint64_t val;
 
     cpu_memory_rw_debug(env_cpu(env), addr, (uint8_t *)&val, 8, 0);
-    return tswap64(val);
+    return sh_swap64(env, val);
 }
 
 static inline uint32_t softmmu_tget32(CPUArchState *env, target_ulong addr)
@@ -25,7 +40,7 @@ static inline uint32_t softmmu_tget32(CPUArchState *env, target_ulong addr)
     uint32_t val;
 
     cpu_memory_rw_debug(env_cpu(env), addr, (uint8_t *)&val, 4, 0);
-    return tswap32(val);
+    return sh_swap32(env, val);
 }
 
 static inline uint32_t softmmu_tget8(CPUArchState *env, target_ulong addr)
@@ -44,14 +59,14 @@ static inline uint32_t softmmu_tget8(CPUArchState *env, target_ulong addr)
 static inline void softmmu_tput64(CPUArchState *env,
                                   target_ulong addr, uint64_t val)
 {
-    val = tswap64(val);
+    val = sh_swap64(env, val);
     cpu_memory_rw_debug(env_cpu(env), addr, (uint8_t *)&val, 8, 1);
 }
 
 static inline void softmmu_tput32(CPUArchState *env,
                                   target_ulong addr, uint32_t val)
 {
-    val = tswap32(val);
+    val = sh_swap32(env, val);
     cpu_memory_rw_debug(env_cpu(env), addr, (uint8_t *)&val, 4, 1);
 }
 #define put_user_u64(arg, p) ({ softmmu_tput64(env, p, arg) ; 0; })
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v3 3/5] tests/tcg/ppc64: Add basic softmmu test support
  2022-04-18 19:10 [RFC PATCH v3 0/5] Port PPC64/PowerNV MMU tests to QEMU Leandro Lupori
  2022-04-18 19:10 ` [RFC PATCH v3 1/5] ppc64: Add semihosting support Leandro Lupori
  2022-04-18 19:10 ` [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le Leandro Lupori
@ 2022-04-18 19:10 ` Leandro Lupori
  2022-04-18 20:27   ` Cédric Le Goater
  2022-04-18 19:10 ` [RFC PATCH v3 4/5] tests/tcg/ppc64: Add MMU test sources Leandro Lupori
  2022-04-18 19:11 ` [RFC PATCH v3 5/5] tests/tcg/ppc64: Build PowerNV and LE tests Leandro Lupori
  4 siblings, 1 reply; 35+ messages in thread
From: Leandro Lupori @ 2022-04-18 19:10 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: Leandro Lupori, danielhb413, richard.henderson, groug, clg,
	pbonzini, alex.bennee, david

Add support to build and run the multiarch hello test, that simply
prints a message and exits, through semihosting operations.

The linker script was imported from
https://github.com/legoater/pnv-test, that are the Microwatt tests
adapted to use a PowerNV console. Boot.S code was inspired on
mmu/head.S, also from pnv-test repo.

Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
---
 MAINTAINERS                             |  2 +
 tests/tcg/ppc64/Makefile.softmmu-target | 56 +++++++++++++++++
 tests/tcg/ppc64/system/include/asm.h    | 68 ++++++++++++++++++++
 tests/tcg/ppc64/system/lib/boot.S       | 84 +++++++++++++++++++++++++
 tests/tcg/ppc64/system/lib/powerpc.lds  | 27 ++++++++
 5 files changed, 237 insertions(+)
 create mode 100644 tests/tcg/ppc64/Makefile.softmmu-target
 create mode 100644 tests/tcg/ppc64/system/include/asm.h
 create mode 100644 tests/tcg/ppc64/system/lib/boot.S
 create mode 100644 tests/tcg/ppc64/system/lib/powerpc.lds

diff --git a/MAINTAINERS b/MAINTAINERS
index 4ad2451e03..54f917f8ea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -266,6 +266,7 @@ M: Cédric Le Goater <clg@kaod.org>
 M: Daniel Henrique Barboza <danielhb413@gmail.com>
 R: David Gibson <david@gibson.dropbear.id.au>
 R: Greg Kurz <groug@kaod.org>
+R: Leandro Lupori <leandro.lupori@eldorado.org.br>
 L: qemu-ppc@nongnu.org
 S: Maintained
 F: target/ppc/
@@ -273,6 +274,7 @@ F: hw/ppc/ppc.c
 F: hw/ppc/ppc_booke.c
 F: include/hw/ppc/ppc.h
 F: disas/ppc.c
+F: tests/tcg/ppc64/
 
 RISC-V TCG CPUs
 M: Palmer Dabbelt <palmer@dabbelt.com>
diff --git a/tests/tcg/ppc64/Makefile.softmmu-target b/tests/tcg/ppc64/Makefile.softmmu-target
new file mode 100644
index 0000000000..948427b70d
--- /dev/null
+++ b/tests/tcg/ppc64/Makefile.softmmu-target
@@ -0,0 +1,56 @@
+#
+# PowerPC64 system tests
+#
+
+# For now, disable tests that are failing
+DISABLED_TESTS := memory
+DISABLED_EXTRA_RUNS := run-gdbstub-memory
+
+PPC64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/ppc64/system
+VPATH+=$(PPC64_SYSTEM_SRC)
+
+# These objects provide the basic boot code and helper functions for all tests
+CRT_PATH=$(PPC64_SYSTEM_SRC)/lib
+CRT_OBJS=boot.o
+
+LINK_SCRIPT=$(CRT_PATH)/powerpc.lds
+# NOTE: --build-id is stored before the first code section in the linked
+#       binary, which causes problems for most tests, that expect to
+#       begin at address 0.
+LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none -static -nostdlib \
+    $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
+TESTS += $(filter-out $(DISABLED_TESTS),$(MULTIARCH_TESTS))
+EXTRA_RUNS += $(filter-out $(DISABLED_EXTRA_RUNS),$(MULTIARCH_RUNS))
+
+# NOTE: -Os doesn't work well with -Wl,--oformat=binary
+#       Some linker generated functions, such as savegpr*/restgpr*,
+#       end up being undefined.
+CFLAGS = -O -g -Wall -std=c99 -msoft-float -mno-vsx -mno-altivec \
+         -fno-stack-protector -ffreestanding \
+         -I $(PPC64_SYSTEM_SRC)/include $(MINILIB_INC) \
+         -mcpu=power8
+
+# Uncomment to test in LE
+# override EXTRA_CFLAGS += -mlittle-endian -mabi=elfv2
+
+# Leave the .elf files, to make debugging easier
+.PRECIOUS: $(CRT_OBJS) $(addsuffix .elf,$(TESTS))
+
+# Build CRT objects
+%.o: $(CRT_PATH)/%.S
+	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -x assembler-with-cpp -c $< -o $@
+
+# Build and link the tests
+
+# The .elf files are just for debugging
+%.elf: %.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
+	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
+
+%: %.c %.elf $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
+	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS) -Wl,--oformat=binary
+
+memory: CFLAGS+=-DCHECK_UNALIGNED=1
+
+# Running
+QEMU_BASE_MACHINE=-cpu power9 -M powernv9 -m 1G -vga none -nographic
+QEMU_OPTS+=$(QEMU_BASE_MACHINE) -serial chardev:output -bios
diff --git a/tests/tcg/ppc64/system/include/asm.h b/tests/tcg/ppc64/system/include/asm.h
new file mode 100644
index 0000000000..127ed46730
--- /dev/null
+++ b/tests/tcg/ppc64/system/include/asm.h
@@ -0,0 +1,68 @@
+#ifndef PPC64_ASM_H
+#define PPC64_ASM_H
+
+#define XCONCAT(a, b)       a ## b
+#define CONCAT(a, b)        XCONCAT(a, b)
+
+/* Load an immediate 64-bit value into a register */
+#define LOAD_IMM64(r, e)                        \
+    lis     r, (e)@highest;                     \
+    ori     r, r, (e)@higher;                   \
+    rldicr  r, r, 32, 31;                       \
+    oris    r, r, (e)@h;                        \
+    ori     r, r, (e)@l;
+
+/* Switch CPU to little-endian mode, if needed */
+#define FIXUP_ENDIAN \
+    tdi   0, 0, 0x48;   /* Reverse endian of b . + 8 */             \
+    b     $ + 44;       /* Skip trampoline if endian is good */     \
+    .long 0xa600607d;   /* mfmsr r11 */                             \
+    .long 0x01006b69;   /* xori r11,r11,1 */                        \
+    .long 0x00004039;   /* li r10,0 */                              \
+    .long 0x6401417d;   /* mtmsrd r10,1 */                          \
+    .long 0x05009f42;   /* bcl 20,31,$+4 */                         \
+    .long 0xa602487d;   /* mflr r10 */                              \
+    .long 0x14004a39;   /* addi r10,r10,20 */                       \
+    .long 0xa6035a7d;   /* mtsrr0 r10 */                            \
+    .long 0xa6037b7d;   /* mtsrr1 r11 */                            \
+    .long 0x2400004c    /* rfid */
+
+/* Handle differences between ELFv1 and ELFv2 ABIs */
+
+#define DOT_LABEL(name)     CONCAT(., name)
+
+#if !defined(_CALL_ELF) || _CALL_ELF == 1
+#define FUNCTION(name)                          \
+    .section ".opd", "aw";                      \
+    .p2align 3;                                 \
+    .globl   name;                              \
+name:                                           \
+    .quad   DOT_LABEL(name), .TOC.@tocbase, 0;  \
+    .previous;                                  \
+DOT_LABEL(name):
+
+#define CALL(fn)                                \
+    LOAD_IMM64(%r12, fn);                       \
+    ld      %r12, 0(%r12);                      \
+    mtctr   %r12;                               \
+    bctrl
+
+#define CALL_LOCAL(fn)                          \
+    bl      DOT_LABEL(fn)
+
+#else
+#define FUNCTION(name)                          \
+    .globl   name;                              \
+name:
+
+#define CALL(fn)                                \
+    LOAD_IMM64(%r12, fn);                       \
+    mtctr   %r12;                               \
+    bctrl
+
+#define CALL_LOCAL(fn)                          \
+    bl      fn
+
+#endif
+
+#endif
diff --git a/tests/tcg/ppc64/system/lib/boot.S b/tests/tcg/ppc64/system/lib/boot.S
new file mode 100644
index 0000000000..b3bcd8a7da
--- /dev/null
+++ b/tests/tcg/ppc64/system/lib/boot.S
@@ -0,0 +1,84 @@
+/* Copyright 2013-2014 IBM Corp.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+ * implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "asm.h"
+
+#define SPR_HID0                        0x3f0
+#define SPR_HID0_POWER9_HILE            0x0800000000000000
+
+#define ADP_STOPPED_APPLICATIONEXIT     0x20026
+
+    .section ".head","ax"
+
+    /* QEMU enters in BE at 0x10 by default */
+    . = 0x10
+.global start
+start:
+    FIXUP_ENDIAN
+
+    /* Setup TOC */
+    LOAD_IMM64(%r2, .TOC.)
+
+    /* Configure interrupt endian */
+#ifdef __LITTLE_ENDIAN__
+    mfspr   %r10, SPR_HID0
+    LOAD_IMM64(%r11, SPR_HID0_POWER9_HILE)
+    or      %r10, %r10, %r11
+    mtspr   SPR_HID0, %r10
+#endif
+
+    /* Clear .bss */
+    LOAD_IMM64(%r10, __bss_start)
+    LOAD_IMM64(%r11, __bss_end)
+    subf    %r11, %r10, %r11
+    addi    %r11, %r11, 63
+    srdi.   %r11, %r11, 6
+    beq     2f
+    mtctr   %r11
+1:  dcbz    0, %r10
+    addi    %r10, %r10, 64
+    bdnz    1b
+
+    /* Setup stack */
+2:  LOAD_IMM64(%r1, __stack_top)
+    li      %r0, 0
+    stdu    %r0, -32(%r1)
+
+    CALL(main)
+
+    /* Terminate on exit */
+    CALL_LOCAL(sys_exit)
+    b       .
+
+FUNCTION(sys_exit)
+    /*
+     * As semihosting operations are executed by non-translated QEMU code,
+     * we shouldn't need to save LR.
+     */
+    LOAD_IMM64(%r4, ADP_STOPPED_APPLICATIONEXIT)
+    std     %r4, -16(%r1)
+    std     %r3, -8(%r1)
+    li      %r3, 0x18
+    addi    %r4, %r1, -16
+    sc      7
+    blr
+
+FUNCTION(__sys_outc)
+    addi    %r4, %r1, -1
+    stb     %r3, 0(%r4)
+    li      %r3, 0x03
+    sc      7
+    blr
diff --git a/tests/tcg/ppc64/system/lib/powerpc.lds b/tests/tcg/ppc64/system/lib/powerpc.lds
new file mode 100644
index 0000000000..db451e1fb9
--- /dev/null
+++ b/tests/tcg/ppc64/system/lib/powerpc.lds
@@ -0,0 +1,27 @@
+SECTIONS
+{
+    . = 0;
+    _start = .;
+    .head : {
+        KEEP(*(.head))
+    }
+    . = ALIGN(0x1000);
+    .text : { *(.text) *(.text.*) *(.rodata) *(.rodata.*) }
+    . = ALIGN(0x1000);
+    .data : { *(.data) *(.data.*) *(.got) *(.toc) }
+    . = ALIGN(0x80);
+    __bss_start = .;
+    .bss : {
+        *(.dynsbss)
+        *(.sbss)
+        *(.scommon)
+        *(.dynbss)
+        *(.bss)
+        *(.common)
+        *(.bss.*)
+    }
+    . = ALIGN(0x80);
+    __bss_end = .;
+    . = . + 0x4000;
+    __stack_top = .;
+}
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v3 4/5] tests/tcg/ppc64: Add MMU test sources
  2022-04-18 19:10 [RFC PATCH v3 0/5] Port PPC64/PowerNV MMU tests to QEMU Leandro Lupori
                   ` (2 preceding siblings ...)
  2022-04-18 19:10 ` [RFC PATCH v3 3/5] tests/tcg/ppc64: Add basic softmmu test support Leandro Lupori
@ 2022-04-18 19:10 ` Leandro Lupori
  2022-04-18 19:11 ` [RFC PATCH v3 5/5] tests/tcg/ppc64: Build PowerNV and LE tests Leandro Lupori
  4 siblings, 0 replies; 35+ messages in thread
From: Leandro Lupori @ 2022-04-18 19:10 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: Leandro Lupori, danielhb413, richard.henderson, groug, clg,
	pbonzini, alex.bennee, david

Add MMU test sources, from https://github.com/legoater/pnv-test,
based on Microwatt tests but with some adaptations.

In particular, the tests that check updates to RC bits were
removed, because, apparently, Microwatt never updates RC bits, but
just raise an exception when they must be updated, leaving the
task to the OS
(https://github.com/antonblanchard/microwatt/blob/master/mmu.vhdl#L402).

Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
---
 tests/tcg/ppc64/system/mmu-head.S | 142 ++++++
 tests/tcg/ppc64/system/mmu.c      | 764 ++++++++++++++++++++++++++++++
 tests/tcg/ppc64/system/mmu.h      |   9 +
 3 files changed, 915 insertions(+)
 create mode 100644 tests/tcg/ppc64/system/mmu-head.S
 create mode 100644 tests/tcg/ppc64/system/mmu.c
 create mode 100644 tests/tcg/ppc64/system/mmu.h

diff --git a/tests/tcg/ppc64/system/mmu-head.S b/tests/tcg/ppc64/system/mmu-head.S
new file mode 100644
index 0000000000..e9e01f0642
--- /dev/null
+++ b/tests/tcg/ppc64/system/mmu-head.S
@@ -0,0 +1,142 @@
+/* Copyright 2013-2014 IBM Corp.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+ * implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "asm.h"
+
+#include "lib/boot.S"
+
+    /* Read a location with translation on */
+FUNCTION(test_read)
+    mfmsr   %r9
+    ori     %r8,%r9,0x10    /* set MSR_DR */
+    mtmsrd  %r8,0
+    mr      %r6,%r3
+    li      %r3,0
+    ld      %r5,0(%r6)
+    li      %r3,1
+    /* land here if DSI occurred */
+    mtmsrd  %r9,0
+    std     %r5,0(%r4)
+    blr
+
+    /* Write a location with translation on */
+FUNCTION(test_write)
+    mfmsr   %r9
+    ori     %r8,%r9,0x10    /* set MSR_DR */
+    mtmsrd  %r8,0
+    mr      %r6,%r3
+    li      %r3,0
+    std     %r4,0(%r6)
+    li      %r3,1
+    /* land here if DSI occurred */
+    mtmsrd  %r9,0
+    blr
+
+    /* Do a dcbz with translation on */
+FUNCTION(test_dcbz)
+    mfmsr   %r9
+    ori     %r8,%r9,0x10    /* set MSR_DR */
+    mtmsrd  %r8,0
+    mr      %r6,%r3
+    li      %r3,0
+    dcbz    0,%r6
+    li      %r3,1
+    /* land here if DSI occurred */
+    mtmsrd  %r9,0
+    blr
+
+FUNCTION(test_exec)
+    mtsrr0  %r4
+    mtsrr1  %r5
+    rfid
+
+#define EXCEPTION(nr)        \
+    .= nr                   ;\
+    li      %r3, (nr >> 4)  ;\
+    CALL_LOCAL(sys_exit)
+
+    /* DSI vector - skip the failing instruction + the next one */
+    . = 0x300
+    mtsprg0 %r10
+    mfsrr0  %r10
+    addi    %r10,%r10,8
+    mtsrr0  %r10
+    rfid
+
+    EXCEPTION(0x380)
+
+    /*
+     * ISI vector - jump to LR to return from the test,
+     * with r3 cleared
+     */
+    . = 0x400
+    li      %r3,0
+    blr
+
+    /* More exception stubs */
+    EXCEPTION(0x480)
+    EXCEPTION(0x500)
+    EXCEPTION(0x600)
+    EXCEPTION(0x700)
+    EXCEPTION(0x800)
+    EXCEPTION(0x900)
+    EXCEPTION(0x980)
+    EXCEPTION(0xa00)
+    EXCEPTION(0xb00)
+
+    /*
+     * System call - used to exit from tests where MSR[PR]
+     * may have been set.
+     */
+    . = 0xc00
+    blr
+
+    EXCEPTION(0xd00)
+    EXCEPTION(0xe00)
+    EXCEPTION(0xe20)
+    EXCEPTION(0xe40)
+    EXCEPTION(0xe60)
+    EXCEPTION(0xe80)
+    EXCEPTION(0xf00)
+    EXCEPTION(0xf20)
+    EXCEPTION(0xf40)
+    EXCEPTION(0xf60)
+    EXCEPTION(0xf80)
+
+    . = 0x1000
+    /*
+     * This page gets mapped at various locations and
+     * the tests try to execute from it.
+     * r3 contains the test number.
+     */
+FUNCTION(test_start)
+    nop
+    nop
+    cmpdi   %r3,1
+    beq     test_1
+    cmpdi   %r3,2
+    beq     test_2
+test_return:
+    li      %r3,1
+    sc
+
+    . = 0x1ff8
+    /* test a branch near the end of a page */
+test_1:     b   test_return
+
+    /* test flowing from one page to the next */
+test_2:     nop
+    b       test_return
diff --git a/tests/tcg/ppc64/system/mmu.c b/tests/tcg/ppc64/system/mmu.c
new file mode 100644
index 0000000000..8e9fca2675
--- /dev/null
+++ b/tests/tcg/ppc64/system/mmu.c
@@ -0,0 +1,764 @@
+#include <stddef.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+#include "minilib.h"
+#include "mmu.h"
+
+#define MSR_LE    0x01
+#define MSR_DR    0x10
+#define MSR_IR    0x20
+#define MSR_HV    0x1000000000000000ul
+#define MSR_SF    0x8000000000000000ul
+
+#ifdef __LITTLE_ENDIAN__
+#define MSR_DFLT    (MSR_SF | MSR_HV | MSR_LE)
+#else
+#define MSR_DFLT    (MSR_SF | MSR_HV)
+#endif
+
+#define XSTR(x)     #x
+#define STR(x)      XSTR(x)
+
+#define RIC_TLB     0
+#define RIC_PWC     1
+#define RIC_ALL     2
+
+#define PRS         1
+
+#define IS(x)       ((unsigned long)(x) << 10)
+#define IS_VA       IS(0)
+#define IS_PID      IS(1)
+#define IS_LPID     IS(2)
+#define IS_ALL      IS(3)
+
+#define TLBIE_5(rb, rs, ric, prs, r)                \
+    __asm__ volatile(".long 0x7c000264 | "          \
+        "%0 << 21 | "                               \
+        STR(ric) " << 18 | "                        \
+        STR(prs) " << 17 | "                        \
+        STR(r) "<< 16 | "                           \
+        "%1 << 11"                                  \
+        : : "r" (rs), "r" (rb) : "memory")
+
+static inline void tlbie_all(int prs)
+{
+    if (prs) {
+        TLBIE_5(IS_ALL, 0, RIC_ALL, 1, 1);
+    } else {
+        TLBIE_5(IS_ALL, 0, RIC_ALL, 0, 1);
+    }
+}
+
+static inline void tlbie_va(unsigned long va, int prs)
+{
+    va &= ~0xffful;
+
+    if (prs) {
+        TLBIE_5(IS_VA | va, 0, RIC_TLB, 1, 1);
+    } else {
+        TLBIE_5(IS_VA | va, 0, RIC_TLB, 0, 1);
+    }
+    __asm__ volatile("eieio; tlbsync; ptesync" : : : "memory");
+}
+
+#define DSISR       18
+#define DAR         19
+#define SRR0        26
+#define SRR1        27
+#define PID         48
+#define LPCR        318
+#define PTCR        464
+
+#define PPC_BIT(x)  (0x8000000000000000ul >> (x))
+
+#define LPCR_UPRT   PPC_BIT(41)
+#define LPCR_HR     PPC_BIT(43)
+
+#define PATE_HR     PPC_BIT(0)
+
+static inline unsigned long mfspr(int sprnum)
+{
+    long val;
+
+    __asm__ volatile("mfspr %0,%1" : "=r" (val) : "i" (sprnum));
+    return val;
+}
+
+static inline void mtspr(int sprnum, unsigned long val)
+{
+    __asm__ volatile("mtspr %0,%1" : : "i" (sprnum), "r" (val));
+}
+
+static inline void store_pte(unsigned long *p, unsigned long pte)
+{
+#ifdef __LITTLE_ENDIAN__
+    __asm__ volatile("stdbrx %1,0,%0" : : "r" (p), "r" (pte) : "memory");
+#else
+    __asm__ volatile("stdx   %1,0,%0" : : "r" (p), "r" (pte) : "memory");
+#endif
+    __asm__ volatile("ptesync" : : : "memory");
+}
+
+#define CACHE_LINE_SIZE    64
+
+void zero_memory(void *ptr, unsigned long nbytes)
+{
+    unsigned long nb, i, nl;
+    void *p;
+
+    for (; nbytes != 0; nbytes -= nb, ptr += nb) {
+        nb = -((unsigned long)ptr) & (CACHE_LINE_SIZE - 1);
+        if (nb == 0 && nbytes >= CACHE_LINE_SIZE) {
+            nl = nbytes / CACHE_LINE_SIZE;
+            p = ptr;
+            for (i = 0; i < nl; ++i) {
+                __asm__ volatile("dcbz 0,%0" : : "r" (p) : "memory");
+                p += CACHE_LINE_SIZE;
+            }
+            nb = nl * CACHE_LINE_SIZE;
+        } else {
+            if (nb > nbytes) {
+                nb = nbytes;
+            }
+            for (i = 0; i < nb; ++i) {
+                ((unsigned char *)ptr)[i] = 0;
+            }
+        }
+    }
+}
+
+#define PAGE_SHIFT      12
+#define PAGE_SIZE       (1ul << PAGE_SHIFT)
+
+/* Partition Page Dir params */
+#define PPD_L1_BITS     5
+#define PPD_L2_BITS     14    /* virtual level 2 PGD address bits */
+#define PPD_PA_INC      (1ul << (PAGE_SHIFT + PPD_L2_BITS))
+
+#define RPTE_V          PPC_BIT(0)
+#define RPTE_L          PPC_BIT(1)
+#define RPTE_RPN_MASK   0x01fffffffffff000ul
+#define RPTE_R          PPC_BIT(55)
+#define RPTE_C          PPC_BIT(56)
+#define RPTE_PRIV       PPC_BIT(60)
+#define RPTE_RD         PPC_BIT(61)
+#define RPTE_RW         PPC_BIT(62)
+#define RPTE_EX         PPC_BIT(63)
+#define RPTE_PERM_ALL   (RPTE_RD | RPTE_RW | RPTE_EX)
+
+#define PERM_EX         RPTE_EX
+#define PERM_WR         RPTE_RW
+#define PERM_RD         RPTE_RD
+#define PERM_PRIV       RPTE_PRIV
+#define ATTR_NC         0x020
+#define CHG             RPTE_C
+#define REF             RPTE_R
+
+#define DFLT_PERM       (PERM_WR | PERM_RD | REF | CHG)
+
+/*
+ * Set up an MMU translation tree using memory starting at the 64k point.
+ * We use 2 levels, mapping 2GB (the minimum size possible), with a
+ * 8kB PGD level pointing to 4kB PTE pages.
+ */
+unsigned long *pgdir = (unsigned long *) 0x10000;
+unsigned long *proc_tbl = (unsigned long *) 0x12000;
+unsigned long *part_tbl = (unsigned long *) 0x13000;
+unsigned long *part_pgdir = (unsigned long *) 0x14000;
+unsigned long free_ptr = 0x15000;
+void *eas_mapped[4];
+int neas_mapped;
+
+void init_mmu(void)
+{
+    int i, n;
+    unsigned long pa, pte;
+
+    /* Select Radix MMU (HR), with HW process table */
+    mtspr(LPCR, mfspr(LPCR) | LPCR_UPRT | LPCR_HR);
+
+    /*
+     * Set up partition page dir, needed to translate process table
+     * addresses.
+     * We use only 1 level, mapping 2GB 1-1, with 32 64M pages.
+     */
+    zero_memory(part_tbl, PAGE_SIZE);
+    store_pte(&part_tbl[0], PATE_HR | (unsigned long) part_pgdir |
+            PPD_L1_BITS);
+
+    for (i = 0, n = 1 << PPD_L1_BITS, pa = 0;
+            i < n; i++, pa += PPD_PA_INC) {
+        pte = RPTE_V | RPTE_L | (pa & RPTE_RPN_MASK) | RPTE_PERM_ALL;
+        store_pte(&part_pgdir[i], pte);
+    }
+
+    /* set up partition table */
+    store_pte(&part_tbl[1], (unsigned long)proc_tbl);
+    /* set up process table */
+    zero_memory(proc_tbl, 512 * sizeof(unsigned long));
+    mtspr(PTCR, (unsigned long)part_tbl);
+    mtspr(PID, 1);
+    zero_memory(pgdir, 1024 * sizeof(unsigned long));
+    /* RTS = 0 (2GB address space), RPDS = 10 (1024-entry top level) */
+    store_pte(&proc_tbl[2 * 1], (unsigned long) pgdir | 10);
+    tlbie_all(0);   /* invalidate all TLB entries */
+}
+
+static unsigned long *read_pgd(unsigned long i)
+{
+    unsigned long ret;
+
+#ifdef __LITTLE_ENDIAN__
+    __asm__ volatile("ldbrx %0,%1,%2" : "=r" (ret) : "b" (pgdir),
+                     "r" (i * sizeof(unsigned long)));
+#else
+    __asm__ volatile("ldx   %0,%1,%2" : "=r" (ret) : "b" (pgdir),
+                     "r" (i * sizeof(unsigned long)));
+#endif
+    return (unsigned long *) (ret & 0x00ffffffffffff00);
+}
+
+void map(void *ea, void *pa, unsigned long perm_attr)
+{
+    unsigned long epn = (unsigned long) ea >> 12;
+    unsigned long i, j;
+    unsigned long *ptep;
+
+    i = (epn >> 9) & 0x3ff;
+    j = epn & 0x1ff;
+    if (pgdir[i] == 0) {
+        zero_memory((void *)free_ptr, 512 * sizeof(unsigned long));
+        store_pte(&pgdir[i], 0x8000000000000000 | free_ptr | 9);
+        free_ptr += 512 * sizeof(unsigned long);
+    }
+    ptep = read_pgd(i);
+    store_pte(&ptep[j], 0xc000000000000000 | ((unsigned long)pa &
+                                              0x00fffffffffff000) | perm_attr);
+    eas_mapped[neas_mapped++] = ea;
+}
+
+void unmap(void *ea)
+{
+    unsigned long epn = (unsigned long) ea >> 12;
+    unsigned long i, j;
+    unsigned long *ptep;
+
+    i = (epn >> 9) & 0x3ff;
+    j = epn & 0x1ff;
+    if (pgdir[i] == 0) {
+        return;
+    }
+    ptep = read_pgd(i);
+    store_pte(&ptep[j], 0);
+    tlbie_va((unsigned long)ea, PRS);
+}
+
+void unmap_all(void)
+{
+    int i;
+
+    for (i = 0; i < neas_mapped; ++i) {
+        unmap(eas_mapped[i]);
+    }
+    neas_mapped = 0;
+}
+
+int mmu_test_1(void)
+{
+    long *ptr = (long *) 0x123000;
+    long val;
+
+    /* this should fail */
+    if (test_read(ptr, &val, 0xdeadbeefd00d)) {
+        return 1;
+    }
+    /* dest reg of load should be unchanged */
+    if (val != 0xdeadbeefd00d) {
+        return 2;
+    }
+    /* DAR and DSISR should be set correctly */
+    if (mfspr(DAR) != (long) ptr || mfspr(DSISR) != 0x40000000) {
+        return 3;
+    }
+    return 0;
+}
+
+int mmu_test_2(void)
+{
+    long *mem = (long *) 0x8000;
+    long *ptr = (long *) 0x124000;
+    long *ptr2 = (long *) 0x1124000;
+    long val;
+
+    /* create PTE */
+    map(ptr, mem, DFLT_PERM);
+    /* initialize the memory content */
+    mem[33] = 0xbadc0ffee;
+    /* this should succeed and be a cache miss */
+    if (!test_read(&ptr[33], &val, 0xdeadbeefd00d)) {
+        return 1;
+    }
+    /* dest reg of load should have the value written */
+    if (val != 0xbadc0ffee) {
+        return 2;
+    }
+    /* load a second TLB entry in the same set as the first */
+    map(ptr2, mem, DFLT_PERM);
+    /* this should succeed and be a cache hit */
+    if (!test_read(&ptr2[33], &val, 0xdeadbeefd00d)) {
+        return 3;
+    }
+    /* dest reg of load should have the value written */
+    if (val != 0xbadc0ffee) {
+        return 4;
+    }
+    /* check that the first entry still works */
+    if (!test_read(&ptr[33], &val, 0xdeadbeefd00d)) {
+        return 5;
+    }
+    if (val != 0xbadc0ffee) {
+        return 6;
+    }
+    return 0;
+}
+
+int mmu_test_3(void)
+{
+    long *mem = (long *) 0x9000;
+    long *ptr = (long *) 0x14a000;
+    long val;
+
+    /* create PTE */
+    map(ptr, mem, DFLT_PERM);
+    /* initialize the memory content */
+    mem[45] = 0xfee1800d4ea;
+    /* this should succeed and be a cache miss */
+    if (!test_read(&ptr[45], &val, 0xdeadbeefd0d0)) {
+        return 1;
+    }
+    /* dest reg of load should have the value written */
+    if (val != 0xfee1800d4ea) {
+        return 2;
+    }
+    /* remove the PTE */
+    unmap(ptr);
+    /* this should fail */
+    if (test_read(&ptr[45], &val, 0xdeadbeefd0d0)) {
+        return 3;
+    }
+    /* dest reg of load should be unchanged */
+    if (val != 0xdeadbeefd0d0) {
+        return 4;
+    }
+    /* DAR and DSISR should be set correctly */
+    if (mfspr(DAR) != (long) &ptr[45] || mfspr(DSISR) != 0x40000000) {
+        return 5;
+    }
+    return 0;
+}
+
+int mmu_test_4(void)
+{
+    long *mem = (long *) 0xa000;
+    long *ptr = (long *) 0x10b000;
+    long *ptr2 = (long *) 0x110b000;
+    long val;
+
+    /* create PTE */
+    map(ptr, mem, DFLT_PERM);
+    /* initialize the memory content */
+    mem[27] = 0xf00f00f00f00;
+    /* this should succeed and be a cache miss */
+    if (!test_write(&ptr[27], 0xe44badc0ffee)) {
+        return 1;
+    }
+    /* memory should now have the value written */
+    if (mem[27] != 0xe44badc0ffee) {
+        return 2;
+    }
+    /* load a second TLB entry in the same set as the first */
+    map(ptr2, mem, DFLT_PERM);
+    /* this should succeed and be a cache hit */
+    if (!test_write(&ptr2[27], 0x6e11ae)) {
+        return 3;
+    }
+    /* memory should have the value written */
+    if (mem[27] != 0x6e11ae) {
+        return 4;
+    }
+    /* check that the first entry still exists */
+    /* (assumes TLB is 2-way associative or more) */
+    if (!test_read(&ptr[27], &val, 0xdeadbeefd00d)) {
+        return 5;
+    }
+    if (val != 0x6e11ae) {
+        return 6;
+    }
+    return 0;
+}
+
+int mmu_test_5(void)
+{
+    long *mem = (long *) 0xbffd;
+    long *ptr = (long *) 0x39fffd;
+    long val;
+
+    /* create PTE */
+    map(ptr, mem, DFLT_PERM);
+    /* this should fail */
+    if (test_read(ptr, &val, 0xdeadbeef0dd0)) {
+        return 1;
+    }
+    /* dest reg of load should be unchanged */
+    if (val != 0xdeadbeef0dd0) {
+        return 2;
+    }
+    /* DAR and DSISR should be set correctly */
+    if (mfspr(DAR) != ((long)ptr & ~0xfff) + 0x1000 ||
+            mfspr(DSISR) != 0x40000000) {
+        return 3;
+    }
+    return 0;
+}
+
+int mmu_test_6(void)
+{
+    long *mem = (long *) 0xbffd;
+    long *ptr = (long *) 0x39fffd;
+
+    /* create PTE */
+    map(ptr, mem, DFLT_PERM);
+    /* initialize memory */
+    *mem = 0x123456789abcdef0;
+    /* this should fail */
+    if (test_write(ptr, 0xdeadbeef0dd0)) {
+        return 1;
+    }
+    /* DAR and DSISR should be set correctly */
+    if (mfspr(DAR) != ((long)ptr & ~0xfff) + 0x1000 ||
+            mfspr(DSISR) != 0x42000000) {
+        return 2;
+    }
+    return 0;
+}
+
+int mmu_test_7(void)
+{
+    long *mem = (long *) 0x8000;
+    long *ptr = (long *) 0x124000;
+    long val;
+
+    *mem = 0x123456789abcdef0;
+    /* create PTE without read or write permission */
+    map(ptr, mem, REF);
+    /* this should fail */
+    if (test_read(ptr, &val, 0xdeadd00dbeef)) {
+        return 1;
+    }
+    /* dest reg of load should be unchanged */
+    if (val != 0xdeadd00dbeef) {
+        return 2;
+    }
+    /* DAR and DSISR should be set correctly */
+    if (mfspr(DAR) != (long) ptr || mfspr(DSISR) != 0x08000000) {
+        return 3;
+    }
+    /* this should fail */
+    if (test_write(ptr, 0xdeadbeef0dd1)) {
+        return 4;
+    }
+    /* DAR and DSISR should be set correctly */
+    if (mfspr(DAR) != (long)ptr || mfspr(DSISR) != 0x0a000000) {
+        return 5;
+    }
+    /* memory should be unchanged */
+    if (*mem != 0x123456789abcdef0) {
+        return 6;
+    }
+    return 0;
+}
+
+int mmu_test_8(void)
+{
+    long *mem = (long *) 0x8000;
+    long *ptr = (long *) 0x124000;
+    long val;
+
+    *mem = 0x123456789abcdef0;
+    /* create PTE with read but not write permission */
+    map(ptr, mem, REF | PERM_RD);
+    /* this should succeed */
+    if (!test_read(ptr, &val, 0xdeadd00dbeef)) {
+        return 1;
+    }
+    /* this should fail */
+    if (test_write(ptr, 0xdeadbeef0dd1)) {
+        return 2;
+    }
+    /* DAR and DSISR should be set correctly */
+    if (mfspr(DAR) != (long)ptr || mfspr(DSISR) != 0x0a000000) {
+        return 3;
+    }
+    /* memory should be unchanged */
+    if (*mem != 0x123456789abcdef0) {
+        return 4;
+    }
+    return 0;
+}
+
+int mmu_test_9(void)
+{
+    unsigned long ptr = 0x523000;
+
+    /* this should fail */
+    if (test_exec(0, ptr, MSR_DFLT | MSR_IR)) {
+        return 1;
+    }
+    /* SRR0 and SRR1 should be set correctly */
+    if (mfspr(SRR0) != (long) ptr ||
+            mfspr(SRR1) != (MSR_DFLT | 0x40000000 | MSR_IR)) {
+        return 2;
+    }
+    return 0;
+}
+
+int mmu_test_10(void)
+{
+    unsigned long mem = 0x1000;
+    unsigned long ptr = 0x324000;
+    unsigned long ptr2 = 0x1324000;
+
+    /* create PTE */
+    map((void *)ptr, (void *)mem, PERM_EX | REF);
+    /* this should succeed and be a cache miss */
+    if (!test_exec(0, ptr, MSR_DFLT | MSR_IR)) {
+        return 1;
+    }
+    /* create a second PTE */
+    map((void *)ptr2, (void *)mem, PERM_EX | REF);
+    /* this should succeed and be a cache hit */
+    if (!test_exec(0, ptr2, MSR_DFLT | MSR_IR)) {
+        return 2;
+    }
+    return 0;
+}
+
+int mmu_test_11(void)
+{
+    unsigned long mem = 0x1000;
+    unsigned long ptr = 0x349000;
+    unsigned long ptr2 = 0x34a000;
+
+    /* create a PTE */
+    map((void *)ptr, (void *)mem, PERM_EX | REF);
+    /* this should succeed */
+    if (!test_exec(1, ptr, MSR_DFLT | MSR_IR)) {
+        return 1;
+    }
+    /* invalidate the PTE */
+    unmap((void *)ptr);
+    /* install a second PTE */
+    map((void *)ptr2, (void *)mem, PERM_EX | REF);
+    /* this should fail */
+    if (test_exec(1, ptr, MSR_DFLT | MSR_IR)) {
+        return 2;
+    }
+    /* SRR0 and SRR1 should be set correctly */
+    if (mfspr(SRR0) != (long) ptr ||
+            mfspr(SRR1) != (MSR_DFLT | 0x40000000 | MSR_IR)) {
+        return 3;
+    }
+    return 0;
+}
+
+int mmu_test_12(void)
+{
+    unsigned long mem = 0x1000;
+    unsigned long mem2 = 0x2000;
+    unsigned long ptr = 0x30a000;
+    unsigned long ptr2 = 0x30b000;
+
+    /* create a PTE */
+    map((void *)ptr, (void *)mem, PERM_EX | REF);
+    /* this should fail due to second page not being mapped */
+    if (test_exec(2, ptr, MSR_DFLT | MSR_IR)) {
+        return 1;
+    }
+    /* SRR0 and SRR1 should be set correctly */
+    if (mfspr(SRR0) != ptr2 ||
+            mfspr(SRR1) != (MSR_DFLT | 0x40000000 | MSR_IR)) {
+        return 2;
+    }
+    /* create a PTE for the second page */
+    map((void *)ptr2, (void *)mem2, PERM_EX | REF);
+    /* this should succeed */
+    if (!test_exec(2, ptr, MSR_DFLT | MSR_IR)) {
+        return 3;
+    }
+    return 0;
+}
+
+int mmu_test_13(void)
+{
+    unsigned long mem = 0x1000;
+    unsigned long ptr = 0x324000;
+
+    /* create a PTE without execute permission */
+    map((void *)ptr, (void *)mem, DFLT_PERM);
+    /* this should fail */
+    if (test_exec(0, ptr, MSR_DFLT | MSR_IR)) {
+        return 1;
+    }
+    /* SRR0 and SRR1 should be set correctly */
+    if (mfspr(SRR0) != ptr ||
+            mfspr(SRR1) != (MSR_DFLT | 0x10000000 | MSR_IR)) {
+        return 2;
+    }
+    return 0;
+}
+
+int mmu_test_14(void)
+{
+    unsigned long mem = 0x1000;
+    unsigned long mem2 = 0x2000;
+    unsigned long ptr = 0x30a000;
+    unsigned long ptr2 = 0x30b000;
+
+    /* create a PTE */
+    map((void *)ptr, (void *)mem, PERM_EX | REF);
+    /* create a PTE for the second page without execute permission */
+    map((void *)ptr2, (void *)mem2, PERM_RD | REF);
+    /* this should fail due to second page being no-execute */
+    if (test_exec(2, ptr, MSR_DFLT | MSR_IR)) {
+        return 1;
+    }
+    /* SRR0 and SRR1 should be set correctly */
+    if (mfspr(SRR0) != ptr2 ||
+            mfspr(SRR1) != (MSR_DFLT | 0x10000000 | MSR_IR)) {
+        return 2;
+    }
+    /* create a PTE for the second page with execute permission */
+    map((void *)ptr2, (void *)mem2, PERM_RD | PERM_EX | REF);
+    /* this should succeed */
+    if (!test_exec(2, ptr, MSR_DFLT | MSR_IR)) {
+        return 3;
+    }
+    return 0;
+}
+
+int mmu_test_15(void)
+{
+    unsigned long mem = 0x1000;
+    unsigned long ptr = 0x349000;
+
+    /* create a PTE without ref or execute permission */
+    map((void *)ptr, (void *)mem, 0);
+    /* this should fail */
+    if (test_exec(2, ptr, MSR_DFLT | MSR_IR)) {
+        return 1;
+    }
+    /* SRR0 and SRR1 should be set correctly */
+    /* RC update fail bit should not be set */
+    if (mfspr(SRR0) != (long) ptr ||
+            mfspr(SRR1) != (MSR_DFLT | 0x10000000 | MSR_IR)) {
+        return 2;
+    }
+    return 0;
+}
+
+int mmu_test_16(void)
+{
+    long *mem = (long *) 0x8000;
+    long *ptr = (long *) 0x124000;
+    long *ptr2 = (long *) 0x1124000;
+
+    /* create PTE */
+    map(ptr, mem, DFLT_PERM);
+    /* this should succeed and be a cache miss */
+    if (!test_dcbz(&ptr[129])) {
+        return 1;
+    }
+    /* create a second PTE */
+    map(ptr2, mem, DFLT_PERM);
+    /* this should succeed and be a cache hit */
+    if (!test_dcbz(&ptr2[130])) {
+        return 2;
+    }
+    return 0;
+}
+
+int mmu_test_17(void)
+{
+    long *mem = (long *) 0x8000;
+    long *ptr = (long *) 0x124000;
+
+    *mem = 0x123456789abcdef0;
+    /* create PTE with read but not write permission */
+    map(ptr, mem, REF | PERM_RD);
+    /* this should fail and create a TLB entry */
+    if (test_write(ptr, 0xdeadbeef0dd1)) {
+        return 1;
+    }
+    /* DAR and DSISR should be set correctly */
+    if (mfspr(DAR) != (long)ptr || mfspr(DSISR) != 0x0a000000) {
+        return 2;
+    }
+    /* Update the PTE to have write permission */
+    map(ptr, mem, REF | CHG | PERM_RD | PERM_WR);
+    /* this should succeed */
+    if (!test_write(ptr, 0xdeadbeef0dd1)) {
+        return 3;
+    }
+    return 0;
+}
+
+int fail;
+
+void do_test(int num, int (*test)(void))
+{
+    int ret;
+
+    mtspr(DSISR, 0);
+    mtspr(DAR, 0);
+    unmap_all();
+    ml_printf("test %d:", num);
+    ret = test();
+    if (ret == 0) {
+        ml_printf("PASS\r\n");
+    } else {
+        fail = 1;
+        ml_printf("FAIL %d", ret);
+        if (num <= 10 || num == 19) {
+            ml_printf(" DAR=%lx DSISR=%lx", mfspr(DAR), mfspr(DSISR));
+        } else {
+            ml_printf(" SRR0=%lx SRR1=%lx", mfspr(SRR0), mfspr(SRR1));
+        }
+        ml_printf("\r\n");
+    }
+}
+
+int main(void)
+{
+    init_mmu();
+
+    do_test(1, mmu_test_1);
+    do_test(2, mmu_test_2);
+    do_test(3, mmu_test_3);
+    do_test(4, mmu_test_4);
+    do_test(5, mmu_test_5);
+    do_test(6, mmu_test_6);
+    do_test(7, mmu_test_7);
+    do_test(8, mmu_test_8);
+    do_test(9, mmu_test_9);
+    do_test(10, mmu_test_10);
+    do_test(11, mmu_test_11);
+    do_test(12, mmu_test_12);
+    do_test(13, mmu_test_13);
+    do_test(14, mmu_test_14);
+    do_test(15, mmu_test_15);
+    do_test(16, mmu_test_16);
+    do_test(17, mmu_test_17);
+
+    return fail;
+}
diff --git a/tests/tcg/ppc64/system/mmu.h b/tests/tcg/ppc64/system/mmu.h
new file mode 100644
index 0000000000..eb191e4bd0
--- /dev/null
+++ b/tests/tcg/ppc64/system/mmu.h
@@ -0,0 +1,9 @@
+#ifndef PPC64_MMU_H
+#define PPC64_MMU_H
+
+int test_read(long *addr, long *ret, long init);
+int test_write(long *addr, long val);
+int test_dcbz(long *addr);
+int test_exec(int testno, unsigned long pc, unsigned long msr);
+
+#endif
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [RFC PATCH v3 5/5] tests/tcg/ppc64: Build PowerNV and LE tests
  2022-04-18 19:10 [RFC PATCH v3 0/5] Port PPC64/PowerNV MMU tests to QEMU Leandro Lupori
                   ` (3 preceding siblings ...)
  2022-04-18 19:10 ` [RFC PATCH v3 4/5] tests/tcg/ppc64: Add MMU test sources Leandro Lupori
@ 2022-04-18 19:11 ` Leandro Lupori
  4 siblings, 0 replies; 35+ messages in thread
From: Leandro Lupori @ 2022-04-18 19:11 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: Leandro Lupori, danielhb413, richard.henderson, groug, clg,
	pbonzini, alex.bennee, david

Each Microwatt/PowerNV test use its own head.S file and thus needs
different build rules.

Also add rules to build and run all tests in LE mode.

Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
---
 tests/tcg/ppc64/Makefile.softmmu-rules  |  34 +++++++
 tests/tcg/ppc64/Makefile.softmmu-target | 121 +++++++++++++++++++-----
 2 files changed, 129 insertions(+), 26 deletions(-)
 create mode 100644 tests/tcg/ppc64/Makefile.softmmu-rules

diff --git a/tests/tcg/ppc64/Makefile.softmmu-rules b/tests/tcg/ppc64/Makefile.softmmu-rules
new file mode 100644
index 0000000000..abe0de0a7f
--- /dev/null
+++ b/tests/tcg/ppc64/Makefile.softmmu-rules
@@ -0,0 +1,34 @@
+#
+# Rules to build PowerPC64 softmmu tests, for both BE and LE
+#
+
+# Build CRT and test objects
+%$(LE_SUFFIX).o: $(CRT_PATH)/%.S
+	$(CC) $(PPC64_CFLAGS) -x assembler-with-cpp -c $< -o $@
+
+%$(LE_SUFFIX).o: %.S
+	$(CC) $(PPC64_CFLAGS) -x assembler-with-cpp -c $< -o $@
+
+%$(LE_SUFFIX).o: $(CRT_PATH)/%.c
+	$(CC) $(PPC64_CFLAGS) -c $< -o $@
+
+%$(LE_SUFFIX).o: %.c
+	$(CC) $(PPC64_CFLAGS) -c $< -o $@
+
+# Build .elf files for debugging
+%$(LE_SUFFIX).elf: %$(LE_SUFFIX).o $(LINK_SCRIPT) $(CRT_DEPS) $(MINILIB_DEPS)
+	$(CC) $(PPC64_CFLAGS) -o $@ $< $(LDFLAGS)
+
+$(PPC64_PNV_ELFS): %$(LE_SUFFIX).elf: %-head$(LE_SUFFIX).o %$(LE_SUFFIX).o \
+                    $(LINK_SCRIPT) $(CRT_DEPS) $(MINILIB_DEPS)
+	$(CC) $(PPC64_CFLAGS) -o $@ $< $*$(LE_SUFFIX).o $(LDFLAGS)
+
+# Build test binaries
+%$(LE_SUFFIX): %$(LE_SUFFIX).o $(LINK_SCRIPT) $(CRT_DEPS) $(MINILIB_DEPS) \
+               %$(LE_SUFFIX).elf
+	$(CC) $(PPC64_CFLAGS) -o $@ $< $(LDFLAGS) -Wl,--oformat=binary
+
+$(PPC64_PNV_TESTS): %$(LE_SUFFIX): %-head$(LE_SUFFIX).o %$(LE_SUFFIX).o \
+                    $(LINK_SCRIPT) $(CRT_DEPS) $(MINILIB_DEPS) %$(LE_SUFFIX).elf
+	$(CC) $(PPC64_CFLAGS) -o $@ $< $*$(LE_SUFFIX).o $(LDFLAGS) \
+				-Wl,--oformat=binary
diff --git a/tests/tcg/ppc64/Makefile.softmmu-target b/tests/tcg/ppc64/Makefile.softmmu-target
index 948427b70d..cf89d2f950 100644
--- a/tests/tcg/ppc64/Makefile.softmmu-target
+++ b/tests/tcg/ppc64/Makefile.softmmu-target
@@ -5,22 +5,52 @@
 # For now, disable tests that are failing
 DISABLED_TESTS := memory
 DISABLED_EXTRA_RUNS := run-gdbstub-memory
+# Disable LE tests too
+DISABLED_TESTS += $(addsuffix -le, $(DISABLED_TESTS))
+DISABLED_EXTRA_RUNS += $(addsuffix -le, $(DISABLED_EXTRA_RUNS))
 
-PPC64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/ppc64/system
-VPATH+=$(PPC64_SYSTEM_SRC)
+PPC64_SRC := $(SRC_PATH)/tests/tcg/ppc64
+PPC64_SYSTEM_SRC := $(PPC64_SRC)/system
+VPATH += $(PPC64_SYSTEM_SRC)
 
 # These objects provide the basic boot code and helper functions for all tests
-CRT_PATH=$(PPC64_SYSTEM_SRC)/lib
-CRT_OBJS=boot.o
+CRT_PATH := $(PPC64_SYSTEM_SRC)/lib
+CRT_OBJS_BE := boot.o
+CRT_OBJS_LE := boot-le.o
+# NOTE: %-head.o replaces boot.o on PowerNV tests
+PNV_CRT_OBJS_BE := $(filter-out boot.o, $(CRT_OBJS_BE))
+PNV_CRT_OBJS_LE := $(filter-out boot-le.o, $(CRT_OBJS_LE))
 
-LINK_SCRIPT=$(CRT_PATH)/powerpc.lds
-# NOTE: --build-id is stored before the first code section in the linked
-#       binary, which causes problems for most tests, that expect to
-#       begin at address 0.
-LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none -static -nostdlib \
-    $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
-TESTS += $(filter-out $(DISABLED_TESTS),$(MULTIARCH_TESTS))
-EXTRA_RUNS += $(filter-out $(DISABLED_EXTRA_RUNS),$(MULTIARCH_RUNS))
+MINILIB_OBJS_BE := $(MINILIB_OBJS)
+MINILIB_OBJS_LE := $(patsubst %.o, %-le.o, $(MINILIB_OBJS))
+
+# Add BE and LE tests
+
+# Each Microwatt/PowerNV test use its own head.S file and thus needs
+# different rules.
+PPC64BE_PNV_TESTS := mmu
+PPC64BE_PNV_ELFS := $(addsuffix .elf, $(PPC64BE_PNV_TESTS))
+PPC64LE_PNV_TESTS := $(addsuffix -le, $(PPC64BE_PNV_TESTS))
+PPC64LE_PNV_ELFS := $(addsuffix .elf, $(PPC64LE_PNV_TESTS))
+
+# Remaining test sources are assumed to be non-PowerNV tests
+PPC64_TEST_SRCS := $(wildcard $(PPC64_SYSTEM_SRC)/*.c)
+PPC64BE_TESTS := $(MULTIARCH_TESTS)
+PPC64BE_TESTS += $(filter-out $(PPC64BE_PNV_TESTS),\
+                 $(patsubst $(PPC64_SYSTEM_SRC)/%.c, %, $(PPC64_TEST_SRCS)))
+PPC64BE_ELFS := $(addsuffix .elf,$(PPC64BE_TESTS))
+PPC64LE_TESTS := $(addsuffix -le, $(PPC64BE_TESTS))
+PPC64LE_ELFS := $(addsuffix .elf,$(PPC64LE_TESTS))
+
+TESTS += $(filter-out $(DISABLED_TESTS), $(PPC64BE_TESTS) $(PPC64LE_TESTS))
+TESTS += $(PPC64BE_PNV_TESTS) $(PPC64LE_PNV_TESTS)
+
+MULTIARCH_RUNS_BE := $(MULTIARCH_RUNS)
+MULTIARCH_RUNS_LE := $(addsuffix -le, $(MULTIARCH_RUNS))
+EXTRA_RUNS += $(filter-out $(DISABLED_EXTRA_RUNS), \
+                           $(MULTIARCH_RUNS_BE) $(MULTIARCH_RUNS_LE))
+
+LINK_SCRIPT := $(CRT_PATH)/powerpc.lds
 
 # NOTE: -Os doesn't work well with -Wl,--oformat=binary
 #       Some linker generated functions, such as savegpr*/restgpr*,
@@ -30,27 +60,66 @@ CFLAGS = -O -g -Wall -std=c99 -msoft-float -mno-vsx -mno-altivec \
          -I $(PPC64_SYSTEM_SRC)/include $(MINILIB_INC) \
          -mcpu=power8
 
-# Uncomment to test in LE
-# override EXTRA_CFLAGS += -mlittle-endian -mabi=elfv2
+# NOTE: --build-id is stored before the first code section in the linked
+#       binary, which causes problems for most tests, that expect to
+#       begin at address 0.
+LDFLAGS = -Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none -static -nostdlib \
+    $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
+
+memory memory-le: CFLAGS+=-DCHECK_UNALIGNED=1
+
+# PowerNV tests build outputs
+PPC64BE_PNV_OUTPUTS := $(PPC64BE_PNV_TESTS) $(PPC64BE_PNV_ELFS)
+PPC64LE_PNV_OUTPUTS := $(PPC64LE_PNV_TESTS) $(PPC64LE_PNV_ELFS)
+# Non-PowerNV tests build outputs
+PPC64BE_OUTPUTS := $(PPC64BE_TESTS) $(PPC64BE_ELFS)
+PPC64LE_OUTPUTS := $(PPC64LE_TESTS) $(PPC64LE_ELFS)
+# Outputs of all tests
+PPC64BE_ALL_OUTPUTS := $(PPC64BE_OUTPUTS) $(PPC64BE_PNV_OUTPUTS)
+PPC64LE_ALL_OUTPUTS := $(PPC64LE_OUTPUTS) $(PPC64LE_PNV_OUTPUTS)
+
+PPC64_CFLAGS = $(CFLAGS) $(EXTRA_CFLAGS) $(PPC64LE_CFLAGS)
 
 # Leave the .elf files, to make debugging easier
-.PRECIOUS: $(CRT_OBJS) $(addsuffix .elf,$(TESTS))
+.PRECIOUS: $(CRT_OBJS_BE) $(CRT_OBJS_LE) $(addsuffix .elf,$(TESTS))
+
+# BE rules
+
+LE_SUFFIX :=
+CRT_DEPS := $(CRT_OBJS_BE)
+MINILIB_DEPS := $(MINILIB_OBJS_BE)
+PPC64_PNV_ELFS := $(PPC64BE_PNV_ELFS)
+PPC64_PNV_TESTS := $(PPC64BE_PNV_TESTS)
+
+$(PPC64BE_ALL_OUTPUTS): LE_SUFFIX =
+$(PPC64BE_ALL_OUTPUTS): PPC64LE_CFLAGS =
+$(PPC64BE_OUTPUTS): CRT_OBJS = $(CRT_OBJS_BE)
+$(PPC64BE_PNV_OUTPUTS): CRT_OBJS = $(PNV_CRT_OBJS_BE)
+$(PPC64BE_ALL_OUTPUTS): MINILIB_OBJS = $(MINILIB_OBJS_BE)
+
+include $(PPC64_SRC)/Makefile.softmmu-rules
 
-# Build CRT objects
-%.o: $(CRT_PATH)/%.S
-	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -x assembler-with-cpp -c $< -o $@
+# LE rules
 
-# Build and link the tests
+LE_SUFFIX := -le
+CRT_DEPS := $(CRT_OBJS_LE)
+MINILIB_DEPS := $(MINILIB_OBJS_LE)
+PPC64_PNV_ELFS := $(PPC64LE_PNV_ELFS)
+PPC64_PNV_TESTS := $(PPC64LE_PNV_TESTS)
 
-# The .elf files are just for debugging
-%.elf: %.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
-	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
+$(PPC64LE_ALL_OUTPUTS): LE_SUFFIX = -le
+$(PPC64LE_ALL_OUTPUTS): PPC64LE_CFLAGS = -mlittle-endian -mabi=elfv2
+$(PPC64LE_OUTPUTS): CRT_OBJS = $(CRT_OBJS_LE)
+$(PPC64LE_PNV_OUTPUTS): CRT_OBJS = $(PNV_CRT_OBJS_LE)
+$(PPC64LE_ALL_OUTPUTS): MINILIB_OBJS = $(MINILIB_OBJS_LE)
 
-%: %.c %.elf $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
-	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS) -Wl,--oformat=binary
+include $(PPC64_SRC)/Makefile.softmmu-rules
 
-memory: CFLAGS+=-DCHECK_UNALIGNED=1
+# Build LE Minilib objs
+%-le.o: $(SYSTEM_MINILIB_SRC)/%.c
+	$(CC) $(PPC64_CFLAGS) -c $< -o $@
 
 # Running
 QEMU_BASE_MACHINE=-cpu power9 -M powernv9 -m 1G -vga none -nographic
-QEMU_OPTS+=$(QEMU_BASE_MACHINE) -serial chardev:output -bios
+QEMU_OPTS+=$(QEMU_BASE_MACHINE) -semihosting-config \
+    enable=on,target=native,chardev=output -bios
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-18 19:10 ` [RFC PATCH v3 1/5] ppc64: Add semihosting support Leandro Lupori
@ 2022-04-18 20:22   ` Cédric Le Goater
  2022-04-20 17:54     ` Leandro Lupori
  2022-04-20 18:09     ` Leandro Lupori
  2022-04-18 23:33   ` Richard Henderson
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 35+ messages in thread
From: Cédric Le Goater @ 2022-04-18 20:22 UTC (permalink / raw)
  To: Leandro Lupori, qemu-devel, qemu-ppc
  Cc: danielhb413, richard.henderson, groug, Nicholas Piggin, pbonzini,
	alex.bennee, david

On 4/18/22 21:10, Leandro Lupori wrote:
> Add semihosting support for PPC64. This implementation is
> based on the standard for ARM semihosting version 2.0, as
> implemented by QEMU and documented in
> 
>      https://github.com/ARM-software/abi-aa/releases
> 
> The PPC64 specific differences are the following:
> 
> Semihosting Trap Instruction: sc 7
> Operation Number Register: r3
> Parameter Register: r4
> Return Register: r3
> Data block field size: 64 bits

'sc' is a good way to implement semi hosting but we should make sure
that it is not colliding with future extensions, at least with the
next POWERPC processor. Is that the case ? if not, then the lev could
be reserved.

I think the part adding POWERPC_EXCP_SEMIHOST should be proposed in a
separate patch.

> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
> ---
>   configs/devices/ppc64-softmmu/default.mak |  3 +++
>   qemu-options.hx                           | 18 ++++++++-----
>   semihosting/arm-compat-semi.c             | 33 +++++++++++++++++++++++
>   target/ppc/cpu.h                          |  3 ++-
>   target/ppc/excp_helper.c                  |  9 +++++++
>   target/ppc/translate.c                    | 14 ++++++++++
>   6 files changed, 72 insertions(+), 8 deletions(-)
> 
> diff --git a/configs/devices/ppc64-softmmu/default.mak b/configs/devices/ppc64-softmmu/default.mak
> index b90e5bf455..43b618fa32 100644
> --- a/configs/devices/ppc64-softmmu/default.mak
> +++ b/configs/devices/ppc64-softmmu/default.mak
> @@ -8,3 +8,6 @@ CONFIG_POWERNV=y
>   
>   # For pSeries
>   CONFIG_PSERIES=y
> +
> +CONFIG_SEMIHOSTING=y
> +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 34e9b32a5c..6e76f7de96 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4527,11 +4527,12 @@ SRST
>   ERST
>   DEF("semihosting", 0, QEMU_OPTION_semihosting,
>       "-semihosting    semihosting mode\n",
> -    QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA |
> -    QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
> +    QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_MIPS |
> +    QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV | QEMU_ARCH_PPC)
>   SRST
>   ``-semihosting``
> -    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V only).
> +    Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V and
> +    PPC only).
>   
>       Note that this allows guest direct access to the host filesystem, so
>       should only be used with a trusted guest OS.
> @@ -4542,12 +4543,12 @@ ERST
>   DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
>       "-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]\n" \
>       "                semihosting configuration\n",
> -QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA |
> -QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
> +QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_MIPS |
> +QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV | QEMU_ARCH_PPC)
>   SRST
>   ``-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]``
> -    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II, RISC-V
> -    only).
> +    Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II,
> +    RISC-V, PPC only).
>   
>       Note that this allows guest direct access to the host filesystem, so
>       should only be used with a trusted guest OS.
> @@ -4563,6 +4564,9 @@ SRST
>   
>       On RISC-V this implements the standard semihosting API, version 0.2.
>   
> +    On PPC this implements the standard Arm semihosting API, version 2.0,
> +    with only the trap instruction and register definitions changed.
> +
>       ``target=native|gdb|auto``
>           Defines where the semihosting calls will be addressed, to QEMU
>           (``native``) or to GDB (``gdb``). The default is ``auto``, which
> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
> index 7a51fd0737..e1279c316c 100644
> --- a/semihosting/arm-compat-semi.c
> +++ b/semihosting/arm-compat-semi.c
> @@ -268,6 +268,31 @@ common_semi_sys_exit_extended(CPUState *cs, int nr)
>   
>   #endif
>   
> +#ifdef TARGET_PPC64

This PPC ifdef in an ARM file seems wrong.

The rest looks OK.

Thanks,

C.


> +static inline target_ulong
> +common_semi_arg(CPUState *cs, int argno)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +    return env->gpr[3 + argno];
> +}
> +
> +static inline void
> +common_semi_set_ret(CPUState *cs, target_ulong ret)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +    env->gpr[3] = ret;
> +}
> +
> +static inline bool
> +common_semi_sys_exit_extended(CPUState *cs, int nr)
> +{
> +    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
> +}
> +
> +#endif
> +
>   /*
>    * Allocate a new guest file descriptor and return it; if we
>    * couldn't allocate a new fd then return -1.
> @@ -450,6 +475,12 @@ static target_ulong common_semi_flen_buf(CPUState *cs)
>   
>       sp = env->gpr[xSP];
>   #endif
> +#ifdef TARGET_PPC64
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +
> +    sp = env->gpr[1];
> +#endif
>   
>       return sp - 64;
>   }
> @@ -780,6 +811,8 @@ static inline bool is_64bit_semihosting(CPUArchState *env)
>       return is_a64(env);
>   #elif defined(TARGET_RISCV)
>       return riscv_cpu_mxl(env) != MXL_RV32;
> +#elif defined(TARGET_PPC64)
> +    return true;
>   #else
>   #error un-handled architecture
>   #endif
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 047b24ba50..349104ad79 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -129,8 +129,9 @@ enum {
>       POWERPC_EXCP_SYSCALL_VECTORED = 102, /* scv exception                     */
>       POWERPC_EXCP_PERFM_EBB = 103,    /* Performance Monitor EBB Exception    */
>       POWERPC_EXCP_EXTERNAL_EBB = 104, /* External EBB Exception               */
> +    POWERPC_EXCP_SEMIHOST = 105,     /* Semihosting Exception                */
>       /* EOL                                                                   */
> -    POWERPC_EXCP_NB       = 105,
> +    POWERPC_EXCP_NB       = 106,
>       /* QEMU exceptions: special cases we want to stop translation            */
>       POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only      */
>   };
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index d3e2cfcd71..af34a57082 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -25,6 +25,7 @@
>   #include "helper_regs.h"
>   
>   #include "trace.h"
> +#include "semihosting/common-semi.h"
>   
>   #ifdef CONFIG_TCG
>   #include "exec/helper-proto.h"
> @@ -100,6 +101,7 @@ static const char *powerpc_excp_name(int excp)
>       case POWERPC_EXCP_SDOOR_HV: return "SDOOR_HV";
>       case POWERPC_EXCP_HVIRT:    return "HVIRT";
>       case POWERPC_EXCP_SYSCALL_VECTORED: return "SYSCALL_VECTORED";
> +    case POWERPC_EXCP_SEMIHOST: return "SEMIHOST";
>       default:
>           g_assert_not_reached();
>       }
> @@ -1327,6 +1329,13 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>       target_ulong msr, new_msr, vector;
>       int srr0, srr1, lev = -1;
>   
> +    /* Handle semihost exceptions first */
> +    if (excp == POWERPC_EXCP_SEMIHOST) {
> +        env->gpr[3] = do_common_semihosting(cs);
> +        env->nip += 4;
> +        return;
> +    }
> +
>       /* new srr1 value excluding must-be-zero bits */
>       msr = env->msr & ~0x783f0000ULL;
>   
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 408ae26173..c013889a84 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4520,6 +4520,20 @@ static void gen_sc(DisasContext *ctx)
>       uint32_t lev;
>   
>       lev = (ctx->opcode >> 5) & 0x7F;
> +
> +#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
> +    /*
> +     * PowerPC semihosting uses the following
> +     * instruction to flag a semihosting call:
> +     *
> +     *      sc 7            0x440000e2
> +     */
> +    if (lev == 7) {
> +        gen_exception(ctx, POWERPC_EXCP_SEMIHOST);
> +        return;
> +    }
> +#endif
> +
>       gen_exception_err(ctx, POWERPC_SYSCALL, lev);
>   }
>   



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 3/5] tests/tcg/ppc64: Add basic softmmu test support
  2022-04-18 19:10 ` [RFC PATCH v3 3/5] tests/tcg/ppc64: Add basic softmmu test support Leandro Lupori
@ 2022-04-18 20:27   ` Cédric Le Goater
  2022-04-18 21:52     ` Daniel Henrique Barboza
  0 siblings, 1 reply; 35+ messages in thread
From: Cédric Le Goater @ 2022-04-18 20:27 UTC (permalink / raw)
  To: Leandro Lupori, qemu-devel, qemu-ppc
  Cc: danielhb413, richard.henderson, groug, pbonzini, alex.bennee, david

On 4/18/22 21:10, Leandro Lupori wrote:
> Add support to build and run the multiarch hello test, that simply
> prints a message and exits, through semihosting operations.
> 
> The linker script was imported from
> https://github.com/legoater/pnv-test, that are the Microwatt tests
> adapted to use a PowerNV console. Boot.S code was inspired on
> mmu/head.S, also from pnv-test repo.
> 
> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
> ---
>   MAINTAINERS                             |  2 +
>   tests/tcg/ppc64/Makefile.softmmu-target | 56 +++++++++++++++++
>   tests/tcg/ppc64/system/include/asm.h    | 68 ++++++++++++++++++++
>   tests/tcg/ppc64/system/lib/boot.S       | 84 +++++++++++++++++++++++++
>   tests/tcg/ppc64/system/lib/powerpc.lds  | 27 ++++++++
>   5 files changed, 237 insertions(+)
>   create mode 100644 tests/tcg/ppc64/Makefile.softmmu-target
>   create mode 100644 tests/tcg/ppc64/system/include/asm.h
>   create mode 100644 tests/tcg/ppc64/system/lib/boot.S
>   create mode 100644 tests/tcg/ppc64/system/lib/powerpc.lds
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4ad2451e03..54f917f8ea 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -266,6 +266,7 @@ M: Cédric Le Goater <clg@kaod.org>
>   M: Daniel Henrique Barboza <danielhb413@gmail.com>
>   R: David Gibson <david@gibson.dropbear.id.au>
>   R: Greg Kurz <groug@kaod.org>
> +R: Leandro Lupori <leandro.lupori@eldorado.org.br>
>   L: qemu-ppc@nongnu.org
>   S: Maintained
>   F: target/ppc/
> @@ -273,6 +274,7 @@ F: hw/ppc/ppc.c
>   F: hw/ppc/ppc_booke.c
>   F: include/hw/ppc/ppc.h
>   F: disas/ppc.c
> +F: tests/tcg/ppc64/


May be separate the ppc64 tests entry, unless you want to become a Reviewer
of the PPC arch. Which is fine for me but I am not sure this is what you
want. If you do separate the ppc64 tests, please declare yourself as a
maintainer because you are clearly among the persons who know the most
about it.

Thanks,

C.


>   RISC-V TCG CPUs
>   M: Palmer Dabbelt <palmer@dabbelt.com>
> diff --git a/tests/tcg/ppc64/Makefile.softmmu-target b/tests/tcg/ppc64/Makefile.softmmu-target
> new file mode 100644
> index 0000000000..948427b70d
> --- /dev/null
> +++ b/tests/tcg/ppc64/Makefile.softmmu-target
> @@ -0,0 +1,56 @@
> +#
> +# PowerPC64 system tests
> +#
> +
> +# For now, disable tests that are failing
> +DISABLED_TESTS := memory
> +DISABLED_EXTRA_RUNS := run-gdbstub-memory
> +
> +PPC64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/ppc64/system
> +VPATH+=$(PPC64_SYSTEM_SRC)
> +
> +# These objects provide the basic boot code and helper functions for all tests
> +CRT_PATH=$(PPC64_SYSTEM_SRC)/lib
> +CRT_OBJS=boot.o
> +
> +LINK_SCRIPT=$(CRT_PATH)/powerpc.lds
> +# NOTE: --build-id is stored before the first code section in the linked
> +#       binary, which causes problems for most tests, that expect to
> +#       begin at address 0.
> +LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none -static -nostdlib \
> +    $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
> +TESTS += $(filter-out $(DISABLED_TESTS),$(MULTIARCH_TESTS))
> +EXTRA_RUNS += $(filter-out $(DISABLED_EXTRA_RUNS),$(MULTIARCH_RUNS))
> +
> +# NOTE: -Os doesn't work well with -Wl,--oformat=binary
> +#       Some linker generated functions, such as savegpr*/restgpr*,
> +#       end up being undefined.
> +CFLAGS = -O -g -Wall -std=c99 -msoft-float -mno-vsx -mno-altivec \
> +         -fno-stack-protector -ffreestanding \
> +         -I $(PPC64_SYSTEM_SRC)/include $(MINILIB_INC) \
> +         -mcpu=power8
> +
> +# Uncomment to test in LE
> +# override EXTRA_CFLAGS += -mlittle-endian -mabi=elfv2
> +
> +# Leave the .elf files, to make debugging easier
> +.PRECIOUS: $(CRT_OBJS) $(addsuffix .elf,$(TESTS))
> +
> +# Build CRT objects
> +%.o: $(CRT_PATH)/%.S
> +	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -x assembler-with-cpp -c $< -o $@
> +
> +# Build and link the tests
> +
> +# The .elf files are just for debugging
> +%.elf: %.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
> +	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> +
> +%: %.c %.elf $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
> +	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS) -Wl,--oformat=binary
> +
> +memory: CFLAGS+=-DCHECK_UNALIGNED=1
> +
> +# Running
> +QEMU_BASE_MACHINE=-cpu power9 -M powernv9 -m 1G -vga none -nographic
> +QEMU_OPTS+=$(QEMU_BASE_MACHINE) -serial chardev:output -bios
> diff --git a/tests/tcg/ppc64/system/include/asm.h b/tests/tcg/ppc64/system/include/asm.h
> new file mode 100644
> index 0000000000..127ed46730
> --- /dev/null
> +++ b/tests/tcg/ppc64/system/include/asm.h
> @@ -0,0 +1,68 @@
> +#ifndef PPC64_ASM_H
> +#define PPC64_ASM_H
> +
> +#define XCONCAT(a, b)       a ## b
> +#define CONCAT(a, b)        XCONCAT(a, b)
> +
> +/* Load an immediate 64-bit value into a register */
> +#define LOAD_IMM64(r, e)                        \
> +    lis     r, (e)@highest;                     \
> +    ori     r, r, (e)@higher;                   \
> +    rldicr  r, r, 32, 31;                       \
> +    oris    r, r, (e)@h;                        \
> +    ori     r, r, (e)@l;
> +
> +/* Switch CPU to little-endian mode, if needed */
> +#define FIXUP_ENDIAN \
> +    tdi   0, 0, 0x48;   /* Reverse endian of b . + 8 */             \
> +    b     $ + 44;       /* Skip trampoline if endian is good */     \
> +    .long 0xa600607d;   /* mfmsr r11 */                             \
> +    .long 0x01006b69;   /* xori r11,r11,1 */                        \
> +    .long 0x00004039;   /* li r10,0 */                              \
> +    .long 0x6401417d;   /* mtmsrd r10,1 */                          \
> +    .long 0x05009f42;   /* bcl 20,31,$+4 */                         \
> +    .long 0xa602487d;   /* mflr r10 */                              \
> +    .long 0x14004a39;   /* addi r10,r10,20 */                       \
> +    .long 0xa6035a7d;   /* mtsrr0 r10 */                            \
> +    .long 0xa6037b7d;   /* mtsrr1 r11 */                            \
> +    .long 0x2400004c    /* rfid */
> +
> +/* Handle differences between ELFv1 and ELFv2 ABIs */
> +
> +#define DOT_LABEL(name)     CONCAT(., name)
> +
> +#if !defined(_CALL_ELF) || _CALL_ELF == 1
> +#define FUNCTION(name)                          \
> +    .section ".opd", "aw";                      \
> +    .p2align 3;                                 \
> +    .globl   name;                              \
> +name:                                           \
> +    .quad   DOT_LABEL(name), .TOC.@tocbase, 0;  \
> +    .previous;                                  \
> +DOT_LABEL(name):
> +
> +#define CALL(fn)                                \
> +    LOAD_IMM64(%r12, fn);                       \
> +    ld      %r12, 0(%r12);                      \
> +    mtctr   %r12;                               \
> +    bctrl
> +
> +#define CALL_LOCAL(fn)                          \
> +    bl      DOT_LABEL(fn)
> +
> +#else
> +#define FUNCTION(name)                          \
> +    .globl   name;                              \
> +name:
> +
> +#define CALL(fn)                                \
> +    LOAD_IMM64(%r12, fn);                       \
> +    mtctr   %r12;                               \
> +    bctrl
> +
> +#define CALL_LOCAL(fn)                          \
> +    bl      fn
> +
> +#endif
> +
> +#endif
> diff --git a/tests/tcg/ppc64/system/lib/boot.S b/tests/tcg/ppc64/system/lib/boot.S
> new file mode 100644
> index 0000000000..b3bcd8a7da
> --- /dev/null
> +++ b/tests/tcg/ppc64/system/lib/boot.S
> @@ -0,0 +1,84 @@
> +/* Copyright 2013-2014 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include "asm.h"
> +
> +#define SPR_HID0                        0x3f0
> +#define SPR_HID0_POWER9_HILE            0x0800000000000000
> +
> +#define ADP_STOPPED_APPLICATIONEXIT     0x20026
> +
> +    .section ".head","ax"
> +
> +    /* QEMU enters in BE at 0x10 by default */
> +    . = 0x10
> +.global start
> +start:
> +    FIXUP_ENDIAN
> +
> +    /* Setup TOC */
> +    LOAD_IMM64(%r2, .TOC.)
> +
> +    /* Configure interrupt endian */
> +#ifdef __LITTLE_ENDIAN__
> +    mfspr   %r10, SPR_HID0
> +    LOAD_IMM64(%r11, SPR_HID0_POWER9_HILE)
> +    or      %r10, %r10, %r11
> +    mtspr   SPR_HID0, %r10
> +#endif
> +
> +    /* Clear .bss */
> +    LOAD_IMM64(%r10, __bss_start)
> +    LOAD_IMM64(%r11, __bss_end)
> +    subf    %r11, %r10, %r11
> +    addi    %r11, %r11, 63
> +    srdi.   %r11, %r11, 6
> +    beq     2f
> +    mtctr   %r11
> +1:  dcbz    0, %r10
> +    addi    %r10, %r10, 64
> +    bdnz    1b
> +
> +    /* Setup stack */
> +2:  LOAD_IMM64(%r1, __stack_top)
> +    li      %r0, 0
> +    stdu    %r0, -32(%r1)
> +
> +    CALL(main)
> +
> +    /* Terminate on exit */
> +    CALL_LOCAL(sys_exit)
> +    b       .
> +
> +FUNCTION(sys_exit)
> +    /*
> +     * As semihosting operations are executed by non-translated QEMU code,
> +     * we shouldn't need to save LR.
> +     */
> +    LOAD_IMM64(%r4, ADP_STOPPED_APPLICATIONEXIT)
> +    std     %r4, -16(%r1)
> +    std     %r3, -8(%r1)
> +    li      %r3, 0x18
> +    addi    %r4, %r1, -16
> +    sc      7
> +    blr
> +
> +FUNCTION(__sys_outc)
> +    addi    %r4, %r1, -1
> +    stb     %r3, 0(%r4)
> +    li      %r3, 0x03
> +    sc      7
> +    blr
> diff --git a/tests/tcg/ppc64/system/lib/powerpc.lds b/tests/tcg/ppc64/system/lib/powerpc.lds
> new file mode 100644
> index 0000000000..db451e1fb9
> --- /dev/null
> +++ b/tests/tcg/ppc64/system/lib/powerpc.lds
> @@ -0,0 +1,27 @@
> +SECTIONS
> +{
> +    . = 0;
> +    _start = .;
> +    .head : {
> +        KEEP(*(.head))
> +    }
> +    . = ALIGN(0x1000);
> +    .text : { *(.text) *(.text.*) *(.rodata) *(.rodata.*) }
> +    . = ALIGN(0x1000);
> +    .data : { *(.data) *(.data.*) *(.got) *(.toc) }
> +    . = ALIGN(0x80);
> +    __bss_start = .;
> +    .bss : {
> +        *(.dynsbss)
> +        *(.sbss)
> +        *(.scommon)
> +        *(.dynbss)
> +        *(.bss)
> +        *(.common)
> +        *(.bss.*)
> +    }
> +    . = ALIGN(0x80);
> +    __bss_end = .;
> +    . = . + 0x4000;
> +    __stack_top = .;
> +}



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 3/5] tests/tcg/ppc64: Add basic softmmu test support
  2022-04-18 20:27   ` Cédric Le Goater
@ 2022-04-18 21:52     ` Daniel Henrique Barboza
  2022-04-20 18:13       ` Leandro Lupori
  0 siblings, 1 reply; 35+ messages in thread
From: Daniel Henrique Barboza @ 2022-04-18 21:52 UTC (permalink / raw)
  To: Cédric Le Goater, Leandro Lupori, qemu-devel, qemu-ppc
  Cc: pbonzini, richard.henderson, alex.bennee, groug, david



On 4/18/22 17:27, Cédric Le Goater wrote:
> On 4/18/22 21:10, Leandro Lupori wrote:
>> Add support to build and run the multiarch hello test, that simply
>> prints a message and exits, through semihosting operations.
>>
>> The linker script was imported from
>> https://github.com/legoater/pnv-test, that are the Microwatt tests
>> adapted to use a PowerNV console. Boot.S code was inspired on
>> mmu/head.S, also from pnv-test repo.
>>
>> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
>> ---
>>   MAINTAINERS                             |  2 +
>>   tests/tcg/ppc64/Makefile.softmmu-target | 56 +++++++++++++++++
>>   tests/tcg/ppc64/system/include/asm.h    | 68 ++++++++++++++++++++
>>   tests/tcg/ppc64/system/lib/boot.S       | 84 +++++++++++++++++++++++++
>>   tests/tcg/ppc64/system/lib/powerpc.lds  | 27 ++++++++
>>   5 files changed, 237 insertions(+)
>>   create mode 100644 tests/tcg/ppc64/Makefile.softmmu-target
>>   create mode 100644 tests/tcg/ppc64/system/include/asm.h
>>   create mode 100644 tests/tcg/ppc64/system/lib/boot.S
>>   create mode 100644 tests/tcg/ppc64/system/lib/powerpc.lds
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 4ad2451e03..54f917f8ea 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -266,6 +266,7 @@ M: Cédric Le Goater <clg@kaod.org>
>>   M: Daniel Henrique Barboza <danielhb413@gmail.com>
>>   R: David Gibson <david@gibson.dropbear.id.au>
>>   R: Greg Kurz <groug@kaod.org>
>> +R: Leandro Lupori <leandro.lupori@eldorado.org.br>
>>   L: qemu-ppc@nongnu.org
>>   S: Maintained
>>   F: target/ppc/
>> @@ -273,6 +274,7 @@ F: hw/ppc/ppc.c
>>   F: hw/ppc/ppc_booke.c
>>   F: include/hw/ppc/ppc.h
>>   F: disas/ppc.c
>> +F: tests/tcg/ppc64/
> 
> 
> May be separate the ppc64 tests entry, unless you want to become a Reviewer
> of the PPC arch. Which is fine for me but I am not sure this is what you
> want. If you do separate the ppc64 tests, please declare yourself as a
> maintainer because you are clearly among the persons who know the most
> about it.

I second that. Leandro, feel free to add yourself as the maintainer of the softmmu
ppc64 tests.


Daniel

> 
> Thanks,
> 
> C.
> 
> 
>>   RISC-V TCG CPUs
>>   M: Palmer Dabbelt <palmer@dabbelt.com>
>> diff --git a/tests/tcg/ppc64/Makefile.softmmu-target b/tests/tcg/ppc64/Makefile.softmmu-target
>> new file mode 100644
>> index 0000000000..948427b70d
>> --- /dev/null
>> +++ b/tests/tcg/ppc64/Makefile.softmmu-target
>> @@ -0,0 +1,56 @@
>> +#
>> +# PowerPC64 system tests
>> +#
>> +
>> +# For now, disable tests that are failing
>> +DISABLED_TESTS := memory
>> +DISABLED_EXTRA_RUNS := run-gdbstub-memory
>> +
>> +PPC64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/ppc64/system
>> +VPATH+=$(PPC64_SYSTEM_SRC)
>> +
>> +# These objects provide the basic boot code and helper functions for all tests
>> +CRT_PATH=$(PPC64_SYSTEM_SRC)/lib
>> +CRT_OBJS=boot.o
>> +
>> +LINK_SCRIPT=$(CRT_PATH)/powerpc.lds
>> +# NOTE: --build-id is stored before the first code section in the linked
>> +#       binary, which causes problems for most tests, that expect to
>> +#       begin at address 0.
>> +LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none -static -nostdlib \
>> +    $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
>> +TESTS += $(filter-out $(DISABLED_TESTS),$(MULTIARCH_TESTS))
>> +EXTRA_RUNS += $(filter-out $(DISABLED_EXTRA_RUNS),$(MULTIARCH_RUNS))
>> +
>> +# NOTE: -Os doesn't work well with -Wl,--oformat=binary
>> +#       Some linker generated functions, such as savegpr*/restgpr*,
>> +#       end up being undefined.
>> +CFLAGS = -O -g -Wall -std=c99 -msoft-float -mno-vsx -mno-altivec \
>> +         -fno-stack-protector -ffreestanding \
>> +         -I $(PPC64_SYSTEM_SRC)/include $(MINILIB_INC) \
>> +         -mcpu=power8
>> +
>> +# Uncomment to test in LE
>> +# override EXTRA_CFLAGS += -mlittle-endian -mabi=elfv2
>> +
>> +# Leave the .elf files, to make debugging easier
>> +.PRECIOUS: $(CRT_OBJS) $(addsuffix .elf,$(TESTS))
>> +
>> +# Build CRT objects
>> +%.o: $(CRT_PATH)/%.S
>> +    $(CC) $(CFLAGS) $(EXTRA_CFLAGS) -x assembler-with-cpp -c $< -o $@
>> +
>> +# Build and link the tests
>> +
>> +# The .elf files are just for debugging
>> +%.elf: %.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
>> +    $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
>> +
>> +%: %.c %.elf $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
>> +    $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS) -Wl,--oformat=binary
>> +
>> +memory: CFLAGS+=-DCHECK_UNALIGNED=1
>> +
>> +# Running
>> +QEMU_BASE_MACHINE=-cpu power9 -M powernv9 -m 1G -vga none -nographic
>> +QEMU_OPTS+=$(QEMU_BASE_MACHINE) -serial chardev:output -bios
>> diff --git a/tests/tcg/ppc64/system/include/asm.h b/tests/tcg/ppc64/system/include/asm.h
>> new file mode 100644
>> index 0000000000..127ed46730
>> --- /dev/null
>> +++ b/tests/tcg/ppc64/system/include/asm.h
>> @@ -0,0 +1,68 @@
>> +#ifndef PPC64_ASM_H
>> +#define PPC64_ASM_H
>> +
>> +#define XCONCAT(a, b)       a ## b
>> +#define CONCAT(a, b)        XCONCAT(a, b)
>> +
>> +/* Load an immediate 64-bit value into a register */
>> +#define LOAD_IMM64(r, e)                        \
>> +    lis     r, (e)@highest;                     \
>> +    ori     r, r, (e)@higher;                   \
>> +    rldicr  r, r, 32, 31;                       \
>> +    oris    r, r, (e)@h;                        \
>> +    ori     r, r, (e)@l;
>> +
>> +/* Switch CPU to little-endian mode, if needed */
>> +#define FIXUP_ENDIAN \
>> +    tdi   0, 0, 0x48;   /* Reverse endian of b . + 8 */             \
>> +    b     $ + 44;       /* Skip trampoline if endian is good */     \
>> +    .long 0xa600607d;   /* mfmsr r11 */                             \
>> +    .long 0x01006b69;   /* xori r11,r11,1 */                        \
>> +    .long 0x00004039;   /* li r10,0 */                              \
>> +    .long 0x6401417d;   /* mtmsrd r10,1 */                          \
>> +    .long 0x05009f42;   /* bcl 20,31,$+4 */                         \
>> +    .long 0xa602487d;   /* mflr r10 */                              \
>> +    .long 0x14004a39;   /* addi r10,r10,20 */                       \
>> +    .long 0xa6035a7d;   /* mtsrr0 r10 */                            \
>> +    .long 0xa6037b7d;   /* mtsrr1 r11 */                            \
>> +    .long 0x2400004c    /* rfid */
>> +
>> +/* Handle differences between ELFv1 and ELFv2 ABIs */
>> +
>> +#define DOT_LABEL(name)     CONCAT(., name)
>> +
>> +#if !defined(_CALL_ELF) || _CALL_ELF == 1
>> +#define FUNCTION(name)                          \
>> +    .section ".opd", "aw";                      \
>> +    .p2align 3;                                 \
>> +    .globl   name;                              \
>> +name:                                           \
>> +    .quad   DOT_LABEL(name), .TOC.@tocbase, 0;  \
>> +    .previous;                                  \
>> +DOT_LABEL(name):
>> +
>> +#define CALL(fn)                                \
>> +    LOAD_IMM64(%r12, fn);                       \
>> +    ld      %r12, 0(%r12);                      \
>> +    mtctr   %r12;                               \
>> +    bctrl
>> +
>> +#define CALL_LOCAL(fn)                          \
>> +    bl      DOT_LABEL(fn)
>> +
>> +#else
>> +#define FUNCTION(name)                          \
>> +    .globl   name;                              \
>> +name:
>> +
>> +#define CALL(fn)                                \
>> +    LOAD_IMM64(%r12, fn);                       \
>> +    mtctr   %r12;                               \
>> +    bctrl
>> +
>> +#define CALL_LOCAL(fn)                          \
>> +    bl      fn
>> +
>> +#endif
>> +
>> +#endif
>> diff --git a/tests/tcg/ppc64/system/lib/boot.S b/tests/tcg/ppc64/system/lib/boot.S
>> new file mode 100644
>> index 0000000000..b3bcd8a7da
>> --- /dev/null
>> +++ b/tests/tcg/ppc64/system/lib/boot.S
>> @@ -0,0 +1,84 @@
>> +/* Copyright 2013-2014 IBM Corp.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + *     http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> + * implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#include "asm.h"
>> +
>> +#define SPR_HID0                        0x3f0
>> +#define SPR_HID0_POWER9_HILE            0x0800000000000000
>> +
>> +#define ADP_STOPPED_APPLICATIONEXIT     0x20026
>> +
>> +    .section ".head","ax"
>> +
>> +    /* QEMU enters in BE at 0x10 by default */
>> +    . = 0x10
>> +.global start
>> +start:
>> +    FIXUP_ENDIAN
>> +
>> +    /* Setup TOC */
>> +    LOAD_IMM64(%r2, .TOC.)
>> +
>> +    /* Configure interrupt endian */
>> +#ifdef __LITTLE_ENDIAN__
>> +    mfspr   %r10, SPR_HID0
>> +    LOAD_IMM64(%r11, SPR_HID0_POWER9_HILE)
>> +    or      %r10, %r10, %r11
>> +    mtspr   SPR_HID0, %r10
>> +#endif
>> +
>> +    /* Clear .bss */
>> +    LOAD_IMM64(%r10, __bss_start)
>> +    LOAD_IMM64(%r11, __bss_end)
>> +    subf    %r11, %r10, %r11
>> +    addi    %r11, %r11, 63
>> +    srdi.   %r11, %r11, 6
>> +    beq     2f
>> +    mtctr   %r11
>> +1:  dcbz    0, %r10
>> +    addi    %r10, %r10, 64
>> +    bdnz    1b
>> +
>> +    /* Setup stack */
>> +2:  LOAD_IMM64(%r1, __stack_top)
>> +    li      %r0, 0
>> +    stdu    %r0, -32(%r1)
>> +
>> +    CALL(main)
>> +
>> +    /* Terminate on exit */
>> +    CALL_LOCAL(sys_exit)
>> +    b       .
>> +
>> +FUNCTION(sys_exit)
>> +    /*
>> +     * As semihosting operations are executed by non-translated QEMU code,
>> +     * we shouldn't need to save LR.
>> +     */
>> +    LOAD_IMM64(%r4, ADP_STOPPED_APPLICATIONEXIT)
>> +    std     %r4, -16(%r1)
>> +    std     %r3, -8(%r1)
>> +    li      %r3, 0x18
>> +    addi    %r4, %r1, -16
>> +    sc      7
>> +    blr
>> +
>> +FUNCTION(__sys_outc)
>> +    addi    %r4, %r1, -1
>> +    stb     %r3, 0(%r4)
>> +    li      %r3, 0x03
>> +    sc      7
>> +    blr
>> diff --git a/tests/tcg/ppc64/system/lib/powerpc.lds b/tests/tcg/ppc64/system/lib/powerpc.lds
>> new file mode 100644
>> index 0000000000..db451e1fb9
>> --- /dev/null
>> +++ b/tests/tcg/ppc64/system/lib/powerpc.lds
>> @@ -0,0 +1,27 @@
>> +SECTIONS
>> +{
>> +    . = 0;
>> +    _start = .;
>> +    .head : {
>> +        KEEP(*(.head))
>> +    }
>> +    . = ALIGN(0x1000);
>> +    .text : { *(.text) *(.text.*) *(.rodata) *(.rodata.*) }
>> +    . = ALIGN(0x1000);
>> +    .data : { *(.data) *(.data.*) *(.got) *(.toc) }
>> +    . = ALIGN(0x80);
>> +    __bss_start = .;
>> +    .bss : {
>> +        *(.dynsbss)
>> +        *(.sbss)
>> +        *(.scommon)
>> +        *(.dynbss)
>> +        *(.bss)
>> +        *(.common)
>> +        *(.bss.*)
>> +    }
>> +    . = ALIGN(0x80);
>> +    __bss_end = .;
>> +    . = . + 0x4000;
>> +    __stack_top = .;
>> +}
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-18 19:10 ` [RFC PATCH v3 1/5] ppc64: Add semihosting support Leandro Lupori
  2022-04-18 20:22   ` Cédric Le Goater
@ 2022-04-18 23:33   ` Richard Henderson
  2022-04-19  9:26   ` Peter Maydell
  2022-04-20 18:05   ` Peter Maydell
  3 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-04-18 23:33 UTC (permalink / raw)
  To: Leandro Lupori, qemu-devel, qemu-ppc
  Cc: danielhb413, groug, clg, pbonzini, alex.bennee, david

On 4/18/22 12:10, Leandro Lupori wrote:
> Add semihosting support for PPC64. This implementation is
> based on the standard for ARM semihosting version 2.0, as
> implemented by QEMU and documented in
> 
>      https://github.com/ARM-software/abi-aa/releases
> 
> The PPC64 specific differences are the following:
> 
> Semihosting Trap Instruction: sc 7
> Operation Number Register: r3
> Parameter Register: r4
> Return Register: r3
> Data block field size: 64 bits
> 
> Signed-off-by: Leandro Lupori<leandro.lupori@eldorado.org.br>
> ---
>   configs/devices/ppc64-softmmu/default.mak |  3 +++
>   qemu-options.hx                           | 18 ++++++++-----
>   semihosting/arm-compat-semi.c             | 33 +++++++++++++++++++++++
>   target/ppc/cpu.h                          |  3 ++-
>   target/ppc/excp_helper.c                  |  9 +++++++
>   target/ppc/translate.c                    | 14 ++++++++++
>   6 files changed, 72 insertions(+), 8 deletions(-)

Modulo whatever sc number yall settle on,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le
  2022-04-18 19:10 ` [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le Leandro Lupori
@ 2022-04-18 23:36   ` Richard Henderson
  2022-04-20 18:42     ` Leandro Lupori
  2022-04-20 19:42   ` Peter Maydell
  1 sibling, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-04-18 23:36 UTC (permalink / raw)
  To: Leandro Lupori, qemu-devel, qemu-ppc
  Cc: danielhb413, groug, clg, pbonzini, alex.bennee, david

On 4/18/22 12:10, Leandro Lupori wrote:
> +static inline uint64_t sh_swap64(CPUArchState *env, uint64_t val)
> +{
> +    return msr_le ? val : tswap64(val);
> +}
> +
> +static inline uint32_t sh_swap32(CPUArchState *env, uint32_t val)
> +{
> +    return msr_le ? val : tswap32(val);
> +}

That doesn't work -- tswap itself is conditional.

You want

   return msr_le ? le32_to_cpu(x) : be32_to_cpu(x);

etc.  One of them will be a nop, and the other will bswap.


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-18 19:10 ` [RFC PATCH v3 1/5] ppc64: Add semihosting support Leandro Lupori
  2022-04-18 20:22   ` Cédric Le Goater
  2022-04-18 23:33   ` Richard Henderson
@ 2022-04-19  9:26   ` Peter Maydell
  2022-04-20 18:20     ` Leandro Lupori
  2022-04-20 18:05   ` Peter Maydell
  3 siblings, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2022-04-19  9:26 UTC (permalink / raw)
  To: Leandro Lupori
  Cc: danielhb413, richard.henderson, qemu-devel, groug, qemu-ppc, clg,
	pbonzini, alex.bennee, david

On Mon, 18 Apr 2022 at 20:15, Leandro Lupori
<leandro.lupori@eldorado.org.br> wrote:
>
> Add semihosting support for PPC64. This implementation is
> based on the standard for ARM semihosting version 2.0, as
> implemented by QEMU and documented in
>
>     https://github.com/ARM-software/abi-aa/releases
>
> The PPC64 specific differences are the following:
>
> Semihosting Trap Instruction: sc 7
> Operation Number Register: r3
> Parameter Register: r4
> Return Register: r3
> Data block field size: 64 bits

Where is the independent specification which defines that
this is the ABI for PPC semihosting? You should provide the
URL for that in a comment somewhere.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-18 20:22   ` Cédric Le Goater
@ 2022-04-20 17:54     ` Leandro Lupori
  2022-04-20 18:18       ` Richard Henderson
  2022-04-20 18:09     ` Leandro Lupori
  1 sibling, 1 reply; 35+ messages in thread
From: Leandro Lupori @ 2022-04-20 17:54 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel, qemu-ppc
  Cc: danielhb413, richard.henderson, groug, Nicholas Piggin, pbonzini,
	alex.bennee, david

On 4/18/22 17:22, Cédric Le Goater wrote:

>> diff --git a/semihosting/arm-compat-semi.c 
>> b/semihosting/arm-compat-semi.c
>> index 7a51fd0737..e1279c316c 100644
>> --- a/semihosting/arm-compat-semi.c
>> +++ b/semihosting/arm-compat-semi.c
>> @@ -268,6 +268,31 @@ common_semi_sys_exit_extended(CPUState *cs, int nr)
>>
>>   #endif
>>
>> +#ifdef TARGET_PPC64
> 
> This PPC ifdef in an ARM file seems wrong.
> 
> The rest looks OK.

IIUC, arm-compat-semi.c is not an ARM specific file, but it's used by 
targets that implement ARM-compatible semihosting. It's currently used 
by ARM and RISC-V and both use target ifdefs in small parts of this file.

Thanks,
Leandro

> 
> Thanks,
> 
> C.
> 
> 
>> +static inline target_ulong
>> +common_semi_arg(CPUState *cs, int argno)
>> +{
>> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +    CPUPPCState *env = &cpu->env;
>> +    return env->gpr[3 + argno];
>> +}
>> +
>> +static inline void
>> +common_semi_set_ret(CPUState *cs, target_ulong ret)
>> +{
>> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +    CPUPPCState *env = &cpu->env;
>> +    env->gpr[3] = ret;
>> +}
>> +
>> +static inline bool
>> +common_semi_sys_exit_extended(CPUState *cs, int nr)
>> +{
>> +    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 
>> 8);
>> +}
>> +
>> +#endif
>> +


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-18 19:10 ` [RFC PATCH v3 1/5] ppc64: Add semihosting support Leandro Lupori
                     ` (2 preceding siblings ...)
  2022-04-19  9:26   ` Peter Maydell
@ 2022-04-20 18:05   ` Peter Maydell
  2022-04-20 18:30     ` Leandro Lupori
  3 siblings, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2022-04-20 18:05 UTC (permalink / raw)
  To: Leandro Lupori
  Cc: danielhb413, richard.henderson, qemu-devel, groug, qemu-ppc, clg,
	pbonzini, alex.bennee, david

On Mon, 18 Apr 2022 at 20:15, Leandro Lupori
<leandro.lupori@eldorado.org.br> wrote:
>
> Add semihosting support for PPC64. This implementation is
> based on the standard for ARM semihosting version 2.0, as
> implemented by QEMU and documented in
>
>     https://github.com/ARM-software/abi-aa/releases
>
> The PPC64 specific differences are the following:
>
> Semihosting Trap Instruction: sc 7
> Operation Number Register: r3
> Parameter Register: r4
> Return Register: r3
> Data block field size: 64 bits
>
> +static inline bool
> +common_semi_sys_exit_extended(CPUState *cs, int nr)
> +{
> +    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
> +}

Does the PPC specification for semihosting really follow the
legacy Arm requirement that the 32-bit version of the EXIT
call doesn't let the caller specify the exit status? It's
not a very sensible choice IMHO if you don't have the legacy
baggage to deal with.

-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-18 20:22   ` Cédric Le Goater
  2022-04-20 17:54     ` Leandro Lupori
@ 2022-04-20 18:09     ` Leandro Lupori
  2022-04-21  2:04       ` Nicholas Piggin
  2022-04-21  6:12       ` Cédric Le Goater
  1 sibling, 2 replies; 35+ messages in thread
From: Leandro Lupori @ 2022-04-20 18:09 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel, qemu-ppc
  Cc: danielhb413, richard.henderson, groug, Nicholas Piggin, pbonzini,
	alex.bennee, david

On 4/18/22 17:22, Cédric Le Goater wrote:
> On 4/18/22 21:10, Leandro Lupori wrote:
>> Add semihosting support for PPC64. This implementation is
>> based on the standard for ARM semihosting version 2.0, as
>> implemented by QEMU and documented in
>>
>>      https://github.com/ARM-software/abi-aa/releases
>>
>> The PPC64 specific differences are the following:
>>
>> Semihosting Trap Instruction: sc 7
>> Operation Number Register: r3
>> Parameter Register: r4
>> Return Register: r3
>> Data block field size: 64 bits
> 
> 'sc' is a good way to implement semi hosting but we should make sure
> that it is not colliding with future extensions, at least with the
> next POWERPC processor. Is that the case ? if not, then the lev could
> be reserved.
> 

Power ISA 3.1B says that LEV values greater that 2 are reserved.
Level 2 is the ultravisor, so I assumed that level 7 was far enough from 
current max level. I don't know if POWER11 will introduce new privilege 
levels. Is this info publicly available somewhere? Or do you have a 
better level in mind to use instead?

> I think the part adding POWERPC_EXCP_SEMIHOST should be proposed in a
> separate patch.
> 

Ok, I can move it to a separate patch. That would be all changes in 
target/ppc/cpu.h and target/ppc/excp_helper.c, right?


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 3/5] tests/tcg/ppc64: Add basic softmmu test support
  2022-04-18 21:52     ` Daniel Henrique Barboza
@ 2022-04-20 18:13       ` Leandro Lupori
  0 siblings, 0 replies; 35+ messages in thread
From: Leandro Lupori @ 2022-04-20 18:13 UTC (permalink / raw)
  To: Daniel Henrique Barboza, Cédric Le Goater, qemu-devel, qemu-ppc
  Cc: pbonzini, richard.henderson, alex.bennee, groug, david

On 4/18/22 18:52, Daniel Henrique Barboza wrote:
> [E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você 
> possa confirmar o remetente e saber que o conteúdo é seguro. Em caso de 
> e-mail suspeito entre imediatamente em contato com o DTI.
> 
> On 4/18/22 17:27, Cédric Le Goater wrote:
>> On 4/18/22 21:10, Leandro Lupori wrote:
>>> Add support to build and run the multiarch hello test, that simply
>>> prints a message and exits, through semihosting operations.
>>>
>>> The linker script was imported from
>>> https://github.com/legoater/pnv-test, that are the Microwatt tests
>>> adapted to use a PowerNV console. Boot.S code was inspired on
>>> mmu/head.S, also from pnv-test repo.
>>>
>>> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
>>> ---
>>>   MAINTAINERS                             |  2 +
>>>   tests/tcg/ppc64/Makefile.softmmu-target | 56 +++++++++++++++++
>>>   tests/tcg/ppc64/system/include/asm.h    | 68 ++++++++++++++++++++
>>>   tests/tcg/ppc64/system/lib/boot.S       | 84 +++++++++++++++++++++++++
>>>   tests/tcg/ppc64/system/lib/powerpc.lds  | 27 ++++++++
>>>   5 files changed, 237 insertions(+)
>>>   create mode 100644 tests/tcg/ppc64/Makefile.softmmu-target
>>>   create mode 100644 tests/tcg/ppc64/system/include/asm.h
>>>   create mode 100644 tests/tcg/ppc64/system/lib/boot.S
>>>   create mode 100644 tests/tcg/ppc64/system/lib/powerpc.lds
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 4ad2451e03..54f917f8ea 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -266,6 +266,7 @@ M: Cédric Le Goater <clg@kaod.org>
>>>   M: Daniel Henrique Barboza <danielhb413@gmail.com>
>>>   R: David Gibson <david@gibson.dropbear.id.au>
>>>   R: Greg Kurz <groug@kaod.org>
>>> +R: Leandro Lupori <leandro.lupori@eldorado.org.br>
>>>   L: qemu-ppc@nongnu.org
>>>   S: Maintained
>>>   F: target/ppc/
>>> @@ -273,6 +274,7 @@ F: hw/ppc/ppc.c
>>>   F: hw/ppc/ppc_booke.c
>>>   F: include/hw/ppc/ppc.h
>>>   F: disas/ppc.c
>>> +F: tests/tcg/ppc64/
>>
>>
>> May be separate the ppc64 tests entry, unless you want to become a 
>> Reviewer
>> of the PPC arch. Which is fine for me but I am not sure this is what you
>> want. If you do separate the ppc64 tests, please declare yourself as a
>> maintainer because you are clearly among the persons who know the most
>> about it.
> 
> I second that. Leandro, feel free to add yourself as the maintainer of 
> the softmmu
> ppc64 tests.
> 

Ok, it was really not my intention to become a reviewer of the PPC arch, 
but only of ppc64 tests. I'll move it to a separate entry.

Thanks,
Leandro

> 
> Daniel
> 
>>
>> Thanks,
>>
>> C.
>>
>>
>>>   RISC-V TCG CPUs
>>>   M: Palmer Dabbelt <palmer@dabbelt.com>
>>> diff --git a/tests/tcg/ppc64/Makefile.softmmu-target 
>>> b/tests/tcg/ppc64/Makefile.softmmu-target
>>> new file mode 100644
>>> index 0000000000..948427b70d
>>> --- /dev/null
>>> +++ b/tests/tcg/ppc64/Makefile.softmmu-target
>>> @@ -0,0 +1,56 @@
>>> +#
>>> +# PowerPC64 system tests
>>> +#
>>> +
>>> +# For now, disable tests that are failing
>>> +DISABLED_TESTS := memory
>>> +DISABLED_EXTRA_RUNS := run-gdbstub-memory
>>> +
>>> +PPC64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/ppc64/system
>>> +VPATH+=$(PPC64_SYSTEM_SRC)
>>> +
>>> +# These objects provide the basic boot code and helper functions for 
>>> all tests
>>> +CRT_PATH=$(PPC64_SYSTEM_SRC)/lib
>>> +CRT_OBJS=boot.o
>>> +
>>> +LINK_SCRIPT=$(CRT_PATH)/powerpc.lds
>>> +# NOTE: --build-id is stored before the first code section in the 
>>> linked
>>> +#       binary, which causes problems for most tests, that expect to
>>> +#       begin at address 0.
>>> +LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none -static -nostdlib \
>>> +    $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
>>> +TESTS += $(filter-out $(DISABLED_TESTS),$(MULTIARCH_TESTS))
>>> +EXTRA_RUNS += $(filter-out $(DISABLED_EXTRA_RUNS),$(MULTIARCH_RUNS))
>>> +
>>> +# NOTE: -Os doesn't work well with -Wl,--oformat=binary
>>> +#       Some linker generated functions, such as savegpr*/restgpr*,
>>> +#       end up being undefined.
>>> +CFLAGS = -O -g -Wall -std=c99 -msoft-float -mno-vsx -mno-altivec \
>>> +         -fno-stack-protector -ffreestanding \
>>> +         -I $(PPC64_SYSTEM_SRC)/include $(MINILIB_INC) \
>>> +         -mcpu=power8
>>> +
>>> +# Uncomment to test in LE
>>> +# override EXTRA_CFLAGS += -mlittle-endian -mabi=elfv2
>>> +
>>> +# Leave the .elf files, to make debugging easier
>>> +.PRECIOUS: $(CRT_OBJS) $(addsuffix .elf,$(TESTS))
>>> +
>>> +# Build CRT objects
>>> +%.o: $(CRT_PATH)/%.S
>>> +    $(CC) $(CFLAGS) $(EXTRA_CFLAGS) -x assembler-with-cpp -c $< -o $@
>>> +
>>> +# Build and link the tests
>>> +
>>> +# The .elf files are just for debugging
>>> +%.elf: %.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
>>> +    $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
>>> +
>>> +%: %.c %.elf $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
>>> +    $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS) 
>>> -Wl,--oformat=binary
>>> +
>>> +memory: CFLAGS+=-DCHECK_UNALIGNED=1
>>> +
>>> +# Running
>>> +QEMU_BASE_MACHINE=-cpu power9 -M powernv9 -m 1G -vga none -nographic
>>> +QEMU_OPTS+=$(QEMU_BASE_MACHINE) -serial chardev:output -bios
>>> diff --git a/tests/tcg/ppc64/system/include/asm.h 
>>> b/tests/tcg/ppc64/system/include/asm.h
>>> new file mode 100644
>>> index 0000000000..127ed46730
>>> --- /dev/null
>>> +++ b/tests/tcg/ppc64/system/include/asm.h
>>> @@ -0,0 +1,68 @@
>>> +#ifndef PPC64_ASM_H
>>> +#define PPC64_ASM_H
>>> +
>>> +#define XCONCAT(a, b)       a ## b
>>> +#define CONCAT(a, b)        XCONCAT(a, b)
>>> +
>>> +/* Load an immediate 64-bit value into a register */
>>> +#define LOAD_IMM64(r, e)                        \
>>> +    lis     r, (e)@highest;                     \
>>> +    ori     r, r, (e)@higher;                   \
>>> +    rldicr  r, r, 32, 31;                       \
>>> +    oris    r, r, (e)@h;                        \
>>> +    ori     r, r, (e)@l;
>>> +
>>> +/* Switch CPU to little-endian mode, if needed */
>>> +#define FIXUP_ENDIAN \
>>> +    tdi   0, 0, 0x48;   /* Reverse endian of b . + 8 */             \
>>> +    b     $ + 44;       /* Skip trampoline if endian is good */     \
>>> +    .long 0xa600607d;   /* mfmsr r11 */                             \
>>> +    .long 0x01006b69;   /* xori r11,r11,1 */                        \
>>> +    .long 0x00004039;   /* li r10,0 */                              \
>>> +    .long 0x6401417d;   /* mtmsrd r10,1 */                          \
>>> +    .long 0x05009f42;   /* bcl 20,31,$+4 */                         \
>>> +    .long 0xa602487d;   /* mflr r10 */                              \
>>> +    .long 0x14004a39;   /* addi r10,r10,20 */                       \
>>> +    .long 0xa6035a7d;   /* mtsrr0 r10 */                            \
>>> +    .long 0xa6037b7d;   /* mtsrr1 r11 */                            \
>>> +    .long 0x2400004c    /* rfid */
>>> +
>>> +/* Handle differences between ELFv1 and ELFv2 ABIs */
>>> +
>>> +#define DOT_LABEL(name)     CONCAT(., name)
>>> +
>>> +#if !defined(_CALL_ELF) || _CALL_ELF == 1
>>> +#define FUNCTION(name)                          \
>>> +    .section ".opd", "aw";                      \
>>> +    .p2align 3;                                 \
>>> +    .globl   name;                              \
>>> +name:                                           \
>>> +    .quad   DOT_LABEL(name), .TOC.@tocbase, 0;  \
>>> +    .previous;                                  \
>>> +DOT_LABEL(name):
>>> +
>>> +#define CALL(fn)                                \
>>> +    LOAD_IMM64(%r12, fn);                       \
>>> +    ld      %r12, 0(%r12);                      \
>>> +    mtctr   %r12;                               \
>>> +    bctrl
>>> +
>>> +#define CALL_LOCAL(fn)                          \
>>> +    bl      DOT_LABEL(fn)
>>> +
>>> +#else
>>> +#define FUNCTION(name)                          \
>>> +    .globl   name;                              \
>>> +name:
>>> +
>>> +#define CALL(fn)                                \
>>> +    LOAD_IMM64(%r12, fn);                       \
>>> +    mtctr   %r12;                               \
>>> +    bctrl
>>> +
>>> +#define CALL_LOCAL(fn)                          \
>>> +    bl      fn
>>> +
>>> +#endif
>>> +
>>> +#endif
>>> diff --git a/tests/tcg/ppc64/system/lib/boot.S 
>>> b/tests/tcg/ppc64/system/lib/boot.S
>>> new file mode 100644
>>> index 0000000000..b3bcd8a7da
>>> --- /dev/null
>>> +++ b/tests/tcg/ppc64/system/lib/boot.S
>>> @@ -0,0 +1,84 @@
>>> +/* Copyright 2013-2014 IBM Corp.
>>> + *
>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>> + * you may not use this file except in compliance with the License.
>>> + * You may obtain a copy of the License at
>>> + *
>>> + *     http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + * Unless required by applicable law or agreed to in writing, software
>>> + * distributed under the License is distributed on an "AS IS" BASIS,
>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>>> + * implied.
>>> + * See the License for the specific language governing permissions and
>>> + * limitations under the License.
>>> + */
>>> +
>>> +#include "asm.h"
>>> +
>>> +#define SPR_HID0                        0x3f0
>>> +#define SPR_HID0_POWER9_HILE            0x0800000000000000
>>> +
>>> +#define ADP_STOPPED_APPLICATIONEXIT     0x20026
>>> +
>>> +    .section ".head","ax"
>>> +
>>> +    /* QEMU enters in BE at 0x10 by default */
>>> +    . = 0x10
>>> +.global start
>>> +start:
>>> +    FIXUP_ENDIAN
>>> +
>>> +    /* Setup TOC */
>>> +    LOAD_IMM64(%r2, .TOC.)
>>> +
>>> +    /* Configure interrupt endian */
>>> +#ifdef __LITTLE_ENDIAN__
>>> +    mfspr   %r10, SPR_HID0
>>> +    LOAD_IMM64(%r11, SPR_HID0_POWER9_HILE)
>>> +    or      %r10, %r10, %r11
>>> +    mtspr   SPR_HID0, %r10
>>> +#endif
>>> +
>>> +    /* Clear .bss */
>>> +    LOAD_IMM64(%r10, __bss_start)
>>> +    LOAD_IMM64(%r11, __bss_end)
>>> +    subf    %r11, %r10, %r11
>>> +    addi    %r11, %r11, 63
>>> +    srdi.   %r11, %r11, 6
>>> +    beq     2f
>>> +    mtctr   %r11
>>> +1:  dcbz    0, %r10
>>> +    addi    %r10, %r10, 64
>>> +    bdnz    1b
>>> +
>>> +    /* Setup stack */
>>> +2:  LOAD_IMM64(%r1, __stack_top)
>>> +    li      %r0, 0
>>> +    stdu    %r0, -32(%r1)
>>> +
>>> +    CALL(main)
>>> +
>>> +    /* Terminate on exit */
>>> +    CALL_LOCAL(sys_exit)
>>> +    b       .
>>> +
>>> +FUNCTION(sys_exit)
>>> +    /*
>>> +     * As semihosting operations are executed by non-translated QEMU 
>>> code,
>>> +     * we shouldn't need to save LR.
>>> +     */
>>> +    LOAD_IMM64(%r4, ADP_STOPPED_APPLICATIONEXIT)
>>> +    std     %r4, -16(%r1)
>>> +    std     %r3, -8(%r1)
>>> +    li      %r3, 0x18
>>> +    addi    %r4, %r1, -16
>>> +    sc      7
>>> +    blr
>>> +
>>> +FUNCTION(__sys_outc)
>>> +    addi    %r4, %r1, -1
>>> +    stb     %r3, 0(%r4)
>>> +    li      %r3, 0x03
>>> +    sc      7
>>> +    blr
>>> diff --git a/tests/tcg/ppc64/system/lib/powerpc.lds 
>>> b/tests/tcg/ppc64/system/lib/powerpc.lds
>>> new file mode 100644
>>> index 0000000000..db451e1fb9
>>> --- /dev/null
>>> +++ b/tests/tcg/ppc64/system/lib/powerpc.lds
>>> @@ -0,0 +1,27 @@
>>> +SECTIONS
>>> +{
>>> +    . = 0;
>>> +    _start = .;
>>> +    .head : {
>>> +        KEEP(*(.head))
>>> +    }
>>> +    . = ALIGN(0x1000);
>>> +    .text : { *(.text) *(.text.*) *(.rodata) *(.rodata.*) }
>>> +    . = ALIGN(0x1000);
>>> +    .data : { *(.data) *(.data.*) *(.got) *(.toc) }
>>> +    . = ALIGN(0x80);
>>> +    __bss_start = .;
>>> +    .bss : {
>>> +        *(.dynsbss)
>>> +        *(.sbss)
>>> +        *(.scommon)
>>> +        *(.dynbss)
>>> +        *(.bss)
>>> +        *(.common)
>>> +        *(.bss.*)
>>> +    }
>>> +    . = ALIGN(0x80);
>>> +    __bss_end = .;
>>> +    . = . + 0x4000;
>>> +    __stack_top = .;
>>> +}
>>



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-20 17:54     ` Leandro Lupori
@ 2022-04-20 18:18       ` Richard Henderson
  2022-04-20 18:38         ` Leandro Lupori
  0 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-04-20 18:18 UTC (permalink / raw)
  To: Leandro Lupori, Cédric Le Goater, qemu-devel, qemu-ppc
  Cc: danielhb413, groug, Nicholas Piggin, pbonzini, alex.bennee, david

On 4/20/22 10:54, Leandro Lupori wrote:
> On 4/18/22 17:22, Cédric Le Goater wrote:
> 
>>> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
>>> index 7a51fd0737..e1279c316c 100644
>>> --- a/semihosting/arm-compat-semi.c
>>> +++ b/semihosting/arm-compat-semi.c
>>> @@ -268,6 +268,31 @@ common_semi_sys_exit_extended(CPUState *cs, int nr)
>>>
>>>   #endif
>>>
>>> +#ifdef TARGET_PPC64
>>
>> This PPC ifdef in an ARM file seems wrong.
>>
>> The rest looks OK.
> 
> IIUC, arm-compat-semi.c is not an ARM specific file, but it's used by targets that 
> implement ARM-compatible semihosting. It's currently used by ARM and RISC-V and both use 
> target ifdefs in small parts of this file.

It would be nice to split these out to target/arch/arm-compat-semi.h or something akin.


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-19  9:26   ` Peter Maydell
@ 2022-04-20 18:20     ` Leandro Lupori
  2022-04-20 19:24       ` Peter Maydell
  0 siblings, 1 reply; 35+ messages in thread
From: Leandro Lupori @ 2022-04-20 18:20 UTC (permalink / raw)
  To: Peter Maydell
  Cc: danielhb413, richard.henderson, qemu-devel, groug, qemu-ppc, clg,
	pbonzini, alex.bennee, david

On 4/19/22 06:26, Peter Maydell wrote:
> On Mon, 18 Apr 2022 at 20:15, Leandro Lupori
> <leandro.lupori@eldorado.org.br> wrote:
>>
>> Add semihosting support for PPC64. This implementation is
>> based on the standard for ARM semihosting version 2.0, as
>> implemented by QEMU and documented in
>>
>>      https://github.com/ARM-software/abi-aa/releases
>>
>> The PPC64 specific differences are the following:
>>
>> Semihosting Trap Instruction: sc 7
>> Operation Number Register: r3
>> Parameter Register: r4
>> Return Register: r3
>> Data block field size: 64 bits
> 
> Where is the independent specification which defines that
> this is the ABI for PPC semihosting? You should provide the
> URL for that in a comment somewhere.
> 

AFAIK, there is no official PPC semihosting specification. Would it be 
ok to just document it somewhere else, e.g. GitHub, as an unofficial 
specification?

Thanks,
Leandro

> thanks
> -- PMM



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-20 18:05   ` Peter Maydell
@ 2022-04-20 18:30     ` Leandro Lupori
  0 siblings, 0 replies; 35+ messages in thread
From: Leandro Lupori @ 2022-04-20 18:30 UTC (permalink / raw)
  To: Peter Maydell
  Cc: danielhb413, richard.henderson, qemu-devel, groug, qemu-ppc, clg,
	pbonzini, alex.bennee, david

On 4/20/22 15:05, Peter Maydell wrote:
> [E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você possa confirmar o remetente e saber que o conteúdo é seguro. Em caso de e-mail suspeito entre imediatamente em contato com o DTI.
> 
> On Mon, 18 Apr 2022 at 20:15, Leandro Lupori
> <leandro.lupori@eldorado.org.br> wrote:
>>
>> Add semihosting support for PPC64. This implementation is
>> based on the standard for ARM semihosting version 2.0, as
>> implemented by QEMU and documented in
>>
>>      https://github.com/ARM-software/abi-aa/releases
>>
>> The PPC64 specific differences are the following:
>>
>> Semihosting Trap Instruction: sc 7
>> Operation Number Register: r3
>> Parameter Register: r4
>> Return Register: r3
>> Data block field size: 64 bits
>>
>> +static inline bool
>> +common_semi_sys_exit_extended(CPUState *cs, int nr)
>> +{
>> +    return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
>> +}
> 
> Does the PPC specification for semihosting really follow the
> legacy Arm requirement that the 32-bit version of the EXIT
> call doesn't let the caller specify the exit status? It's
> not a very sensible choice IMHO if you don't have the legacy
> baggage to deal with.
> 

As we are actually writing an unofficial PPC specification for 
semihosting now, I guess it makes sense to leave the legacy 32-bit 
version of EXIT out of it. I'll change this part to always return true.
By the way, this initial implementation supports only 64-bit PPC.

Thanks,
Leandro

> -- PMM



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-20 18:18       ` Richard Henderson
@ 2022-04-20 18:38         ` Leandro Lupori
  0 siblings, 0 replies; 35+ messages in thread
From: Leandro Lupori @ 2022-04-20 18:38 UTC (permalink / raw)
  To: Richard Henderson, Cédric Le Goater, qemu-devel, qemu-ppc
  Cc: danielhb413, groug, Nicholas Piggin, pbonzini, alex.bennee, david

On 4/20/22 15:18, Richard Henderson wrote:
> [E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você 
> possa confirmar o remetente e saber que o conteúdo é seguro. Em caso de 
> e-mail suspeito entre imediatamente em contato com o DTI.
> 
> On 4/20/22 10:54, Leandro Lupori wrote:
>> On 4/18/22 17:22, Cédric Le Goater wrote:
>>
>>>> diff --git a/semihosting/arm-compat-semi.c 
>>>> b/semihosting/arm-compat-semi.c
>>>> index 7a51fd0737..e1279c316c 100644
>>>> --- a/semihosting/arm-compat-semi.c
>>>> +++ b/semihosting/arm-compat-semi.c
>>>> @@ -268,6 +268,31 @@ common_semi_sys_exit_extended(CPUState *cs, int 
>>>> nr)
>>>>
>>>>   #endif
>>>>
>>>> +#ifdef TARGET_PPC64
>>>
>>> This PPC ifdef in an ARM file seems wrong.
>>>
>>> The rest looks OK.
>>
>> IIUC, arm-compat-semi.c is not an ARM specific file, but it's used by 
>> targets that
>> implement ARM-compatible semihosting. It's currently used by ARM and 
>> RISC-V and both use
>> target ifdefs in small parts of this file.
> 
> It would be nice to split these out to target/arch/arm-compat-semi.h or 
> something akin.
> 

Ah, ok, I'll try to move the target specific parts to their own header 
files then.

Thanks,
Leandro

> 
> r~



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le
  2022-04-18 23:36   ` Richard Henderson
@ 2022-04-20 18:42     ` Leandro Lupori
  0 siblings, 0 replies; 35+ messages in thread
From: Leandro Lupori @ 2022-04-20 18:42 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, qemu-ppc
  Cc: danielhb413, groug, clg, pbonzini, alex.bennee, david

On 4/18/22 20:36, Richard Henderson wrote:
> [E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você 
> possa confirmar o remetente e saber que o conteúdo é seguro. Em caso de 
> e-mail suspeito entre imediatamente em contato com o DTI.
> 
> On 4/18/22 12:10, Leandro Lupori wrote:
>> +static inline uint64_t sh_swap64(CPUArchState *env, uint64_t val)
>> +{
>> +    return msr_le ? val : tswap64(val);
>> +}
>> +
>> +static inline uint32_t sh_swap32(CPUArchState *env, uint32_t val)
>> +{
>> +    return msr_le ? val : tswap32(val);
>> +}
> 
> That doesn't work -- tswap itself is conditional.
> 
> You want
> 
>    return msr_le ? le32_to_cpu(x) : be32_to_cpu(x);
> 
> etc.  One of them will be a nop, and the other will bswap.
> 

Right, I'll make this change.

Thanks,
Leandro

> 
> r~



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-20 18:20     ` Leandro Lupori
@ 2022-04-20 19:24       ` Peter Maydell
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Maydell @ 2022-04-20 19:24 UTC (permalink / raw)
  To: Leandro Lupori
  Cc: danielhb413, richard.henderson, qemu-devel, groug, qemu-ppc, clg,
	pbonzini, alex.bennee, david

On Wed, 20 Apr 2022 at 19:20, Leandro Lupori
<leandro.lupori@eldorado.org.br> wrote:
>
> On 4/19/22 06:26, Peter Maydell wrote:
> > On Mon, 18 Apr 2022 at 20:15, Leandro Lupori
> > <leandro.lupori@eldorado.org.br> wrote:
> >>
> >> Add semihosting support for PPC64. This implementation is
> >> based on the standard for ARM semihosting version 2.0, as
> >> implemented by QEMU and documented in
> >>
> >>      https://github.com/ARM-software/abi-aa/releases
> >>
> >> The PPC64 specific differences are the following:
> >>
> >> Semihosting Trap Instruction: sc 7
> >> Operation Number Register: r3
> >> Parameter Register: r4
> >> Return Register: r3
> >> Data block field size: 64 bits
> >
> > Where is the independent specification which defines that
> > this is the ABI for PPC semihosting? You should provide the
> > URL for that in a comment somewhere.
> >
>
> AFAIK, there is no official PPC semihosting specification. Would it be
> ok to just document it somewhere else, e.g. GitHub, as an unofficial
> specification?

I'm going to push back on this in the same way I did for
the RISC-V folks. If this is an official PPC semihosting
specification, intended to be supported by multiple
different pieces of software, it needs to have an
independent spec document somewhere (even if that
spec document just cross-refers to the Arm spec for
most of the detail). If this is an ad-hoc "add this
thing for PPC in a purely QEMU-specific way" patchset,
then no, we shouldn't implement it.

Semihosting is an ABI, and when QEMU implements an ABI
it should be because it's an external pre-existing one.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le
  2022-04-18 19:10 ` [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le Leandro Lupori
  2022-04-18 23:36   ` Richard Henderson
@ 2022-04-20 19:42   ` Peter Maydell
  2022-04-20 19:52     ` Richard Henderson
  1 sibling, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2022-04-20 19:42 UTC (permalink / raw)
  To: Leandro Lupori
  Cc: danielhb413, richard.henderson, qemu-devel, groug, qemu-ppc, clg,
	pbonzini, alex.bennee, david

On Mon, 18 Apr 2022 at 20:19, Leandro Lupori
<leandro.lupori@eldorado.org.br> wrote:
>
> PPC64 CPUs can change its endian dynamically, so semihosting code
> must check its MSR at run time to determine if byte swapping is
> needed.

Arm CPUs also change endianness dynamically, so why is this
change PPC-specific ?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le
  2022-04-20 19:42   ` Peter Maydell
@ 2022-04-20 19:52     ` Richard Henderson
  2022-04-21  8:46       ` Alex Bennée
  2022-04-21  9:13       ` Peter Maydell
  0 siblings, 2 replies; 35+ messages in thread
From: Richard Henderson @ 2022-04-20 19:52 UTC (permalink / raw)
  To: Peter Maydell, Leandro Lupori
  Cc: danielhb413, qemu-devel, groug, qemu-ppc, clg, pbonzini,
	alex.bennee, david

On 4/20/22 12:42, Peter Maydell wrote:
> On Mon, 18 Apr 2022 at 20:19, Leandro Lupori
> <leandro.lupori@eldorado.org.br> wrote:
>>
>> PPC64 CPUs can change its endian dynamically, so semihosting code
>> must check its MSR at run time to determine if byte swapping is
>> needed.
> 
> Arm CPUs also change endianness dynamically, so why is this
> change PPC-specific ?

I'm reasonably certain that we simply don't test armbe or aarch64_be semihosting.  Leandro 
found this because qemu-system-ppc64 defaults to BE and qemu-system-aarch64 defaults to LE.


r~


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-20 18:09     ` Leandro Lupori
@ 2022-04-21  2:04       ` Nicholas Piggin
  2022-04-21  6:21         ` Cédric Le Goater
  2022-04-28  3:56         ` Nicholas Piggin
  2022-04-21  6:12       ` Cédric Le Goater
  1 sibling, 2 replies; 35+ messages in thread
From: Nicholas Piggin @ 2022-04-21  2:04 UTC (permalink / raw)
  To: Cédric Le Goater, Leandro Lupori, qemu-devel, qemu-ppc
  Cc: danielhb413, richard.henderson, groug, pbonzini, alex.bennee, david

Excerpts from Leandro Lupori's message of April 21, 2022 4:09 am:
> On 4/18/22 17:22, Cédric Le Goater wrote:
>> On 4/18/22 21:10, Leandro Lupori wrote:
>>> Add semihosting support for PPC64. This implementation is
>>> based on the standard for ARM semihosting version 2.0, as
>>> implemented by QEMU and documented in
>>>
>>>      https://github.com/ARM-software/abi-aa/releases
>>>
>>> The PPC64 specific differences are the following:
>>>
>>> Semihosting Trap Instruction: sc 7
>>> Operation Number Register: r3
>>> Parameter Register: r4
>>> Return Register: r3
>>> Data block field size: 64 bits
>> 
>> 'sc' is a good way to implement semi hosting but we should make sure
>> that it is not colliding with future extensions, at least with the
>> next POWERPC processor. Is that the case ? if not, then the lev could
>> be reserved.
>> 
> 
> Power ISA 3.1B says that LEV values greater that 2 are reserved.
> Level 2 is the ultravisor, so I assumed that level 7 was far enough from 
> current max level. I don't know if POWER11 will introduce new privilege 
> levels. Is this info publicly available somewhere? Or do you have a 
> better level in mind to use instead?

It's not available but there are no plans to use LEV=7.

It would be fine in practice I think, but it's kind of ugly and not 
great precedent -- how would we find out all the projects which use 
reserved instructions or values for something? Nominally the onus is on 
the software to accept breakage, but in reality important software that
breaks causes a headache for the ISA.

IBM's systemsim emulator actually has an instruction to call out to the 
emulator to do various things like IO. It uses the opcode

  .long 0x000eaeb0

That is the primary op 0 reserved space, and there is actually another 
op 'attn' or 'sp_attn' there which IBM CPUs implement, it is similar in 
spirit (it calls out to the service processor and/or chip error handling 
system to deal with a condition out-of-band). You don't want to use attn 
here because the core under emulation might implement it, I'm just 
noting the precedent with similar functionality under this primary 
opcode.

So I think the systemsim emulator instruction should be a good choice. 
But it should really be documented. I will bring this up at the Open 
Power ISA working group meeting next week and see what the options are 
with getting it formally allocated for semihosting emulators (or what 
the alternatives are).

Thanks,
Nick



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-20 18:09     ` Leandro Lupori
  2022-04-21  2:04       ` Nicholas Piggin
@ 2022-04-21  6:12       ` Cédric Le Goater
  1 sibling, 0 replies; 35+ messages in thread
From: Cédric Le Goater @ 2022-04-21  6:12 UTC (permalink / raw)
  To: Leandro Lupori, qemu-devel, qemu-ppc
  Cc: danielhb413, richard.henderson, groug, Nicholas Piggin, pbonzini,
	alex.bennee, david

>> I think the part adding POWERPC_EXCP_SEMIHOST should be proposed in
>> a separate patch.
> 
> Ok, I can move it to a separate patch. That would be all changes in 
> target/ppc/cpu.h and target/ppc/excp_helper.c, right?

yes.

Thanks,

C.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-21  2:04       ` Nicholas Piggin
@ 2022-04-21  6:21         ` Cédric Le Goater
  2022-04-21  9:56           ` Nicholas Piggin
  2022-04-28  3:56         ` Nicholas Piggin
  1 sibling, 1 reply; 35+ messages in thread
From: Cédric Le Goater @ 2022-04-21  6:21 UTC (permalink / raw)
  To: Nicholas Piggin, Leandro Lupori, qemu-devel, qemu-ppc
  Cc: danielhb413, richard.henderson, groug, pbonzini, alex.bennee, david

On 4/21/22 04:04, Nicholas Piggin wrote:
> Excerpts from Leandro Lupori's message of April 21, 2022 4:09 am:
>> On 4/18/22 17:22, Cédric Le Goater wrote:
>>> On 4/18/22 21:10, Leandro Lupori wrote:
>>>> Add semihosting support for PPC64. This implementation is
>>>> based on the standard for ARM semihosting version 2.0, as
>>>> implemented by QEMU and documented in
>>>>
>>>>       https://github.com/ARM-software/abi-aa/releases
>>>>
>>>> The PPC64 specific differences are the following:
>>>>
>>>> Semihosting Trap Instruction: sc 7
>>>> Operation Number Register: r3
>>>> Parameter Register: r4
>>>> Return Register: r3
>>>> Data block field size: 64 bits
>>>
>>> 'sc' is a good way to implement semi hosting but we should make sure
>>> that it is not colliding with future extensions, at least with the
>>> next POWERPC processor. Is that the case ? if not, then the lev could
>>> be reserved.
>>>
>>
>> Power ISA 3.1B says that LEV values greater that 2 are reserved.
>> Level 2 is the ultravisor, so I assumed that level 7 was far enough from
>> current max level. I don't know if POWER11 will introduce new privilege
>> levels. Is this info publicly available somewhere? Or do you have a
>> better level in mind to use instead?
> 
> It's not available but there are no plans to use LEV=7.
> 
> It would be fine in practice I think, but it's kind of ugly and not
> great precedent -- how would we find out all the projects which use
> reserved instructions or values for something? Nominally the onus is on
> the software to accept breakage, but in reality important software that
> breaks causes a headache for the ISA.
> 
> IBM's systemsim emulator actually has an instruction to call out to the
> emulator to do various things like IO. It uses the opcode
> 
>    .long 0x000eaeb0
> 
> That is the primary op 0 reserved space, and there is actually another
> op 'attn' or 'sp_attn' there which IBM CPUs implement, it is similar in
> spirit (it calls out to the service processor and/or chip error handling
> system to deal with a condition out-of-band). You don't want to use attn
> here because the core under emulation might implement it, I'm just
> noting the precedent with similar functionality under this primary
> opcode.
> 
> So I think the systemsim emulator instruction should be a good choice.

yeah. It's not a major change.

> But it should really be documented. I will bring this up at the Open
> Power ISA working group meeting next week and see what the options are
> with getting it formally allocated for semihosting emulators (or what
> the alternatives are).

It would be nice to invite Leandro to this meeting since he started
implementing.

Thanks,

C.



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le
  2022-04-20 19:52     ` Richard Henderson
@ 2022-04-21  8:46       ` Alex Bennée
  2022-04-21  9:13       ` Peter Maydell
  1 sibling, 0 replies; 35+ messages in thread
From: Alex Bennée @ 2022-04-21  8:46 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Peter Maydell, Leandro Lupori, danielhb413, qemu-devel, groug,
	qemu-ppc, clg, pbonzini, david


Richard Henderson <richard.henderson@linaro.org> writes:

> On 4/20/22 12:42, Peter Maydell wrote:
>> On Mon, 18 Apr 2022 at 20:19, Leandro Lupori
>> <leandro.lupori@eldorado.org.br> wrote:
>>>
>>> PPC64 CPUs can change its endian dynamically, so semihosting code
>>> must check its MSR at run time to determine if byte swapping is
>>> needed.
>> Arm CPUs also change endianness dynamically, so why is this
>> change PPC-specific ?
>
> I'm reasonably certain that we simply don't test armbe or aarch64_be
> semihosting.  Leandro found this because qemu-system-ppc64 defaults to
> BE and qemu-system-aarch64 defaults to LE.

Maybe it is time to have a generic endianess variable in CPUState so we
can avoid having arch specific hacks in the semihosting code. That said
is endianess binary? I seem to recall on ARM the instruction stream is
always in one endianess so it only really affects CPU data loads and
stores. Is it the same for PPC?


>
>
> r~


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le
  2022-04-20 19:52     ` Richard Henderson
  2022-04-21  8:46       ` Alex Bennée
@ 2022-04-21  9:13       ` Peter Maydell
  2022-04-21  9:43         ` Alex Bennée
  1 sibling, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2022-04-21  9:13 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Leandro Lupori, danielhb413, qemu-devel, groug, qemu-ppc, clg,
	pbonzini, alex.bennee, david

On Wed, 20 Apr 2022 at 20:52, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/20/22 12:42, Peter Maydell wrote:
> > On Mon, 18 Apr 2022 at 20:19, Leandro Lupori
> > <leandro.lupori@eldorado.org.br> wrote:
> >>
> >> PPC64 CPUs can change its endian dynamically, so semihosting code
> >> must check its MSR at run time to determine if byte swapping is
> >> needed.
> >
> > Arm CPUs also change endianness dynamically, so why is this
> > change PPC-specific ?
>
> I'm reasonably certain that we simply don't test armbe or aarch64_be semihosting.  Leandro
> found this because qemu-system-ppc64 defaults to BE and qemu-system-aarch64 defaults to LE.

Right, so if there's an existing bug here on arm we should fix that,
and that probably means that the abstraction split between
"arch-specific thing" and "non-arch-specific code" is different
from "PPC just overrides the entire swap function".

-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le
  2022-04-21  9:13       ` Peter Maydell
@ 2022-04-21  9:43         ` Alex Bennée
  2022-04-21 10:01           ` Peter Maydell
  0 siblings, 1 reply; 35+ messages in thread
From: Alex Bennée @ 2022-04-21  9:43 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Leandro Lupori, danielhb413, Richard Henderson, qemu-devel,
	groug, qemu-ppc, clg, pbonzini, david


Peter Maydell <peter.maydell@linaro.org> writes:

> On Wed, 20 Apr 2022 at 20:52, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 4/20/22 12:42, Peter Maydell wrote:
>> > On Mon, 18 Apr 2022 at 20:19, Leandro Lupori
>> > <leandro.lupori@eldorado.org.br> wrote:
>> >>
>> >> PPC64 CPUs can change its endian dynamically, so semihosting code
>> >> must check its MSR at run time to determine if byte swapping is
>> >> needed.
>> >
>> > Arm CPUs also change endianness dynamically, so why is this
>> > change PPC-specific ?
>>
>> I'm reasonably certain that we simply don't test armbe or aarch64_be semihosting.  Leandro
>> found this because qemu-system-ppc64 defaults to BE and qemu-system-aarch64 defaults to LE.
>
> Right, so if there's an existing bug here on arm we should fix that,
> and that probably means that the abstraction split between
> "arch-specific thing" and "non-arch-specific code" is different
> from "PPC just overrides the entire swap function".

I think the helper function cpu_virtio_is_big_endian is really just a
proxy for the data endianess mode of the guest. Perhaps it could be
re-named and then used by the semihosting code?

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-21  6:21         ` Cédric Le Goater
@ 2022-04-21  9:56           ` Nicholas Piggin
  0 siblings, 0 replies; 35+ messages in thread
From: Nicholas Piggin @ 2022-04-21  9:56 UTC (permalink / raw)
  To: Cédric Le Goater, Leandro Lupori, qemu-devel, qemu-ppc
  Cc: danielhb413, richard.henderson, groug, pbonzini, alex.bennee, david

Excerpts from Cédric Le Goater's message of April 21, 2022 4:21 pm:
> On 4/21/22 04:04, Nicholas Piggin wrote:
>> Excerpts from Leandro Lupori's message of April 21, 2022 4:09 am:
>>> On 4/18/22 17:22, Cédric Le Goater wrote:
>>>> On 4/18/22 21:10, Leandro Lupori wrote:
>>>>> Add semihosting support for PPC64. This implementation is
>>>>> based on the standard for ARM semihosting version 2.0, as
>>>>> implemented by QEMU and documented in
>>>>>
>>>>>       https://github.com/ARM-software/abi-aa/releases
>>>>>
>>>>> The PPC64 specific differences are the following:
>>>>>
>>>>> Semihosting Trap Instruction: sc 7
>>>>> Operation Number Register: r3
>>>>> Parameter Register: r4
>>>>> Return Register: r3
>>>>> Data block field size: 64 bits
>>>>
>>>> 'sc' is a good way to implement semi hosting but we should make sure
>>>> that it is not colliding with future extensions, at least with the
>>>> next POWERPC processor. Is that the case ? if not, then the lev could
>>>> be reserved.
>>>>
>>>
>>> Power ISA 3.1B says that LEV values greater that 2 are reserved.
>>> Level 2 is the ultravisor, so I assumed that level 7 was far enough from
>>> current max level. I don't know if POWER11 will introduce new privilege
>>> levels. Is this info publicly available somewhere? Or do you have a
>>> better level in mind to use instead?
>> 
>> It's not available but there are no plans to use LEV=7.
>> 
>> It would be fine in practice I think, but it's kind of ugly and not
>> great precedent -- how would we find out all the projects which use
>> reserved instructions or values for something? Nominally the onus is on
>> the software to accept breakage, but in reality important software that
>> breaks causes a headache for the ISA.
>> 
>> IBM's systemsim emulator actually has an instruction to call out to the
>> emulator to do various things like IO. It uses the opcode
>> 
>>    .long 0x000eaeb0
>> 
>> That is the primary op 0 reserved space, and there is actually another
>> op 'attn' or 'sp_attn' there which IBM CPUs implement, it is similar in
>> spirit (it calls out to the service processor and/or chip error handling
>> system to deal with a condition out-of-band). You don't want to use attn
>> here because the core under emulation might implement it, I'm just
>> noting the precedent with similar functionality under this primary
>> opcode.
>> 
>> So I think the systemsim emulator instruction should be a good choice.
> 
> yeah. It's not a major change.
> 
>> But it should really be documented. I will bring this up at the Open
>> Power ISA working group meeting next week and see what the options are
>> with getting it formally allocated for semihosting emulators (or what
>> the alternatives are).
> 
> It would be nice to invite Leandro to this meeting since he started
> implementing.

Good point. I'll organize with him offline.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le
  2022-04-21  9:43         ` Alex Bennée
@ 2022-04-21 10:01           ` Peter Maydell
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Maydell @ 2022-04-21 10:01 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Leandro Lupori, danielhb413, Richard Henderson, qemu-devel,
	groug, qemu-ppc, clg, pbonzini, david

On Thu, 21 Apr 2022 at 10:44, Alex Bennée <alex.bennee@linaro.org> wrote:
> I think the helper function cpu_virtio_is_big_endian is really just a
> proxy for the data endianess mode of the guest. Perhaps it could be
> re-named and then used by the semihosting code?

We specifically named and documented that as "don't use this unless
you're the silly legacy virtio devices":

    /**
     * @virtio_is_big_endian: Callback to return %true if a CPU which supports
     * runtime configurable endianness is currently big-endian.
     * Non-configurable CPUs can use the default implementation of this method.
     * This method should not be used by any callers other than the pre-1.0
     * virtio devices.
     */

I think you're correct that it is also the right thing for semihosting,
but we should be a bit careful with the naming and commenting so we
can retain the "this is the wrong thing for most situations and
definitely not something to be calling in device model code" information
for developers and code reviewers.

-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-21  2:04       ` Nicholas Piggin
  2022-04-21  6:21         ` Cédric Le Goater
@ 2022-04-28  3:56         ` Nicholas Piggin
  2022-05-03 15:50           ` Fabiano Rosas
  1 sibling, 1 reply; 35+ messages in thread
From: Nicholas Piggin @ 2022-04-28  3:56 UTC (permalink / raw)
  To: Cédric Le Goater, Leandro Lupori, qemu-devel, qemu-ppc
  Cc: danielhb413, richard.henderson, groug, pbonzini, alex.bennee, david

Excerpts from Nicholas Piggin's message of April 21, 2022 12:04 pm:
> Excerpts from Leandro Lupori's message of April 21, 2022 4:09 am:
>> On 4/18/22 17:22, Cédric Le Goater wrote:
>>> On 4/18/22 21:10, Leandro Lupori wrote:
>>>> Add semihosting support for PPC64. This implementation is
>>>> based on the standard for ARM semihosting version 2.0, as
>>>> implemented by QEMU and documented in
>>>>
>>>>      https://github.com/ARM-software/abi-aa/releases
>>>>
>>>> The PPC64 specific differences are the following:
>>>>
>>>> Semihosting Trap Instruction: sc 7
>>>> Operation Number Register: r3
>>>> Parameter Register: r4
>>>> Return Register: r3
>>>> Data block field size: 64 bits
>>> 
>>> 'sc' is a good way to implement semi hosting but we should make sure
>>> that it is not colliding with future extensions, at least with the
>>> next POWERPC processor. Is that the case ? if not, then the lev could
>>> be reserved.
>>> 
>> 
>> Power ISA 3.1B says that LEV values greater that 2 are reserved.
>> Level 2 is the ultravisor, so I assumed that level 7 was far enough from 
>> current max level. I don't know if POWER11 will introduce new privilege 
>> levels. Is this info publicly available somewhere? Or do you have a 
>> better level in mind to use instead?
> 
> It's not available but there are no plans to use LEV=7.
> 
> It would be fine in practice I think, but it's kind of ugly and not 
> great precedent -- how would we find out all the projects which use 
> reserved instructions or values for something? Nominally the onus is on 
> the software to accept breakage, but in reality important software that
> breaks causes a headache for the ISA.
> 
> IBM's systemsim emulator actually has an instruction to call out to the 
> emulator to do various things like IO. It uses the opcode
> 
>   .long 0x000eaeb0
> 
> That is the primary op 0 reserved space, and there is actually another 
> op 'attn' or 'sp_attn' there which IBM CPUs implement, it is similar in 
> spirit (it calls out to the service processor and/or chip error handling 
> system to deal with a condition out-of-band). You don't want to use attn 
> here because the core under emulation might implement it, I'm just 
> noting the precedent with similar functionality under this primary 
> opcode.
> 
> So I think the systemsim emulator instruction should be a good choice. 
> But it should really be documented. I will bring this up at the Open 
> Power ISA working group meeting next week and see what the options are 
> with getting it formally allocated for semihosting emulators (or what 
> the alternatives are).

Update on the ISA TWG meeting

Semihosting was well received, the idea is not so new so I think it was
easily understood by attendees.

There were no objections to allocating a new opcode for this purpose.
The preference was a new opcode rather than using a reserved sc LEV
value.

The primary opcode 0 space is possibly unsuitable because it is said
to be "allocated to specific purposes that are outside the scope of the
Power ISA." whereas I think we want a first class instruction for this,
it may have implementation-dependent behaviour but on processors that
do not implement it, we would like it to have well-defined behaviour.

So we can probably just pick an opcode and submit a patch RFC to the
ISA (I can try help with that). First, there are a few questions to
resolve:

- What behaviour do we want for CPUs which do not implement it or
  disable it? E.g., no-op or illegal instruction interrupt. Ideally
  we would choose an opcode such that the architecture is compatible
  with existing CPUs.

- Would it be useful for KVM to implement semihosting support for
  guests on hard processors?

- Is there value in an endian-agnostic instruction? (Assuming we can
  find one). This question only comes to me because our BMC gdbserver
  for debugging the host CPUs implements breakpoints by inserting an
  'attn' instruction in the host code, and that does not work if the
  host switches endian. Any possibility the semihosting instruction
  would ever be injected out-of-band? Seems not so likely.

There were also some thoughts about bringing the semihosting spec
under the Open Power group but that's outside the scope of the ISA
group. This may be a possibility we could consider but I think for
now it should be enough to document it like riscv and put it
somewhere (even in the QEMU tree should be okay for now IMO).

Thanks,
Nick


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-04-28  3:56         ` Nicholas Piggin
@ 2022-05-03 15:50           ` Fabiano Rosas
  2022-05-10  9:41             ` Nicholas Piggin
  0 siblings, 1 reply; 35+ messages in thread
From: Fabiano Rosas @ 2022-05-03 15:50 UTC (permalink / raw)
  To: Nicholas Piggin, Cédric Le Goater, Leandro Lupori,
	qemu-devel, qemu-ppc
  Cc: danielhb413, richard.henderson, groug, pbonzini, alex.bennee, david

Nicholas Piggin <npiggin@gmail.com> writes:

> Excerpts from Nicholas Piggin's message of April 21, 2022 12:04 pm:
>> Excerpts from Leandro Lupori's message of April 21, 2022 4:09 am:
>>> On 4/18/22 17:22, Cédric Le Goater wrote:
>>>> On 4/18/22 21:10, Leandro Lupori wrote:
>>>>> Add semihosting support for PPC64. This implementation is
>>>>> based on the standard for ARM semihosting version 2.0, as
>>>>> implemented by QEMU and documented in
>>>>>
>>>>>      https://github.com/ARM-software/abi-aa/releases
>>>>>
>>>>> The PPC64 specific differences are the following:
>>>>>
>>>>> Semihosting Trap Instruction: sc 7
>>>>> Operation Number Register: r3
>>>>> Parameter Register: r4
>>>>> Return Register: r3
>>>>> Data block field size: 64 bits
>>>> 
>>>> 'sc' is a good way to implement semi hosting but we should make sure
>>>> that it is not colliding with future extensions, at least with the
>>>> next POWERPC processor. Is that the case ? if not, then the lev could
>>>> be reserved.
>>>> 
>>> 
>>> Power ISA 3.1B says that LEV values greater that 2 are reserved.
>>> Level 2 is the ultravisor, so I assumed that level 7 was far enough from 
>>> current max level. I don't know if POWER11 will introduce new privilege 
>>> levels. Is this info publicly available somewhere? Or do you have a 
>>> better level in mind to use instead?
>> 
>> It's not available but there are no plans to use LEV=7.
>> 
>> It would be fine in practice I think, but it's kind of ugly and not 
>> great precedent -- how would we find out all the projects which use 
>> reserved instructions or values for something? Nominally the onus is on 
>> the software to accept breakage, but in reality important software that
>> breaks causes a headache for the ISA.
>> 
>> IBM's systemsim emulator actually has an instruction to call out to the 
>> emulator to do various things like IO. It uses the opcode
>> 
>>   .long 0x000eaeb0
>> 
>> That is the primary op 0 reserved space, and there is actually another 
>> op 'attn' or 'sp_attn' there which IBM CPUs implement, it is similar in 
>> spirit (it calls out to the service processor and/or chip error handling 
>> system to deal with a condition out-of-band). You don't want to use attn 
>> here because the core under emulation might implement it, I'm just 
>> noting the precedent with similar functionality under this primary 
>> opcode.
>> 
>> So I think the systemsim emulator instruction should be a good choice. 
>> But it should really be documented. I will bring this up at the Open 
>> Power ISA working group meeting next week and see what the options are 
>> with getting it formally allocated for semihosting emulators (or what 
>> the alternatives are).
>
> Update on the ISA TWG meeting
>
> Semihosting was well received, the idea is not so new so I think it was
> easily understood by attendees.
>
> There were no objections to allocating a new opcode for this purpose.
> The preference was a new opcode rather than using a reserved sc LEV
> value.
>
> The primary opcode 0 space is possibly unsuitable because it is said
> to be "allocated to specific purposes that are outside the scope of the
> Power ISA." whereas I think we want a first class instruction for this,
> it may have implementation-dependent behaviour but on processors that
> do not implement it, we would like it to have well-defined behaviour.
>
> So we can probably just pick an opcode and submit a patch RFC to the
> ISA (I can try help with that). First, there are a few questions to
> resolve:
>
> - What behaviour do we want for CPUs which do not implement it or
>   disable it? E.g., no-op or illegal instruction interrupt. Ideally
>   we would choose an opcode such that the architecture is compatible
>   with existing CPUs.

I think that since semihosting is not a one-shot thing it would be
better to have it trap so that the "host" could remediate in some
way. Or even carry on with servicing the semihosting anyway.

> - Would it be useful for KVM to implement semihosting support for
>   guests on hard processors?

Are there any undesirable implications to using an instruction that
traps to the HV? I'd say let's get it if we can but otherwise it's not a
big deal.

I had two use-cases in mind:

1) Improving our interactions with gdbstub and having the guest use
   sys_exit to report some debugging events like watchpoints or
   singlestep;

2) Bootstrapping with KVM. We had instances in the past of needing to
   write guest code from scratch and having to rely solely in GDB for a
   while before setting up the console.

But I realise that these use-cases conflate semihosting with general
debugging and regular PAPR features, respectively. So it might be a
stretch.

> - Is there value in an endian-agnostic instruction? (Assuming we can
>   find one). This question only comes to me because our BMC gdbserver
>   for debugging the host CPUs implements breakpoints by inserting an
>   'attn' instruction in the host code, and that does not work if the
>   host switches endian. Any possibility the semihosting instruction
>   would ever be injected out-of-band? Seems not so likely.

Semihosting requires some sort of register setup, I don't think having
it done out-of-band would be practical. So we possibly don't need an
endian-agnostic instruction.

> There were also some thoughts about bringing the semihosting spec
> under the Open Power group but that's outside the scope of the ISA
> group. This may be a possibility we could consider but I think for
> now it should be enough to document it like riscv and put it
> somewhere (even in the QEMU tree should be okay for now IMO).
>
> Thanks,
> Nick


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [RFC PATCH v3 1/5] ppc64: Add semihosting support
  2022-05-03 15:50           ` Fabiano Rosas
@ 2022-05-10  9:41             ` Nicholas Piggin
  0 siblings, 0 replies; 35+ messages in thread
From: Nicholas Piggin @ 2022-05-10  9:41 UTC (permalink / raw)
  To: Cédric Le Goater, Fabiano Rosas, Leandro Lupori, qemu-devel,
	qemu-ppc
  Cc: alex.bennee, danielhb413, david, groug, pbonzini, richard.henderson

Excerpts from Fabiano Rosas's message of May 4, 2022 1:50 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
> 
>> Excerpts from Nicholas Piggin's message of April 21, 2022 12:04 pm:
>>> Excerpts from Leandro Lupori's message of April 21, 2022 4:09 am:
>>>> On 4/18/22 17:22, Cédric Le Goater wrote:
>>>>> On 4/18/22 21:10, Leandro Lupori wrote:
>>>>>> Add semihosting support for PPC64. This implementation is
>>>>>> based on the standard for ARM semihosting version 2.0, as
>>>>>> implemented by QEMU and documented in
>>>>>>
>>>>>>      https://github.com/ARM-software/abi-aa/releases
>>>>>>
>>>>>> The PPC64 specific differences are the following:
>>>>>>
>>>>>> Semihosting Trap Instruction: sc 7
>>>>>> Operation Number Register: r3
>>>>>> Parameter Register: r4
>>>>>> Return Register: r3
>>>>>> Data block field size: 64 bits
>>>>> 
>>>>> 'sc' is a good way to implement semi hosting but we should make sure
>>>>> that it is not colliding with future extensions, at least with the
>>>>> next POWERPC processor. Is that the case ? if not, then the lev could
>>>>> be reserved.
>>>>> 
>>>> 
>>>> Power ISA 3.1B says that LEV values greater that 2 are reserved.
>>>> Level 2 is the ultravisor, so I assumed that level 7 was far enough from 
>>>> current max level. I don't know if POWER11 will introduce new privilege 
>>>> levels. Is this info publicly available somewhere? Or do you have a 
>>>> better level in mind to use instead?
>>> 
>>> It's not available but there are no plans to use LEV=7.
>>> 
>>> It would be fine in practice I think, but it's kind of ugly and not 
>>> great precedent -- how would we find out all the projects which use 
>>> reserved instructions or values for something? Nominally the onus is on 
>>> the software to accept breakage, but in reality important software that
>>> breaks causes a headache for the ISA.
>>> 
>>> IBM's systemsim emulator actually has an instruction to call out to the 
>>> emulator to do various things like IO. It uses the opcode
>>> 
>>>   .long 0x000eaeb0
>>> 
>>> That is the primary op 0 reserved space, and there is actually another 
>>> op 'attn' or 'sp_attn' there which IBM CPUs implement, it is similar in 
>>> spirit (it calls out to the service processor and/or chip error handling 
>>> system to deal with a condition out-of-band). You don't want to use attn 
>>> here because the core under emulation might implement it, I'm just 
>>> noting the precedent with similar functionality under this primary 
>>> opcode.
>>> 
>>> So I think the systemsim emulator instruction should be a good choice. 
>>> But it should really be documented. I will bring this up at the Open 
>>> Power ISA working group meeting next week and see what the options are 
>>> with getting it formally allocated for semihosting emulators (or what 
>>> the alternatives are).
>>
>> Update on the ISA TWG meeting
>>
>> Semihosting was well received, the idea is not so new so I think it was
>> easily understood by attendees.
>>
>> There were no objections to allocating a new opcode for this purpose.
>> The preference was a new opcode rather than using a reserved sc LEV
>> value.
>>
>> The primary opcode 0 space is possibly unsuitable because it is said
>> to be "allocated to specific purposes that are outside the scope of the
>> Power ISA." whereas I think we want a first class instruction for this,
>> it may have implementation-dependent behaviour but on processors that
>> do not implement it, we would like it to have well-defined behaviour.
>>
>> So we can probably just pick an opcode and submit a patch RFC to the
>> ISA (I can try help with that). First, there are a few questions to
>> resolve:
>>
>> - What behaviour do we want for CPUs which do not implement it or
>>   disable it? E.g., no-op or illegal instruction interrupt. Ideally
>>   we would choose an opcode such that the architecture is compatible
>>   with existing CPUs.
> 
> I think that since semihosting is not a one-shot thing it would be
> better to have it trap so that the "host" could remediate in some
> way. Or even carry on with servicing the semihosting anyway.

Yeah I think I agree, and semihosting code that is intended to compile
away or be dynamically disableable can implement that itself if
needed.

> 
>> - Would it be useful for KVM to implement semihosting support for
>>   guests on hard processors?
> 
> Are there any undesirable implications to using an instruction that
> traps to the HV? I'd say let's get it if we can but otherwise it's not a
> big deal.
> 
> I had two use-cases in mind:
> 
> 1) Improving our interactions with gdbstub and having the guest use
>    sys_exit to report some debugging events like watchpoints or
>    singlestep;
> 
> 2) Bootstrapping with KVM. We had instances in the past of needing to
>    write guest code from scratch and having to rely solely in GDB for a
>    while before setting up the console.
> 
> But I realise that these use-cases conflate semihosting with general
> debugging and regular PAPR features, respectively. So it might be a
> stretch.

I don't see downsides actually. A userspace can already induce some HV
interrupts.

So we can use any unallocated opcode, and in the specification we can 
say processors which do not support the semihosting facility
should treat it as an illegal instruction. That means the spec is 
backward compatible. It also gives you the trap to HV with HEAI 
interrupt on existing processors so KVM can implement it for guests.

Maybe it's as simple as that?

> 
>> - Is there value in an endian-agnostic instruction? (Assuming we can
>>   find one). This question only comes to me because our BMC gdbserver
>>   for debugging the host CPUs implements breakpoints by inserting an
>>   'attn' instruction in the host code, and that does not work if the
>>   host switches endian. Any possibility the semihosting instruction
>>   would ever be injected out-of-band? Seems not so likely.
> 
> Semihosting requires some sort of register setup, I don't think having
> it done out-of-band would be practical. So we possibly don't need an
> endian-agnostic instruction.

Okay.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2022-05-10 10:06 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-18 19:10 [RFC PATCH v3 0/5] Port PPC64/PowerNV MMU tests to QEMU Leandro Lupori
2022-04-18 19:10 ` [RFC PATCH v3 1/5] ppc64: Add semihosting support Leandro Lupori
2022-04-18 20:22   ` Cédric Le Goater
2022-04-20 17:54     ` Leandro Lupori
2022-04-20 18:18       ` Richard Henderson
2022-04-20 18:38         ` Leandro Lupori
2022-04-20 18:09     ` Leandro Lupori
2022-04-21  2:04       ` Nicholas Piggin
2022-04-21  6:21         ` Cédric Le Goater
2022-04-21  9:56           ` Nicholas Piggin
2022-04-28  3:56         ` Nicholas Piggin
2022-05-03 15:50           ` Fabiano Rosas
2022-05-10  9:41             ` Nicholas Piggin
2022-04-21  6:12       ` Cédric Le Goater
2022-04-18 23:33   ` Richard Henderson
2022-04-19  9:26   ` Peter Maydell
2022-04-20 18:20     ` Leandro Lupori
2022-04-20 19:24       ` Peter Maydell
2022-04-20 18:05   ` Peter Maydell
2022-04-20 18:30     ` Leandro Lupori
2022-04-18 19:10 ` [RFC PATCH v3 2/5] ppc64: Fix semihosting on ppc64le Leandro Lupori
2022-04-18 23:36   ` Richard Henderson
2022-04-20 18:42     ` Leandro Lupori
2022-04-20 19:42   ` Peter Maydell
2022-04-20 19:52     ` Richard Henderson
2022-04-21  8:46       ` Alex Bennée
2022-04-21  9:13       ` Peter Maydell
2022-04-21  9:43         ` Alex Bennée
2022-04-21 10:01           ` Peter Maydell
2022-04-18 19:10 ` [RFC PATCH v3 3/5] tests/tcg/ppc64: Add basic softmmu test support Leandro Lupori
2022-04-18 20:27   ` Cédric Le Goater
2022-04-18 21:52     ` Daniel Henrique Barboza
2022-04-20 18:13       ` Leandro Lupori
2022-04-18 19:10 ` [RFC PATCH v3 4/5] tests/tcg/ppc64: Add MMU test sources Leandro Lupori
2022-04-18 19:11 ` [RFC PATCH v3 5/5] tests/tcg/ppc64: Build PowerNV and LE tests Leandro Lupori

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.