[PATCH v2 04/25] KVM: arm64: nv: Add sanitising to EL2 configuration registers

[Joey Gouly]

On Fri, Feb 02, 2024 at 03:10:26PM +0000, Marc Zyngier wrote:
> On Thu, 01 Feb 2024 14:56:07 +0000,
> Joey Gouly <joey.gouly at arm.com> wrote:
> > 
> > On Tue, Jan 30, 2024 at 08:45:11PM +0000, Marc Zyngier wrote:
> > > We can now start making use of our sanitising masks by setting them
> > > to values that depend on the guest's configuration.
> > > 
> > > First up are VTTBR_EL2, VTCR_EL2, VMPIDR_EL2 and HCR_EL2.
> > > 
> > > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > > ---
> > >  arch/arm64/kvm/nested.c | 56 ++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 55 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > > index c976cd4b8379..ee461e630527 100644
> > > --- a/arch/arm64/kvm/nested.c
> > > +++ b/arch/arm64/kvm/nested.c
> > > @@ -181,7 +181,7 @@ u64 kvm_vcpu_sanitise_vncr_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg sr)
> > >  	return v;
> > >  }
> > >  
> > > -static void __maybe_unused set_sysreg_masks(struct kvm *kvm, int sr, u64 res0, u64 res1)
> > > +static void set_sysreg_masks(struct kvm *kvm, int sr, u64 res0, u64 res1)
> > >  {
> > >  	int i = sr - __VNCR_START__;
> > >  
> > > @@ -191,6 +191,7 @@ static void __maybe_unused set_sysreg_masks(struct kvm *kvm, int sr, u64 res0, u
> > >  
> > >  int kvm_init_nv_sysregs(struct kvm *kvm)
> > >  {
> > > +	u64 res0, res1;
> > >  	int ret = 0;
> > >  
> > >  	mutex_lock(&kvm->arch.config_lock);
> > > @@ -209,6 +210,59 @@ int kvm_init_nv_sysregs(struct kvm *kvm)
> > >  		kvm->arch.id_regs[i] = limit_nv_id_reg(IDX_IDREG(i),
> > >  						       kvm->arch.id_regs[i]);
> > >  
> > > +	/* VTTBR_EL2 */
> > > +	res0 = res1 = 0;
> > > +	if (!kvm_has_feat_enum(kvm, ID_AA64MMFR1_EL1, VMIDBits, 16))
> > > +		res0 |= GENMASK(63, 56);
> > > +	set_sysreg_masks(kvm, VTTBR_EL2, res0, res1);
> > > +
> > > +	/* VTCR_EL2 */
> > > +	res0 = GENMASK(63, 32) | GENMASK(30, 20);
> > > +	res1 = BIT(31);
> > > +	set_sysreg_masks(kvm, VTCR_EL2, res0, res1);
> > > +
> > > +	/* VMPIDR_EL2 */
> > > +	res0 = GENMASK(63, 40) | GENMASK(30, 24);
> > > +	res1 = BIT(31);
> > > +	set_sysreg_masks(kvm, VMPIDR_EL2, res0, res1);
> > > +
> > > +	/* HCR_EL2 */
> > > +	res0 = BIT(48);
> > > +	res1 = HCR_RW;
> > 
> > Just want to clarify this bit actually, this is restricting the (first level
> > only?) nested EL1 to run as AArch64?
> 
> This is restricting it at all levels. The guest hypervisor will
> eventually write its own view of HCR_EL2.RW in the VNCR page, and if
> it has promised AArch32 to its own guest (at any level), it is in for
> a treat, because we will forcefully reset this bit to 1.

Alright, thanks, I'll have to work through that some more.

> 
> > Should we be sanitising ID_AA64PFR0_EL1.EL1=0b0001?
> 
> We already do. See limit_nv_id_reg() and how it constraints
> ID_AA64PFR0_EL1.ELx to 0b0001.
> 

I forgot about that, was looking at read_sanitised_id_aa64pfr0_el1() in the non-
NV parts.

Thanks,
Joey