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