[RFC PATCH 0/5] Exception handling in HYP mode on ARMv7-A
Currently, when U-Boot is running in hypervisor mode on ARMv7-A CPUs with virtualization extensions, the exception handling does not work. A couple things need to change which are detailed in my earlier message to the u-boot mailing list with the subject "Exception handling in HYP mode on ARMv7-A".
I have verified that this patch series works on the ODroid XU4 and the Raspberry Pi 3B in Aarch32 mode when running in hypervisor mode. One simple way to verify is by running the miscellaneous "exception" command (CMD_EXCEPTION).
Jim Posen (5): Compile for ARMv7-A with virtualization extensions Hypervisor mode interrupt vector table Set HVBAR register correctly Remove dead code Fix PC adjustment logic in exception handlers
arch/arm/Makefile | 4 + arch/arm/cpu/armv7/start.S | 5 ++ arch/arm/include/asm/u-boot-arm.h | 14 ++-- arch/arm/lib/interrupts.c | 26 +++--- arch/arm/lib/relocate.S | 8 ++ arch/arm/lib/vectors.S | 129 +++++++++++++++++++++++------- arch/arm/mach-bcm283x/Kconfig | 3 + arch/arm/mach-exynos/Kconfig | 1 + 8 files changed, 139 insertions(+), 51 deletions(-)
In order to assemble instructions that read the ELR_hyp register, which is part of the ARMv7-A virtualization extensions, the assembler target architecture needs to be changed.
Signed-off-by: Jim Posen jim.posen@gmail.com ---
arch/arm/Makefile | 4 ++++ arch/arm/mach-bcm283x/Kconfig | 3 +++ arch/arm/mach-exynos/Kconfig | 1 + 3 files changed, 8 insertions(+)
diff --git a/arch/arm/Makefile b/arch/arm/Makefile index 6c9a00c5a4..68c6b06a0e 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -14,8 +14,12 @@ arch-$(CONFIG_CPU_SA1100) =-march=armv4 arch-$(CONFIG_CPU_PXA) = arch-$(CONFIG_CPU_ARM1136) =-march=armv5t arch-$(CONFIG_CPU_ARM1176) =-march=armv5t +ifeq ($(CONFIG_CPU_V7_HAS_VIRT),y) +arch-$(CONFIG_CPU_V7A) =-march=armv7ve +else arch-$(CONFIG_CPU_V7A) =$(call cc-option, -march=armv7-a, \ $(call cc-option, -march=armv7)) +endif arch-$(CONFIG_CPU_V7M) =-march=armv7-m arch-$(CONFIG_CPU_V7R) =-march=armv7-r ifeq ($(CONFIG_ARM64_CRC32),y) diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig index b3287ce8bc..500a504c35 100644 --- a/arch/arm/mach-bcm283x/Kconfig +++ b/arch/arm/mach-bcm283x/Kconfig @@ -8,6 +8,7 @@ config BCM2836 depends on ARCH_BCM283X select ARMV7_LPAE select CPU_V7A + select CPU_V7_HAS_VIRT
config BCM2837 bool "Broadcom BCM2837 SoC support" @@ -19,6 +20,7 @@ config BCM2837_32B select BCM2837 select ARMV7_LPAE select CPU_V7A + select CPU_V7_HAS_VIRT
config BCM2837_64B bool "Broadcom BCM2837 SoC 64-bit support" @@ -36,6 +38,7 @@ config BCM2711_32B select BCM2711 select ARMV7_LPAE select CPU_V7A + select CPU_V7_HAS_VIRT select PHYS_64BIT
config BCM2711_64B diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig index 7df0e17617..6d0e95ba44 100644 --- a/arch/arm/mach-exynos/Kconfig +++ b/arch/arm/mach-exynos/Kconfig @@ -19,6 +19,7 @@ config ARCH_EXYNOS5 bool "Exynos5 SoC family" select BOARD_EARLY_INIT_F select CPU_V7A + select CPU_V7_HAS_VIRT select SHA_HW_ACCEL imply CMD_HASH imply CRC32_VERIFY
Create separate vector table for exceptions taken to PL2, which happens when U-Boot is running in hypervisor mode. The handler logic is different enough that a separate vector table is the simplest way to handle it.
Signed-off-by: Jim Posen jim.posen@gmail.com ---
arch/arm/lib/vectors.S | 95 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+)
diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S index 56f3681558..89b91b27da 100644 --- a/arch/arm/lib/vectors.S +++ b/arch/arm/lib/vectors.S @@ -117,6 +117,39 @@ _not_used: .word not_used _irq: .word irq _fiq: .word fiq
+#ifdef CONFIG_CPU_V7_HAS_VIRT +/* + ************************************************************************* + * + * Hypervisor mode interrupt vectors table + * + ************************************************************************* + */ + + .globl hyp_vector_table + + .align 5 +hyp_vector_table: + ldr pc, _hyp_reset + ldr pc, _hyp_undefined_instruction + ldr pc, _hyp_software_interrupt + ldr pc, _hyp_prefetch_abort + ldr pc, _hyp_data_abort + ldr pc, _hyp_not_used + ldr pc, _hyp_irq + ldr pc, _hyp_fiq + +_hyp_reset: .word reset +_hyp_undefined_instruction: .word hyp_undefined_instruction +_hyp_software_interrupt: .word hyp_software_interrupt +_hyp_prefetch_abort: .word hyp_prefetch_abort +_hyp_data_abort: .word hyp_data_abort +_hyp_not_used: .word hyp_not_used +_hyp_irq: .word hyp_irq +_hyp_fiq: .word hyp_fiq + +#endif /* CONFIG_CPU_V7_HAS_VIR + .balignl 16,0xdeadbeef
/* @@ -139,6 +172,15 @@ data_abort: not_used: irq: fiq: +#ifdef CONFIG_CPU_V7_HAS_VIRT +hyp_undefined_instruction: +hyp_software_interrupt: +hyp_prefetch_abort: +hyp_data_abort: +hyp_not_used: +hyp_irq: +hyp_fiq: +#endif /* CONFIG_CPU_V7_HAS_VIRT 1: b 1b /* hang and never return */
@@ -289,4 +331,57 @@ fiq: bad_save_user_regs bl do_fiq
+#ifdef CONFIG_CPU_V7_HAS_VIRT + +/** + * Hypervisor mode exception handlers + * + * If exception is taken to EL2, U-Boot must have been running in hypervisor + * mode. Use the existing stack to save register values onto because + * overwriting the stack pointer would leave no way to inspect its value. + */ + + .macro hyp_enter_exception + str sp, [sp, #(-S_FRAME_SIZE + S_SP)] + sub sp, sp, #S_FRAME_SIZE + stmia sp, {r0 - r12} + mrs r1, elr_hyp + mrs r2, spsr + str lr, [sp, #S_LR] + str r1, [sp, #S_PC] + str r2, [sp, #S_PSR] + str r0, [sp, #S_OLD_R0] + mov r0, sp + .endm + +hyp_undefined_instruction: + hyp_enter_exception + bl do_undefined_instruction + +hyp_software_interrupt: + hyp_enter_exception + bl do_software_interrupt + +hyp_prefetch_abort: + hyp_enter_exception + bl do_prefetch_abort + +hyp_data_abort: + hyp_enter_exception + bl do_data_abort + +hyp_not_used: + hyp_enter_exception + bl do_not_used + +hyp_irq: + hyp_enter_exception + bl do_irq + +hyp_fiq: + hyp_enter_exception + bl do_fiq + +#endif /* CONFIG_CPU_V7_HAS_VIRT */ + #endif /* CONFIG_SPL_BUILD */
This is the base address register for the vector table when in hypervisor mode.
Signed-off-by: Jim Posen jim.posen@gmail.com ---
arch/arm/cpu/armv7/start.S | 5 +++++ arch/arm/lib/relocate.S | 8 ++++++++ 2 files changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 698e15b8e1..bce8ddb1b0 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -113,6 +113,11 @@ switch_to_hypervisor_ret: ldr r0, =_start mcr p15, 0, r0, c12, c0, 0 @Set VBAR #endif +#ifdef CPU_V7_HAS_VIRT + /* Set vector address in CP15 VBAR register */ + ldr r0, =hyp_vector_table + mcr p15, 0, r0, c12, c0, 0 @Set HVBAR +#endif #endif
/* the mask ROM code should have PLL and others stable */ diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S index 14b7f61c1a..3ebf6a519e 100644 --- a/arch/arm/lib/relocate.S +++ b/arch/arm/lib/relocate.S @@ -60,6 +60,14 @@ ENTRY(relocate_vectors) ldmia r0!, {r2-r8,r10} stmia r1!, {r2-r8,r10} #endif +#ifdef CONFIG_CPU_V7_HAS_VIRT + /* + * If the ARM processor has the virtualization extensions, + * use HVBAR to relocate the EL2 exception vectors. + */ + ldr r0, =hyp_vector_table /* load dynamically relocated address */ + mcr p15, 4, r0, c12, c0, 0 /* Set HVBAR */ +#endif #endif bx lr
This code was missed in commit 01abae4d0486 ("Remove various unused interrupt related code")
Signed-off-by: Jim Posen jim.posen@gmail.com ---
arch/arm/lib/vectors.S | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-)
diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S index 89b91b27da..a36e3b7a43 100644 --- a/arch/arm/lib/vectors.S +++ b/arch/arm/lib/vectors.S @@ -224,8 +224,7 @@ IRQ_STACK_START_IN: #define I_BIT 0x80
/* - * use bad_save_user_regs for abort/prefetch/undef/swi ... - * use irq_save_user_regs / irq_restore_user_regs for IRQ/FIQ handling + * use bad_save_user_regs for all exception types */
.macro bad_save_user_regs @@ -242,27 +241,6 @@ IRQ_STACK_START_IN: mov r0, sp @ save current stack into r0 (param register) .endm
- .macro irq_save_user_regs - sub sp, sp, #S_FRAME_SIZE - stmia sp, {r0 - r12} @ Calling r0-r12 - @ !!!! R8 NEEDS to be saved !!!! a reserved stack spot would be good. - add r8, sp, #S_PC - stmdb r8, {sp, lr}^ @ Calling SP, LR - str lr, [r8, #0] @ Save calling PC - mrs r6, spsr - str r6, [r8, #4] @ Save CPSR - str r0, [r8, #8] @ Save OLD_R0 - mov r0, sp - .endm - - .macro irq_restore_user_regs - ldmia sp, {r0 - lr}^ @ Calling r0 - lr - mov r0, r0 - ldr lr, [sp, #S_PC] @ Get PC - add sp, sp, #S_FRAME_SIZE - subs pc, lr, #4 @ return & move spsr_svc into cpsr - .endm - .macro get_bad_stack ldr r13, IRQ_STACK_START_IN @ setup our mode stack
@@ -276,14 +254,6 @@ IRQ_STACK_START_IN: movs pc, lr @ jump to next instruction & switch modes. .endm
- .macro get_irq_stack @ setup IRQ stack - ldr sp, IRQ_STACK_START - .endm - - .macro get_fiq_stack @ setup FIQ stack - ldr sp, FIQ_STACK_START - .endm - /* * exception handlers */
The link register is at a different offset depending on processor mode, so ensure it applies the correct adjustment.
Signed-off-by: Jim Posen jim.posen@gmail.com ---
arch/arm/include/asm/u-boot-arm.h | 14 +++++++------- arch/arm/lib/interrupts.c | 26 +++++++++++++------------- arch/arm/lib/vectors.S | 4 +++- 3 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/arch/arm/include/asm/u-boot-arm.h b/arch/arm/include/asm/u-boot-arm.h index 0b93cc48c5..6ebefa7c86 100644 --- a/arch/arm/include/asm/u-boot-arm.h +++ b/arch/arm/include/asm/u-boot-arm.h @@ -41,17 +41,17 @@ int board_init(void); struct pt_regs;
void bad_mode(void); -void do_undefined_instruction(struct pt_regs *pt_regs); -void do_software_interrupt(struct pt_regs *pt_regs); -void do_prefetch_abort(struct pt_regs *pt_regs); -void do_data_abort(struct pt_regs *pt_regs); -void do_not_used(struct pt_regs *pt_regs); +void do_undefined_instruction(struct pt_regs *pt_regs, bool hyp_mode); +void do_software_interrupt(struct pt_regs *pt_regs, bool hyp_mode); +void do_prefetch_abort(struct pt_regs *pt_regs, bool hyp_mode); +void do_data_abort(struct pt_regs *pt_regs, bool hyp_mode); +void do_not_used(struct pt_regs *pt_regs, bool hyp_mode); #ifdef CONFIG_ARM64 void do_fiq(struct pt_regs *pt_regs, unsigned int esr); void do_irq(struct pt_regs *pt_regs, unsigned int esr); #else -void do_fiq(struct pt_regs *pt_regs); -void do_irq(struct pt_regs *pt_regswq); +void do_fiq(struct pt_regs *pt_regs, bool hyp_mode); +void do_irq(struct pt_regs *pt_regswq, bool hyp_mode); #endif
void reset_misc(void); diff --git a/arch/arm/lib/interrupts.c b/arch/arm/lib/interrupts.c index 6dc27d1d58..498401cdc5 100644 --- a/arch/arm/lib/interrupts.c +++ b/arch/arm/lib/interrupts.c @@ -135,17 +135,17 @@ static inline void fixup_pc(struct pt_regs *regs, int offset) regs->ARM_pc = pc | (regs->ARM_pc & PCMASK); }
-void do_undefined_instruction (struct pt_regs *pt_regs) +void do_undefined_instruction(struct pt_regs *pt_regs, bool hyp_mode) { efi_restore_gd(); printf ("undefined instruction\n"); - fixup_pc(pt_regs, -4); + fixup_pc(pt_regs, hyp_mode ? 0 : thumb_mode(regs) ? -2 : -4); show_regs (pt_regs); show_efi_loaded_images(pt_regs); bad_mode (); }
-void do_software_interrupt (struct pt_regs *pt_regs) +void do_software_interrupt(struct pt_regs *pt_regs, bool _hyp_mode) { efi_restore_gd(); printf ("software interrupt\n"); @@ -155,51 +155,51 @@ void do_software_interrupt (struct pt_regs *pt_regs) bad_mode (); }
-void do_prefetch_abort (struct pt_regs *pt_regs) +void do_prefetch_abort(struct pt_regs *pt_regs, bool hyp_mode) { efi_restore_gd(); printf ("prefetch abort\n"); - fixup_pc(pt_regs, -8); + fixup_pc(pt_regs, hyp_mode ? 0 : -4); show_regs (pt_regs); show_efi_loaded_images(pt_regs); bad_mode (); }
-void do_data_abort (struct pt_regs *pt_regs) +void do_data_abort(struct pt_regs *pt_regs, bool hyp_mode) { efi_restore_gd(); printf ("data abort\n"); - fixup_pc(pt_regs, -8); + fixup_pc(pt_regs, hyp_mode ? 0 : -8); show_regs (pt_regs); show_efi_loaded_images(pt_regs); bad_mode (); }
-void do_not_used (struct pt_regs *pt_regs) +void do_not_used(struct pt_regs *pt_regs, bool hyp_mode) { efi_restore_gd(); printf ("not used\n"); - fixup_pc(pt_regs, -8); + fixup_pc(pt_regs, hyp_mode ? 0 : -8); show_regs (pt_regs); show_efi_loaded_images(pt_regs); bad_mode (); }
-void do_fiq (struct pt_regs *pt_regs) +void do_fiq(struct pt_regs *pt_regs, bool hyp_mode) { efi_restore_gd(); printf ("fast interrupt request\n"); - fixup_pc(pt_regs, -8); + fixup_pc(pt_regs, hyp_mode ? 0 : -4); show_regs (pt_regs); show_efi_loaded_images(pt_regs); bad_mode (); }
-void do_irq (struct pt_regs *pt_regs) +void do_irq(struct pt_regs *pt_regs, bool hyp_mode) { efi_restore_gd(); printf ("interrupt request\n"); - fixup_pc(pt_regs, -8); + fixup_pc(pt_regs, hyp_mode ? 0 : -4); show_regs (pt_regs); show_efi_loaded_images(pt_regs); bad_mode (); diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S index a36e3b7a43..dff2155dad 100644 --- a/arch/arm/lib/vectors.S +++ b/arch/arm/lib/vectors.S @@ -239,6 +239,7 @@ IRQ_STACK_START_IN: mov r1, lr stmia r5, {r0 - r3} @ save sp_SVC, lr_SVC, pc, cpsr mov r0, sp @ save current stack into r0 (param register) + mov r1, #0 @ set r1 param to 0, not hypervisor mode .endm
.macro get_bad_stack @@ -321,7 +322,8 @@ fiq: str r1, [sp, #S_PC] str r2, [sp, #S_PSR] str r0, [sp, #S_OLD_R0] - mov r0, sp + mov r0, sp @ set r0 param to struct pt_regs on stack + mov r1, #1 @ set r1 param to 1, hypervisor mode .endm
hyp_undefined_instruction:
On Sun, Oct 24, 2021 at 07:58:03PM -0400, Jim Posen wrote:
Currently, when U-Boot is running in hypervisor mode on ARMv7-A CPUs with virtualization extensions, the exception handling does not work. A couple things need to change which are detailed in my earlier message to the u-boot mailing list with the subject "Exception handling in HYP mode on ARMv7-A".
I have verified that this patch series works on the ODroid XU4 and the Raspberry Pi 3B in Aarch32 mode when running in hypervisor mode. One simple way to verify is by running the miscellaneous "exception" command (CMD_EXCEPTION).
Jim Posen (5): Compile for ARMv7-A with virtualization extensions Hypervisor mode interrupt vector table Set HVBAR register correctly Remove dead code Fix PC adjustment logic in exception handlers
arch/arm/Makefile | 4 + arch/arm/cpu/armv7/start.S | 5 ++ arch/arm/include/asm/u-boot-arm.h | 14 ++-- arch/arm/lib/interrupts.c | 26 +++--- arch/arm/lib/relocate.S | 8 ++ arch/arm/lib/vectors.S | 129 +++++++++++++++++++++++------- arch/arm/mach-bcm283x/Kconfig | 3 + arch/arm/mach-exynos/Kconfig | 1 + 8 files changed, 139 insertions(+), 51 deletions(-)
This all seems fine, but I don't know enough about the specifics here to comment on the implementation. Is there any we we could also test this via QEMU for example?
I sent a new version of the patch series with a couple of fixes and
instructions for testing with QEMU.
On Tue, Nov 23, 2021 at 10:24 AM Tom Rini trini@konsulko.com wrote:
On Sun, Oct 24, 2021 at 07:58:03PM -0400, Jim Posen wrote:
Currently, when U-Boot is running in hypervisor mode on ARMv7-A CPUs with virtualization extensions, the exception handling does not work. A couple things need to change which are detailed in my earlier message to the u-boot mailing list with the subject "Exception handling in HYP mode on ARMv7-A".
I have verified that this patch series works on the ODroid XU4 and the Raspberry Pi 3B in Aarch32 mode when running in hypervisor mode. One simple way to verify is by running the miscellaneous "exception" command (CMD_EXCEPTION).
Jim Posen (5): Compile for ARMv7-A with virtualization extensions Hypervisor mode interrupt vector table Set HVBAR register correctly Remove dead code Fix PC adjustment logic in exception handlers
arch/arm/Makefile | 4 + arch/arm/cpu/armv7/start.S | 5 ++ arch/arm/include/asm/u-boot-arm.h | 14 ++-- arch/arm/lib/interrupts.c | 26 +++--- arch/arm/lib/relocate.S | 8 ++ arch/arm/lib/vectors.S | 129 +++++++++++++++++++++++------- arch/arm/mach-bcm283x/Kconfig | 3 + arch/arm/mach-exynos/Kconfig | 1 + 8 files changed, 139 insertions(+), 51 deletions(-)
This all seems fine, but I don't know enough about the specifics here to comment on the implementation. Is there any we we could also test this via QEMU for example?
-- Tom
participants (2)
-
Jim Posen -
Tom Rini