Message ID | 1318489205-315-4-git-send-email-konrad@gaisler.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
From: Konrad Eisele <konrad@gaisler.com> Date: Thu, 13 Oct 2011 09:00:04 +0200 > -/* In kernel these functions don't return a value. > - * One should use macros in asm/string.h for that purpose. > - * We return 0, so that bugs are more apparent. > - */ > -#define SETUP_RETL > -#define RETL_INSN clr %o0 > +#define SETUP_RETL mov %o0, %g6 > +#define RETL_INSN mov %g6, %o0 Sigh... The kernel uses %g6 as the global thread register, see include/asm/thread_info_32.h: register struct thread_info *current_thread_info_reg asm("g6"); Your kernel would have crashed early in the boot if you actually tested this patch. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller wrote: > From: Konrad Eisele <konrad@gaisler.com> > Date: Thu, 13 Oct 2011 09:00:04 +0200 > >> -/* In kernel these functions don't return a value. >> - * One should use macros in asm/string.h for that purpose. >> - * We return 0, so that bugs are more apparent. >> - */ >> -#define SETUP_RETL >> -#define RETL_INSN clr %o0 >> +#define SETUP_RETL mov %o0, %g6 >> +#define RETL_INSN mov %g6, %o0 > > Sigh... > > The kernel uses %g6 as the global thread register, see > include/asm/thread_info_32.h: > > register struct thread_info *current_thread_info_reg asm("g6"); > > Your kernel would have crashed early in the boot if you actually > tested this patch. > > Isnt the simplest way to apply the original gdbstub.c patch instead? - ptr += strlen(strcpy(ptr, "thread:")); + strcpy(ptr, "thread:"); + ptr += strlen(ptr); Or even a simple strcpy(ptr, "thread:"); ptr += 7; In that case its the "[PATCH 4/4] kernel,debug: SPARC KGDB stub strcpy fix" from [09/30/2011 03:47 PM]. instead of the memcpy patch. It seems to me that memcpy is tightly programmed. Is saving 2 lines of gdbstub.c diff worth a memcpy rewrite? -- Konrad -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Konrad Eisele <konrad@gaisler.com> Date: Wed, 19 Oct 2011 10:33:25 +0200 > Isnt the simplest way to apply the original gdbstub.c patch instead? It's wrong, memcpy() is not implemented properly on 32-bit sparc, fix it. > It seems to me that memcpy is tightly programmed. Is saving 2 lines of gdbstub.c > diff worth a memcpy rewrite? You really can't find another register to save the initial %o0 value in? Have you even tried? -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sparc/lib/memcpy.S b/arch/sparc/lib/memcpy.S index 34fe657..835014b 100644 --- a/arch/sparc/lib/memcpy.S +++ b/arch/sparc/lib/memcpy.S @@ -19,12 +19,8 @@ x: #undef FASTER_NONALIGNED #define FASTER_ALIGNED -/* In kernel these functions don't return a value. - * One should use macros in asm/string.h for that purpose. - * We return 0, so that bugs are more apparent. - */ -#define SETUP_RETL -#define RETL_INSN clr %o0 +#define SETUP_RETL mov %o0, %g6 +#define RETL_INSN mov %g6, %o0 #else
Gcc can optimize constant strcpy to a memcpy call. However the return value of strcpy is used in KGDB. Return the standard return value instead of 0 Signed-off-by: Konrad Eisele <konrad@gaisler.com> --- arch/sparc/lib/memcpy.S | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)