Re: [PATCH 09/38] arm64: replace local_daif helpers

On 9/17/26 09:14, Jinjie Ruan wrote:

在 2026/9/14 18:20, Vladimir Murzin 写道:

quoted

From: Ada Couprie Diaz <redacted>

Replace existing `local_daif_...` usage with the new `local_all_irqs...`
helpers, adding the necessary save-state struct where required.

Slightly rework `apei_claim_sea()`'s handling of interrupt masking to
fit with the new APIs.  Currently, it saves the DAIF flags to restore
later and the `local_irq` ones to check if interrupts are disabled.
It then masks AIF to signal the SEA (forcing DAIF masking), and will
unmask to IF (re-enabling PMR masking if pseudo-NMIs are in use) if it
interrupted a task with interrupts unmaksed, restoring with the DAIF
flags from the beginning afterwards.

As we want to preserve a "mask/unmask" process, change the logic from
"mask AIF/unmask A/unmask IF" to "mask IF/mask A/unmask A/unmask IF" :
first mask with `local_irq_save()`, mask/unmask further around the
SEA signaling, and restore at the end with `local_irq_restore()`.
Now that both APIs properly work with DAIF and PMR, this works
as expected and also removes the need for a "save flags without masking"
function for the new helpers, as this was the only user.

Remove the use of GIC_PRIO_PSR_I_SET in `kvm/hyp/nvhe/switch.c`,
as we now check both PMR and DAIF everywhere and we don't need to signal
interrupts being masked in DAIF in the PMR anymore.

Signed-off-by: Ada Couprie Diaz <redacted>
Signed-off-by: Vladimir Murzin <redacted>
Reviewed-by: Jinjie Ruan <redacted>
---
 arch/arm64/kernel/acpi.c          | 15 +++++++--------
 arch/arm64/kernel/hibernate.c     | 14 +++++++-------
 arch/arm64/kernel/machine_kexec.c |  4 ++--
 arch/arm64/kernel/setup.c         |  4 ++--
 arch/arm64/kernel/smp.c           | 10 +++++-----
 arch/arm64/kernel/suspend.c       | 14 +++++++-------
 arch/arm64/kvm/hyp/nvhe/switch.c  |  2 +-
 arch/arm64/kvm/hyp/vgic-v3-sr.c   |  7 ++++---
 arch/arm64/kvm/hyp/vhe/switch.c   | 14 ++++++++------
 arch/arm64/mm/mmu.c               |  7 ++++---
 10 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 681aa2bbc399..ea9a9da7e412 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c

@@ -33,7 +33,7 @@
 #include <acpi/processor.h>
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
-#include <asm/daifflags.h>
+#include <asm/interrupts/masking.h>
 #include <asm/smp_plat.h>
 
 int acpi_noirq = 1;		/* skip ACPI IRQ initialization */

@@ -391,15 +391,14 @@ int apei_claim_sea(struct pt_regs *regs)
 	int err = -ENOENT;
 	bool return_to_irqs_enabled;
 	unsigned long current_flags;
+	arm64_exc_hwstates_t hwstate;
 
 	if (!IS_ENABLED(CONFIG_ACPI_APEI_GHES))
 		return err;
 
-	current_flags = local_daif_save_flags();
-
-	/* current_flags isn't useful here as daif doesn't tell us about pNMI */
-	return_to_irqs_enabled = !irqs_disabled_flags(arch_local_save_flags());
+	local_irq_save(current_flags);
 
+	return_to_irqs_enabled = !irqs_disabled_flags(current_flags);
 	if (regs)
 		return_to_irqs_enabled = !regs_irqs_disabled(regs);
 

@@ -407,10 +406,11 @@ int apei_claim_sea(struct pt_regs *regs)
 	 * SEA can interrupt SError, mask it and describe this as an NMI so
 	 * that APEI defers the handling.
 	 */
-	local_daif_restore(DAIF_ERRCTX);
+	hwstate = local_exceptions_save_mask(ERROR_CONTEXT);
 	nmi_enter();
 	err = ghes_notify_sea();
 	nmi_exit();
+	local_exceptions_restore(hwstate);

Hi Vladimir,

I think mixing local_irq_save()/local_irq_restore() and
local_exceptions_save_mask()/local_exceptions_restore here may be
problematic, because arch_local_irq_save() does not take into full
account the state of DAIF and the pseudo-NMI. For example, when the
pseudo-NMI is enabled, arch_local_irq_save() only sets PMR to
GIC_PRIO_IRQOFF without taking DAIF into consideration, whereas
local_exceptions_save_mask(NOIRQ_CONTEXT) additionally clears all the
DAIF bits.

But whereas local_exceptions_save_mask() returns exactly the current
hwstate arch_local_irq_save() sets, and local_exceptions_restore() also
needs to restore a unified state.

Hi Jinjie,

Yeah, that nesting makes my head spin. Would it be cleaner to apply
the masking sequentially, something like:

int apei_claim_sea(struct pt_regs *regs)
{
	arm64_exc_hwstates_t hwstate;
	bool irqs_disabled;
	int err = -ENOENT;

	if (!IS_ENABLED(CONFIG_ACPI_APEI_GHES))
		return err;

	/*
	 * SEA can interrupt SError, mask it and describe this as an NMI so
	 * that APEI defers the handling.
	 */
	hwstate = local_exceptions_save_mask(ERROR_CONTEXT);
	nmi_enter();
	err = ghes_notify_sea();
	nmi_exit();
	local_exceptions_restore(hwstate);

	if (err)
		return err;

	/*
	 * APEI NMI-like notifications are deferred to irq_work. Unless
	 * we interrupted irqs-masked code, we can do that now.
	 */
	irqs_disabled = regs ? regs_irqs_disabled(regs) : arch_irqs_disabled_flags(hwstate.saved.flags);

	if (!irqs_disabled) {
		hwstate = local_exceptions_save_mask(NOIRQ_CONTEXT);
		__irq_enter();
		irq_work_run();
		__irq_exit();
		local_exceptions_restore(hwstate);
	} else {
		pr_warn_ratelimited("APEI work queued but not completed\n");
		err = -EINPROGRESS;
	}

	return err;
}


Cheers
Vladimir

Best regards,
Jinjie

quoted

 
 	/*
 	 * APEI NMI-like notifications are deferred to irq_work. Unless

@@ -418,7 +418,6 @@ int apei_claim_sea(struct pt_regs *regs)
 	 */
 	if (!err) {
 		if (return_to_irqs_enabled) {
-			local_daif_restore(DAIF_PROCCTX_NOIRQ);
 			__irq_enter();
 			irq_work_run();
 			__irq_exit();

@@ -428,7 +427,7 @@ int apei_claim_sea(struct pt_regs *regs)
 		}
 	}
 
-	local_daif_restore(current_flags);
+	local_irq_restore(current_flags);
 
 	return err;
 }