Re: [PATCH V4 2/7] x86/entry: Switch the stack after error_entry() returns

[Lai Jiangshan]

On Mon, Apr 11, 2022 at 5:35 PM Borislav Petkov <bp@xxxxxxxxx> wrote:
>
> On Fri, Mar 18, 2022 at 10:30:11PM +0800, Lai Jiangshan wrote:
> > From: Lai Jiangshan <jiangshan.ljs@xxxxxxxxxxxx>

>
> > Switching to the stack after error_entry() makes the code simpler and
> > intuitive.
> >
> > The behavior/logic is unchanged:
> >   1) (opt) feed fixup_bad_iret() with the pt_regs pushed by ASM code
>
> opt?

I meant it as optional.

I will rewrite it as

1) feed fixup_bad_iret() with the pt_regs pushed by the ASM code if it
is a fault
caused by bad IRET.

>
> >   2) (opt) fixup_bad_iret() moves the partial pt_regs up
> >   3) feed sync_regs() with the pt_regs pushed by ASM code or returned
> >      by fixup_bad_iret()
> >   4) sync_regs() copies the whole pt_regs to kernel stack if needed
> >   5) after error_entry() and switching %rsp, it is in kernel stack with
> >      the pt_regs


>
> > After this change, error_entry() will not do fancy things with the stack
> > except when in the prolog which will be fixed in the next patch ("move
> > PUSH_AND_CLEAR_REGS out of error_entry").  This patch and the next patch
>
> "This patch" is tautology, as already said.
>
> There's no "next patch" in git.
>
> > can't be swapped because the next patch relies on this patch's stopping
> > fiddling with the return-address of error_entry(), otherwise the objtool
> > would complain.
>
> If that is the case, then those two should me merged into one!

This patch moves the epilog (switching stack) of error_entry() out of
error_entry().  The next patch moves the prolog (pushing pt_regs) out
of error_entry().  They can be separated patches.

I don't think anything wrong if the order of these two patches
is swapped.  Peter Z asked info about the ordering of other patches
and I tried moving the next patch up and saw the complaint from
the objtool.

I wanted to explain the ordering of the patches.  This explanation
should be put in the cover letter instead of in the commit message.

Thanks
Lai