From: Will Deacon <will@kernel.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Ard Biesheuvel <ardb+git@google.com>,
linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Mark Rutland <mark.rutland@arm.com>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH v3 6/8] arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state
Date: Fri, 19 Sep 2025 14:54:59 +0100 [thread overview]
Message-ID: <aM1gs9rhKbrB2Val@willie-the-truck> (raw)
In-Reply-To: <CAMj1kXHDtTNMzih7OoTYU0vN4M3mOmFL3YOfaPUKReyJQA6uAQ@mail.gmail.com>
On Fri, Sep 19, 2025 at 03:42:12PM +0200, Ard Biesheuvel wrote:
> On Fri, 19 Sept 2025 at 13:35, Will Deacon <will@kernel.org> wrote:
> >
> > On Thu, Sep 18, 2025 at 12:30:17PM +0200, Ard Biesheuvel wrote:
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > Replace the spinlock in the arm64 glue code with a mutex, so that
> > > the CPU can preempted while running the EFI runtime service.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > > arch/arm64/kernel/efi.c | 13 ++++++++++---
> > > 1 file changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > > index 0d52414415f3..4372fafde8e9 100644
> > > --- a/arch/arm64/kernel/efi.c
> > > +++ b/arch/arm64/kernel/efi.c
> > > @@ -166,15 +166,22 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
> > > return s;
> > > }
> > >
> > > -static DEFINE_RAW_SPINLOCK(efi_rt_lock);
> > > +static DEFINE_MUTEX(efi_rt_lock);
> > >
> > > bool arch_efi_call_virt_setup(void)
> > > {
> > > if (!may_use_simd())
> > > return false;
> > >
> > > + /*
> > > + * This might be called from a non-sleepable context so try to take the
> > > + * lock but don't block on it. This should never fail in practice, as
> > > + * all EFI runtime calls are serialized under the efi_runtime_lock.
> > > + */
> > > + if (WARN_ON(!mutex_trylock(&efi_rt_lock)))
> > > + return false;
> >
> > If it will never fail in practice, why do we need the lock at all? Can we
> > just assert that the efi_runtime_lock is held instead and rely on that?
> >
>
> Excellent point.
>
> Do you mean a lockdep assert? efi_runtime_lock is a semaphore, so
> there is no is_locked() API that we can BUG() on here.
Yes, I was thinking of lockdep. Even though lockdep doesn't tend to be
enabled in production, just having it in the code is useful documentation
imo.
Will
next prev parent reply other threads:[~2025-09-19 13:55 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-18 10:30 [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
2025-09-18 10:30 ` [PATCH v3 1/8] efi: Add missing static initializer for efi_mm::cpus_allowed_lock Ard Biesheuvel
2025-09-18 10:30 ` [PATCH v3 2/8] efi/runtime: Return success/failure from arch_efi_call_virt_setup() Ard Biesheuvel
2025-09-18 10:30 ` [PATCH v3 3/8] efi/runtime: Deal with arch_efi_call_virt_setup() returning failure Ard Biesheuvel
2025-09-18 10:30 ` [PATCH v3 4/8] arm64/fpsimd: Permit kernel mode NEON with IRQs off Ard Biesheuvel
2025-09-19 11:33 ` Will Deacon
2025-09-18 10:30 ` [PATCH v3 5/8] arm64/fpsimd: Drop special handling for EFI runtime services Ard Biesheuvel
2025-09-18 11:57 ` Ard Biesheuvel
2025-09-18 13:10 ` Mark Brown
2025-09-22 6:55 ` Ard Biesheuvel
2025-09-18 10:30 ` [PATCH v3 6/8] arm64/efi: Use a mutex to protect the EFI stack and FP/SIMD state Ard Biesheuvel
2025-09-19 11:35 ` Will Deacon
2025-09-19 13:42 ` Ard Biesheuvel
2025-09-19 13:54 ` Will Deacon [this message]
2025-09-18 10:30 ` [PATCH v3 7/8] arm64/efi: Move uaccess en/disable out of efi_set_pgd() Ard Biesheuvel
2025-09-19 11:36 ` Will Deacon
2025-09-18 10:30 ` [PATCH v3 8/8] arm64/efi: Call EFI runtime services without disabling preemption Ard Biesheuvel
2025-09-19 11:36 ` Will Deacon
2025-09-18 11:33 ` [PATCH v3 0/8] arm64: Make EFI calls preemptible Ard Biesheuvel
2025-09-18 11:44 ` Will Deacon
2025-09-18 11:48 ` Ard Biesheuvel
2025-09-19 11:38 ` Will Deacon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aM1gs9rhKbrB2Val@willie-the-truck \
--to=will@kernel.org \
--cc=ardb+git@google.com \
--cc=ardb@kernel.org \
--cc=bigeasy@linutronix.de \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox