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

[Marc Zyngier <maz@kernel.org>]

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?

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).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.