2017-10-31 23:51:45

by James Hogan

[permalink] [raw]
Subject: Re: [PATCH] MIPS: Fix exception entry when CONFIG_EVA enabled

On Wed, Oct 11, 2017 at 08:12:31AM -0500, Corey Minyard wrote:
> On 10/11/2017 03:59 AM, Matt Redfearn wrote:
> > Commit 9fef68686317b ("MIPS: Make SAVE_SOME more standard") made several
> > changes to the order in which registers are saved in the SAVE_SOME
> > macro, used by exception handlers to save the processor state. In
> > particular, it removed the
> > move k1, sp
> > in the delay slot of the branch testing if the processor is already in
> > kernel mode. This is replaced later in the macro by a
> > move k0, sp
> > When CONFIG_EVA is disabled, this instruction actually appears in the
> > delay slot of the branch. However, when CONFIG_EVA is enabled, instead
> > the RPS workaround of
> > MFC0 k0, CP0_ENTRYHI
> > appears in the delay slot. This results in k0 not containing the stack
> > pointer, but some unrelated value, which is then saved to the kernel
> > stack. On exit from the exception, this bogus value is restored to the
> > stack pointer, resulting in an OOPS.
> >
> > Fix this by moving the save of SP in k0 explicitly in the delay slot of
> > the branch, outside of the CONFIG_EVA section, restoring the expected
> > instruction ordering when CONFIG_EVA is active.
> >
> > Fixes: 9fef68686317b ("MIPS: Make SAVE_SOME more standard")
> > Signed-off-by: Matt Redfearn <[email protected]>
> > Reported-by: Vladimir Kondratiev <[email protected]>
>
> I looked this over pretty carefully and it looks correct to me.  It
> makes no difference
> in the instructions generated by the non-EVA case.  I shouldn't have
> missed this :(.
>
> Reviewed-by: Corey Minyard <[email protected]>

Yeh, having stared at it for a little while it looks correct to me too.

Reviewed-by: James Hogan <[email protected]>

Cheers
James


Attachments:
(No filename) (1.77 kB)
signature.asc (849.00 B)
Digital signature
Download all attachments