Date: Fri, 02 Feb 2024 15:10:26 +0000
Message-ID: <86cyte7vh9.wl-maz@kernel.org>
From: Marc Zyngier <maz@kernel.org>
To: Joey Gouly <joey.gouly@arm.com>
Cc: kvmarm@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH v2 04/25] KVM: arm64: nv: Add sanitising to EL2 configuration registers
In-Reply-To: <20240201145607.GH2284078@e124191.cambridge.arm.com>
References: <20240130204533.693853-1-maz@kernel.org>
	<20240130204533.693853-5-maz@kernel.org>
	<20240201145607.GH2284078@e124191.cambridge.arm.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.1
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
Precedence: bulk
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII

On Thu, 01 Feb 2024 14:56:07 +0000,
Joey Gouly <joey.gouly@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@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.

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

Thanks,

	M.

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