• Mark Rutland's avatar
    arm64: defer clearing DAIF.D · 080297be
    Mark Rutland authored
    For historical reasons we unmask debug exceptions in __cpu_setup(), but
    it's not necessary to unmask debug exceptions this early in the
    boot/idle entry paths. It would be better to unmask debug exceptions
    later in C code as this simplifies the current code and will make it
    easier to rework exception masking logic to handle non-DAIF bits in
    future (e.g. PSTATE.{ALLINT,PM}).
    
    We started clearing DAIF.D in __cpu_setup() in commit:
    
      2ce39ad1 ("arm64: debug: unmask PSTATE.D earlier")
    
    At the time, we needed to ensure that DAIF.D was clear on the primary
    CPU before scheduling and preemption were possible, and chose to do this
    in __cpu_setup() so that this occurred in the same place for primary and
    secondary CPUs. As we cannot handle debug exceptions this early, we
    placed an ISB between initializing MDSCR_EL1 and clearing DAIF.D so that
    no exceptions should be triggered.
    
    Subsequently we rewrote the return-from-{idle,suspend} paths to use
    __cpu_setup() in commit:
    
      cabe1c81 ("arm64: Change cpu_resume() to enable mmu early then access sleep_sp by va")
    
    ... which allowed for earlier use of the MMU and had the desirable
    property of using the same code to reset the CPU in the cold and warm
    boot paths. This introduced a bug: DAIF.D was clear while
    cpu_do_resume() restored MDSCR_EL1 and other control registers (e.g.
    breakpoint/watchpoint control/value registers), and so we could
    unexpectedly take debug exceptions.
    
    We fixed that in commit:
    
      744c6c37 ("arm64: kernel: Fix unmasked debug exceptions when restoring mdscr_el1")
    
    ... by having cpu_do_resume() use the `disable_dbg` macro to set DAIF.D
    before restoring MDSCR_EL1 and other control registers. This relies on
    DAIF.D being subsequently cleared again in cpu_resume().
    
    Subsequently we reworked DAIF masking in commit:
    
      0fbeb318 ("arm64: explicitly mask all exceptions")
    
    ... where we began enforcing a policy that DAIF.D being set implies all
    other DAIF bits are set, and so e.g. we cannot take an IRQ while DAIF.D
    is set. As part of this the use of `disable_dbg` in cpu_resume() was
    replaced with `disable_daif` for consistency with the rest of the
    kernel.
    
    These days, there's no need to clear DAIF.D early within __cpu_setup():
    
    * setup_arch() clears DAIF.DA before scheduling and preemption are
      possible on the primary CPU, avoiding the problem we we originally
      trying to work around.
    
      Note: DAIF.IF get cleared later when interrupts are enabled for the
      first time.
    
    * secondary_start_kernel() clears all DAIF bits before scheduling and
      preemption are possible on secondary CPUs.
    
      Note: with pseudo-NMI, the PMR is initialized here before any DAIF
      bits are cleared. Similar will be necessary for the architectural NMI.
    
    * cpu_suspend() restores all DAIF bits when returning from idle,
      ensuring that we don't unexpectedly leave DAIF.D clear or set.
    
      Note: with pseudo-NMI, the PMR is initialized here before DAIF is
      cleared. Similar will be necessary for the architectural NMI.
    
    This patch removes the unmasking of debug exceptions from __cpu_setup(),
    relying on the above locations to initialize DAIF. This allows some
    other cleanups:
    
    * It is no longer necessary for cpu_resume() to explicitly mask debug
      (or other) exceptions, as it is always called with all DAIF bits set.
      Thus we drop the use of `disable_daif`.
    
    * The `enable_dbg` macro is no longer used, and so is dropped.
    
    * It is no longer necessary to have an ISB immediately after
      initializing MDSCR_EL1 in __cpu_setup(), and we can revert to relying
      on the context synchronization that occurs when the MMU is enabled
      between __cpu_setup() and code which clears DAIF.D
    
    Comments are added to setup_arch() and secondary_start_kernel() to
    explain the initial unmasking of the DAIF bits.
    Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
    Cc: Catalin Marinas <catalin.marinas@arm.com>
    Cc: Mark Brown <broonie@kernel.org>
    Cc: Will Deacon <will@kernel.org>
    Link: https://lore.kernel.org/r/20240422113523.4070414-3-mark.rutland@arm.comSigned-off-by: default avatarWill Deacon <will@kernel.org>
    080297be
smp.c 26.5 KB