Hello.

Jason Wessel wrote:

> I rolled up several patches along with my changes to fix HW breakpoints 
> on x86_64 and i386.   I also deprecated some of the #defines in favor of 
> using the kgdb_ops to control the HW breakpoints for other non IA archs 
> at the point that these are implemented.
> 
> Until such time that there is a common interface for kernel HW 
> breakpoints, if a user space application uses HW breakpoints the kernel 
> breakpoints will disappear.  I tested execution, data access and data 
> write breakpoints and all work with gdb 6.6.
> 
> If there are no objections, I am going to replace the i386.patch with 
> i386_hw_breakpoints.patch and x86_64_breakpoints.patch and merge the 
> core changes for arch breakpoints to core-lite.patch.
> 
> Signed-off-by: Jason Wessel <[EMAIL PROTECTED]>
> 
> 
> 
> ------------------------------------------------------------------------
> 
> ---
>  arch/i386/kernel/kgdb.c    |   92 +++++++++++++++++++++++++++++------
>  arch/x86_64/kernel/kgdb.c  |  117 
> +++++++++++++++++++++++++++++++++++++++++++--
>  include/asm-generic/kgdb.h |   40 ---------------
>  include/linux/kgdb.h       |    2 
>  kernel/kgdb.c              |   12 ++--
>  lib/Kconfig.kgdb           |    8 ---
>  6 files changed, 199 insertions(+), 72 deletions(-)
> 
> Index: linux-2.6.21.1/lib/Kconfig.kgdb
> ===================================================================
> --- linux-2.6.21.1.orig/lib/Kconfig.kgdb
> +++ linux-2.6.21.1/lib/Kconfig.kgdb
> @@ -21,14 +21,6 @@ config KGDB
>         at http://kgdb.sourceforge.net as well as in DocBook form
>         in Documentation/DocBook/.  If unsure, say N.
>  
> -config KGDB_ARCH_HAS_HARDWARE_BREAKPOINTS
> -     bool
> -     depends on KGDB
> -     default n
> -     help
> -       If you say Y here, KGDB will make use of hardware breakpoints
> -       rather than software breakpoints.  If unsure, say N.
> -
>  config KGDB_ARCH_HAS_SHADOW_INFO
>       bool
>  
> Index: linux-2.6.21.1/arch/i386/kernel/kgdb.c
> ===================================================================
> --- linux-2.6.21.1.orig/arch/i386/kernel/kgdb.c
> +++ linux-2.6.21.1/arch/i386/kernel/kgdb.c
[...]
> @@ -126,8 +128,53 @@ static struct hw_breakpoint {
>       { .enabled = 0 },
>  };
>  
> -#ifdef CONFIG_KGDB_ARCH_HAS_HARDWARE_BREAKPOINTS
> -int kgdb_remove_hw_break(unsigned long addr)
> +static void kgdb_correct_hw_break(void)
> +{
> +     int breakno;
> +     int correctit;
> +     int breakbit;
> +     unsigned long dr7;
> +
> +     get_debugreg(dr7, 7);
> +     correctit = 0;
> +     for (breakno = 0; breakno < 3; 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) << 16) <<
> +                         (breakno << 2);
> +                     switch (breakno) {
> +                     case 0:
> +                             set_debugreg(breakinfo[breakno].addr, 0);
> +                             break;
> +
> +                     case 1:
> +                             set_debugreg(breakinfo[breakno].addr, 1);
> +                             break;
> +
> +                     case 2:
> +                             set_debugreg(breakinfo[breakno].addr, 2);
> +                             break;
> +
> +                     case 3:
> +                             set_debugreg(breakinfo[breakno].addr, 3);
> +                             break;
> +                     }

    Erm, why not replace all this switch it in one line:

        set_debugreg(breakinfo[breakno].addr, breakno);

