arch/arm64/kvm/arm.c | 53 +++++-------- arch/arm64/kvm/hyp/include/nvhe/spinlock.h | 5 ++ arch/arm64/kvm/hyp/nvhe/ffa.c | 86 +++++++++----------- arch/arm64/kvm/hyp/nvhe/mm.c | 37 +++------ arch/arm64/kvm/hyp/nvhe/page_alloc.c | 13 +-- arch/arm64/kvm/hyp/nvhe/pkvm.c | 122 ++++++++++++----------------- arch/arm64/kvm/mmu.c | 95 +++++++++------------- arch/arm64/kvm/pkvm.c | 19 ++--- arch/arm64/kvm/psci.c | 19 ++--- arch/arm64/kvm/reset.c | 8 +- 10 files changed, 182 insertions(+), 275 deletions(-)
Hi everyone,
I stumbled upon a lock leak while reviewing Sebastien's recent GIC Hardening
series [1] on an early error path. It reminded me that we've run into this exact
kind of bug pretty often while working on pKVM. Since we're going to be
hopefully upstream even more pKVM code soon, I'd like to try and fix this
pattern once and for all.
This series refactors a good chunk of KVM/arm64 codebase—covering both the host
at EL1 and the hypervisor at EL2—to use the guard() and scoped_guard() macros
from <linux/cleanup.h>.
By tying our locks to the scope:
- We can safely use early returns (like return -EINVAL;) without accidentally
leaking locks.
- We can get rid of a lot of tangled goto exit labels.
- Reviewing patches gets a lot easier since we don't have to mentally track lock
states across giant functions.
A common concern with these RAII-style macros is that they might bloat the
generated assembly. However, looking at the compiled code, it actually does the
opposite. As an example, looking at the EL2 object file
arch/arm64/kvm/hyp/nvhe/pkvm.nvhe.o before and after this series:
- Instruction count: Dropped by 39 lines of assembly.
- .text segment size: Shrank by 166 bytes.
This happens because the compiler no longer has to generate dedicated branch
targets for exit blocks, nor the stack frame unwinding jumps just to call
hyp_spin_unlock. It just efficiently inlines the unlock instruction right where
the return happens.
The series is split into two parts:
- Patches 1-5 (EL2): Adds the DEFINE_LOCK_GUARD_1 wrapper for hyp_spinlock and
cleans up the nvhe/ directory.
- Patches 6-10 (EL1): Cleans up manual mutex_lock and spin_lock calls across the
standard KVM host code (mmu.c, arm.c, psci.c, reset.c, pkvm.c).
There are definitely other parts of the KVM codebase that could benefit from
this (especially the vGIC), but I'm stopping here for now to see what everyone
thinks of the approach before touching anything else.
Based on Linux 7.0-rc3.
Cheers,
/fuad
[1] https://lore.kernel.org/all/CA+EHjTy051kjdReuESmk3HpMsBEW=s3DxhJXa8vQJFHz--wG2Q@mail.gmail.com/
To:
---
Fuad Tabba (10):
KVM: arm64: Add scoped resource management (guard) for hyp_spinlock
KVM: arm64: Use guard(hyp_spinlock) in page_alloc.c
KVM: arm64: Use guard(hyp_spinlock) in ffa.c
KVM: arm64: Use guard(hyp_spinlock) in mm.c
KVM: arm64: Use guard(hyp_spinlock) in pkvm.c
KVM: arm64: Use guard(mutex) in mmu.c
KVM: arm64: Use scoped resource management in arm.c
KVM: arm64: Use guard(spinlock) in psci.c
KVM: arm64: Use guard(spinlock) in reset.c
KVM: arm64: Use guard(mutex) in pkvm.c
arch/arm64/kvm/arm.c | 53 +++++--------
arch/arm64/kvm/hyp/include/nvhe/spinlock.h | 5 ++
arch/arm64/kvm/hyp/nvhe/ffa.c | 86 +++++++++-----------
arch/arm64/kvm/hyp/nvhe/mm.c | 37 +++------
arch/arm64/kvm/hyp/nvhe/page_alloc.c | 13 +--
arch/arm64/kvm/hyp/nvhe/pkvm.c | 122 ++++++++++++-----------------
arch/arm64/kvm/mmu.c | 95 +++++++++-------------
arch/arm64/kvm/pkvm.c | 19 ++---
arch/arm64/kvm/psci.c | 19 ++---
arch/arm64/kvm/reset.c | 8 +-
10 files changed, 182 insertions(+), 275 deletions(-)
---
base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681
change-id: 20260316-tabba-el2_guard-2f7e3fa06f69
Best regards,
--
Fuad Tabba <tabba@google.com>
On Mon, 16 Mar 2026 17:35:21 +0000, Fuad Tabba <tabba@google.com> wrote: > > Hi everyone, > > I stumbled upon a lock leak while reviewing Sebastien's recent GIC Hardening > series [1] on an early error path. It reminded me that we've run into this exact > kind of bug pretty often while working on pKVM. Since we're going to be > hopefully upstream even more pKVM code soon, I'd like to try and fix this > pattern once and for all. I'm of the opposite opinion. I'd rather convert things as code gets modified, because this otherwise makes backporting patches a real pain (we have been through that exercise with the irq stack, and it wasn't an enjoyable experience). So while I'm not opposed to this in general, I'd rather see it as a prefix for new features, instead of a standalone series. [...] > There are definitely other parts of the KVM codebase that could benefit from > this (especially the vGIC), but I'm stopping here for now to see what everyone > thinks of the approach before touching anything else. The vgic is specially difficult to convert because of some of the constructs that take a lock in a function and release it in another, defeating the scope-based locking. I'll have a look at the series anyway. Thanks, M. -- Without deviation from the norm, progress is not possible.
Hi Marc, On Tue, 17 Mar 2026 at 08:20, Marc Zyngier <maz@kernel.org> wrote: > > On Mon, 16 Mar 2026 17:35:21 +0000, > Fuad Tabba <tabba@google.com> wrote: > > > > Hi everyone, > > > > I stumbled upon a lock leak while reviewing Sebastien's recent GIC Hardening > > series [1] on an early error path. It reminded me that we've run into this exact > > kind of bug pretty often while working on pKVM. Since we're going to be > > hopefully upstream even more pKVM code soon, I'd like to try and fix this > > pattern once and for all. > > I'm of the opposite opinion. I'd rather convert things as code gets > modified, because this otherwise makes backporting patches a real pain > (we have been through that exercise with the irq stack, and it wasn't > an enjoyable experience). > > So while I'm not opposed to this in general, I'd rather see it as a > prefix for new features, instead of a standalone series. I'm happy to do whatever is easier for you. My thinking was that, since the coming pKVM series (assuming Will's series goes in soon) is going to be pretty long, it would be easier to have this done earlier. > > [...] > > > There are definitely other parts of the KVM codebase that could benefit from > > this (especially the vGIC), but I'm stopping here for now to see what everyone > > thinks of the approach before touching anything else. > > The vgic is specially difficult to convert because of some of the > constructs that take a lock in a function and release it in another, > defeating the scope-based locking. > > I'll have a look at the series anyway. Thanks! In case you think this is a good idea, let me know and I'll respin without those "Change-ids". Cheers, /fuad > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
Hi again, On Mon, 16 Mar 2026 at 17:35, Fuad Tabba <tabba@google.com> wrote: > > Hi everyone, This was my first time using b4, and I thought it would strip the Change-id's, and this was supposed to be a "reflect" run, but obviously something went wrong there :) Other than that, this series is ready for review. Sorry for the Change-id noise! Cheers, /fuad > > I stumbled upon a lock leak while reviewing Sebastien's recent GIC Hardening > series [1] on an early error path. It reminded me that we've run into this exact > kind of bug pretty often while working on pKVM. Since we're going to be > hopefully upstream even more pKVM code soon, I'd like to try and fix this > pattern once and for all. > > This series refactors a good chunk of KVM/arm64 codebase—covering both the host > at EL1 and the hypervisor at EL2—to use the guard() and scoped_guard() macros > from <linux/cleanup.h>. > > By tying our locks to the scope: > - We can safely use early returns (like return -EINVAL;) without accidentally > leaking locks. > - We can get rid of a lot of tangled goto exit labels. > - Reviewing patches gets a lot easier since we don't have to mentally track lock > states across giant functions. > > A common concern with these RAII-style macros is that they might bloat the > generated assembly. However, looking at the compiled code, it actually does the > opposite. As an example, looking at the EL2 object file > arch/arm64/kvm/hyp/nvhe/pkvm.nvhe.o before and after this series: > - Instruction count: Dropped by 39 lines of assembly. > - .text segment size: Shrank by 166 bytes. > > This happens because the compiler no longer has to generate dedicated branch > targets for exit blocks, nor the stack frame unwinding jumps just to call > hyp_spin_unlock. It just efficiently inlines the unlock instruction right where > the return happens. > > The series is split into two parts: > - Patches 1-5 (EL2): Adds the DEFINE_LOCK_GUARD_1 wrapper for hyp_spinlock and > cleans up the nvhe/ directory. > - Patches 6-10 (EL1): Cleans up manual mutex_lock and spin_lock calls across the > standard KVM host code (mmu.c, arm.c, psci.c, reset.c, pkvm.c). > > There are definitely other parts of the KVM codebase that could benefit from > this (especially the vGIC), but I'm stopping here for now to see what everyone > thinks of the approach before touching anything else. > > Based on Linux 7.0-rc3. > > Cheers, > /fuad > > [1] https://lore.kernel.org/all/CA+EHjTy051kjdReuESmk3HpMsBEW=s3DxhJXa8vQJFHz--wG2Q@mail.gmail.com/ > > To: > > --- > Fuad Tabba (10): > KVM: arm64: Add scoped resource management (guard) for hyp_spinlock > KVM: arm64: Use guard(hyp_spinlock) in page_alloc.c > KVM: arm64: Use guard(hyp_spinlock) in ffa.c > KVM: arm64: Use guard(hyp_spinlock) in mm.c > KVM: arm64: Use guard(hyp_spinlock) in pkvm.c > KVM: arm64: Use guard(mutex) in mmu.c > KVM: arm64: Use scoped resource management in arm.c > KVM: arm64: Use guard(spinlock) in psci.c > KVM: arm64: Use guard(spinlock) in reset.c > KVM: arm64: Use guard(mutex) in pkvm.c > > arch/arm64/kvm/arm.c | 53 +++++-------- > arch/arm64/kvm/hyp/include/nvhe/spinlock.h | 5 ++ > arch/arm64/kvm/hyp/nvhe/ffa.c | 86 +++++++++----------- > arch/arm64/kvm/hyp/nvhe/mm.c | 37 +++------ > arch/arm64/kvm/hyp/nvhe/page_alloc.c | 13 +-- > arch/arm64/kvm/hyp/nvhe/pkvm.c | 122 ++++++++++++----------------- > arch/arm64/kvm/mmu.c | 95 +++++++++------------- > arch/arm64/kvm/pkvm.c | 19 ++--- > arch/arm64/kvm/psci.c | 19 ++--- > arch/arm64/kvm/reset.c | 8 +- > 10 files changed, 182 insertions(+), 275 deletions(-) > --- > base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681 > change-id: 20260316-tabba-el2_guard-2f7e3fa06f69 > > Best regards, > -- > Fuad Tabba <tabba@google.com> >
© 2016 - 2026 Red Hat, Inc.