[PATCH v2 22/36] KVM: arm64: gic-v5: Trap and mask guest PPI register accesses

[Sascha Bischoff]

On Wed, 2026-01-07 at 15:17 +0000, Jonathan Cameron wrote:
> On Fri, 19 Dec 2025 15:52:43 +0000
> Sascha Bischoff <Sascha.Bischoff at arm.com> wrote:
> 
> > A guest should not be able to detect if a PPI that is not exposed
> > to
> > the guest is implemented or not. If the writes to the PPI registers
> > are not masked, it becomes possible for the guest to detect the
> > presence of all implemented PPIs on the host.
> > 
> > Guest writes to the following registers are masked:
> > 
> > ICC_CACTIVERx_EL1
> > ICC_SACTIVERx_EL1
> > ICC_CPENDRx_EL1
> > ICC_SPENDRx_EL1
> > ICC_ENABLERx_EL1
> > ICC_PRIORITYRx_EL1
> > 
> > When a guest writes these registers, the write is masked with the
> > set
> > of PPIs actually exposed to the guest, and the state is written
> > back
> > to KVM's shadow state..
> 
> One . seems enough.
> 
> > 
> > Reads for the above registers are not masked. When the guest is
> > running and reads from the above registers, it is presented with
> > what
> > KVM provides in the ICH_PPI_x_EL2 registers, which is the masked
> > version of what the guest last 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, and
> > hence this must also be trapped, but only for reads. The content of
> > the HMRs is calculated and masked when finalising the PPI state for
> > the guest.
> > 
> > Signed-off-by: Sascha Bischoff <sascha.bischoff at arm.com>
> A few bits inline but nothing significant so I'll assume you tidy
> those up
> Reviewed-by: Jonathan Cameron <jonathan.cameron at huawei.com>

I've left this tag off for now as this code has gone a bit of a rework
(mostly reducing the scope) and I don't think it really applies
anymore.

Instead of trapping many of the writes to the PPI registers, the set
has been reduced to just the ICH_PPI_ENABLERx_EL1. By trapping and
masking writes to the PPI enable, we stop the guest from ever enabling
any interrupt that is not exposed to it, and are able to forgo trapping
the rest of the writes.

Thanks,
Sascha

> 
> > ---
> >  arch/arm64/kvm/config.c   |  22 ++++++-
> >  arch/arm64/kvm/sys_regs.c | 133
> > ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 153 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
> > index eb0c6f4d95b6d..f81bfdadd12fb 100644
> > --- a/arch/arm64/kvm/config.c
> > +++ b/arch/arm64/kvm/config.c
> > @@ -1586,8 +1586,26 @@ 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
> 
> need 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;
> > +}
> 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 383ada0d75922..cef13bf6bb3a1 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -696,6 +696,111 @@ 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 (p->is_write)
> > +		return ignore_write(vcpu, p);
> > +
> > +	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];
> > +	}
> 
> No {} as single line statements in all legs.
> 
> However, I'd be tempted to use a local variable for the index like
> you've
> done in many other cases
> 	
> 	unsigned int index;
> 
> ...
> 
> 	index = p->Op2 == 0 ? 0 : 1;
> 	p->regval = vcpu->arch.vgic_cpu.vgic_v5.vgic_ppi_hrm[index];
> 
> Or use the p->Op2 % 2 as you do in ppi_enabler.
> 
> 
> > +
> > +	return true;
> > +}
> > +
> > +static bool access_gicv5_ppi_enabler(struct kvm_vcpu *vcpu,
> > +				     struct sys_reg_params *p,
> > +				     const struct sys_reg_desc *r)
> > +{
> > +	struct vgic_v5_cpu_if *cpu_if = &vcpu-
> > >arch.vgic_cpu.vgic_v5;
> > +	u64 masked_write;
> > +
> > +	/* We never expect to get here with a read! */
> > +	if (WARN_ON_ONCE(!p->is_write))
> > +		return undef_access(vcpu, p, r);
> > +
> > +	masked_write = p->regval & cpu_if->vgic_ppi_mask[p->Op2 %
> > 2];
> > +	cpu_if->vgic_ich_ppi_enabler_entry[p->Op2 % 2] =
> > masked_write;
> > +
> > +	return true;
> > +}
> > +
> > +static bool access_gicv5_ppi_pendr(struct kvm_vcpu *vcpu,
> > +				   struct sys_reg_params *p,
> > +				   const struct sys_reg_desc *r)
> > +{
> > +	struct vgic_v5_cpu_if *cpu_if = &vcpu-
> > >arch.vgic_cpu.vgic_v5;
> > +	u64 masked_write;
> > +
> > +	/* We never expect to get here with a read! */
> > +	if (WARN_ON_ONCE(!p->is_write))
> > +		return undef_access(vcpu, p, r);
> > +
> > +	masked_write = p->regval & cpu_if->vgic_ppi_mask[p->Op2 %
> > 2];
> > +
> > +	if (p->Op2 & 0x2) {	/* SPENDRx */
> > +		cpu_if->vgic_ppi_pendr_entry[p->Op2 % 2] |=
> > masked_write;
> > +	} else {		/* CPENDRx */
> > +		cpu_if->vgic_ppi_pendr_entry[p->Op2 % 2] &=
> > ~masked_write;
> > +	}
> 
> No {} wanted in kernel style when all legs are single line
> statements.
> Same applies in a few other cases that follow.
> 
> > +
> > +	return true;
> > +}
> > +
>