Re: nios2: is the ptrace ABI correct?
On 2015-03-09 at 18:02:17 +0100, Chung-Lin Tang wrote: > On 2015/3/10 12:54 AM, Chung-Lin Tang wrote: > > It appears that some of the ways nios2 has organized the > > ucontext/pt_regs/etc. are remnants of the pre-generic code, some > > basically because the port was based off m68k. > > > > I've re-organized the headers a bit: nios2/include/asm/ucontext.h is > > deleted, and re-definition of struct sigcontext now allows use of > > uapi/asm-generic/ucontext.h directly. Note that the reorg, despite > > effectively renaming some fields, is still binary compatible. I'll > > probably update the corresponding glibc definitions later. > > > > struct pt_regs is now not exported, and all exported register sets are > > now supposed to follow the 49 register set defined as in GDB now. > > > > Tobias, Ley Foon, how do you think this looks? > > Sorry, accidentally attached unrelated GCC patch instead, this one's the > correct one. Sorry for jumping into the discussion so late. The patch looks fine to me as well. Only one minor nit below. Since Ley Foon already applied it as is, I'll just send a followup patch. Thanks Ezequiel, Chung-Lin and Ley Foon for pushing this! Looking forward to have strace support on nios2. [...] > diff --git a/arch/nios2/include/uapi/asm/Kbuild > b/arch/nios2/include/uapi/asm/Kbuild > index 4f07ca3f8..37613119 100644 > --- a/arch/nios2/include/uapi/asm/Kbuild > +++ b/arch/nios2/include/uapi/asm/Kbuild > @@ -2,3 +2,5 @@ include include/uapi/asm-generic/Kbuild.asm > > header-y += elf.h > header-y += ucontext.h This should no longer be needed now that the non-generic ucontext.h is gone. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On 03/11/2015 04:48 AM, Ley Foon Tan wrote: > On Tue, Mar 10, 2015 at 2:17 PM, Chung-Lin Tang > wrote: >> On 2015/3/10 10:54 AM, Ley Foon Tan wrote: >>> On Tue, Mar 10, 2015 at 1:05 AM, Ezequiel Garcia >>> wrote: On 03/09/2015 02:02 PM, Chung-Lin Tang wrote: > On 2015/3/10 12:54 AM, Chung-Lin Tang wrote: >> It appears that some of the ways nios2 has organized the >> ucontext/pt_regs/etc. are remnants of the pre-generic code, some >> basically because the port was based off m68k. >> >> I've re-organized the headers a bit: nios2/include/asm/ucontext.h is >> deleted, and re-definition of struct sigcontext now allows use of >> uapi/asm-generic/ucontext.h directly. Note that the reorg, despite >> effectively renaming some fields, is still binary compatible. I'll >> probably update the corresponding glibc definitions later. >> >> struct pt_regs is now not exported, and all exported register sets are >> now supposed to follow the 49 register set defined as in GDB now. >> >> Tobias, Ley Foon, how do you think this looks? > > Sorry, accidentally attached unrelated GCC patch instead, this one's the > correct one. > Looks good. I'm wondering if... +/* User structures for general purpose registers. */ +struct user_pt_regs { + __u32 regs[49]; }; Can we expose the registers explicitly here? Like this: struct user_pt_regs { __u32 r0; __u32 r1; ... __u32 sp; __u32 gp; __u32 estatus; }; It looks self-documenting and thus easier to use. >>> >>> Hi Chung-Lin, >>> >>> Your patch look good to me. >>> Do you have any problem to change the struct user_pt_regs based on >>> Ezequiel's suggestion? >> >> Well, exposing the register names like that sort of defeats the purpose of >> the PTR_* defines. >> >> Judging from the overall trend of style in arch/*/include/uapi/asm/ptrace.h >> across ports, I would prefer to stay with the array field. >> > Okay, I will include your patch. > That'd be great. I'll wait until Linus takes the change, and then will submit the strace support to strace mailing list. Thanks for the help! -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On Tue, Mar 10, 2015 at 2:17 PM, Chung-Lin Tang wrote: > On 2015/3/10 10:54 AM, Ley Foon Tan wrote: >> On Tue, Mar 10, 2015 at 1:05 AM, Ezequiel Garcia >> wrote: >>> >>> >>> On 03/09/2015 02:02 PM, Chung-Lin Tang wrote: On 2015/3/10 12:54 AM, Chung-Lin Tang wrote: > It appears that some of the ways nios2 has organized the > ucontext/pt_regs/etc. are remnants of the pre-generic code, some > basically because the port was based off m68k. > > I've re-organized the headers a bit: nios2/include/asm/ucontext.h is > deleted, and re-definition of struct sigcontext now allows use of > uapi/asm-generic/ucontext.h directly. Note that the reorg, despite > effectively renaming some fields, is still binary compatible. I'll > probably update the corresponding glibc definitions later. > > struct pt_regs is now not exported, and all exported register sets are > now supposed to follow the 49 register set defined as in GDB now. > > Tobias, Ley Foon, how do you think this looks? Sorry, accidentally attached unrelated GCC patch instead, this one's the correct one. >>> >>> Looks good. I'm wondering if... >>> >>> +/* User structures for general purpose registers. */ >>> +struct user_pt_regs { >>> + __u32 regs[49]; >>> }; >>> >>> Can we expose the registers explicitly here? Like this: >>> >>> struct user_pt_regs { >>> __u32 r0; >>> __u32 r1; >>> ... >>> __u32 sp; >>> __u32 gp; >>> __u32 estatus; >>> }; >>> >>> It looks self-documenting and thus easier to use. >> >> Hi Chung-Lin, >> >> Your patch look good to me. >> Do you have any problem to change the struct user_pt_regs based on >> Ezequiel's suggestion? > > Well, exposing the register names like that sort of defeats the purpose of > the PTR_* defines. > > Judging from the overall trend of style in arch/*/include/uapi/asm/ptrace.h > across ports, I would prefer to stay with the array field. > Okay, I will include your patch. Regards Ley Foon -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On 2015/3/10 10:54 AM, Ley Foon Tan wrote: > On Tue, Mar 10, 2015 at 1:05 AM, Ezequiel Garcia > wrote: >> >> >> On 03/09/2015 02:02 PM, Chung-Lin Tang wrote: >>> On 2015/3/10 12:54 AM, Chung-Lin Tang wrote: It appears that some of the ways nios2 has organized the ucontext/pt_regs/etc. are remnants of the pre-generic code, some basically because the port was based off m68k. I've re-organized the headers a bit: nios2/include/asm/ucontext.h is deleted, and re-definition of struct sigcontext now allows use of uapi/asm-generic/ucontext.h directly. Note that the reorg, despite effectively renaming some fields, is still binary compatible. I'll probably update the corresponding glibc definitions later. struct pt_regs is now not exported, and all exported register sets are now supposed to follow the 49 register set defined as in GDB now. Tobias, Ley Foon, how do you think this looks? >>> >>> Sorry, accidentally attached unrelated GCC patch instead, this one's the >>> correct one. >>> >> >> Looks good. I'm wondering if... >> >> +/* User structures for general purpose registers. */ >> +struct user_pt_regs { >> + __u32 regs[49]; >> }; >> >> Can we expose the registers explicitly here? Like this: >> >> struct user_pt_regs { >> __u32 r0; >> __u32 r1; >> ... >> __u32 sp; >> __u32 gp; >> __u32 estatus; >> }; >> >> It looks self-documenting and thus easier to use. > > Hi Chung-Lin, > > Your patch look good to me. > Do you have any problem to change the struct user_pt_regs based on > Ezequiel's suggestion? Well, exposing the register names like that sort of defeats the purpose of the PTR_* defines. Judging from the overall trend of style in arch/*/include/uapi/asm/ptrace.h across ports, I would prefer to stay with the array field. Thanks, Chung-Lin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On Tue, Mar 10, 2015 at 1:05 AM, Ezequiel Garcia wrote: > > > On 03/09/2015 02:02 PM, Chung-Lin Tang wrote: >> On 2015/3/10 12:54 AM, Chung-Lin Tang wrote: >>> It appears that some of the ways nios2 has organized the >>> ucontext/pt_regs/etc. are remnants of the pre-generic code, some >>> basically because the port was based off m68k. >>> >>> I've re-organized the headers a bit: nios2/include/asm/ucontext.h is >>> deleted, and re-definition of struct sigcontext now allows use of >>> uapi/asm-generic/ucontext.h directly. Note that the reorg, despite >>> effectively renaming some fields, is still binary compatible. I'll >>> probably update the corresponding glibc definitions later. >>> >>> struct pt_regs is now not exported, and all exported register sets are >>> now supposed to follow the 49 register set defined as in GDB now. >>> >>> Tobias, Ley Foon, how do you think this looks? >> >> Sorry, accidentally attached unrelated GCC patch instead, this one's the >> correct one. >> > > Looks good. I'm wondering if... > > +/* User structures for general purpose registers. */ > +struct user_pt_regs { > + __u32 regs[49]; > }; > > Can we expose the registers explicitly here? Like this: > > struct user_pt_regs { > __u32 r0; > __u32 r1; > ... > __u32 sp; > __u32 gp; > __u32 estatus; > }; > > It looks self-documenting and thus easier to use. Hi Chung-Lin, Your patch look good to me. Do you have any problem to change the struct user_pt_regs based on Ezequiel's suggestion? If not, can you please resend the new patch. Thanks. Regards Ley Foon -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On 03/09/2015 02:02 PM, Chung-Lin Tang wrote: > On 2015/3/10 12:54 AM, Chung-Lin Tang wrote: >> It appears that some of the ways nios2 has organized the >> ucontext/pt_regs/etc. are remnants of the pre-generic code, some >> basically because the port was based off m68k. >> >> I've re-organized the headers a bit: nios2/include/asm/ucontext.h is >> deleted, and re-definition of struct sigcontext now allows use of >> uapi/asm-generic/ucontext.h directly. Note that the reorg, despite >> effectively renaming some fields, is still binary compatible. I'll >> probably update the corresponding glibc definitions later. >> >> struct pt_regs is now not exported, and all exported register sets are >> now supposed to follow the 49 register set defined as in GDB now. >> >> Tobias, Ley Foon, how do you think this looks? > > Sorry, accidentally attached unrelated GCC patch instead, this one's the > correct one. > Looks good. I'm wondering if... +/* User structures for general purpose registers. */ +struct user_pt_regs { + __u32 regs[49]; }; Can we expose the registers explicitly here? Like this: struct user_pt_regs { __u32 r0; __u32 r1; ... __u32 sp; __u32 gp; __u32 estatus; }; It looks self-documenting and thus easier to use. -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On 2015/3/10 12:54 AM, Chung-Lin Tang wrote: > It appears that some of the ways nios2 has organized the > ucontext/pt_regs/etc. are remnants of the pre-generic code, some > basically because the port was based off m68k. > > I've re-organized the headers a bit: nios2/include/asm/ucontext.h is > deleted, and re-definition of struct sigcontext now allows use of > uapi/asm-generic/ucontext.h directly. Note that the reorg, despite > effectively renaming some fields, is still binary compatible. I'll > probably update the corresponding glibc definitions later. > > struct pt_regs is now not exported, and all exported register sets are > now supposed to follow the 49 register set defined as in GDB now. > > Tobias, Ley Foon, how do you think this looks? Sorry, accidentally attached unrelated GCC patch instead, this one's the correct one. Chung-Lin diff --git a/arch/nios2/include/asm/ptrace.h b/arch/nios2/include/asm/ptrace.h index 20fb1cf..6424621 100644 --- a/arch/nios2/include/asm/ptrace.h +++ b/arch/nios2/include/asm/ptrace.h @@ -15,7 +15,54 @@ #include +/* This struct defines the way the registers are stored on the + stack during a system call. */ + #ifndef __ASSEMBLY__ +struct pt_regs { + unsigned long r8; /* r8-r15 Caller-saved GP registers */ + unsigned long r9; + unsigned long r10; + unsigned long r11; + unsigned long r12; + unsigned long r13; + unsigned long r14; + unsigned long r15; + unsigned long r1; /* Assembler temporary */ + unsigned long r2; /* Retval LS 32bits */ + unsigned long r3; /* Retval MS 32bits */ + unsigned long r4; /* r4-r7 Register arguments */ + unsigned long r5; + unsigned long r6; + unsigned long r7; + unsigned long orig_r2; /* Copy of r2 ?? */ + unsigned long ra; /* Return address */ + unsigned long fp; /* Frame pointer */ + unsigned long sp; /* Stack pointer */ + unsigned long gp; /* Global pointer */ + unsigned long estatus; + unsigned long ea; /* Exception return address (pc) */ + unsigned long orig_r7; +}; + +/* + * This is the extended stack used by signal handlers and the context + * switcher: it's pushed after the normal "struct pt_regs". + */ +struct switch_stack { + unsigned long r16; /* r16-r23 Callee-saved GP registers */ + unsigned long r17; + unsigned long r18; + unsigned long r19; + unsigned long r20; + unsigned long r21; + unsigned long r22; + unsigned long r23; + unsigned long fp; + unsigned long gp; + unsigned long ra; +}; + #define user_mode(regs)(((regs)->estatus & ESTATUS_EU)) #define instruction_pointer(regs) ((regs)->ra) diff --git a/arch/nios2/include/asm/ucontext.h b/arch/nios2/include/asm/ucontext.h deleted file mode 100644 index 2c87614..000 --- a/arch/nios2/include/asm/ucontext.h +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright (C) 2010 Tobias Klauser - * Copyright (C) 2004 Microtronix Datacom Ltd - * - * This file is subject to the terms and conditions of the GNU General Public - * License. See the file "COPYING" in the main directory of this archive - * for more details. - */ - -#ifndef _ASM_NIOS2_UCONTEXT_H -#define _ASM_NIOS2_UCONTEXT_H - -typedef int greg_t; -#define NGREG 32 -typedef greg_t gregset_t[NGREG]; - -struct mcontext { - int version; - gregset_t gregs; -}; - -#define MCONTEXT_VERSION 2 - -struct ucontext { - unsigned long uc_flags; - struct ucontext *uc_link; - stack_t uc_stack; - struct mcontext uc_mcontext; - sigset_t uc_sigmask; /* mask last for extensibility */ -}; - -#endif diff --git a/arch/nios2/include/uapi/asm/Kbuild b/arch/nios2/include/uapi/asm/Kbuild index 4f07ca3f8..37613119 100644 --- a/arch/nios2/include/uapi/asm/Kbuild +++ b/arch/nios2/include/uapi/asm/Kbuild @@ -2,3 +2,5 @@ include include/uapi/asm-generic/Kbuild.asm header-y += elf.h header-y += ucontext.h + +generic-y += ucontext.h diff --git a/arch/nios2/include/uapi/asm/elf.h b/arch/nios2/include/uapi/asm/elf.h index a5b91ae..6f06d3b 100644 --- a/arch/nios2/include/uapi/asm/elf.h +++ b/arch/nios2/include/uapi/asm/elf.h @@ -50,9 +50,7 @@ typedef unsigned long elf_greg_t; -#define ELF_NGREG \ - ((sizeof(struct pt_regs) + sizeof(struct switch_stack)) / \ - sizeof(elf_greg_t)) +#define ELF_NGREG 49 typedef elf_greg_t elf_gregset_t[ELF_NGREG]; typedef unsigned long elf_fpregset_t; diff --git a/arch/nios2/include/uapi/asm/ptrace.h b/arch/nios2/include/uapi/asm/ptrace.h index e83a7c9..71a3305 100644 --- a/arch/nios2/include/uapi/asm/ptrace.h +++ b/arch/nios2/include/uapi/asm/ptrace.h @@ -67,53 +67,9 @@ #define NUM_PTRACE_REG (PTR_TLBMISC + 1) -/* this struct defines the way the registers are stored on the - stack duri
Re: nios2: is the ptrace ABI correct?
On 2015/2/27 07:19 PM, Ezequiel Garcia wrote: > Right now, my biggest problem is how to remove the pt_regs from being > exported to userspace, without breaking things. The struct is defined > and used in a few UAPI headers: > > arch/nios2/include/uapi/asm/ptrace.h > arch/nios2/include/uapi/asm/sigcontext.h > arch/nios2/include/uapi/asm/elf.h > > For ucontext, we would need to remove > arch/nios2/include/uapi/asm/sigcontext.h and avoid using > asm-generic/ucontext.h. > > For ptrace, we could move pt_regs from the UAPI header to some internal > header. That way userspace is not exposed the wrong struct. > > For the elf coredump, I have no idea yet :) It appears that some of the ways nios2 has organized the ucontext/pt_regs/etc. are remnants of the pre-generic code, some basically because the port was based off m68k. I've re-organized the headers a bit: nios2/include/asm/ucontext.h is deleted, and re-definition of struct sigcontext now allows use of uapi/asm-generic/ucontext.h directly. Note that the reorg, despite effectively renaming some fields, is still binary compatible. I'll probably update the corresponding glibc definitions later. struct pt_regs is now not exported, and all exported register sets are now supposed to follow the 49 register set defined as in GDB now. Tobias, Ley Foon, how do you think this looks? Thanks, Chung-Lin Index: gcc/config/nios2/nios2-protos.h === --- gcc/config/nios2/nios2-protos.h (revision 446123) +++ gcc/config/nios2/nios2-protos.h (working copy) @@ -32,7 +32,7 @@ extern void nios2_function_profiler (FILE *, int); #ifdef RTX_CODE extern bool nios2_emit_move_sequence (rtx *, enum machine_mode); extern void nios2_emit_expensive_div (rtx *, enum machine_mode); -extern void nios2_adjust_call_address (rtx *); +extern void nios2_adjust_call_address (rtx *, rtx); extern rtx nios2_get_return_address (int); extern void nios2_set_return_address (rtx, rtx); Index: gcc/config/nios2/nios2.c === --- gcc/config/nios2/nios2.c(revision 446123) +++ gcc/config/nios2/nios2.c(working copy) @@ -1511,12 +1511,12 @@ nios2_unspec_offset (rtx loc, int unspec) /* Generate GOT pointer based address with large offset. */ static rtx -nios2_large_got_address (rtx offset) +nios2_large_got_address (rtx offset, rtx tmp) { - rtx addr = gen_reg_rtx (Pmode); - emit_insn (gen_add3_insn (addr, pic_offset_table_rtx, - force_reg (Pmode, offset))); - return addr; + if (!tmp) +tmp = gen_reg_rtx (Pmode); + emit_move_insn (tmp, offset); + return gen_rtx_PLUS (Pmode, tmp, pic_offset_table_rtx); } /* Generate a GOT pointer based address. */ @@ -1527,7 +1527,7 @@ nios2_got_address (rtx loc, int unspec) crtl->uses_pic_offset_table = 1; if (nios2_large_offset_p (unspec)) -return nios2_large_got_address (offset); +return force_reg (Pmode, nios2_large_got_address (offset, NULL_RTX)); return gen_rtx_PLUS (Pmode, pic_offset_table_rtx, offset); } @@ -2080,13 +2080,17 @@ nios2_load_pic_register (void) /* Generate a PIC address as a MEM rtx. */ static rtx -nios2_load_pic_address (rtx sym, int unspec) +nios2_load_pic_address (rtx sym, int unspec, rtx tmp) { if (flag_pic == 2 && GET_CODE (sym) == SYMBOL_REF && nios2_symbol_binds_local_p (sym)) /* Under -fPIC, generate a GOTOFF address for local symbols. */ -return nios2_got_address (sym, UNSPEC_PIC_GOTOFF_SYM); +{ + rtx offset = nios2_unspec_offset (sym, UNSPEC_PIC_GOTOFF_SYM); + crtl->uses_pic_offset_table = 1; + return nios2_large_got_address (offset, tmp); +} return gen_const_mem (Pmode, nios2_got_address (sym, unspec)); } @@ -2122,7 +2126,7 @@ nios2_legitimize_constant_address (rtx addr) if (nios2_tls_symbol_p (base)) base = nios2_legitimize_tls_address (base); else if (flag_pic) -base = nios2_load_pic_address (base, UNSPEC_PIC_SYM); +base = nios2_load_pic_address (base, UNSPEC_PIC_SYM, NULL_RTX); else return addr; @@ -2266,18 +2270,24 @@ nios2_emit_move_sequence (rtx *operands, enum mach /* The function with address *ADDR is being called. If the address needs to be loaded from the GOT, emit the instruction to do so and - update *ADDR to point to the rtx for the loaded value. */ + update *ADDR to point to the rtx for the loaded value. + If REG != NULL_RTX, it is used as the target/scratch register in the + GOT address calculation. */ void -nios2_adjust_call_address (rtx *call_op) +nios2_adjust_call_address (rtx *call_op, rtx reg) { - rtx addr; - gcc_assert (MEM_P (*call_op)); - addr = XEXP (*call_op, 0); + if (MEM_P (*call_op)) +call_op = &XEXP (*call_op, 0); + + rtx addr = *call_op; if (flag_pic && CONSTANT_P (addr)) { - rtx reg = gen_reg_rtx (Pmode); - emit_move_insn (reg, nios2_load_pic_address (addr, UNSPEC_PIC_
Re: nios2: is the ptrace ABI correct?
Hi all, I have here a patch to rework most of the UAPI headers, so the info exported to userspace is sane. The ABI itself is left untouched. This will allow to support strace (which is ready since a few weeks now) and will sanitize the UAPIs. The ABI itself is ugly as hell, but I don't want to break it. Given the lack of replies on this thread, I guess nobody cares much about nios2 mainline. Anyway... I'll try to submit a patchset soon and I'll appreciate if the libc can take a look and make sure everything is OK. -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
Hi Chung-Lin Tang, On 02/27/2015 05:57 AM, Chung-Lin Tang wrote: > On 15/2/25 10:07 PM, Arnd Bergmann wrote: >> On Wednesday 25 February 2015 08:33:16 Ezequiel Garcia wrote: >>> >>> /me is more confused now >>> >>> In arch/nios2/include/asm/ucontext.h >>> >>> struct ucontext { >>> unsigned long uc_flags; >>> struct ucontext *uc_link; >>> stack_t uc_stack; >>> struct mcontext uc_mcontext; >>> sigset_t uc_sigmask; >>> }; >>> >>> And in include/uapi/asm-generic/ucontext.h: >>> >>> struct ucontext { >>> unsigned long uc_flags; >>> struct ucontext *uc_link; >>> stack_t uc_stack; >>> struct sigcontext uc_mcontext; >>> sigset_t uc_sigmask; >>> }; >>> >>> Which one is the one that userspace sees? And why does the kernel has >>> two different structures? >> >> Userspace sees the asm-generic header, which I assume is a bug >> in this case. > > Yes, I believe nios2 doesn't not need this asm-generic/ucontext.h > header; OTOH it just isn't used; no real harm done, so easily fixed. > >>> Given this oddities, I'm wondering how troublesome would be to just >>> re-do this and break the ptrace and signal ABI. For instance, just >>> pushing pt_regs in PTRACE_GETREGSET would make things much clearer. >> >> Could you change pt_regs to match the layout you have for PTRACE_GETREGSET >> instead? It seems much more intuitive. > > There is a reason for this pt_regs arrangement: the nios2 syscall > interface uses r4-r9 for parameters, while the usual C conventions use > only r4-r7, placing r8-r9 at the start of pt_regs creates a natural > stack layout for entering C code after the asm shims in entry.S > >>> I guess Linus would burn me for even suggesting to breaking users... but >>> do we have any users at all? This arch has just been mainlined. Altera's >>> out-of-tree is already ABI-incompatible with mainline so that's not an >>> issue. >>> >>> The only one using this ABI is gdb, which is easily fixed. >> >> You can change anything you like as long as nobody complains about >> regressions. > > PTRACE_GET/SETREGSET is a new feature in nios2-linux that we're still > about to support in upstream GDB, so things could be fixed if needed, > but why can't you just use the [0...] ordering in userspace? > Sure, that's doable. However, I believe the problem is the pt_regs struct is exported in ptrace.h so we seem to be telling userspace to use it. > BTW, it's even that way in signal stacks as well; nios2 does not > use/export sigcontext inside struct ucontext. We just use a int[32] > array there. > I think we are more or less on the same page. Right now, my biggest problem is how to remove the pt_regs from being exported to userspace, without breaking things. The struct is defined and used in a few UAPI headers: arch/nios2/include/uapi/asm/ptrace.h arch/nios2/include/uapi/asm/sigcontext.h arch/nios2/include/uapi/asm/elf.h For ucontext, we would need to remove arch/nios2/include/uapi/asm/sigcontext.h and avoid using asm-generic/ucontext.h. For ptrace, we could move pt_regs from the UAPI header to some internal header. That way userspace is not exposed the wrong struct. For the elf coredump, I have no idea yet :) -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On 15/2/25 10:07 PM, Arnd Bergmann wrote: > On Wednesday 25 February 2015 08:33:16 Ezequiel Garcia wrote: >> >> /me is more confused now >> >> In arch/nios2/include/asm/ucontext.h >> >> struct ucontext { >> unsigned long uc_flags; >> struct ucontext *uc_link; >> stack_t uc_stack; >> struct mcontext uc_mcontext; >> sigset_t uc_sigmask; >> }; >> >> And in include/uapi/asm-generic/ucontext.h: >> >> struct ucontext { >> unsigned long uc_flags; >> struct ucontext *uc_link; >> stack_t uc_stack; >> struct sigcontext uc_mcontext; >> sigset_t uc_sigmask; >> }; >> >> Which one is the one that userspace sees? And why does the kernel has >> two different structures? > > Userspace sees the asm-generic header, which I assume is a bug > in this case. Yes, I believe nios2 doesn't not need this asm-generic/ucontext.h header; OTOH it just isn't used; no real harm done, so easily fixed. >> Given this oddities, I'm wondering how troublesome would be to just >> re-do this and break the ptrace and signal ABI. For instance, just >> pushing pt_regs in PTRACE_GETREGSET would make things much clearer. > > Could you change pt_regs to match the layout you have for PTRACE_GETREGSET > instead? It seems much more intuitive. There is a reason for this pt_regs arrangement: the nios2 syscall interface uses r4-r9 for parameters, while the usual C conventions use only r4-r7, placing r8-r9 at the start of pt_regs creates a natural stack layout for entering C code after the asm shims in entry.S >> I guess Linus would burn me for even suggesting to breaking users... but >> do we have any users at all? This arch has just been mainlined. Altera's >> out-of-tree is already ABI-incompatible with mainline so that's not an >> issue. >> >> The only one using this ABI is gdb, which is easily fixed. > > You can change anything you like as long as nobody complains about > regressions. PTRACE_GET/SETREGSET is a new feature in nios2-linux that we're still about to support in upstream GDB, so things could be fixed if needed, but why can't you just use the [0...] ordering in userspace? BTW, it's even that way in signal stacks as well; nios2 does not use/export sigcontext inside struct ucontext. We just use a int[32] array there. Thanks, Chung-Lin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On Wednesday 25 February 2015 08:33:16 Ezequiel Garcia wrote: > > /me is more confused now > > In arch/nios2/include/asm/ucontext.h > > struct ucontext { > unsigned long uc_flags; > struct ucontext *uc_link; > stack_t uc_stack; > struct mcontext uc_mcontext; > sigset_t uc_sigmask; > }; > > And in include/uapi/asm-generic/ucontext.h: > > struct ucontext { > unsigned long uc_flags; > struct ucontext *uc_link; > stack_t uc_stack; > struct sigcontext uc_mcontext; > sigset_t uc_sigmask; > }; > > Which one is the one that userspace sees? And why does the kernel has > two different structures? Userspace sees the asm-generic header, which I assume is a bug in this case. > Given this oddities, I'm wondering how troublesome would be to just > re-do this and break the ptrace and signal ABI. For instance, just > pushing pt_regs in PTRACE_GETREGSET would make things much clearer. Could you change pt_regs to match the layout you have for PTRACE_GETREGSET instead? It seems much more intuitive. > I guess Linus would burn me for even suggesting to breaking users... but > do we have any users at all? This arch has just been mainlined. Altera's > out-of-tree is already ABI-incompatible with mainline so that's not an > issue. > > The only one using this ABI is gdb, which is easily fixed. You can change anything you like as long as nobody complains about regressions. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On 02/24/2015 04:25 PM, Arnd Bergmann wrote: > On Tuesday 24 February 2015 12:28:41 Ezequiel Garcia wrote: >> >> Gah, no, you are right. I got confused. >> >> So it would be OK to avoid remove pt_regs from the uapi headers? >> How does this affect the signal handling nios2 implementation? >> > > We have a number of architectures that don't provide this structure: > > $ git grep -L pt_regs arch/*/include/uapi/asm/ptrace.h > arch/frv/include/uapi/asm/ptrace.h > arch/metag/include/uapi/asm/ptrace.h > arch/openrisc/include/uapi/asm/ptrace.h > arch/s390/include/uapi/asm/ptrace.h > > so I'd assume it's ok in general not to have it. However, on > nios2, struct pt_regs is embedded inside of struct sigcontext. > If I read the code in arch/nios2/kernel/signal.c correctly, > this is actually a bug and you should use a different structure > there too, because pt_regs does not match the layout of the > stack either. This means that the (rare) user programs that > would know about the architecture to modify signal stacks > are currently broken. > /me is more confused now In arch/nios2/include/asm/ucontext.h struct ucontext { unsigned long uc_flags; struct ucontext *uc_link; stack_t uc_stack; struct mcontext uc_mcontext; sigset_t uc_sigmask; }; And in include/uapi/asm-generic/ucontext.h: struct ucontext { unsigned long uc_flags; struct ucontext *uc_link; stack_t uc_stack; struct sigcontext uc_mcontext; sigset_t uc_sigmask; }; Which one is the one that userspace sees? And why does the kernel has two different structures? Given this oddities, I'm wondering how troublesome would be to just re-do this and break the ptrace and signal ABI. For instance, just pushing pt_regs in PTRACE_GETREGSET would make things much clearer. I guess Linus would burn me for even suggesting to breaking users... but do we have any users at all? This arch has just been mainlined. Altera's out-of-tree is already ABI-incompatible with mainline so that's not an issue. The only one using this ABI is gdb, which is easily fixed. -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On Tuesday 24 February 2015 12:28:41 Ezequiel Garcia wrote: > > Gah, no, you are right. I got confused. > > So it would be OK to avoid remove pt_regs from the uapi headers? > How does this affect the signal handling nios2 implementation? > We have a number of architectures that don't provide this structure: $ git grep -L pt_regs arch/*/include/uapi/asm/ptrace.h arch/frv/include/uapi/asm/ptrace.h arch/metag/include/uapi/asm/ptrace.h arch/openrisc/include/uapi/asm/ptrace.h arch/s390/include/uapi/asm/ptrace.h so I'd assume it's ok in general not to have it. However, on nios2, struct pt_regs is embedded inside of struct sigcontext. If I read the code in arch/nios2/kernel/signal.c correctly, this is actually a bug and you should use a different structure there too, because pt_regs does not match the layout of the stack either. This means that the (rare) user programs that would know about the architecture to modify signal stacks are currently broken. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
Hi Arnd, On 02/24/2015 05:54 AM, Arnd Bergmann wrote: > On Tuesday 24 February 2015 00:04:21 Ezequiel Garcia wrote: >> So, tried a different approach and removed pt_regs from the UAPI ptrace.h, >> replacing it with a new user_regs that describes how registers are passed >> to user. This however is also problematic, as pt_regs is already used >> by glibc (not really sure what for). >> > > I've looked at glibc and could not find a use for pt_regs there. Where > did you find it? It's quite possible that it's incorrect as well > if the structures don't match. > Gah, no, you are right. I got confused. So it would be OK to avoid remove pt_regs from the uapi headers? How does this affect the signal handling nios2 implementation? -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: nios2: is the ptrace ABI correct?
On Tuesday 24 February 2015 00:04:21 Ezequiel Garcia wrote: > So, tried a different approach and removed pt_regs from the UAPI ptrace.h, > replacing it with a new user_regs that describes how registers are passed > to user. This however is also problematic, as pt_regs is already used > by glibc (not really sure what for). > I've looked at glibc and could not find a use for pt_regs there. Where did you find it? It's quite possible that it's incorrect as well if the structures don't match. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
nios2: is the ptrace ABI correct?
Hi everyone, I've been trying to port strace to Nios-II, but came across some oddities in the ptrace ABI. The pt_regs structure is exposed to userspace through the ptrace.h UAPI header: arch/nios2/include/uapi/asm/ptrace.h Such pt_regs has the following layout: struct pt_regs { unsigned long r8; unsigned long r9; unsigned long r10; [and so on] } But the regset implementation in arch/nios2/kernel/ptrace.c pushes a different layout to userspace, as it uses the PTR_ macros, also in the UAPI header: #define PTR_R0 0 #define PTR_R1 1 #define PTR_R2 2 #define PTR_R3 3 #define PTR_R4 4 #define PTR_R5 5 #define PTR_R6 6 #define PTR_R7 7 #define PTR_R8 8 [..] In other words, the PTRACE_GETREGSET will pass to userspace register r8 at offset 8*4, but the pt_regs structure says it's at offset 0. This difference appears because ptrace returns one layout to userspace, and pushes the registers to the stack with another layout. I've fixed this and managed to port strace by changing the genregs_get implementation, so it returns a layout that matches pt_regs, and therefore matches the stack. Having this one-to-one correspondence, has a nice benefit. The implementation is trivial: static int genregs_get(struct task_struct *target, const struct user_regset *regset, unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf) { const struct pt_regs *regs = task_pt_regs(target); const struct switch_stack *sw = (struct switch_stack *)regs - 1; int ret; ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, regs, 0, PT_REGS_SIZE); if (!ret) ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, sw, 0, SWITCH_STACK_SIZE); return ret; } However, there's a problem. The ABI is already different, and current gdb seems to rely in the current layout. However, it does it by *not* using the pt_regs structure. Give the ABI is already used, I'm reluctant to change it as I don't want to break our beloved users. So, tried a different approach and removed pt_regs from the UAPI ptrace.h, replacing it with a new user_regs that describes how registers are passed to user. This however is also problematic, as pt_regs is already used by glibc (not really sure what for). In conclusion, I'm lost and have no clue how a proper fix would look like. (Or perhaps, I'm really lost and there's no fix needed.) Any ideas? -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
nios2: is the ptrace ABI correct?
Hi everyone, I've been trying to port strace to Nios-II, but came across some oddities in the ptrace ABI. The pt_regs structure is exposed to userspace through the ptrace.h UAPI header: arch/nios2/include/uapi/asm/ptrace.h Such pt_regs has the following layout: struct pt_regs { unsigned long r8; unsigned long r9; unsigned long r10; [and so on] } But the regset implementation in arch/nios2/kernel/ptrace.c pushes a different layout to userspace, as it uses the PTR_ macros, also in the UAPI header: #define PTR_R0 0 #define PTR_R1 1 #define PTR_R2 2 #define PTR_R3 3 #define PTR_R4 4 #define PTR_R5 5 #define PTR_R6 6 #define PTR_R7 7 #define PTR_R8 8 [..] In other words, the PTRACE_GETREGSET will pass to userspace register r8 at offset 8*4, but the pt_regs structure says it's at offset 0. This difference appears because ptrace returns one layout to userspace, and pushes the registers to the stack with another layout. I've fixed this and managed to port strace by changing the genregs_get implementation, so it returns a layout that matches pt_regs, and therefore matches the stack. Having this one-to-one correspondence, has a nice benefit. The implementation is trivial: static int genregs_get(struct task_struct *target, const struct user_regset *regset, unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf) { const struct pt_regs *regs = task_pt_regs(target); const struct switch_stack *sw = (struct switch_stack *)regs - 1; int ret; ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, regs, 0, PT_REGS_SIZE); if (!ret) ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, sw, 0, SWITCH_STACK_SIZE); return ret; } However, there's a problem. The ABI is already different, and current gdb seems to rely in the current layout. However, it does it by *not* using the pt_regs structure. Give the ABI is already used, I'm reluctant to change it as I don't want to break our beloved users. So, tried a different approach and removed pt_regs from the UAPI ptrace.h, replacing it with a new user_regs that describes how registers are passed to user. This however is also problematic, as pt_regs is already used by glibc (not really sure what for). In conclusion, I'm lost and have no clue how a proper fix would look like. (Or perhaps, I'm really lost and there's no fix needed.) Any ideas? -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/