Re: [kvm-unit-tests PATCH 14/32] powerpc: general interrupt tests

Thomas Huth <thuth@xxxxxxxxxx> · Fri, 1 Mar 2024 13:41:22 +0100

On 26/02/2024 11.12, Nicholas Piggin wrote:
Add basic testing of various kinds of interrupts, machine check,
page fault, illegal, decrementer, trace, syscall, etc.

This has a known failure on QEMU TCG pseries machines where MSR[ME]
can be incorrectly set to 0.

Two questions out of curiosity:

Any chance that this could be fixed easily in QEMU?

Or is there a way to detect TCG from within the test? (for example, we have 
a host_is_tcg() function for s390x so we can e.g. use report_xfail() for 
tests that are known to fail on TCG there)

Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx>
---
  lib/powerpc/asm/processor.h |   4 +
  lib/powerpc/asm/reg.h       |  17 ++
  lib/powerpc/setup.c         |  11 +
  lib/ppc64/asm/ptrace.h      |  16 ++
  powerpc/Makefile.common     |   3 +-
  powerpc/interrupts.c        | 415 ++++++++++++++++++++++++++++++++++++
  powerpc/unittests.cfg       |   3 +
  7 files changed, 468 insertions(+), 1 deletion(-)
  create mode 100644 powerpc/interrupts.c

diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index cf1b9d8ff..eed37d1f4 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -11,7 +11,11 @@ void do_handle_exception(struct pt_regs *regs);
  #endif /* __ASSEMBLY__ */
  
  extern bool cpu_has_hv;
+extern bool cpu_has_power_mce;
+extern bool cpu_has_siar;
  extern bool cpu_has_heai;
+extern bool cpu_has_prefix;
+extern bool cpu_has_sc_lev;
  
  static inline uint64_t mfspr(int nr)
  {
diff --git a/lib/powerpc/asm/reg.h b/lib/powerpc/asm/reg.h
index 782e75527..d6097f48f 100644
--- a/lib/powerpc/asm/reg.h
+++ b/lib/powerpc/asm/reg.h
@@ -5,8 +5,15 @@
  
  #define UL(x) _AC(x, UL)
  
+#define SPR_DSISR	0x012
+#define SPR_DAR		0x013
+#define SPR_DEC		0x016
  #define SPR_SRR0	0x01a
  #define SPR_SRR1	0x01b
+#define   SRR1_PREFIX		UL(0x20000000)
+#define SPR_FSCR	0x099
+#define   FSCR_PREFIX		UL(0x2000)
+#define SPR_HFSCR	0x0be
  #define SPR_TB		0x10c
  #define SPR_SPRG0	0x110
  #define SPR_SPRG1	0x111
@@ -22,12 +29,17 @@
  #define   PVR_VER_POWER8	UL(0x004d0000)
  #define   PVR_VER_POWER9	UL(0x004e0000)
  #define   PVR_VER_POWER10	UL(0x00800000)
+#define SPR_HDEC	0x136
  #define SPR_HSRR0	0x13a
  #define SPR_HSRR1	0x13b
+#define SPR_LPCR	0x13e
+#define   LPCR_HDICE		UL(0x1)
+#define SPR_HEIR	0x153
  #define SPR_MMCR0	0x31b
  #define   MMCR0_FC		UL(0x80000000)
  #define   MMCR0_PMAE		UL(0x04000000)
  #define   MMCR0_PMAO		UL(0x00000080)
+#define SPR_SIAR	0x31c
  
  /* Machine State Register definitions: */
  #define MSR_LE_BIT	0
@@ -35,6 +47,11 @@
  #define MSR_HV_BIT	60			/* Hypervisor mode */
  #define MSR_SF_BIT	63			/* 64-bit mode */
  
+#define MSR_DR		UL(0x0010)
+#define MSR_IR		UL(0x0020)
+#define MSR_BE		UL(0x0200)		/* Branch Trace Enable */
+#define MSR_SE		UL(0x0400)		/* Single Step Enable */
+#define MSR_EE		UL(0x8000)
  #define MSR_ME		UL(0x1000)
  
  #endif
diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index 3c81aee9e..9b665f59c 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -87,7 +87,11 @@ static void cpu_set(int fdtnode, u64 regval, void *info)
  }
  
  bool cpu_has_hv;
+bool cpu_has_power_mce; /* POWER CPU machine checks */
+bool cpu_has_siar;
  bool cpu_has_heai;
+bool cpu_has_prefix;
+bool cpu_has_sc_lev; /* sc interrupt has LEV field in SRR1 */
  
  static void cpu_init(void)
  {
@@ -112,15 +116,22 @@ static void cpu_init(void)
  
  	switch (mfspr(SPR_PVR) & PVR_VERSION_MASK) {
  	case PVR_VER_POWER10:
+		cpu_has_prefix = true;
+		cpu_has_sc_lev = true;
  	case PVR_VER_POWER9:
  	case PVR_VER_POWER8E:
  	case PVR_VER_POWER8NVL:
  	case PVR_VER_POWER8:
+		cpu_has_power_mce = true;
  		cpu_has_heai = true;
+		cpu_has_siar = true;
  		break;
  	default:
  		break;
  	}
+
+	if (!cpu_has_hv) /* HEIR is HV register */
+		cpu_has_heai = false;
  }
  
  static void mem_init(phys_addr_t freemem_start)
diff --git a/lib/ppc64/asm/ptrace.h b/lib/ppc64/asm/ptrace.h
index 12de7499b..db263a59e 100644
--- a/lib/ppc64/asm/ptrace.h
+++ b/lib/ppc64/asm/ptrace.h
@@ -5,6 +5,9 @@
  #define STACK_FRAME_OVERHEAD    112     /* size of minimum stack frame */
  
  #ifndef __ASSEMBLY__
+
+#include <asm/reg.h>
+
  struct pt_regs {
  	unsigned long gpr[32];
  	unsigned long nip;
@@ -17,6 +20,19 @@ struct pt_regs {
  	unsigned long _pad; /* stack must be 16-byte aligned */
  };
  
+static inline bool regs_is_prefix(volatile struct pt_regs *regs)
+{
+	return regs->msr & SRR1_PREFIX;
+}
+
+static inline void regs_advance_insn(struct pt_regs *regs)
+{
+	if (regs_is_prefix(regs))
+		regs->nip += 8;
+	else
+		regs->nip += 4;
+}
+
  #define STACK_INT_FRAME_SIZE    (sizeof(struct pt_regs) + \
  				 STACK_FRAME_OVERHEAD + KERNEL_REDZONE_SIZE)
  
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 1e181da69..68165fc25 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -12,7 +12,8 @@ tests-common = \
  	$(TEST_DIR)/rtas.elf \
  	$(TEST_DIR)/emulator.elf \
  	$(TEST_DIR)/tm.elf \
-	$(TEST_DIR)/sprs.elf
+	$(TEST_DIR)/sprs.elf \
+	$(TEST_DIR)/interrupts.elf
  
  tests-all = $(tests-common) $(tests)
  all: directories $(TEST_DIR)/boot_rom.bin $(tests-all)
diff --git a/powerpc/interrupts.c b/powerpc/interrupts.c
new file mode 100644
index 000000000..442f8c569
--- /dev/null
+++ b/powerpc/interrupts.c
@@ -0,0 +1,415 @@
+/*
+ * Test interrupts
+ *
+ * Copyright 2024 Nicholas Piggin, IBM Corp.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.

I know, we're using this line in a lot of source files ... but maybe we 
should do better for new files at least: "LGPL, version 2" is a little bit 
ambiguous: Does it mean the "Library GPL version 2.0" or the "Lesser GPL 
version 2.1"? Maybe you could clarify by additionally providing a SPDX 
identifier here, or by explicitly writing 2.0 or 2.1.

 Thanks,
  Thomas