> Index: linux-2.6.21.1/arch/x86_64/kernel/kgdb.c
> ===================================================================
> --- linux-2.6.21.1.orig/arch/x86_64/kernel/kgdb.c
> +++ linux-2.6.21.1/arch/x86_64/kernel/kgdb.c
[...]
> @@ -136,10 +137,115 @@ enabled:0}, {
>  enabled:0}, {
>  enabled:0}};
>  
> +static void kgdb_correct_hw_break(void)
> +{
> +     int breakno;
> +     int correctit;
> +     int breakbit;
> +     unsigned long dr7;
> +
> +     get_debugreg(dr7, 7);
> +     correctit = 0;
> +     for (breakno = 0; breakno < 3; 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) << 16) <<
> +                         (breakno << 2);
> +                     switch (breakno) {
> +                     case 0:
> +                             set_debugreg(breakinfo[breakno].addr, 0);
> +                             break;
> +
> +                     case 1:
> +                             set_debugreg(breakinfo[breakno].addr, 1);
> +                             break;
> +
> +                     case 2:
> +                             set_debugreg(breakinfo[breakno].addr, 2);
> +                             break;
> +
> +                     case 3:
> +                             set_debugreg(breakinfo[breakno].addr, 3);
> +                             break;
> +                     }

    Same question here...

> Index: linux-2.6.21.1/include/asm-generic/kgdb.h
> ===================================================================
> --- linux-2.6.21.1.orig/include/asm-generic/kgdb.h
> +++ linux-2.6.21.1/include/asm-generic/kgdb.h
> @@ -60,46 +60,6 @@ extern void kgdb_disable_hw_debug(struct
>  #define kgdb_post_master_code(regs, v, c)    do { } while (0)
>  #endif
>  
> -#ifdef CONFIG_KGDB_ARCH_HAS_HARDWARE_BREAKPOINTS
> -/**
> - *   kgdb_set_hw_break - Set a hardware breakpoint at @addr.
> - *   @addr: The address to set a hardware breakpoint at.
> - */
> -extern int kgdb_set_hw_break(unsigned long addr);
> -
> -/**
> - *   kgdb_remove_hw_break - Remove a hardware breakpoint at @addr.
> - *   @addr: The address to remove a hardware breakpoint from.
> - */
> -extern int kgdb_remove_hw_break(unsigned long addr);
> -
> -/**
> - *   kgdb_remove_all_hw_break - Clear all hardware breakpoints.
> - */
> -extern void kgdb_remove_all_hw_break(void);
> -
> -/**
> - *   kgdb_correct_hw_break - Correct hardware breakpoints.
> - *
> - *   A hook to allow for changes to the hardware breakpoint, called
> - *   after a single step (s) or continue (c) packet, and once we're about
> - *   to let the kernel continue running.
> - *
> - *   This is used to set the hardware breakpoint registers for all the
> - *   slave cpus on an SMP configuration. This must be called after any
> - *   changes are made to the hardware breakpoints (such as by a single
> - *   step (s) or continue (c) packet. This is only required on
> - *   architectures that support SMP and every processor has its own set
> - *   of breakpoint registers.
> - */
> -extern void kgdb_correct_hw_break(void);
> -#else
> -#define kgdb_set_hw_break(addr)                      0
> -#define kgdb_remove_hw_break(addr)           0
> -#define kgdb_remove_all_hw_break()           do { } while (0)
> -#define kgdb_correct_hw_break()                      do { } while (0)
> -#endif
> -
> Index: linux-2.6.21.1/include/linux/kgdb.h
> ===================================================================
> --- linux-2.6.21.1.orig/include/linux/kgdb.h
> +++ linux-2.6.21.1/include/linux/kgdb.h
> @@ -220,6 +220,8 @@ struct kgdb_arch {
>       int (*remove_breakpoint)(unsigned long, char *);
>       int (*set_hw_breakpoint)(unsigned long, int, enum kgdb_bptype);
>       int (*remove_hw_breakpoint)(unsigned long, int, enum kgdb_bptype);
> +     void (*remove_all_hw_break)(void);
> +     void (*correct_hw_break)(void);
>  };

    I must note that the whole b/p mechanism has been a mess -- there are 
these {set|remove}_breakpoint() methods that never get called by KGDB and 
there are
overridable kgdb_arch_{set|remove}_breakpoint() functions which get actually 
called. The situation was with h/w breakpoints was even worse -- both direct 
and indirect calls were in use... :-/
    I'd suggest to remove those s/w breakpoint related indirect calls.

WBR, Sergei

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Kgdb-bugreport mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Reply via email to