mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Antony Pavlov <antonynpavlov@gmail.com>
To: Denis Orlov <denorl2009@gmail.com>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 0/1] MIPS: fix optimised memset returning garbage
Date: Thu, 10 Mar 2022 17:32:53 +0300	[thread overview]
Message-ID: <20220310173253.de205299b310b63f0094730e@gmail.com> (raw)
In-Reply-To: <20220310134040.30631-1-denorl2009@gmail.com>

On Thu, 10 Mar 2022 16:40:39 +0300
Denis Orlov <denorl2009@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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


  parent reply	other threads:[~2022-03-10 14:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-10 13:40 Denis Orlov
2022-03-10 13:40 ` [PATCH 1/1] " Denis Orlov
2022-03-10 14:32 ` Antony Pavlov [this message]
2022-03-10 20:42   ` [PATCH v2] " Denis Orlov
2022-03-14  9:03     ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220310173253.de205299b310b63f0094730e@gmail.com \
    --to=antonynpavlov@gmail.com \
    --cc=barebox@lists.infradead.org \
    --cc=denorl2009@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox