[lkml]   [2008]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] x86: Use v8086_mode helper, trivial unification
    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.


    #ifdef CONFIG_X86_64
    #define stack_addr(regs) ((unsigned long *)regs->sp)
    * "&regs->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 &regs->sp happens to
    * point to the top of the pre-int3 stack.
    #define stack_addr(regs) ((unsigned long *)&regs->sp)

    > > /* 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 ;-)


     \ /
      Last update: 2008-01-18 02:17    [W:0.023 / U:3.100 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site