[PATCH 0/1] MIPS: fix optimised memset returning garbage

[Antony Pavlov]

On Thu, 10 Mar 2022 16:40:39 +0300
Denis Orlov <denorl2009 at gmail.com> wrote:

Hi Denis!

I'm very glad to see that you have found this anoying bug!

Some tips:

* you comment from "PATCH 0/1" is very informative but after applying "PATCH 1/1"
to the mainline this very informative comment remain in maillist archive but not
in the barebox git repo; Could you please combine "PATCH 0/1" comment and "PATCH 1/1"
comment and resend single "PATCH v2" ?

* I propose this ammendment for your comment:

> --- a/PATCH-0/1
> +++ b/PATCH-0/1
> @@ -108,8 +108,8 @@ List-Archive: <https://lore.barebox.org/barebox/>
>  There is currently a bug in memset implementation from
>  arch/mips/lib/memset.S that is used when MIPS_OPTIMIZED_STRING_FUNCTIONS
>  option is enabled. It stems from a missing '.set noreorder' directive
> -before the first branch. This results in an assembler stuffing a 'nop'
> -instruction in a delay slot instead of an instruction writing the return
> +before the first beqz instruction. This results in an assembler stuffing a 'nop'
> +instruction in the beqz delay slot instead of an instruction writing the return
>  value into v0 register that was supposed to go there. That leads to an
>  error that appears when reading memory from SPI Flash and that was
>  mentioned here:

-- 
Best regards,
  Antony Pavlov



> There is currently a bug in memset implementation from
> arch/mips/lib/memset.S that is used when MIPS_OPTIMIZED_STRING_FUNCTIONS
> option is enabled. It stems from a missing '.set noreorder' directive
> before the first branch. This results in an assembler stuffing a 'nop'
> instruction in a delay slot instead of an instruction writing the return
> value into v0 register that was supposed to go there. That leads to an
> error that appears when reading memory from SPI Flash and that was
> mentioned here:
> http://lists.infradead.org/pipermail/barebox/2021-June/036557.html.
> 
> There are a few ways of fixing this bug. The directive '.set noreorder'
> that shows up a bit later in code could be moved up and placed right
> before the memset label (as it was originally in Linux codebase - see
> a583158c9ce822c96a718fbf877cec1e5f9ad75d). Or, rather, the move and the
> branch instruction could be swapped, thus allowing the assembler to
> properly place move in delay slot (as it is done in Linux now - see
> 68dec269ee29c3abfd09596fbee7e40d875a6ab3) - that is what is done in
> patch.
> 
> Denis Orlov (1):
>   MIPS: fix optimised memset returning garbage
> 
>  arch/mips/lib/memset.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox