Re: [PATCH v4 04/21] arm64: Provide an 'upgrade to VHE' stub hypercall

From: Marc Zyngier <maz@kernel.org>
To: David Brazdil <dbrazdil@google.com>
Cc: linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Ard Biesheuvel <ardb@kernel.org>,
	Jing Zhang <jingzhangos@google.com>,
	Ajay Patil <pajay@qti.qualcomm.com>,
	Prasad Sodagudi <psodagud@codeaurora.org>,
	Srinivas Ramana <sramana@codeaurora.org>,
	James Morse <james.morse@arm.com>,
	Julien Thierry <julien.thierry.kdev@gmail.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	kernel-team@android.com
Subject: Re: [PATCH v4 04/21] arm64: Provide an 'upgrade to VHE' stub hypercall
Date: Sun, 24 Jan 2021 18:44:11 +0000	[thread overview]
Message-ID: <87sg6qjhtw.wl-maz@kernel.org> (raw)
In-Reply-To: <20210118112516.6a7lnrtbjvey4iiv@google.com>

On Mon, 18 Jan 2021 11:25:16 +0000,
David Brazdil <dbrazdil@google.com> wrote:
> 
> On Mon, Jan 18, 2021 at 09:45:16AM +0000, Marc Zyngier wrote:
> > As we are about to change the way a VHE system boots, let's
> > provide the core helper, in the form of a stub hypercall that
> > enables VHE and replicates the full EL1 context at EL2, thanks
> > to EL1 and VHE-EL2 being extremely similar.
> > 
> > On exception return, the kernel carries on at EL2. Fancy!
> > 
> > Nothing calls this new hypercall yet, so no functional change.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/include/asm/virt.h |  7 +++-
> >  arch/arm64/kernel/hyp-stub.S  | 67 +++++++++++++++++++++++++++++++++--
> >  2 files changed, 71 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
> > index ee6a48df89d9..7379f35ae2c6 100644
> > --- a/arch/arm64/include/asm/virt.h
> > +++ b/arch/arm64/include/asm/virt.h
> > @@ -35,8 +35,13 @@
> >   */
> >  #define HVC_RESET_VECTORS 2
> >  
> > +/*
> > + * HVC_VHE_RESTART - Upgrade the CPU from EL1 to EL2, if possible
> > + */
> > +#define HVC_VHE_RESTART	3
> > +
> >  /* Max number of HYP stub hypercalls */
> > -#define HVC_STUB_HCALL_NR 3
> > +#define HVC_STUB_HCALL_NR 4
> >  
> >  /* Error returned when an invalid stub number is passed into x0 */
> >  #define HVC_STUB_ERR	0xbadca11
> > diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> > index 160f5881a0b7..fb12398b5c28 100644
> > --- a/arch/arm64/kernel/hyp-stub.S
> > +++ b/arch/arm64/kernel/hyp-stub.S
> > @@ -8,9 +8,9 @@
> >  
> >  #include <linux/init.h>
> >  #include <linux/linkage.h>
> > -#include <linux/irqchip/arm-gic-v3.h>
> >  
> >  #include <asm/assembler.h>
> > +#include <asm/el2_setup.h>
> >  #include <asm/kvm_arm.h>
> >  #include <asm/kvm_asm.h>
> >  #include <asm/ptrace.h>
> > @@ -47,10 +47,13 @@ SYM_CODE_END(__hyp_stub_vectors)
> >  
> >  SYM_CODE_START_LOCAL(el1_sync)
> >  	cmp	x0, #HVC_SET_VECTORS
> > -	b.ne	2f
> > +	b.ne	1f
> >  	msr	vbar_el2, x1
> >  	b	9f
> >  
> > +1:	cmp	x0, #HVC_VHE_RESTART
> > +	b.eq	mutate_to_vhe
> > +
> >  2:	cmp	x0, #HVC_SOFT_RESTART
> >  	b.ne	3f
> >  	mov	x0, x2
> > @@ -70,6 +73,66 @@ SYM_CODE_START_LOCAL(el1_sync)
> >  	eret
> >  SYM_CODE_END(el1_sync)
> >  
> > +// nVHE? No way! Give me the real thing!
> > +SYM_CODE_START_LOCAL(mutate_to_vhe)
> > +	// Sanity check: MMU *must* be off
> > +	mrs	x0, sctlr_el2
> > +	tbnz	x0, #0, 1f
> > +
> > +	// Needs to be VHE capable, obviously
> > +	mrs	x0, id_aa64mmfr1_el1
> > +	ubfx	x0, x0, #ID_AA64MMFR1_VHE_SHIFT, #4
> > +	cbz	x0, 1f
> 
> nit: There is a HVC_STUB_ERR that you could return if these sanity
> checks fail.  The documentation also states that it should be
> returned on error.

Good point. I've now added it, but how the error can be handled is
still up in the air. For now, I've decided to let the kernel continue
its (probably doomed) course.

Thanks,

	M.

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