Re: [PATCH 21/25] KVM: arm64: Make TLBI OS/Range UNDEF if not advertised to the guest

[Marc Zyngier <maz@kernel.org>]

On Thu, 25 Jan 2024 13:30:47 +0000,
Joey Gouly <joey.gouly@arm.com> wrote:
> 
> Question,
> 
> On Mon, Jan 22, 2024 at 08:18:48PM +0000, Marc Zyngier wrote:
> > Outer Shareable and Range TLBI instructions shouldn't be made available
> > to the guest if they are not advertised. Use FGU to disable those,
> > and set HCR_EL2.TLBIOS in the case the host doesn't have FGT.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index a62efd8a2959..3c939ea4a28f 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -3949,6 +3949,9 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu)
> >  
> >  	mutex_lock(&kvm->arch.config_lock);
> >  
> > +	if (!kvm_has_feat(kvm, ID_AA64ISAR0_EL1, TLB, OS))
> > +		vcpu->arch.hcr_el2 |= HCR_TTLBOS;
> > +
> >  	if (test_bit(KVM_ARCH_FLAG_FGU_INITIALIZED, &kvm->arch.flags))
> >  		goto out;
> >  
> > @@ -3961,6 +3964,32 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu)
> >  				       HFGxTR_EL2_nSMPRI_EL1_MASK	|
> >  				       HFGxTR_EL2_nTPIDR2_EL0_MASK);
> >  
> > +	if (!kvm_has_feat(kvm, ID_AA64ISAR0_EL1, TLB, OS))
> > +		kvm->arch.fgu[HFGITR_GROUP] |= (HFGITR_EL2_TLBIRVAALE1OS|
> > +						HFGITR_EL2_TLBIRVALE1OS	|
> > +						HFGITR_EL2_TLBIRVAAE1OS	|
> > +						HFGITR_EL2_TLBIRVAE1OS	|
> > +						HFGITR_EL2_TLBIVAALE1OS	|
> > +						HFGITR_EL2_TLBIVALE1OS	|
> > +						HFGITR_EL2_TLBIVAAE1OS	|
> > +						HFGITR_EL2_TLBIASIDE1OS	|
> > +						HFGITR_EL2_TLBIVAE1OS	|
> > +						HFGITR_EL2_TLBIVMALLE1OS);
> > +
> > +	if (!kvm_has_feat(kvm, ID_AA64ISAR0_EL1, TLB, RANGE))
> > +		kvm->arch.fgu[HFGITR_GROUP] |= (HFGITR_EL2_TLBIRVAALE1	|
> > +						HFGITR_EL2_TLBIRVALE1	|
> > +						HFGITR_EL2_TLBIRVAAE1	|
> > +						HFGITR_EL2_TLBIRVAE1	|
> > +						HFGITR_EL2_TLBIRVAALE1IS|
> > +						HFGITR_EL2_TLBIRVALE1IS	|
> > +						HFGITR_EL2_TLBIRVAAE1IS	|
> > +						HFGITR_EL2_TLBIRVAE1IS	|
> > +						HFGITR_EL2_TLBIRVAALE1OS|
> > +						HFGITR_EL2_TLBIRVALE1OS	|
> > +						HFGITR_EL2_TLBIRVAAE1OS	|
> > +						HFGITR_EL2_TLBIRVAE1OS);
> > +
> >  	set_bit(KVM_ARCH_FLAG_FGU_INITIALIZED, &kvm->arch.flags);
> >  out:
> >  	mutex_unlock(&kvm->arch.config_lock);
> 
> I think I'm right in saying..
> 
> If the VM is running on a platform with ID_AA64ISAR0_EL1.TLB=0b010 (Outer
> Shareable and TLB range maintenance instructions are implemented.) but without
> support for FEAT_FGT, and the VMM sets the ID reg to ID_AA64ISAR0_EL1.TLB=0,
> this change will trap the TLBI *OS instructions but not the following: 
> 
>     TLBI RVAAE1
>     TLBI RVAAE1IS
>     TLBI RVAALE1
>     TLBI RVAALE1IS
>     TLBI RVAE1
>     TLBI RVAE1IS
>     TLBI RVALE1
>     TLBI RVALE1IS
> 
> These TLB range instructions only trap with HCR_EL2.TTLB, however that traps
> all TLB instructions. You may have left this off intentionally, if so can you
> add something to the commit message.

You got it exactly right. Setting TTLB would be overkill, and is one
of the cases where we have to strike a balance between efficiency and
enforcement. I'll leave a comment to that effect to capture this.

Thanks,

	M.

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