Messages in this thread | | | Subject | Re: [PATCH] x86: Use v8086_mode helper, trivial unification | From | Harvey Harrison <> | Date | Thu, 17 Jan 2008 17:14:29 -0800 |
| |
On Thu, 2008-01-17 at 19:59 -0500, H. Peter Anvin wrote: > Harvey Harrison wrote: > > > > Sorry, missed that detail in ptrace.h, I notice now. > > > > Is there some better way this could be organized, would the following > > be an improvement, as opposed to two long ifdef sections? > > > > Patch will follow if you think it's a good idea. > > It is actually quite a bit easier to read.
I'll send along a patch along soon, any thoughts on how to order it in the file?
> > > > > static inline unsigned long stack_pointer(struct pt_regs *regs) > > { > > #ifdef CONFIG_X86_32 > > return (unsigned long)regs; > > #else > > return regs->sp; > > #endif > > } > > This one is kind of strange. In particular, the 32-bit definition isn't > exactly what one would expect. It makes me concerned that it actually > refers to two different kinds of stack pointers?
This tripped up the kprobes unification as well, see the stack_addr() helper that was introduced there. Would be good to figure this out and put a big fat comment on it.
kprobes.c
#ifdef CONFIG_X86_64 #define stack_addr(regs) ((unsigned long *)regs->sp) #else /* * "®s->sp" looks wrong, but it's correct for x86_32. x86_32 CPUs * don't save the ss and esp registers if the CPU is already in kernel * mode when it traps. So for kprobes, regs->sp and regs->ss are not * the [nonexistent] saved stack pointer and ss register, but rather * the top 8 bytes of the pre-int3 stack. So ®s->sp happens to * point to the top of the pre-int3 stack. */ #define stack_addr(regs) ((unsigned long *)®s->sp) #endif
> > /* still need a define here, as one is long and one is unsigned long. > > * but this is another target for unification I guess. */ > > #define regs_return_value(regs) ((regs)->ax) > > Indeed...
I think this comes out of Roland's patches unifying some names eip/rip, eax/rax, etc.
CC'd in case he felt like more work ;-)
Harvey
| |