Re: [PATCH 11/32] KVM: arm64: gic-v5: Trap and emulate ICH_PPI_HMRx_EL1 accesses

[Marc Zyngier <maz@kernel.org>]

On Fri, 12 Dec 2025 15:22:39 +0000,
Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
> 
> The ICC_PPI_HMRx_EL1 register is used to determine which PPIs use
> Level-sensitive semantics, and which use Edge. For a GICv5 guest, the
> correct view of the virtual PPIs must be provided to the guest.

s/ICH/ICC/ in $SUBJECT

> 
> The GICv5 architecture doesn't provide an ICV_PPI_HMRx_EL1 or

The spec disagree with you here (see 9.5.4).

> ICH_PPI_HMRx_EL2 register, and therefore all guest accesses must be
> trapped to avoid the guest directly accessing the host's
> ICC_PPI_HMRx_EL1 state. This change hence configures the FGTs to
> always trap and emulate guest accesses to the HMR running a
> GICv5-based guest.

The real question is what we gain by emulating this register, given
that virtual PPIs are only guaranteed to exist if the physical version
exist. If they exist, then the handling mode is defined by the
that HW, and we can't deviate from it.

Given that, I can't really see the point in trapping something that is
bound to be the same thing as the host, unless this comes with
additional restrictions, for example a mask of interrupts that are
actually exposed to the guest.

Or am I missing something?

> 
> This change also introduces the struct vgic_v5_cpu_if, which includes
> the vgic_hmr. This is not yet populated as it can only be correctly
> populated at vcpu reset time. This will be introduced in a subsquent
> change.
> 
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
>  arch/arm64/kvm/config.c   |  6 +++++-
>  arch/arm64/kvm/sys_regs.c | 26 ++++++++++++++++++++++++++
>  include/kvm/arm_vgic.h    |  5 +++++
>  3 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
> index cbdd8ac90f4d0..7683407ce052a 100644
> --- a/arch/arm64/kvm/config.c
> +++ b/arch/arm64/kvm/config.c
> @@ -1586,8 +1586,12 @@ static void __compute_ich_hfgrtr(struct kvm_vcpu *vcpu)
>  {
>  	__compute_fgt(vcpu, ICH_HFGRTR_EL2);
>  
> -	/* ICC_IAFFIDR_EL1 *always* needs to be trapped when running a guest */
> +	/*
> +	 * ICC_IAFFIDR_EL1 and ICH_PPI_HMRx_EL1 *always* needs to be
> +	 * trapped when running a guest.
> +	 **/
>  	*vcpu_fgt(vcpu, ICH_HFGRTR_EL2) &= ~ICH_HFGRTR_EL2_ICC_IAFFIDR_EL1;
> +	*vcpu_fgt(vcpu, ICH_HFGRTR_EL2) &= ~ICH_HFGRTR_EL2_ICC_PPI_HMRn_EL1;
>  }
>  
>  void kvm_vcpu_load_fgt(struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 31c08fd591d08..a4ae034340040 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -699,6 +699,30 @@ static bool access_gicv5_iaffid(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>  	return true;
>  }
>  
> +static bool access_gicv5_ppi_hmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> +				 const struct sys_reg_desc *r)
> +{
> +	if (!vgic_is_v5(vcpu->kvm))
> +		return undef_access(vcpu, p, r);
> +
> +	if (p->is_write)
> +		return ignore_write(vcpu, p);
> +
> +	/*
> +	 * For GICv5 VMs, the IAFFID value is the same as the VPE ID. The VPE ID
> +	 * is the same as the VCPU's ID.
> +	 */

Unrelated comment?

> +
> +	if (p->Op2 == 0) {	/* ICC_PPI_HMR0_EL1 */
> +		p->regval = vcpu->arch.vgic_cpu.vgic_v5.vgic_ppi_hmr[0];
> +	} else {		/* ICC_PPI_HMR1_EL1 */
> +		p->regval = vcpu->arch.vgic_cpu.vgic_v5.vgic_ppi_hmr[1];
> +	}

nit: Can probably be written as:

	p->regval = vcpu->arch.vgic_cpu.vgic_v5.vgic_ppi_hmr[p->Op2];

Thanks,

	M.

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