lkml.org 
[lkml]   [2012]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/2] kgdb,debug_core,kgdbts: End DEBUG_RODATA limitation using kprobe breakpoints
    (2012/03/22 2:55), Jason Wessel wrote:
    > There has long been a limitation using software breakpoints with a
    > kernel compiled with CONFIG_DEBUG_RODATA. The kprobe breakpoint code
    > has its own text_poke() function which accommodates writing a
    > breakpoint into a read-only page. The debug_core can make use of the
    > text_poke() capabilities by using the kprobes API, specifically
    > arch_arm_kprobe() and arch_disarm_kprobe(). For now it is safe to use
    > a single statically allocated kprobe structure to call the kprobes API
    > because the debug_core breakpoint API is only used when the kernel is
    > in the debug state.

    You might misunderstand it. arch_*_kprobe() are not open APIs.
    Those are kprobes internal APIs (which means that those functions
    should be used only by kprobes).


    > The debug_core will first attempt to use the traditional
    > probe_kernel_write(), and next try using a kprobe breakpoint. The
    > kgdb test suite was updated to run all the software breakpoint tests
    > when using a kernel with built with CONFIG_DEBUG_RODATA.
    >
    > Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

    Nak.

    [...]
    > @@ -165,17 +173,48 @@ int __weak kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
    > {
    > int err;
    >
    > + bpt->type = BP_BREAKPOINT;
    > err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr,
    > BREAK_INSTR_SIZE);
    > if (err)
    > return err;
    > err = probe_kernel_write((char *)bpt->bpt_addr,
    > arch_kgdb_ops.gdb_bpt_instr, BREAK_INSTR_SIZE);
    > +#if defined(CONFIG_KPROBES) && defined(CONFIG_DEBUG_RODATA)
    > + if (!err)
    > + return err;
    > + probe_write_tmp.addr = (kprobe_opcode_t *)bpt->bpt_addr;
    > + arch_arm_kprobe(&probe_write_tmp);

    No, please don't use kprobes internal function this way, because
    you can't ensure that the arch_arm_kprobe() has no side-effect.

    Why don't you use text_poke()? I see that the text_poke()
    is only for x86, but you already have arch/x86/kernel/kgdb.c for
    making your own wrapper function.

    > + err = probe_kernel_read(&probe_write_tmp.opcode, (char *)bpt->bpt_addr,
    > + BREAK_INSTR_SIZE);
    > + if (err)
    > + return err;
    > + if (memcmp(&probe_write_tmp.opcode, arch_kgdb_ops.gdb_bpt_instr,
    > + BREAK_INSTR_SIZE))
    > + return -EINVAL;
    > + bpt->type = BP_KPROBE_BREAKPOINT;
    > +#endif /* CONFIG_KPROBES && CONFIG_DEBUG_RODATA */
    > return err;
    > }
    >
    > int __weak kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
    > {
    > +#if defined(CONFIG_KPROBES) && defined(CONFIG_DEBUG_RODATA)
    > + int err;
    > +
    > + if (bpt->type != BP_KPROBE_BREAKPOINT)
    > + goto knl_write;
    > + probe_write_tmp.addr = (kprobe_opcode_t *)bpt->bpt_addr;
    > + memcpy(&probe_write_tmp.opcode, bpt->saved_instr, BREAK_INSTR_SIZE);
    > + arch_disarm_kprobe(&probe_write_tmp);

    Ditto.

    > + err = probe_kernel_read(&probe_write_tmp.opcode, (char *)bpt->bpt_addr,
    > + BREAK_INSTR_SIZE);
    > + if (err ||
    > + memcmp(&probe_write_tmp.opcode, bpt->saved_instr, BREAK_INSTR_SIZE))
    > + goto knl_write;
    > + return err;
    > +knl_write:
    > +#endif /* CONFIG_KPROBES && CONFIG_DEBUG_RODATA */
    > return probe_kernel_write((char *)bpt->bpt_addr,
    > (char *)bpt->saved_instr, BREAK_INSTR_SIZE);
    > }
    > @@ -294,7 +333,6 @@ int dbg_set_sw_break(unsigned long addr)
    > return -E2BIG;
    >
    > kgdb_break[breakno].state = BP_SET;
    > - kgdb_break[breakno].type = BP_BREAKPOINT;
    > kgdb_break[breakno].bpt_addr = addr;
    >
    > return 0;

    Thank you,


    --
    Masami HIRAMATSU
    Software Platform Research Dept. Linux Technology Center
    Hitachi, Ltd., Yokohama Research Laboratory
    E-mail: masami.hiramatsu.pt@hitachi.com


    \
     
     \ /
      Last update: 2012-03-22 03:57    [W:0.028 / U:62.120 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site