Hi Christoph, Thank you for review. Christoph Hellwig wrote: > You might want to run this past linux-arch to make sure this is suitable > for other architectures. Frankly, I'm not sure about linux-arch, could you explain it? Anyway, I'm interested in that idea. >> --- a/arch/x86/include/asm/ptrace.h >> +++ b/arch/x86/include/asm/ptrace.h >> @@ -7,6 +7,7 @@ >> >> #ifdef __KERNEL__ >> #include <asm/segment.h> >> +#include <asm/page_types.h> >> #endif > > I really wonder if we should split asm/ptrace.h into one file > just defining pt_regs both for userspace and the kernel, and one with > all kinds of register access helpers and maybe another one for the > ptrace architecture interface. Agreed, pt_regs is used more broadly than ptrace itself in kernel. > Unforuntately we would have to keep the ptrace.h name for the one > carrying pt_regs as it's exposed to userland. Perhaps, we should split pt_regs from ptrace.h, like as ptrace-regs.h. >> +static inline unsigned long get_register(struct pt_regs *regs, unsigned offset) >> +{ > > I woner if all these names aren't a bit generic. Shoud we maybe add a > regs_ prefix to make it clear it operates on a pt_regs register set? Indeed. > Also some kerneldoc documentation for all these helpers would be nice. Sure. >> +/* Get Nth argument at function call */ >> +static inline unsigned long get_argument_nth(struct pt_regs *regs, unsigned n) >> +{ >> +#ifdef CONFIG_X86_32 >> +#define NR_REGPARMS 3 > > I think completely separate version for 32 vs 64 bit for this one would > be cleaner. Agreed, > >> + if (n < NR_REGPARMS) { >> + switch (n) { >> + case 0: return regs->ax; >> + case 1: return regs->dx; >> + case 2: return regs->cx; >> + } > > Normal kernel style would be > > switch (n) { > case 0: > return regs->ax; > case 1: > return regs->dx; > case 2: > return regs->cx; > } Oops, thanks, > >> +#define REG_OFFSET(r) offsetof(struct pt_regs, r) >> +#define REG_OFFSET_NAME(r) {.name = #r, .offset = REG_OFFSET(r)} >> +#define REG_OFFSET_END {.name = NULL, .offset = 0} > > At least the REG_OFFSET macro seems superflous to me. > Exactly. Thank you again! -- Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: mhiramat@xxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html