Implement a basic exception handler that dumps the CPU state to the
console, as well as the code required to set the correct exception
vector table's base address in setup.c.
Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
---
v2:
- Place {h_,}exception_common in .text.exceptions section
- Use assembler macro instead of CPP macro for ISR definition
- Tabulate ISR definitions
xen/arch/ppc/include/asm/processor.h | 31 +++++++
xen/arch/ppc/ppc64/Makefile | 2 +
xen/arch/ppc/ppc64/asm-offsets.c | 1 +
xen/arch/ppc/ppc64/exceptions-asm.S | 129 +++++++++++++++++++++++++++
xen/arch/ppc/ppc64/exceptions.c | 102 +++++++++++++++++++++
xen/arch/ppc/setup.c | 11 +++
6 files changed, 277 insertions(+), 1 deletion(-)
create mode 100644 xen/arch/ppc/ppc64/exceptions-asm.S
create mode 100644 xen/arch/ppc/ppc64/exceptions.c
diff --git a/xen/arch/ppc/include/asm/processor.h b/xen/arch/ppc/include/asm/processor.h
index d3dd943c20..a01b62b8a4 100644
--- a/xen/arch/ppc/include/asm/processor.h
+++ b/xen/arch/ppc/include/asm/processor.h
@@ -103,6 +103,37 @@
#define PVR_BE 0x0070
#define PVR_PA6T 0x0090
+/* Exception Definitions */
+#define EXC_SYSTEM_RESET 0x0100 /* System Reset Interrupt */
+#define EXC_MACHINE_CHECK 0x0200 /* Machine Check Interrupt */
+#define EXC_DATA_STORAGE 0x0300 /* Data Storage Interrupt */
+#define EXC_DATA_SEGMENT 0x0380 /* Data Segment Interrupt */
+#define EXC_INSN_STORAGE 0x0400 /* Instruction Storage Interrupt */
+#define EXC_INSN_SEGMENT 0x0480 /* Instruction Segment Interrupt */
+#define EXC_EXTERNAL 0x0500 /* External Interrupt */
+#define EXC_ALIGNMENT 0x0600 /* Alignment Interrupt */
+#define EXC_PROGRAM 0x0700 /* Program Interrupt */
+#define EXC_FPU_UNAVAIL 0x0800 /* Floating-Point Unavailable Interrupt */
+#define EXC_DECREMENTER 0x0900 /* Decrementer Interrupt */
+#define EXC_H_DECREMENTER 0x0980 /* Hypervisor Decrementer Interrupt */
+#define EXC_PRIV_DOORBELL 0x0A00 /* Directed Privileged Doorbell Interrupt */
+#define EXC_SYSTEM_CALL 0x0C00 /* System Call Interrupt */
+#define EXC_TRACE 0x0D00 /* Trace Interrupt */
+#define EXC_H_DATA_STORAGE 0x0E00 /* Hypervisor Data Storage Interrupt */
+#define EXC_H_INSN_STORAGE 0x0E20 /* Hypervisor Instruction Storage Interrupt */
+#define EXC_H_EMUL_ASST 0x0E40 /* Hypervisor Emulation Assistance Interrupt */
+#define EXC_H_MAINTENANCE 0x0E60 /* Hypervisor Maintenance Interrupt */
+#define EXC_H_DOORBELL 0x0E80 /* Directed Hypervisor Doorbell Interrupt */
+#define EXC_H_VIRT 0x0EA0 /* Hypervisor Virtualization Interrupt */
+#define EXC_PERF_MON 0x0F00 /* Performance Monitor Interrupt */
+#define EXC_VECTOR_UNAVAIL 0x0F20 /* Vector Unavailable Interrupt */
+#define EXC_VSX_UNAVAIL 0x0F40 /* VSX Unavailable Interrupt */
+#define EXC_FACIL_UNAVAIL 0x0F60 /* Facility Unavailable Interrupt */
+#define EXC_H_FACIL_UNAVAIL 0x0F80 /* Hypervisor Facility Unavailable Interrupt */
+
+/* Base address of interrupt vector table when LPCR[AIL]=3 */
+#define AIL_VECTOR_BASE _AC(0xc000000000004000, UL)
+
#ifndef __ASSEMBLY__
#include <xen/types.h>
diff --git a/xen/arch/ppc/ppc64/Makefile b/xen/arch/ppc/ppc64/Makefile
index 5b88355bb2..914bb21c40 100644
--- a/xen/arch/ppc/ppc64/Makefile
+++ b/xen/arch/ppc/ppc64/Makefile
@@ -1,2 +1,4 @@
+obj-y += exceptions.o
+obj-y += exceptions-asm.o
obj-y += head.o
obj-y += opal-calls.o
diff --git a/xen/arch/ppc/ppc64/asm-offsets.c b/xen/arch/ppc/ppc64/asm-offsets.c
index c15c1bf136..634d7260e3 100644
--- a/xen/arch/ppc/ppc64/asm-offsets.c
+++ b/xen/arch/ppc/ppc64/asm-offsets.c
@@ -46,6 +46,7 @@ void __dummy__(void)
OFFSET(UREGS_dsisr, struct cpu_user_regs, dsisr);
OFFSET(UREGS_cr, struct cpu_user_regs, cr);
OFFSET(UREGS_fpscr, struct cpu_user_regs, fpscr);
+ OFFSET(UREGS_entry_vector, struct cpu_user_regs, entry_vector);
DEFINE(UREGS_sizeof, sizeof(struct cpu_user_regs));
OFFSET(OPAL_base, struct opal, base);
diff --git a/xen/arch/ppc/ppc64/exceptions-asm.S b/xen/arch/ppc/ppc64/exceptions-asm.S
new file mode 100644
index 0000000000..a7a111463f
--- /dev/null
+++ b/xen/arch/ppc/ppc64/exceptions-asm.S
@@ -0,0 +1,129 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <asm/asm-defns.h>
+#include <asm/processor.h>
+
+ .section .text.exceptions, "ax", %progbits
+
+ /* Helper to dump CPU state to struct cpu_user_regs pointed to by r1. */
+ENTRY(exception_common)
+ /* Save GPRs 1-31 */
+ SAVE_GPRS(1, 31, %r1)
+
+ /* Save LR, CTR, CR */
+ mflr %r0
+ std %r0, UREGS_lr(%r1)
+ mfctr %r0
+ std %r0, UREGS_ctr(%r1)
+ mfcr %r0
+ stw %r0, UREGS_cr(%r1) /* 32-bit */
+
+ /* Save Exception Registers */
+ mfsrr0 %r0
+ std %r0, UREGS_pc(%r1)
+ mfsrr1 %r0
+ std %r0, UREGS_msr(%r1)
+ mfdsisr %r0
+ stw %r0, UREGS_dsisr(%r1) /* 32-bit */
+ mfdar %r0
+ std %r0, UREGS_dar(%r1)
+ li %r0, -1 /* OS's SRR0/SRR1 have been clobbered */
+ std %r0, UREGS_srr0(%r1)
+ std %r0, UREGS_srr1(%r1)
+
+ /* Setup TOC and a stack frame then call C exception handler */
+ mr %r3, %r1
+ bcl 20, 31, 1f
+1: mflr %r12
+ addis %r2, %r12, .TOC.-1b@ha
+ addi %r2, %r2, .TOC.-1b@l
+
+ li %r0, 0
+ stdu %r0, -STACK_FRAME_OVERHEAD(%r1)
+ bl exception_handler
+
+ .size exception_common, . - exception_common
+ .type exception_common, %function
+
+ /* Same as exception_common, but for exceptions that set HSRR{0,1} */
+ENTRY(h_exception_common)
+ /* Save GPRs 1-31 */
+ SAVE_GPRS(1, 31, %r1)
+
+ /* Save LR, CTR, CR */
+ mflr %r0
+ std %r0, UREGS_lr(%r1)
+ mfctr %r0
+ std %r0, UREGS_ctr(%r1)
+ mfcr %r0
+ stw %r0, UREGS_cr(%r1) /* 32-bit */
+
+ /* Save Exception Registers */
+ mfhsrr0 %r0
+ std %r0, UREGS_pc(%r1)
+ mfhsrr1 %r0
+ std %r0, UREGS_msr(%r1)
+ mfsrr0 %r0
+ std %r0, UREGS_srr0(%r1)
+ mfsrr1 %r0
+ std %r0, UREGS_srr1(%r1)
+ mfdsisr %r0
+ stw %r0, UREGS_dsisr(%r1) /* 32-bit */
+ mfdar %r0
+ std %r0, UREGS_dar(%r1)
+
+ /* Setup TOC and a stack frame then call C exception handler */
+ mr %r3, %r1
+ bcl 20, 31, 1f
+1: mflr %r12
+ addis %r2, %r12, .TOC.-1b@ha
+ addi %r2, %r2, .TOC.-1b@l
+
+ li %r0, 0
+ stdu %r0, -STACK_FRAME_OVERHEAD(%r1)
+ bl exception_handler
+
+ .size h_exception_common, . - h_exception_common
+ .type h_exception_common, %function
+
+/*
+ * Declare an ISR for the provided exception that jumps to the specified handler
+ */
+.macro ISR name, exc, handler
+ . = (AIL_VECTOR_BASE - EXCEPTION_VECTORS_START) + \exc
+ ENTRY(\name)
+ /* TODO: switch stack */
+
+ /* Reserve space for struct cpu_user_regs */
+ subi %r1, %r1, UREGS_sizeof
+
+ /* Save r0 immediately so we can use it as scratch space */
+ SAVE_GPR(0, %r1)
+
+ /* Save exception vector number */
+ li %r0, \exc
+ std %r0, UREGS_entry_vector(%r1)
+
+ /* Branch to common code */
+ b \handler
+
+ .size \name, . - \name
+ .type \name, %function
+.endm
+
+
+ISR exc_sysreset, EXC_SYSTEM_RESET, exception_common
+ISR exc_mcheck, EXC_MACHINE_CHECK, exception_common
+ISR exc_dstore, EXC_DATA_STORAGE, exception_common
+ISR exc_dsegment, EXC_DATA_SEGMENT, exception_common
+ISR exc_istore, EXC_INSN_STORAGE, exception_common
+ISR exc_isegment, EXC_INSN_SEGMENT, exception_common
+ISR exc_extern, EXC_EXTERNAL, exception_common
+ISR exc_align, EXC_ALIGNMENT, exception_common
+ISR exc_program, EXC_PROGRAM, exception_common
+ISR exc_fpu, EXC_FPU_UNAVAIL, exception_common
+ISR exc_dec, EXC_DECREMENTER, exception_common
+ISR exc_h_dec, EXC_H_DECREMENTER, h_exception_common
+/* EXC_PRIV_DOORBELL ... EXC_TRACE */
+ISR exc_h_dstore, EXC_H_DATA_STORAGE, h_exception_common
+ISR exc_h_istore, EXC_H_INSN_STORAGE, h_exception_common
diff --git a/xen/arch/ppc/ppc64/exceptions.c b/xen/arch/ppc/ppc64/exceptions.c
new file mode 100644
index 0000000000..ad5ab545f0
--- /dev/null
+++ b/xen/arch/ppc/ppc64/exceptions.c
@@ -0,0 +1,102 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#include <xen/lib.h>
+
+#include <asm/processor.h>
+
+static const char *exception_name_from_vec(uint32_t vec)
+{
+ switch ( vec )
+ {
+ case EXC_SYSTEM_RESET:
+ return "System Reset Interrupt";
+ case EXC_MACHINE_CHECK:
+ return "Machine Check Interrupt";
+ case EXC_DATA_STORAGE:
+ return "Data Storage Interrupt";
+ case EXC_DATA_SEGMENT:
+ return "Data Segment Interrupt";
+ case EXC_INSN_STORAGE:
+ return "Instruction Storage Interrupt";
+ case EXC_INSN_SEGMENT:
+ return "Instruction Segment Interrupt";
+ case EXC_EXTERNAL:
+ return "External Interrupt";
+ case EXC_ALIGNMENT:
+ return "Alignment Interrupt";
+ case EXC_PROGRAM:
+ return "Program Interrupt";
+ case EXC_FPU_UNAVAIL:
+ return "Floating-Point Unavailable Interrupt";
+ case EXC_DECREMENTER:
+ return "Decrementer Interrupt";
+ case EXC_H_DECREMENTER:
+ return "Hypervisor Decrementer Interrupt";
+ case EXC_PRIV_DOORBELL:
+ return "Directed Privileged Doorbell Interrupt";
+ case EXC_SYSTEM_CALL:
+ return "System Call Interrupt";
+ case EXC_TRACE:
+ return "Trace Interrupt";
+ case EXC_H_DATA_STORAGE:
+ return "Hypervisor Data Storage Interrupt";
+ case EXC_H_INSN_STORAGE:
+ return "Hypervisor Instruction Storage Interrupt";
+ case EXC_H_EMUL_ASST:
+ return "Hypervisor Emulation Assistance Interrupt";
+ case EXC_H_MAINTENANCE:
+ return "Hypervisor Maintenance Interrupt";
+ case EXC_H_DOORBELL:
+ return "Directed Hypervisor Doorbell Interrupt";
+ case EXC_H_VIRT:
+ return "Hypervisor Virtualization Interrupt";
+ case EXC_PERF_MON:
+ return "Performance Monitor Interrupt";
+ case EXC_VECTOR_UNAVAIL:
+ return "Vector Unavailable Interrupt";
+ case EXC_VSX_UNAVAIL:
+ return "VSX Unavailable Interrupt";
+ case EXC_FACIL_UNAVAIL:
+ return "Facility Unavailable Interrupt";
+ case EXC_H_FACIL_UNAVAIL:
+ return "Hypervisor Facility Unavailable Interrupt";
+ default:
+ return "(unknown)";
+ }
+}
+
+void exception_handler(struct cpu_user_regs *regs)
+{
+ /* TODO: this is currently only useful for debugging */
+
+ printk("UNRECOVERABLE EXCEPTION: %s (0x%04x)\n\n"
+ "GPR 0-3 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
+ "GPR 4-7 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
+ "GPR 8-11 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
+ "GPR 12-15 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
+ "GPR 16-19 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
+ "GPR 20-23 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
+ "GPR 24-27 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
+ "GPR 28-31 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n\n",
+ exception_name_from_vec(regs->entry_vector), regs->entry_vector,
+ regs->gprs[0], regs->gprs[1], regs->gprs[2], regs->gprs[3],
+ regs->gprs[4], regs->gprs[5], regs->gprs[6], regs->gprs[7],
+ regs->gprs[8], regs->gprs[9], regs->gprs[10], regs->gprs[11],
+ regs->gprs[12], regs->gprs[13], regs->gprs[14], regs->gprs[15],
+ regs->gprs[16], regs->gprs[17], regs->gprs[18], regs->gprs[19],
+ regs->gprs[20], regs->gprs[21], regs->gprs[22], regs->gprs[23],
+ regs->gprs[24], regs->gprs[25], regs->gprs[26], regs->gprs[27],
+ regs->gprs[28], regs->gprs[29], regs->gprs[30], regs->gprs[31]);
+ printk("LR : 0x%016lx\n"
+ "CTR : 0x%016lx\n"
+ "CR : 0x%08x\n"
+ "PC : 0x%016lx\n"
+ "MSR : 0x%016lx\n"
+ "SRR0 : 0x%016lx\n"
+ "SRR1 : 0x%016lx\n"
+ "DAR : 0x%016lx\n"
+ "DSISR : 0x%08x\n",
+ regs->lr, regs->ctr, regs->cr, regs->pc, regs->msr, regs->srr0,
+ regs->srr1, regs->dar, regs->dsisr);
+
+ die();
+}
diff --git a/xen/arch/ppc/setup.c b/xen/arch/ppc/setup.c
index 959c1454a0..101bdd8bb6 100644
--- a/xen/arch/ppc/setup.c
+++ b/xen/arch/ppc/setup.c
@@ -11,6 +11,15 @@
/* Xen stack for bringing up the first CPU. */
unsigned char __initdata cpu0_boot_stack[STACK_SIZE] __aligned(STACK_SIZE);
+void setup_exceptions(void)
+{
+ unsigned long lpcr;
+
+ /* Set appropriate interrupt location in LPCR */
+ lpcr = mfspr(SPRN_LPCR);
+ mtspr(SPRN_LPCR, lpcr | LPCR_AIL_3);
+}
+
void __init noreturn start_xen(unsigned long r3, unsigned long r4,
unsigned long r5, unsigned long r6,
unsigned long r7)
@@ -26,6 +35,8 @@ void __init noreturn start_xen(unsigned long r3, unsigned long r4,
boot_opal_init((void *)r3);
}
+ setup_exceptions();
+
setup_initial_pagetables();
early_printk("Hello, ppc64le!\n");
--
2.30.2On 13.10.2023 20:13, Shawn Anastasio wrote:
> --- /dev/null
> +++ b/xen/arch/ppc/ppc64/exceptions-asm.S
> @@ -0,0 +1,129 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include <asm/asm-defns.h>
> +#include <asm/processor.h>
> +
> + .section .text.exceptions, "ax", %progbits
> +
> + /* Helper to dump CPU state to struct cpu_user_regs pointed to by r1. */
> +ENTRY(exception_common)
> + /* Save GPRs 1-31 */
> + SAVE_GPRS(1, 31, %r1)
This won't save the entry value of %r1, but the one that was already updated.
If this is not a problem, perhaps worth a comment?
> + /* Save LR, CTR, CR */
> + mflr %r0
> + std %r0, UREGS_lr(%r1)
> + mfctr %r0
> + std %r0, UREGS_ctr(%r1)
> + mfcr %r0
> + stw %r0, UREGS_cr(%r1) /* 32-bit */
> +
> + /* Save Exception Registers */
> + mfsrr0 %r0
> + std %r0, UREGS_pc(%r1)
> + mfsrr1 %r0
> + std %r0, UREGS_msr(%r1)
> + mfdsisr %r0
> + stw %r0, UREGS_dsisr(%r1) /* 32-bit */
> + mfdar %r0
> + std %r0, UREGS_dar(%r1)
> + li %r0, -1 /* OS's SRR0/SRR1 have been clobbered */
> + std %r0, UREGS_srr0(%r1)
> + std %r0, UREGS_srr1(%r1)
> +
> + /* Setup TOC and a stack frame then call C exception handler */
> + mr %r3, %r1
> + bcl 20, 31, 1f
> +1: mflr %r12
> + addis %r2, %r12, .TOC.-1b@ha
> + addi %r2, %r2, .TOC.-1b@l
> +
> + li %r0, 0
> + stdu %r0, -STACK_FRAME_OVERHEAD(%r1)
> + bl exception_handler
> +
> + .size exception_common, . - exception_common
> + .type exception_common, %function
> +
> + /* Same as exception_common, but for exceptions that set HSRR{0,1} */
> +ENTRY(h_exception_common)
> + /* Save GPRs 1-31 */
> + SAVE_GPRS(1, 31, %r1)
> +
> + /* Save LR, CTR, CR */
> + mflr %r0
> + std %r0, UREGS_lr(%r1)
> + mfctr %r0
> + std %r0, UREGS_ctr(%r1)
> + mfcr %r0
> + stw %r0, UREGS_cr(%r1) /* 32-bit */
> +
> + /* Save Exception Registers */
> + mfhsrr0 %r0
> + std %r0, UREGS_pc(%r1)
> + mfhsrr1 %r0
> + std %r0, UREGS_msr(%r1)
> + mfsrr0 %r0
> + std %r0, UREGS_srr0(%r1)
> + mfsrr1 %r0
> + std %r0, UREGS_srr1(%r1)
Except for these four lines the two functions look identical. Is it
really good to duplicate all of the rest of the code?
> + mfdsisr %r0
> + stw %r0, UREGS_dsisr(%r1) /* 32-bit */
> + mfdar %r0
> + std %r0, UREGS_dar(%r1)
> +
> + /* Setup TOC and a stack frame then call C exception handler */
> + mr %r3, %r1
> + bcl 20, 31, 1f
> +1: mflr %r12
> + addis %r2, %r12, .TOC.-1b@ha
> + addi %r2, %r2, .TOC.-1b@l
> +
> + li %r0, 0
> + stdu %r0, -STACK_FRAME_OVERHEAD(%r1)
> + bl exception_handler
> +
> + .size h_exception_common, . - h_exception_common
> + .type h_exception_common, %function
> +
> +/*
> + * Declare an ISR for the provided exception that jumps to the specified handler
> + */
> +.macro ISR name, exc, handler
> + . = (AIL_VECTOR_BASE - EXCEPTION_VECTORS_START) + \exc
> + ENTRY(\name)
> + /* TODO: switch stack */
> +
> + /* Reserve space for struct cpu_user_regs */
> + subi %r1, %r1, UREGS_sizeof
> +
> + /* Save r0 immediately so we can use it as scratch space */
> + SAVE_GPR(0, %r1)
> +
> + /* Save exception vector number */
> + li %r0, \exc
> + std %r0, UREGS_entry_vector(%r1)
> +
> + /* Branch to common code */
> + b \handler
> +
> + .size \name, . - \name
> + .type \name, %function
> +.endm
> +
> +
Nit: No double blank lines please.
> +ISR exc_sysreset, EXC_SYSTEM_RESET, exception_common
> +ISR exc_mcheck, EXC_MACHINE_CHECK, exception_common
> +ISR exc_dstore, EXC_DATA_STORAGE, exception_common
> +ISR exc_dsegment, EXC_DATA_SEGMENT, exception_common
> +ISR exc_istore, EXC_INSN_STORAGE, exception_common
> +ISR exc_isegment, EXC_INSN_SEGMENT, exception_common
> +ISR exc_extern, EXC_EXTERNAL, exception_common
> +ISR exc_align, EXC_ALIGNMENT, exception_common
> +ISR exc_program, EXC_PROGRAM, exception_common
> +ISR exc_fpu, EXC_FPU_UNAVAIL, exception_common
> +ISR exc_dec, EXC_DECREMENTER, exception_common
> +ISR exc_h_dec, EXC_H_DECREMENTER, h_exception_common
> +/* EXC_PRIV_DOORBELL ... EXC_TRACE */
> +ISR exc_h_dstore, EXC_H_DATA_STORAGE, h_exception_common
> +ISR exc_h_istore, EXC_H_INSN_STORAGE, h_exception_common
Aiui these are required to be in order, or else the assembler will choke.
Maybe worth another comment?
> --- /dev/null
> +++ b/xen/arch/ppc/ppc64/exceptions.c
> @@ -0,0 +1,102 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#include <xen/lib.h>
> +
> +#include <asm/processor.h>
> +
> +static const char *exception_name_from_vec(uint32_t vec)
> +{
> + switch ( vec )
> + {
> + case EXC_SYSTEM_RESET:
> + return "System Reset Interrupt";
> + case EXC_MACHINE_CHECK:
> + return "Machine Check Interrupt";
> + case EXC_DATA_STORAGE:
> + return "Data Storage Interrupt";
> + case EXC_DATA_SEGMENT:
> + return "Data Segment Interrupt";
> + case EXC_INSN_STORAGE:
> + return "Instruction Storage Interrupt";
> + case EXC_INSN_SEGMENT:
> + return "Instruction Segment Interrupt";
> + case EXC_EXTERNAL:
> + return "External Interrupt";
> + case EXC_ALIGNMENT:
> + return "Alignment Interrupt";
> + case EXC_PROGRAM:
> + return "Program Interrupt";
> + case EXC_FPU_UNAVAIL:
> + return "Floating-Point Unavailable Interrupt";
> + case EXC_DECREMENTER:
> + return "Decrementer Interrupt";
> + case EXC_H_DECREMENTER:
> + return "Hypervisor Decrementer Interrupt";
> + case EXC_PRIV_DOORBELL:
> + return "Directed Privileged Doorbell Interrupt";
> + case EXC_SYSTEM_CALL:
> + return "System Call Interrupt";
> + case EXC_TRACE:
> + return "Trace Interrupt";
> + case EXC_H_DATA_STORAGE:
> + return "Hypervisor Data Storage Interrupt";
> + case EXC_H_INSN_STORAGE:
> + return "Hypervisor Instruction Storage Interrupt";
> + case EXC_H_EMUL_ASST:
> + return "Hypervisor Emulation Assistance Interrupt";
> + case EXC_H_MAINTENANCE:
> + return "Hypervisor Maintenance Interrupt";
> + case EXC_H_DOORBELL:
> + return "Directed Hypervisor Doorbell Interrupt";
> + case EXC_H_VIRT:
> + return "Hypervisor Virtualization Interrupt";
> + case EXC_PERF_MON:
> + return "Performance Monitor Interrupt";
> + case EXC_VECTOR_UNAVAIL:
> + return "Vector Unavailable Interrupt";
> + case EXC_VSX_UNAVAIL:
> + return "VSX Unavailable Interrupt";
> + case EXC_FACIL_UNAVAIL:
> + return "Facility Unavailable Interrupt";
> + case EXC_H_FACIL_UNAVAIL:
> + return "Hypervisor Facility Unavailable Interrupt";
Every string here has Interrupt at the end - any chance of collapsing
all of them?
> + default:
> + return "(unknown)";
> + }
> +}
> +
> +void exception_handler(struct cpu_user_regs *regs)
> +{
> + /* TODO: this is currently only useful for debugging */
> +
> + printk("UNRECOVERABLE EXCEPTION: %s (0x%04x)\n\n"
> + "GPR 0-3 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
> + "GPR 4-7 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
> + "GPR 8-11 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
> + "GPR 12-15 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
> + "GPR 16-19 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
> + "GPR 20-23 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
> + "GPR 24-27 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
> + "GPR 28-31 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n\n",
> + exception_name_from_vec(regs->entry_vector), regs->entry_vector,
> + regs->gprs[0], regs->gprs[1], regs->gprs[2], regs->gprs[3],
> + regs->gprs[4], regs->gprs[5], regs->gprs[6], regs->gprs[7],
> + regs->gprs[8], regs->gprs[9], regs->gprs[10], regs->gprs[11],
> + regs->gprs[12], regs->gprs[13], regs->gprs[14], regs->gprs[15],
> + regs->gprs[16], regs->gprs[17], regs->gprs[18], regs->gprs[19],
> + regs->gprs[20], regs->gprs[21], regs->gprs[22], regs->gprs[23],
> + regs->gprs[24], regs->gprs[25], regs->gprs[26], regs->gprs[27],
> + regs->gprs[28], regs->gprs[29], regs->gprs[30], regs->gprs[31]);
> + printk("LR : 0x%016lx\n"
> + "CTR : 0x%016lx\n"
> + "CR : 0x%08x\n"
> + "PC : 0x%016lx\n"
> + "MSR : 0x%016lx\n"
> + "SRR0 : 0x%016lx\n"
> + "SRR1 : 0x%016lx\n"
> + "DAR : 0x%016lx\n"
> + "DSISR : 0x%08x\n",
> + regs->lr, regs->ctr, regs->cr, regs->pc, regs->msr, regs->srr0,
> + regs->srr1, regs->dar, regs->dsisr);
While surely a matter of taste, I'd still like to ask: In dumping like
this, are 0x prefixes (taking space) actually useful?
> --- a/xen/arch/ppc/setup.c
> +++ b/xen/arch/ppc/setup.c
> @@ -11,6 +11,15 @@
> /* Xen stack for bringing up the first CPU. */
> unsigned char __initdata cpu0_boot_stack[STACK_SIZE] __aligned(STACK_SIZE);
>
> +void setup_exceptions(void)
> +{
> + unsigned long lpcr;
> +
> + /* Set appropriate interrupt location in LPCR */
> + lpcr = mfspr(SPRN_LPCR);
> + mtspr(SPRN_LPCR, lpcr | LPCR_AIL_3);
> +}
Please forgive my lack of PPC knowledge: There's no use of any address
here afaict. Where's the link to (presumably) AIL_VECTOR_BASE? If that
address is kind of magic, I'm then lacking a connection between
XEN_VIRT_START and that constant. (If Xen needed moving around in
address space, it would be nice if changing a single constant would
suffice.)
Also you only OR in LPCR_AIL_3 - is it guaranteed that the respective
field is zero when control is passed to Xen?
Jan
On 10/18/23 10:43 AM, Jan Beulich wrote:
> On 13.10.2023 20:13, Shawn Anastasio wrote:
>> --- /dev/null
>> +++ b/xen/arch/ppc/ppc64/exceptions-asm.S
>> @@ -0,0 +1,129 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +
>> +#include <asm/asm-defns.h>
>> +#include <asm/processor.h>
>> +
>> + .section .text.exceptions, "ax", %progbits
>> +
>> + /* Helper to dump CPU state to struct cpu_user_regs pointed to by r1. */
>> +ENTRY(exception_common)
>> + /* Save GPRs 1-31 */
>> + SAVE_GPRS(1, 31, %r1)
>
> This won't save the entry value of %r1, but the one that was already updated.
> If this is not a problem, perhaps worth a comment?
>
This is indeed not a problem for now, but agreed that it warrants a
comment. Will do.
>> + /* Save LR, CTR, CR */
>> + mflr %r0
>> + std %r0, UREGS_lr(%r1)
>> + mfctr %r0
>> + std %r0, UREGS_ctr(%r1)
>> + mfcr %r0
>> + stw %r0, UREGS_cr(%r1) /* 32-bit */
>> +
>> + /* Save Exception Registers */
>> + mfsrr0 %r0
>> + std %r0, UREGS_pc(%r1)
>> + mfsrr1 %r0
>> + std %r0, UREGS_msr(%r1)
>> + mfdsisr %r0
>> + stw %r0, UREGS_dsisr(%r1) /* 32-bit */
>> + mfdar %r0
>> + std %r0, UREGS_dar(%r1)
>> + li %r0, -1 /* OS's SRR0/SRR1 have been clobbered */
>> + std %r0, UREGS_srr0(%r1)
>> + std %r0, UREGS_srr1(%r1)
>> +
>> + /* Setup TOC and a stack frame then call C exception handler */
>> + mr %r3, %r1
>> + bcl 20, 31, 1f
>> +1: mflr %r12
>> + addis %r2, %r12, .TOC.-1b@ha
>> + addi %r2, %r2, .TOC.-1b@l
>> +
>> + li %r0, 0
>> + stdu %r0, -STACK_FRAME_OVERHEAD(%r1)
>> + bl exception_handler
>> +
>> + .size exception_common, . - exception_common
>> + .type exception_common, %function
>> +
>> + /* Same as exception_common, but for exceptions that set HSRR{0,1} */
>> +ENTRY(h_exception_common)
>> + /* Save GPRs 1-31 */
>> + SAVE_GPRS(1, 31, %r1)
>> +
>> + /* Save LR, CTR, CR */
>> + mflr %r0
>> + std %r0, UREGS_lr(%r1)
>> + mfctr %r0
>> + std %r0, UREGS_ctr(%r1)
>> + mfcr %r0
>> + stw %r0, UREGS_cr(%r1) /* 32-bit */
>> +
>> + /* Save Exception Registers */
>> + mfhsrr0 %r0
>> + std %r0, UREGS_pc(%r1)
>> + mfhsrr1 %r0
>> + std %r0, UREGS_msr(%r1)
>> + mfsrr0 %r0
>> + std %r0, UREGS_srr0(%r1)
>> + mfsrr1 %r0
>> + std %r0, UREGS_srr1(%r1)
>
> Except for these four lines the two functions look identical. Is it
> really good to duplicate all of the rest of the code?
>
Andrew also pointed this out and as I mentioned to him, I expect the two
routines to diverge more significantly in the future, so I'd rather keep
them separate even if in this early stage there's not much difference.
>> + mfdsisr %r0
>> + stw %r0, UREGS_dsisr(%r1) /* 32-bit */
>> + mfdar %r0
>> + std %r0, UREGS_dar(%r1)
>> +
>> + /* Setup TOC and a stack frame then call C exception handler */
>> + mr %r3, %r1
>> + bcl 20, 31, 1f
>> +1: mflr %r12
>> + addis %r2, %r12, .TOC.-1b@ha
>> + addi %r2, %r2, .TOC.-1b@l
>> +
>> + li %r0, 0
>> + stdu %r0, -STACK_FRAME_OVERHEAD(%r1)
>> + bl exception_handler
>> +
>> + .size h_exception_common, . - h_exception_common
>> + .type h_exception_common, %function
>> +
>> +/*
>> + * Declare an ISR for the provided exception that jumps to the specified handler
>> + */
>> +.macro ISR name, exc, handler
>> + . = (AIL_VECTOR_BASE - EXCEPTION_VECTORS_START) + \exc
>> + ENTRY(\name)
>> + /* TODO: switch stack */
>> +
>> + /* Reserve space for struct cpu_user_regs */
>> + subi %r1, %r1, UREGS_sizeof
>> +
>> + /* Save r0 immediately so we can use it as scratch space */
>> + SAVE_GPR(0, %r1)
>> +
>> + /* Save exception vector number */
>> + li %r0, \exc
>> + std %r0, UREGS_entry_vector(%r1)
>> +
>> + /* Branch to common code */
>> + b \handler
>> +
>> + .size \name, . - \name
>> + .type \name, %function
>> +.endm
>> +
>> +
>
> Nit: No double blank lines please.
>
Will fix.
>> +ISR exc_sysreset, EXC_SYSTEM_RESET, exception_common
>> +ISR exc_mcheck, EXC_MACHINE_CHECK, exception_common
>> +ISR exc_dstore, EXC_DATA_STORAGE, exception_common
>> +ISR exc_dsegment, EXC_DATA_SEGMENT, exception_common
>> +ISR exc_istore, EXC_INSN_STORAGE, exception_common
>> +ISR exc_isegment, EXC_INSN_SEGMENT, exception_common
>> +ISR exc_extern, EXC_EXTERNAL, exception_common
>> +ISR exc_align, EXC_ALIGNMENT, exception_common
>> +ISR exc_program, EXC_PROGRAM, exception_common
>> +ISR exc_fpu, EXC_FPU_UNAVAIL, exception_common
>> +ISR exc_dec, EXC_DECREMENTER, exception_common
>> +ISR exc_h_dec, EXC_H_DECREMENTER, h_exception_common
>> +/* EXC_PRIV_DOORBELL ... EXC_TRACE */
>> +ISR exc_h_dstore, EXC_H_DATA_STORAGE, h_exception_common
>> +ISR exc_h_istore, EXC_H_INSN_STORAGE, h_exception_common
>
> Aiui these are required to be in order, or else the assembler will choke.
> Maybe worth another comment?
>
Correct, the ordering does matter here. I'll add a comment.
>> --- /dev/null
>> +++ b/xen/arch/ppc/ppc64/exceptions.c
>> @@ -0,0 +1,102 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +#include <xen/lib.h>
>> +
>> +#include <asm/processor.h>
>> +
>> +static const char *exception_name_from_vec(uint32_t vec)
>> +{
>> + switch ( vec )
>> + {
>> + case EXC_SYSTEM_RESET:
>> + return "System Reset Interrupt";
>> + case EXC_MACHINE_CHECK:
>> + return "Machine Check Interrupt";
>> + case EXC_DATA_STORAGE:
>> + return "Data Storage Interrupt";
>> + case EXC_DATA_SEGMENT:
>> + return "Data Segment Interrupt";
>> + case EXC_INSN_STORAGE:
>> + return "Instruction Storage Interrupt";
>> + case EXC_INSN_SEGMENT:
>> + return "Instruction Segment Interrupt";
>> + case EXC_EXTERNAL:
>> + return "External Interrupt";
>> + case EXC_ALIGNMENT:
>> + return "Alignment Interrupt";
>> + case EXC_PROGRAM:
>> + return "Program Interrupt";
>> + case EXC_FPU_UNAVAIL:
>> + return "Floating-Point Unavailable Interrupt";
>> + case EXC_DECREMENTER:
>> + return "Decrementer Interrupt";
>> + case EXC_H_DECREMENTER:
>> + return "Hypervisor Decrementer Interrupt";
>> + case EXC_PRIV_DOORBELL:
>> + return "Directed Privileged Doorbell Interrupt";
>> + case EXC_SYSTEM_CALL:
>> + return "System Call Interrupt";
>> + case EXC_TRACE:
>> + return "Trace Interrupt";
>> + case EXC_H_DATA_STORAGE:
>> + return "Hypervisor Data Storage Interrupt";
>> + case EXC_H_INSN_STORAGE:
>> + return "Hypervisor Instruction Storage Interrupt";
>> + case EXC_H_EMUL_ASST:
>> + return "Hypervisor Emulation Assistance Interrupt";
>> + case EXC_H_MAINTENANCE:
>> + return "Hypervisor Maintenance Interrupt";
>> + case EXC_H_DOORBELL:
>> + return "Directed Hypervisor Doorbell Interrupt";
>> + case EXC_H_VIRT:
>> + return "Hypervisor Virtualization Interrupt";
>> + case EXC_PERF_MON:
>> + return "Performance Monitor Interrupt";
>> + case EXC_VECTOR_UNAVAIL:
>> + return "Vector Unavailable Interrupt";
>> + case EXC_VSX_UNAVAIL:
>> + return "VSX Unavailable Interrupt";
>> + case EXC_FACIL_UNAVAIL:
>> + return "Facility Unavailable Interrupt";
>> + case EXC_H_FACIL_UNAVAIL:
>> + return "Hypervisor Facility Unavailable Interrupt";
>
> Every string here has Interrupt at the end - any chance of collapsing
> all of them?
>
Fair enough, I'll drop " Interrupt" from all the strings.
>> + default:
>> + return "(unknown)";
>> + }
>> +}
>> +
>> +void exception_handler(struct cpu_user_regs *regs)
>> +{
>> + /* TODO: this is currently only useful for debugging */
>> +
>> + printk("UNRECOVERABLE EXCEPTION: %s (0x%04x)\n\n"
>> + "GPR 0-3 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
>> + "GPR 4-7 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
>> + "GPR 8-11 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
>> + "GPR 12-15 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
>> + "GPR 16-19 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
>> + "GPR 20-23 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
>> + "GPR 24-27 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n"
>> + "GPR 28-31 : 0x%016lx 0x%016lx 0x%016lx 0x%016lx\n\n",
>> + exception_name_from_vec(regs->entry_vector), regs->entry_vector,
>> + regs->gprs[0], regs->gprs[1], regs->gprs[2], regs->gprs[3],
>> + regs->gprs[4], regs->gprs[5], regs->gprs[6], regs->gprs[7],
>> + regs->gprs[8], regs->gprs[9], regs->gprs[10], regs->gprs[11],
>> + regs->gprs[12], regs->gprs[13], regs->gprs[14], regs->gprs[15],
>> + regs->gprs[16], regs->gprs[17], regs->gprs[18], regs->gprs[19],
>> + regs->gprs[20], regs->gprs[21], regs->gprs[22], regs->gprs[23],
>> + regs->gprs[24], regs->gprs[25], regs->gprs[26], regs->gprs[27],
>> + regs->gprs[28], regs->gprs[29], regs->gprs[30], regs->gprs[31]);
>> + printk("LR : 0x%016lx\n"
>> + "CTR : 0x%016lx\n"
>> + "CR : 0x%08x\n"
>> + "PC : 0x%016lx\n"
>> + "MSR : 0x%016lx\n"
>> + "SRR0 : 0x%016lx\n"
>> + "SRR1 : 0x%016lx\n"
>> + "DAR : 0x%016lx\n"
>> + "DSISR : 0x%08x\n",
>> + regs->lr, regs->ctr, regs->cr, regs->pc, regs->msr, regs->srr0,
>> + regs->srr1, regs->dar, regs->dsisr);
>
> While surely a matter of taste, I'd still like to ask: In dumping like
> this, are 0x prefixes (taking space) actually useful?
>
I personally prefer the prefixes, but it is definitely just a matter of
taste.
>> --- a/xen/arch/ppc/setup.c
>> +++ b/xen/arch/ppc/setup.c
>> @@ -11,6 +11,15 @@
>> /* Xen stack for bringing up the first CPU. */
>> unsigned char __initdata cpu0_boot_stack[STACK_SIZE] __aligned(STACK_SIZE);
>>
>> +void setup_exceptions(void)
>> +{
>> + unsigned long lpcr;
>> +
>> + /* Set appropriate interrupt location in LPCR */
>> + lpcr = mfspr(SPRN_LPCR);
>> + mtspr(SPRN_LPCR, lpcr | LPCR_AIL_3);
>> +}
>
> Please forgive my lack of PPC knowledge: There's no use of any address
> here afaict. Where's the link to (presumably) AIL_VECTOR_BASE? If that
> address is kind of magic, I'm then lacking a connection between
> XEN_VIRT_START and that constant. (If Xen needed moving around in
> address space, it would be nice if changing a single constant would
> suffice.)
>
AIL_VECTOR_BASE is indeed a magic address defined by the ISA for AIL=3.
As for the second part of your question, I'm a bit confused as to what
you're asking. The ISRs are placed at a position relative to
the start of the .text.exceptions section (EXCEPTION_VECTORS_START), so
Xen can be arbitrarily shuffled around in address space as long as
EXCEPTION_VECTORS_START lies at or before AIL_VECTOR_BASE.
> Also you only OR in LPCR_AIL_3 - is it guaranteed that the respective
> field is zero when control is passed to Xen?
>
As AIL=3 corresponds to 0b11, the intial state of the AIL field is
irrelevant and OR'ing will always yield the desired result.
> Jan
Thanks,
Shawn
On 19.10.2023 22:02, Shawn Anastasio wrote:
> On 10/18/23 10:43 AM, Jan Beulich wrote:
>> On 13.10.2023 20:13, Shawn Anastasio wrote:
>>> --- a/xen/arch/ppc/setup.c
>>> +++ b/xen/arch/ppc/setup.c
>>> @@ -11,6 +11,15 @@
>>> /* Xen stack for bringing up the first CPU. */
>>> unsigned char __initdata cpu0_boot_stack[STACK_SIZE] __aligned(STACK_SIZE);
>>>
>>> +void setup_exceptions(void)
>>> +{
>>> + unsigned long lpcr;
>>> +
>>> + /* Set appropriate interrupt location in LPCR */
>>> + lpcr = mfspr(SPRN_LPCR);
>>> + mtspr(SPRN_LPCR, lpcr | LPCR_AIL_3);
>>> +}
>>
>> Please forgive my lack of PPC knowledge: There's no use of any address
>> here afaict. Where's the link to (presumably) AIL_VECTOR_BASE? If that
>> address is kind of magic, I'm then lacking a connection between
>> XEN_VIRT_START and that constant. (If Xen needed moving around in
>> address space, it would be nice if changing a single constant would
>> suffice.)
>>
>
> AIL_VECTOR_BASE is indeed a magic address defined by the ISA for AIL=3.
> As for the second part of your question, I'm a bit confused as to what
> you're asking. The ISRs are placed at a position relative to
> the start of the .text.exceptions section (EXCEPTION_VECTORS_START), so
> Xen can be arbitrarily shuffled around in address space as long as
> EXCEPTION_VECTORS_START lies at or before AIL_VECTOR_BASE.
Well, AIL_VECTOR_BASE is #define-d to a plain constant, not derived
from EXCEPTION_VECTORS_START. In turn EXCEPTION_VECTORS_START is
#define-d to a plain constant in patch 1, not derived from
XEN_VIRT_START. Therefore moving Xen around would require to change
(at least) 3 #define-s afaict.
Jan
On 10/20/23 1:22 AM, Jan Beulich wrote:
> On 19.10.2023 22:02, Shawn Anastasio wrote:
>> On 10/18/23 10:43 AM, Jan Beulich wrote:
>>> On 13.10.2023 20:13, Shawn Anastasio wrote:
>>>> --- a/xen/arch/ppc/setup.c
>>>> +++ b/xen/arch/ppc/setup.c
>>>> @@ -11,6 +11,15 @@
>>>> /* Xen stack for bringing up the first CPU. */
>>>> unsigned char __initdata cpu0_boot_stack[STACK_SIZE] __aligned(STACK_SIZE);
>>>>
>>>> +void setup_exceptions(void)
>>>> +{
>>>> + unsigned long lpcr;
>>>> +
>>>> + /* Set appropriate interrupt location in LPCR */
>>>> + lpcr = mfspr(SPRN_LPCR);
>>>> + mtspr(SPRN_LPCR, lpcr | LPCR_AIL_3);
>>>> +}
>>>
>>> Please forgive my lack of PPC knowledge: There's no use of any address
>>> here afaict. Where's the link to (presumably) AIL_VECTOR_BASE? If that
>>> address is kind of magic, I'm then lacking a connection between
>>> XEN_VIRT_START and that constant. (If Xen needed moving around in
>>> address space, it would be nice if changing a single constant would
>>> suffice.)
>>>
>>
>> AIL_VECTOR_BASE is indeed a magic address defined by the ISA for AIL=3.
>> As for the second part of your question, I'm a bit confused as to what
>> you're asking. The ISRs are placed at a position relative to
>> the start of the .text.exceptions section (EXCEPTION_VECTORS_START), so
>> Xen can be arbitrarily shuffled around in address space as long as
>> EXCEPTION_VECTORS_START lies at or before AIL_VECTOR_BASE.
>
> Well, AIL_VECTOR_BASE is #define-d to a plain constant, not derived
> from EXCEPTION_VECTORS_START. In turn EXCEPTION_VECTORS_START is
> #define-d to a plain constant in patch 1, not derived from
> XEN_VIRT_START. Therefore moving Xen around would require to change
> (at least) 3 #define-s afaict.
>
AIL_VECTOR_BASE needs to be a plain constant since it's fixed by the
ISA. EXCEPTION_VECTORS_START could be defined in terms of XEN_VIRT_START
I suppose, but that would require introducing another plain constant for
representing the offset of EXCEPTION_VECTORS_START from XEN_VIRT_START.
I think the current approach is reasonable, especially since patch 1
introduces a linker assertion that ensures that EXCEPTION_VECTORS_START
matches the actual location of _stext_exceptions, so if one was to
change Xen's address and forgot to change the #define they'd get a
build error.
If you would prefer this to be handled in a different way though, I'm
not opposed.
> Jan
Thanks,
Shawn
© 2016 - 2026 Red Hat, Inc.