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

From: Marc Zyngier <maz@kernel.org>
To: Jing Zhang <jingzhangos@google.com>
Cc: kernel-team@android.com,
	Catalin Marinas <catalin.marinas@arm.com>,
	Ard Biesheuvel <ardb@kernel.org>, Will Deacon <will@kernel.org>,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 04/17] arm64: Provide an 'upgrade to VHE' stub hypercall
Date: Tue, 05 Jan 2021 21:03:37 +0000	[thread overview]
Message-ID: <87zh1njdue.wl-maz@kernel.org> (raw)
In-Reply-To: <CAAdAUtiYe9Y6o2QLD_y94y-nVJM8ZquV-_ZETNSzc_u+rKM0-w@mail.gmail.com>

On Tue, 05 Jan 2021 20:35:35 +0000,
Jing Zhang <jingzhangos@google.com> wrote:
> 
> On Mon, Jan 4, 2021 at 7:50 AM Marc Zyngier <maz@kernel.org> 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  | 70 ++++++++++++++++++++++++++++++++++-
> >  2 files changed, 74 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..6ffdc1f7778b 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,16 @@ 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.ne    2f
> > +       mov     x0, #HVC_SOFT_RESTART
> > +       adr     x1, mutate_to_vhe
> > +       // fall through...
> Does "br x1" work here instead of falling through into
> HVC_SOFT_RESTART? Just curious about the reason for falling-through.

Absolutely.

This is the leftover from an initial (and pretty over-engineered)
approach where the caller was using HVC_SOFT_RESTART with the address
of mutate_to_vhe passed as the target. It was an interesting idea, but
obtaining the PA for the function ended when the MMU is on created a
separate, harder to maintain path.

Cleanup time, I guess.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm