Re: [PATCH v13 03/18] arm64: hyp-stub: Move el1_sync into the vectors

From: Marc Zyngier <maz@kernel.org>
To: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: James Morris <jmorris@namei.org>, Sasha Levin <sashal@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	kexec mailing list <kexec@lists.infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	James Morse <james.morse@arm.com>,
	Vladimir Murzin <vladimir.murzin@arm.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	linux-mm <linux-mm@kvack.org>,
	Mark Rutland <mark.rutland@arm.com>,
	steve.capper@arm.com, rfontana@redhat.com,
	Thomas Gleixner <tglx@linutronix.de>,
	Selin Dag <selindag@gmail.com>,
	Tyler Hicks <tyhicks@linux.microsoft.com>,
	Pingfan Liu <kernelfans@gmail.com>
Subject: Re: [PATCH v13 03/18] arm64: hyp-stub: Move el1_sync into the vectors
Date: Thu, 08 Apr 2021 16:01:40 +0100	[thread overview]
Message-ID: <87v98wom5n.wl-maz@kernel.org> (raw)
In-Reply-To: <CA+CK2bDc+9EsH_TLTgg9Pqv_-rZfZfPohdZEfsYjT8ss+8skjg@mail.gmail.com>

On Thu, 08 Apr 2021 15:45:18 +0100,
Pavel Tatashin <pasha.tatashin@soleen.com> wrote:
> 
> On Thu, Apr 8, 2021 at 6:24 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On 2021-04-08 05:05, Pavel Tatashin wrote:
> > > From: James Morse <james.morse@arm.com>
> > >
> > > The hyp-stub's el1_sync code doesn't do very much, this can easily fit
> > > in the vectors.
> > >
> > > With this, all of the hyp-stubs behaviour is contained in its vectors.
> > > This lets kexec and hibernate copy the hyp-stub when they need its
> > > behaviour, instead of re-implementing it.
> > >
> > > Signed-off-by: James Morse <james.morse@arm.com>
> > >
> > > [Fixed merging issues]
> >
> > That's a pretty odd fix IMO.
> >
> > >
> > > Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> > > ---
> > >  arch/arm64/kernel/hyp-stub.S | 59 ++++++++++++++++++------------------
> > >  1 file changed, 29 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/hyp-stub.S
> > > b/arch/arm64/kernel/hyp-stub.S
> > > index ff329c5c074d..d1a73d0f74e0 100644
> > > --- a/arch/arm64/kernel/hyp-stub.S
> > > +++ b/arch/arm64/kernel/hyp-stub.S
> > > @@ -21,6 +21,34 @@ SYM_CODE_START_LOCAL(\label)
> > >       .align 7
> > >       b       \label
> > >  SYM_CODE_END(\label)
> > > +.endm
> > > +
> > > +.macro hyp_stub_el1_sync
> > > +SYM_CODE_START_LOCAL(hyp_stub_el1_sync)
> > > +     .align 7
> > > +     cmp     x0, #HVC_SET_VECTORS
> > > +     b.ne    2f
> > > +     msr     vbar_el2, x1
> > > +     b       9f
> > > +
> > > +2:   cmp     x0, #HVC_SOFT_RESTART
> > > +     b.ne    3f
> > > +     mov     x0, x2
> > > +     mov     x2, x4
> > > +     mov     x4, x1
> > > +     mov     x1, x3
> > > +     br      x4                              // no return
> > > +
> > > +3:   cmp     x0, #HVC_RESET_VECTORS
> > > +     beq     9f                              // Nothing to reset!
> > > +
> > > +     /* Someone called kvm_call_hyp() against the hyp-stub... */
> > > +     mov_q   x0, HVC_STUB_ERR
> > > +     eret
> > > +
> > > +9:   mov     x0, xzr
> > > +     eret
> > > +SYM_CODE_END(hyp_stub_el1_sync)
> >
> > You said you tested this on a TX2. I guess you don't care whether
> > it runs VHE or not...
> 
> Hi Marc,
> 
> Thank you for noticing this. Not sure how this missmerge happened. I
> have added the missing case, and VHE is initialized correctly during
> boot.
> [   14.698175] kvm [1]: VHE mode initialized successfully
> 
> During normal boot, kexec reboot, and kdump reboot. I will respin the
> series and send the version 14 soon.

Please give people a chance to review this lot first. This isn't code
that is easy to digest, and immediate re-spinning does more harm than
good (this isn't targeting 5.13, I would assume).

Thanks,

	M.

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