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

[Sascha Bischoff]

On Tue, 2025-12-16 at 10:41 +0000, Marc Zyngier wrote:
> On Fri, 12 Dec 2025 15:22:39 +0000,
> Sascha Bischoff <Sascha.Bischoff at 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?

No, I think you're quite correct, and this doesn't add meaningful
value.

This all stems from my misunderstanding that GICv5 vPPIs are
independent from the physical PPIs. This is not the case, however, as
the set of implemented virtual PPIs matches the physically implemented
PPIs. The handling mode for each PPI will be reflected in the
ICC/ICV_PPI_HMRx_EL1 sysregs.

This actually has wider impacts:

   1. It makes sense to drop this commit altogether.
   
   2. When initialising the GICv5 PPIs ("KVM: arm64: gic-v5: Init
   Private IRQs (PPIs) for GICv5"), we skip setting their config
   (LEVEL/EDGE).
   
   3. In vgic_v5_reset ("KVM: arm64: gic-v5: Reset vcpu state"), sync
   the host's PPI HMR state (ICC_PPI_HMRx_EL1) to KVM's vPPI shadow
   state as the virtual PPIs should match that, and we need that to
   correctly handle SW-driven PPI injection. Currently, this code
   actually calculates the HMR contents for trapping and emulating,
   which again can be dropped altogether.
   
   4. vgic_hmr can be dropped from the vgic_v5 CPUIF too.

Does this sound reasonable to you?

Thanks,
Sascha

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