Re: [PATCH v2 4/5] KVM: selftests: Add exception handling support for aarch64

[Marc Zyngier <maz@kernel.org>]

On Mon, 03 May 2021 20:12:21 +0100,
Ricardo Koller <ricarkol@google.com> wrote:
> 
> On Mon, May 03, 2021 at 11:32:39AM +0100, Marc Zyngier wrote:
> > On Sat, 01 May 2021 00:24:06 +0100,
> > Ricardo Koller <ricarkol@google.com> wrote:

[...]

> > > +	.if \vector >= 8
> > > +	mrs	x1, sp_el0
> > 
> > I'm still a bit perplexed by this. SP_EL0 is never changed, since you
> > always run in handler mode. Therefore, saving/restoring it is only
> > overhead. If an exception handler wants to introspect it, it is
> > already available in the relevant system register.
> > 
> > Or did you have something else in mind for it?
> > 
> 
> Not really. The reason for saving sp_el0 in there was just for
> consistency, so that handlers for both el0 and el1 exceptions could
> get the sp at regs->sp.

We already have sp_el0 consistency by virtue of having it stored in in
a sysreg.

> Restoring sp_el0 might be too much. So, what do you think of this
> v3: we keep the saving of sp_el0 into regs->sp (to keep things the
> same between el0 and el1) and delete the restoring of sp_el0?

To me, the whole purpose of saving some some context is to allow the
exception handling code to run C code and introspect the interrupted
state. But saving things that are not affected by the context change
seems a bit pointless.

One thing I'd like to see though is to save sp_el1 as it was at the
point of the exception (because that is meaningful to get the
exception context -- think of an unaligned EL1 stack for example),
which means correcting the value that gets saved.

So I would suggest to *only* save sp_el1, to always save it
(irrespective of the exception coming from EL0 or EL1), and to save a
retro-corrected value so that the handler can directly know where the
previous stack pointer was.

Thanks,

	M.

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