lkml.org 
[lkml]   [2010]   [Jan]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 1/4] x86,hw_breakpoint,kgdb: kgdb to use hw_breakpointAPI
    New version which loses the call back registration, because if we
    register as a different breakpoint type, perf won't reject it. I didn't
    see that loop hole in the API the first time around.

    Also the dr7 fixup in hw_breakpoint.c is not needed.

    The trickery for entering twice into the __kgdb_notify is gone too.

    All the regression tests are still passing.

    ---

    arch/x86/kernel/kgdb.c | 173
    ++++++++++++++++++++++++++++++++-----------------
    kernel/kgdb.c | 3
    2 files changed, 118 insertions(+), 58 deletions(-)

    --- a/arch/x86/kernel/kgdb.c
    +++ b/arch/x86/kernel/kgdb.c
    @@ -42,6 +42,7 @@
    #include <linux/init.h>
    #include <linux/smp.h>
    #include <linux/nmi.h>
    +#include <linux/hw_breakpoint.h>

    #include <asm/debugreg.h>
    #include <asm/apicdef.h>
    @@ -204,40 +205,38 @@ void gdb_regs_to_pt_regs(unsigned long *

    static struct hw_breakpoint {
    unsigned enabled;
    - unsigned type;
    - unsigned len;
    unsigned long addr;
    + int len;
    + int type;
    + struct perf_event **pev;
    } breakinfo[4];

    static void kgdb_correct_hw_break(void)
    {
    - unsigned long dr7;
    - int correctit = 0;
    - int breakbit;
    int breakno;

    - get_debugreg(dr7, 7);
    for (breakno = 0; breakno < 4; breakno++) {
    - breakbit = 2 << (breakno << 1);
    - if (!(dr7 & breakbit) && breakinfo[breakno].enabled) {
    - correctit = 1;
    - dr7 |= breakbit;
    - dr7 &= ~(0xf0000 << (breakno << 2));
    - dr7 |= ((breakinfo[breakno].len << 2) |
    - breakinfo[breakno].type) <<
    - ((breakno << 2) + 16);
    - set_debugreg(breakinfo[breakno].addr, breakno);
    -
    - } else {
    - if ((dr7 & breakbit) && !breakinfo[breakno].enabled) {
    - correctit = 1;
    - dr7 &= ~breakbit;
    - dr7 &= ~(0xf0000 << (breakno << 2));
    - }
    - }
    + struct perf_event *bp;
    + struct arch_hw_breakpoint *info;
    + int val;
    + int cpu = raw_smp_processor_id();
    + if (!breakinfo[breakno].enabled)
    + continue;
    + bp = *per_cpu_ptr(breakinfo[breakno].pev, cpu);
    + info = counter_arch_bp(bp);
    + if (bp->attr.disabled != 1)
    + continue;
    + bp->attr.bp_addr = breakinfo[breakno].addr;
    + bp->attr.bp_len = breakinfo[breakno].len;
    + bp->attr.bp_type = breakinfo[breakno].type;
    + info->address = breakinfo[breakno].addr;
    + info->len = breakinfo[breakno].len;
    + info->type = breakinfo[breakno].type;
    + val = arch_install_hw_breakpoint(bp);
    + if (!val)
    + bp->attr.disabled = 0;
    }
    - if (correctit)
    - set_debugreg(dr7, 7);
    + hw_breakpoint_restore();
    }

    static int
    @@ -259,15 +258,23 @@ kgdb_remove_hw_break(unsigned long addr,
    static void kgdb_remove_all_hw_break(void)
    {
    int i;
    + int cpu = raw_smp_processor_id();
    + struct perf_event *bp;

    - for (i = 0; i < 4; i++)
    - memset(&breakinfo[i], 0, sizeof(struct hw_breakpoint));
    + for (i = 0; i < 4; i++) {
    + if (!breakinfo[i].enabled)
    + continue;
    + bp = *per_cpu_ptr(breakinfo[i].pev, cpu);
    + if (bp->attr.disabled == 1)
    + continue;
    + arch_uninstall_hw_breakpoint(bp);
    + bp->attr.disabled = 1;
    + }
    }

    static int
    kgdb_set_hw_break(unsigned long addr, int len, enum kgdb_bptype bptype)
    {
    - unsigned type;
    int i;

    for (i = 0; i < 4; i++)
    @@ -278,27 +285,38 @@ kgdb_set_hw_break(unsigned long addr, in

    switch (bptype) {
    case BP_HARDWARE_BREAKPOINT:
    - type = 0;
    - len = 1;
    + len = 1;
    + breakinfo[i].type = X86_BREAKPOINT_EXECUTE;
    break;
    case BP_WRITE_WATCHPOINT:
    - type = 1;
    + breakinfo[i].type = X86_BREAKPOINT_WRITE;
    break;
    case BP_ACCESS_WATCHPOINT:
    - type = 3;
    + breakinfo[i].type = X86_BREAKPOINT_RW;
    break;
    default:
    return -1;
    }
    -
    - if (len == 1 || len == 2 || len == 4)
    - breakinfo[i].len = len - 1;
    - else
    - return -1;
    -
    - breakinfo[i].enabled = 1;
    + switch (len) {
    + case 1:
    + breakinfo[i].len = X86_BREAKPOINT_LEN_1;
    + break;
    + case 2:
    + breakinfo[i].len = X86_BREAKPOINT_LEN_2;
    + break;
    + case 4:
    + breakinfo[i].len = X86_BREAKPOINT_LEN_4;
    + break;
    +#ifdef CONFIG_X86_64
    + case 8:
    + breakinfo[i].len = X86_BREAKPOINT_LEN_8;
    + break;
    +#endif
    + default:
    + return -1;
    + }
    breakinfo[i].addr = addr;
    - breakinfo[i].type = type;
    + breakinfo[i].enabled = 1;

    return 0;
    }
    @@ -313,8 +331,21 @@ kgdb_set_hw_break(unsigned long addr, in
    */
    void kgdb_disable_hw_debug(struct pt_regs *regs)
    {
    + int i;
    + int cpu = raw_smp_processor_id();
    + struct perf_event *bp;
    +
    /* Disable hardware debugging while we are in kgdb: */
    set_debugreg(0UL, 7);
    + for (i = 0; i < 4; i++) {
    + if (!breakinfo[i].enabled)
    + continue;
    + bp = *per_cpu_ptr(breakinfo[i].pev, cpu);
    + if (bp->attr.disabled == 1)
    + continue;
    + arch_uninstall_hw_breakpoint(bp);
    + bp->attr.disabled = 1;
    + }
    }

    /**
    @@ -378,7 +409,6 @@ int kgdb_arch_handle_exception(int e_vec
    struct pt_regs *linux_regs)
    {
    unsigned long addr;
    - unsigned long dr6;
    char *ptr;
    int newPC;

    @@ -404,20 +434,6 @@ int kgdb_arch_handle_exception(int e_vec
    raw_smp_processor_id());
    }

    - get_debugreg(dr6, 6);
    - if (!(dr6 & 0x4000)) {
    - int breakno;
    -
    - for (breakno = 0; breakno < 4; breakno++) {
    - if (dr6 & (1 << breakno) &&
    - breakinfo[breakno].type == 0) {
    - /* Set restore flag: */
    - linux_regs->flags |= X86_EFLAGS_RF;
    - break;
    - }
    - }
    - }
    - set_debugreg(0UL, 6);
    kgdb_correct_hw_break();

    return 0;
    @@ -485,8 +501,7 @@ static int __kgdb_notify(struct die_args
    break;

    case DIE_DEBUG:
    - if (atomic_read(&kgdb_cpu_doing_single_step) ==
    - raw_smp_processor_id()) {
    + if (atomic_read(&kgdb_cpu_doing_single_step) != -1) {
    if (user_mode(regs))
    return single_step_cont(regs, args);
    break;
    @@ -539,7 +554,42 @@ static struct notifier_block kgdb_notifi
    */
    int kgdb_arch_init(void)
    {
    - return register_die_notifier(&kgdb_notifier);
    + int i, cpu;
    + int ret;
    + struct perf_event_attr attr;
    + struct perf_event **pevent;
    +
    + ret = register_die_notifier(&kgdb_notifier);
    + if (ret != 0)
    + return ret;
    + /*
    + * Pre-allocate the hw breakpoint structions in the non-atomic
    + * portion of kgdb because this operation requires mutexs to
    + * complete.
    + */
    + attr.bp_addr = (unsigned long)kgdb_arch_init;
    + attr.type = PERF_TYPE_BREAKPOINT;
    + attr.bp_len = HW_BREAKPOINT_LEN_1;
    + attr.bp_type = HW_BREAKPOINT_W;
    + attr.disabled = 1;
    + for (i = 0; i < 4; i++) {
    + breakinfo[i].pev = register_wide_hw_breakpoint(&attr, NULL);
    + if (IS_ERR(breakinfo[i].pev)) {
    + printk(KERN_ERR "kgdb: Could not allocate hw breakpoints\n");
    + breakinfo[i].pev = NULL;
    + kgdb_arch_exit();
    + return -1;
    + }
    + for_each_online_cpu(cpu) {
    + pevent = per_cpu_ptr(breakinfo[i].pev, cpu);
    + pevent[0]->hw.sample_period = 1;
    + if (pevent[0]->destroy != NULL) {
    + pevent[0]->destroy = NULL;
    + release_bp_slot(*pevent);
    + }
    + }
    + }
    + return ret;
    }

    /**
    @@ -550,6 +600,13 @@ int kgdb_arch_init(void)
    */
    void kgdb_arch_exit(void)
    {
    + int i;
    + for (i = 0; i < 4; i++) {
    + if (breakinfo[i].pev) {
    + unregister_wide_hw_breakpoint(breakinfo[i].pev);
    + breakinfo[i].pev = NULL;
    + }
    + }
    unregister_die_notifier(&kgdb_notifier);
    }

    --- a/kernel/kgdb.c
    +++ b/kernel/kgdb.c
    @@ -578,6 +578,9 @@ static void kgdb_wait(struct pt_regs *re
    smp_wmb();
    atomic_set(&cpu_in_kgdb[cpu], 1);

    + /* Disable any cpu specific hw breakpoints */
    + kgdb_disable_hw_debug(regs);
    +
    /* Wait till primary CPU is done with debugging */
    while (atomic_read(&passive_cpu_wait[cpu]))
    cpu_relax();


    \
     
     \ /
      Last update: 2010-01-28 21:03    [W:0.037 / U:0.432 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site