Hi Andrew, On 08/02/2025 00:02, Andrew Cooper wrote:
While fixing some common/arch boundaries for UBSAN support on other architectures, the following debugging patch:diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index c1f2d1b89d43..58d1d048d339 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -504,6 +504,8 @@ void asmlinkage __init start_xen(unsigned long fdt_paddr) system_state = SYS_STATE_active; + dump_execution_state(); + for_each_domain( d ) domain_unpause_by_systemcontroller(d); fails with: (XEN) *** Serial input to DOM0 (type 'CTRL-a' three times to switch input) (XEN) CPU0: Unexpected Trap: Undefined Instruction (XEN) ----[ Xen-4.20-rc arm32 debug=n Not tainted ]---- (XEN) CPU: 0 <snip> (XEN) (XEN) **************************************** (XEN) Panic on CPU 0: (XEN) CPU0: Unexpected Trap: Undefined Instruction (XEN) **************************************** This is because the condition for init text is wrong. While there's nothing interesting from that point onwards in start_xen(), it's also wrong for any livepatch which brings in an adjusted BUG_FRAME(). Use is_active_kernel_text() which is the correct test for this purpose, and is aware of init and livepatch regions too. Commit c8d4b6304a5e ("xen/arm: add support for run_in_exception_handler()"), made run_in_exception_handler() work, but didn't complete the TODO left in commit 3e802c6ca1fb ("xen/arm: Correctly support WARN_ON"). Do so, to make ARM consistent with other architectures.
This was done on purpose. If you look at the current implementation of run_in_exception_handler(), it will clobber some registers.
With your patch #2, the function should only clobber one. It is a bit better, but it still not great. So I think we need to stick with WARN() on Arm (+ maybe a comment explaning why it is implemented differently).
Cheers, -- Julien Grall
