Re: [PATCH v5 14/36] KVM: arm64: gic-v5: Add vgic-v5 save/restore hyp interface

Sascha Bischoff
2026-03-04T11:33:15Z

On Tue, 2026-03-03 at 17:10 +0000, Marc Zyngier wrote:
> On Thu, 26 Feb 2026 15:59:02 +0000,
> Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
> >
> > Introduce hyp functions to save/restore the following GICv5 state:
> >
> > * ICC_ICSR_EL1
> > * ICH_APR_EL2
> > * ICH_PPI_ACTIVERx_EL2
> > * ICH_PPI_DVIRx_EL2
> > * ICH_PPI_ENABLERx_EL2
> > * ICH_PPI_PENDRRx_EL2
> > * ICH_PPI_PRIORITYRx_EL2
> > * ICH_VMCR_EL2
> >
> > All of these are saved/restored to/from the KVM vgic_v5 CPUIF
> > shadow
> > state, with the exception of the active, pending, and enable
> > state. The pending state is saved and restored from kvm_host_data
> > as
> > any changes here need to be tracked and propagated back to the
> > vgic_irq shadow structures (coming in a future commit). Therefore,
> > an
> > entry and an exit copy is required. The active and enable state is
> > restored from the vgic_v5 CPUIF, but is saved to kvm_host_data.
> > Again,
> > this needs to by synced back into the shadow data structures.
> >
> > The ICSR must be save/restored as this register is shared between
> > host
> > and guest. Therefore, to avoid leaking host state to the guest,
> > this
> > must be saved and restored. Moreover, as this can by used by the
> > host
> > at any time, it must be save/restored eagerly. Note: the host state
> > is
> > not preserved as the host should only use this register when
> > preemption is disabled.
> >
> > As part of restoring the ICH_VMCR_EL2 and ICH_APR_EL2, GICv3-compat
> > mode is also disabled by setting the ICH_VCTLR_EL2.V3 bit to 0. The
> > correspoinding GICv3-compat mode enable is part of the VMCR & APR
> > restore for a GICv3 guest as it only takes effect when actually
> > running a guest.
> >
> > Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > ---
> >  arch/arm64/include/asm/kvm_asm.h   |   4 +
> >  arch/arm64/include/asm/kvm_host.h  |  16 ++++
> >  arch/arm64/include/asm/kvm_hyp.h   |   8 ++
> >  arch/arm64/kvm/hyp/nvhe/Makefile   |   2 +-
> >  arch/arm64/kvm/hyp/nvhe/hyp-main.c |  32 ++++++++
> >  arch/arm64/kvm/hyp/vgic-v5-sr.c    | 123
> > +++++++++++++++++++++++++++++
> >  arch/arm64/kvm/hyp/vhe/Makefile    |   2 +-
> >  include/kvm/arm_vgic.h             |  21 +++++
> >  8 files changed, 206 insertions(+), 2 deletions(-)
> >  create mode 100644 arch/arm64/kvm/hyp/vgic-v5-sr.c
> >
> > diff --git a/arch/arm64/include/asm/kvm_asm.h
> > b/arch/arm64/include/asm/kvm_asm.h
> > index a1ad12c72ebf1..fe8d4adfc281d 100644
> > --- a/arch/arm64/include/asm/kvm_asm.h
> > +++ b/arch/arm64/include/asm/kvm_asm.h
> > @@ -89,6 +89,10 @@ enum __kvm_host_smccc_func {
> >   __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_load,
> >   __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_put,
> >   __KVM_HOST_SMCCC_FUNC___pkvm_tlb_flush_vmid,
> > + __KVM_HOST_SMCCC_FUNC___vgic_v5_save_apr,
> > + __KVM_HOST_SMCCC_FUNC___vgic_v5_restore_vmcr_apr,
> > + __KVM_HOST_SMCCC_FUNC___vgic_v5_save_ppi_state,
> > + __KVM_HOST_SMCCC_FUNC___vgic_v5_restore_ppi_state,
> >  };
> >
> >  #define DECLARE_KVM_VHE_SYM(sym) extern char sym[]
> > diff --git a/arch/arm64/include/asm/kvm_host.h
> > b/arch/arm64/include/asm/kvm_host.h
> > index 332114bd44d2a..60da84071c86e 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -797,6 +797,22 @@ struct kvm_host_data {
> >   /* Number of debug breakpoints/watchpoints for this CPU
> > (minus 1) */
> >   unsigned int debug_brps;
> >   unsigned int debug_wrps;
> > +
> > + /* PPI state tracking for GICv5-based guests */
> > + struct {
> > + /*
> > + * For tracking the PPI pending state, we need
> > both
> > + * the entry state and exit state to correctly
> > detect
> > + * edges as it is possible that an interrupt has
> > been
> > + * injected in software in the interim.
> > + */
> > + u64 pendr_entry[2];
> > + u64 pendr_exit[2];
> > +
> > + /* The saved state of the regs when leaving the
> > guest */
> > + u64 activer_exit[2];
> > + u64 enabler_exit[2];
> > + } vgic_v5_ppi_state;
> >  };
> >
> >  struct kvm_host_psci_config {
> > diff --git a/arch/arm64/include/asm/kvm_hyp.h
> > b/arch/arm64/include/asm/kvm_hyp.h
> > index 76ce2b94bd97e..3dcec1df87e9e 100644
> > --- a/arch/arm64/include/asm/kvm_hyp.h
> > +++ b/arch/arm64/include/asm/kvm_hyp.h
> > @@ -87,6 +87,14 @@ void __vgic_v3_save_aprs(struct vgic_v3_cpu_if
> > *cpu_if);
> >  void __vgic_v3_restore_vmcr_aprs(struct vgic_v3_cpu_if *cpu_if);
> >  int __vgic_v3_perform_cpuif_access(struct kvm_vcpu *vcpu);
> >
> > +/* GICv5 */
> > +void __vgic_v5_save_apr(struct vgic_v5_cpu_if *cpu_if);
> > +void __vgic_v5_restore_vmcr_apr(struct vgic_v5_cpu_if *cpu_if);
> > +void __vgic_v5_save_ppi_state(struct vgic_v5_cpu_if *cpu_if);
> > +void __vgic_v5_restore_ppi_state(struct vgic_v5_cpu_if *cpu_if);
> > +void __vgic_v5_save_state(struct vgic_v5_cpu_if *cpu_if);
> > +void __vgic_v5_restore_state(struct vgic_v5_cpu_if *cpu_if);
>
> The last two are not plugged as hypercalls? How do they get called?

Right you are - they are the GICv5 equivalents of what we have for
GICv3. They are plumbed a bit differently.

On VHE we call these via vgic_restore_state() and vgic_save_state(),
and with NVHE/hVHE & friends we call these __hyp_vgic_restore_state()
and __hyp_vgic_save_state() in the switch code (nvhe/switch.c). This
means that we don't actually need the hypercalls all as we're always
calling them directly.

I'll re-work the commit message to make this explicit.

However, this made me look more closely at this code again, and the PPI
save/restore code has the same properties in that it is called from the
EXACT same places. I just tested this, and we can do away with the
hypercalls there too, so I'll drop those hypercalls too when I refresh
the series.

>
> Overall, it would be good to describe what gets saved/restored when.
> I'm sure there is a logic behind it all, and maybe it is very close
> to
> what v3 requires, but that's not completely apparent in this patch
> (we
> don't see the call sites).

Yeah, I'll make this much more apparent in the commit message.

Thanks,
Sascha

>
> Thanks,
>
> M.
>