On 14/10/2025 10:46, Mark Rutland wrote:
On Tue, Oct 14, 2025 at 10:25:36AM +0100, Ada Couprie Diaz wrote:
EL0 exception handlers should always call `exit_to_user_mode()` with interrupts unmasked. When handling a completed single-step, we skip the if block and `local_daif_restore(DAIF_PROCCTX)` never gets called, which ends up calling `exit_to_user_mode()` with interrupts masked.
This is broken if pNMI is in use, as `do_notify_resume()` will try to enable interrupts, but `local_irq_enable()` will only change the PMR, leaving interrupts masked via DAIF.
I think we might want to spell thius out a bit more, e.g.
That's fair, your version lays it out better and is probably more accessible !
| We intend that EL0 exception handlers unmask all DAIF exceptions | before calling exit_to_user_mode(). | | When completing single-step of a suspended breakpoint, we do not call | local_daif_restore(DAIF_PROCCTX) before calling exit_to_user_mode(), | leaving all DAIF exceptions masked. | | When pseudo-NMIs are not in use this is benign. | | When pseudo-NMIs are in use, this is unsound. At this point interrupts | are masked by both DAIF.IF and PMR_EL1, and subsequent irq flag | manipulation may not work correctly. For example, a subsequent | local_irq_enable() within exit_to_user_mode_loop() will only unmask | interrupts via PMR_EL1 (leaving those masked via DAIF.IF), and | anything depending on interrupts being unmasked (e.g. delivery of | signals) will not work correctly. | | This can by detected by CONFIG_ARM64_DEBUG_PRIORITY_MASKING.
This could be "This was detected [...]", as that's what I did to detect the error, but happy with either : we don't need to bikeshed this !
With or without that, this looks good to me, so:
Acked-by: Mark Rutland mark.rutland@arm.com
Mark.
Thanks for the quick review, Ada