[PATCH v2 10/36] KVM: arm64: gic-v5: Sanitize ID_AA64PFR2_EL1.GCIE

[Sascha Bischoff]

On Wed, 2026-01-07 at 10:58 +0000, Jonathan Cameron wrote:
> On Fri, 19 Dec 2025 15:52:39 +0000
> Sascha Bischoff <Sascha.Bischoff at arm.com> wrote:
> 
> > Set the guest's view of the GCIE field to IMP when running a GICv5
> > VM,
> > NI otherwise. Reject any writes to the register that try to do
> > anything but set GCIE to IMP when running a GICv5 VM.
> > 
> > As part of this change, we also introduce vgic_is_v5(kvm), in order
> > to
> > check if the guest is a GICv5-native VM. We're also required to
> > extend
> > vgic_is_v3_compat to check for the actual vgic_model. This has one
> > potential issue - if any of the vgic_is_v* checks are used prior to
> > setting the vgic_model (that is, before kvm_vgic_create) then
> > vgic_model will be set to 0, which can result in a false-positive.
> > 
> > Co-authored-by: Timothy Hayes <timothy.hayes at arm.com>
> > Signed-off-by: Timothy Hayes <timothy.hayes at arm.com>
> > Signed-off-by: Sascha Bischoff <sascha.bischoff at arm.com>
> Hi Sascha, Timothy
> 
> The masking of val has me a little confused in the sanitize function.
> Probably needs a slightly rewrite.

Yeah, agreed.

> 
> Jonathan
> 
> > ---
> >  arch/arm64/kvm/sys_regs.c  | 39 ++++++++++++++++++++++++++++++----
> > ----
> >  arch/arm64/kvm/vgic/vgic.h | 10 +++++++++-
> >  2 files changed, 40 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index c8fd7c6a12a13..a065f8939bc8f 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -1758,6 +1758,7 @@ static u8 pmuver_to_perfmon(u8 pmuver)
> >  
> >  static u64 sanitise_id_aa64pfr0_el1(const struct kvm_vcpu *vcpu,
> > u64 val);
> >  static u64 sanitise_id_aa64pfr1_el1(const struct kvm_vcpu *vcpu,
> > u64 val);
> > +static u64 sanitise_id_aa64pfr2_el1(const struct kvm_vcpu *vcpu,
> > u64 val);
> >  static u64 sanitise_id_aa64dfr0_el1(const struct kvm_vcpu *vcpu,
> > u64 val);
> >  
> >  /* Read a sanitised cpufeature ID register by sys_reg_desc */
> > @@ -1783,10 +1784,7 @@ static u64 __kvm_read_sanitised_id_reg(const
> > struct kvm_vcpu *vcpu,
> >  		val = sanitise_id_aa64pfr1_el1(vcpu, val);
> >  		break;
> >  	case SYS_ID_AA64PFR2_EL1:
> > -		val &= ID_AA64PFR2_EL1_FPMR |
> > -			(kvm_has_mte(vcpu->kvm) ?
> > -			 ID_AA64PFR2_EL1_MTEFAR |
> > ID_AA64PFR2_EL1_MTESTOREONLY :
> > -			 0);
> > +		val = sanitise_id_aa64pfr2_el1(vcpu, val);
> >  		break;
> >  	case SYS_ID_AA64ISAR1_EL1:
> >  		if (!vcpu_has_ptrauth(vcpu))
> > @@ -2024,6 +2022,20 @@ static u64 sanitise_id_aa64pfr1_el1(const
> > struct kvm_vcpu *vcpu, u64 val)
> >  	return val;
> >  }
> >  
> > +static u64 sanitise_id_aa64pfr2_el1(const struct kvm_vcpu *vcpu,
> > u64 val)
> > +{
> 
> The code flow in here seems confusing, so maybe needs a rethink even
> if it
> works.  Feels like we need a mask first of everything the kernel
> understands,
> then specific masking out / setting of parts for each feature.
> I'm not sure if the initial mask is handled by the caller (didn't
> check but
> it's in the register array structure).
> Also I love crossing specs where the gicv5 spec says all the other
> fields are
> reserved and they aren't any more.  Would have been better if that
> had
> just said see arm arm for the other parts of this register.
> 
> > +	val &= ID_AA64PFR2_EL1_FPMR |
> > +		(kvm_has_mte(vcpu->kvm) ?
> > +			ID_AA64PFR2_EL1_MTEFAR |
> > ID_AA64PFR2_EL1_MTESTOREONLY : 0);
> 
> So this either masks out everything other than FPRM or masks out
> everything other
> than EL1_MTEFAR, EL1_MTESTORE_ONLY and FPMR.
> 
> Hence...
> 
> > +
> > +	if (vgic_is_v5(vcpu->kvm)) {
> > +		val &= ~ID_AA64PFR2_EL1_GCIE_MASK;
> 
> This is doing nothing as that field isn't set anyway in either of the
> earlier
> possible maskings of val.
> 
> > +		val |= SYS_FIELD_PREP_ENUM(ID_AA64PFR2_EL1, GCIE,
> > IMP);
> > +	}
> > +
> > +	return val;
> > +}
> 
> 

I've taken your advice here, and the flow is now much clearer. Thanks.

We now first allow through all FPMR and MTE* fields, then filter out
the MTE fields if MTE isn't supported. Finally, the GCIE field is set
for GICv5-based guests.

Sascha