From: "Amit S. Kale" <amitkale@emsyssoft.com>
To: Tom Rini <trini@kernel.crashing.org>,
Kernel Mailing List <linux-kernel@vger.kernel.org>,
kgdb-bugreport@lists.sourceforge.net
Subject: Re: [PATCH][KGDB] Move i386 hw breakpoint bits into non-lite
Date: Tue, 30 Mar 2004 13:16:41 +0530 [thread overview]
Message-ID: <200403301316.42033.amitkale@emsyssoft.com> (raw)
In-Reply-To: <20040329163443.GG2895@smtp.west.cox.net>
hw-breakpoint code has been broken since kgdb 1.8. I have to get that working
some day.
Please checkin.
-Amit
On Monday 29 Mar 2004 10:04 pm, Tom Rini wrote:
> Hello. The following interdiffs accomplish the following:
> - core-lite: Add an API to clear all hw breakpoints, and clarify the
> logic of handling z[01] packets. Also rename set_break/remove_break
> to kgdb_set/remove_sw_break.
> - i386-lite: Remove all of the hw-breakpoint code, as it might only be
> working for 'y'/'Y' packets and most certainly not for anything else
> right now.
> - i386.patch: Consolidate both versions of the hw-breakpoint code into
> one set of functions. This is incomplete, and I'm not convinced that
> it worked before with a 'y' / 'Y' packet.
> - core.patch: Make this apply still and add the hw_breakpoint hooks to
> the documentation. I plan on re-organizing this a bit more, and will
> hopefully have another patch, against this later today. (Not posted
> since interdiff wasn't liking it).
>
> This has been tested on i386 + 8250, and I'd like to check this in
> noon -0700, 30 March.
>
> diff -u linux-2.6.4/include/linux/kgdb.h linux-2.6.4/include/linux/kgdb.h
> --- linux-2.6.4/include/linux/kgdb.h 2004-03-19 08:22:37.143170735 -0700
> +++ linux-2.6.4/include/linux/kgdb.h 2004-03-29 08:50:26.510296479 -0700
> @@ -84,8 +84,9 @@
> extern int kgdb_arch_handle_exception(int vector, int signo, int err_code,
> char *InBuffer, char *outBuffer,
> struct pt_regs *regs);
> -extern int kgdb_arch_set_break(unsigned long addr, int type);
> -extern int kgdb_arch_remove_break(unsigned long addr, int type);
> +extern int kgdb_set_hw_break(unsigned long addr);
> +extern int kgdb_remove_hw_break(unsigned long addr);
> +extern void kgdb_remove_all_hw_break(void);
> extern void kgdb_correct_hw_break(void);
> extern void kgdb_shadowinfo(struct pt_regs *regs, char *buffer,
> unsigned threadid);
> diff -u linux-2.6.4/kernel/kgdb.c linux-2.6.4/kernel/kgdb.c
> --- linux-2.6.4/kernel/kgdb.c 2004-03-19 08:22:37.147169789 -0700
> +++ linux-2.6.4/kernel/kgdb.c 2004-03-29 08:51:14.137620137 -0700
> @@ -157,18 +157,23 @@
> }
>
> int __attribute__ ((weak))
> - kgdb_arch_set_break(unsigned long addr, int type)
> + kgdb_set_hw_break(unsigned long addr)
> {
> return 0;
> }
>
> int __attribute__ ((weak))
> - kgdb_arch_remove_break(unsigned long addr, int type)
> + kgdb_remove_hw_break(unsigned long addr)
> {
> return 0;
> }
>
> void __attribute__ ((weak))
> + kgdb_remove_all_hw_break(void)
> +{
> +}
> +
> +void __attribute__ ((weak))
> kgdb_correct_hw_break(void)
> {
> }
> @@ -206,7 +211,6 @@
> {
> unsigned char checksum;
> unsigned char xmitcsum;
> - int i;
> int count;
> char ch;
>
> @@ -540,7 +544,7 @@
> return 0;
> }
>
> -static int set_break(unsigned long addr)
> +static int kgdb_set_sw_break(unsigned long addr)
> {
> int i, breakno = -1;
> int error;
> @@ -576,7 +580,7 @@
> return 0;
> }
>
> -static int remove_break(unsigned long addr)
> +static int kgdb_remove_sw_break(unsigned long addr)
> {
> int i;
> int error;
> @@ -602,6 +606,8 @@
> {
> int i;
> int error;
> +
> + /* Clear memory breakpoints. */
> for (i = 0; i < MAX_BREAKPOINTS; i++) {
> if (kgdb_break[i].state == bp_enabled) {
> unsigned long addr = kgdb_break[i].bpt_addr;
> @@ -615,6 +621,10 @@
> }
> kgdb_break[i].state = bp_disabled;
> }
> +
> + /* Clear hardware breakpoints. */
> + kgdb_remove_all_hw_break();
> +
> return 0;
> }
>
> @@ -710,7 +720,7 @@
> kgdb_usethreadid = shadow_pid(current->pid);
>
> while (1) {
> - int bpt_type = 0;
> + char *bpt_type;
> error = 0;
>
> /* Clear the out buffer. */
> @@ -964,41 +974,39 @@
> else
> error_packet(remcom_out_buffer, -EINVAL);
> break;
> + /* Since GDB-5.3, it's been drafted that '0' is a software
> + * breakpoint, '1' is a hardware breakpoint, so let's do
> + * that.
> + */
> case 'z':
> case 'Z':
> + bpt_type = &remcom_in_buffer[1];
> ptr = &remcom_in_buffer[2];
> +
> + if (*bpt_type != '0' && *bpt_type != '1')
> + /* Unsupported. */
> + break;
> + /* Test if this is a hardware breakpoint, and
> + * if we support it. */
> + if (*bpt_type == '1' && !(kgdb_ops->flags &
> + KGDB_HW_BREAKPOINT))
> + /* Unsupported. */
> + break;
> +
> if (*(ptr++) != ',') {
> error_packet(remcom_out_buffer, -EINVAL);
> break;
> }
> kgdb_hex2long(&ptr, &addr);
>
> - bpt_type = remcom_in_buffer[1];
> - if (bpt_type != bp_breakpoint) {
> - if (bpt_type == bp_hardware_breakpoint &&
> - !(kgdb_ops->flags & KGDB_HW_BREAKPOINT))
> - break;
> -
> - /* if set_break is not defined, then
> - * remove_break does not matter
> - */
> - if (!(kgdb_ops->flags & KGDB_HW_BREAKPOINT))
> - break;
> - }
> -
> - if (remcom_in_buffer[0] == 'Z') {
> - if (bpt_type == bp_breakpoint)
> - error = set_break(addr);
> - else
> - error = kgdb_arch_set_break(addr,
> - bpt_type);
> - } else {
> - if (bpt_type == bp_breakpoint)
> - error = remove_break(addr);
> - else
> - error = kgdb_arch_remove_break(addr,
> - bpt_type);
> - }
> + if (remcom_in_buffer[0] == 'Z' && *bpt_type == '0')
> + error = kgdb_set_sw_break(addr);
> + else if (remcom_in_buffer[0] == 'Z' && *bpt_type == '1')
> + error = kgdb_set_hw_break(addr);
> + else if (remcom_in_buffer[0] == 'z' && *bpt_type == '0')
> + error = kgdb_remove_sw_break(addr);
> + else if (remcom_in_buffer[0] == 'z' && *bpt_type == '1')
> + error = kgdb_remove_hw_break(addr);
>
> if (error == 0)
> strcpy(remcom_out_buffer, "OK");
>
> diff -u linux-2.6.4/arch/i386/kernel/kgdb.c
> linux-2.6.4/arch/i386/kernel/kgdb.c ---
> linux-2.6.4/arch/i386/kernel/kgdb.c 2004-03-19 08:29:47.155575704 -0700 +++
> linux-2.6.4/arch/i386/kernel/kgdb.c 2004-03-29 08:55:53.752914399 -0700 @@
> -124,11 +124,12 @@
> unsigned type;
> unsigned len;
> unsigned addr;
> -} breakinfo[4] = { {
> -enabled:0}, {
> -enabled:0}, {
> -enabled:0}, {
> -enabled:0}};
> +} breakinfo[4] = {
> + { .enabled = 0 },
> + { .enabled = 0 },
> + { .enabled = 0 },
> + { .enabled = 0 },
> +};
>
> void kgdb_correct_hw_break(void)
> {
> @@ -189,62 +190,6 @@
> }
> }
>
> -int kgdb_remove_hw_break(unsigned long addr, int type)
> -{
> - int i, idx = -1;
> - for (i = 0; i < 4; i++) {
> - if (breakinfo[i].addr == addr && breakinfo[i].enabled) {
> - idx = i;
> - break;
> - }
> - }
> - if (idx == -1)
> - return -1;
> -
> - breakinfo[idx].enabled = 0;
> - return 0;
> -}
> -
> -int kgdb_set_hw_break(unsigned long addr, int type)
> -{
> - int i, idx = -1;
> - for (i = 0; i < 4; i++) {
> - if (!breakinfo[i].enabled) {
> - idx = i;
> - break;
> - }
> - }
> - if (idx == -1)
> - return -1;
> -
> - breakinfo[idx].enabled = 1;
> - breakinfo[idx].type = type;
> - breakinfo[idx].len = 1;
> - breakinfo[idx].addr = addr;
> - return 0;
> -}
> -
> -int remove_hw_break(unsigned breakno)
> -{
> - if (!breakinfo[breakno].enabled) {
> - return -1;
> - }
> - breakinfo[breakno].enabled = 0;
> - return 0;
> -}
> -
> -int set_hw_break(unsigned breakno, unsigned type, unsigned len, unsigned
> addr) -{
> - if (breakinfo[breakno].enabled) {
> - return -1;
> - }
> - breakinfo[breakno].enabled = 1;
> - breakinfo[breakno].type = type;
> - breakinfo[breakno].len = len;
> - breakinfo[breakno].addr = addr;
> - return 0;
> -}
> -
> void kgdb_printexceptioninfo(int exceptionNo, int errorcode, char *buffer)
> {
> unsigned dr6;
> @@ -293,8 +238,8 @@
> char *remcom_out_buffer,
> struct pt_regs *linux_regs)
> {
> - long addr, length;
> - long breakno, breaktype;
> + long addr;
> + long breakno;
> char *ptr;
> int newPC;
> int dr6;
> @@ -343,38 +288,8 @@
> return (0);
> -
> - case 'Y':
> - ptr = &remcom_in_buffer[1];
> - kgdb_hex2long(&ptr, &breakno);
> - ptr++;
> - kgdb_hex2long(&ptr, &breaktype);
> - ptr++;
> - kgdb_hex2long(&ptr, &length);
> - ptr++;
> - kgdb_hex2long(&ptr, &addr);
> - if (set_hw_break(breakno & 0x3, breaktype & 0x3,
> - length & 0x3, addr) == 0) {
> - strcpy(remcom_out_buffer, "OK");
> - } else {
> - strcpy(remcom_out_buffer, "ERROR");
> - }
> - break;
> -
> - /* Remove hardware breakpoint */
> - case 'y':
> - ptr = &remcom_in_buffer[1];
> - kgdb_hex2long(&ptr, &breakno);
> - if (remove_hw_break(breakno & 0x3) == 0) {
> - strcpy(remcom_out_buffer, "OK");
> - } else {
> - strcpy(remcom_out_buffer, "ERROR");
> - }
> - break;
> -
> } /* switch */
> return -1; /* this means that we do not want to exit from the handler */
> }
>
> struct kgdb_arch arch_kgdb_ops = {
> .gdb_bpt_instr = {0xcc},
> - .flags = KGDB_HW_BREAKPOINT,
> };
>
> --- linux-2.6.4.orig/arch/i386/kernel/kgdb.c 2004-03-29 08:55:53.752914399
> -0700 +++ linux-2.6.4/arch/i386/kernel/kgdb.c 2004-03-29 08:54:49.382357765
> -0700 @@ -190,6 +190,54 @@
> }
> }
>
> +int kgdb_remove_hw_break(unsigned long addr)
> +{
> + int i, idx = -1;
> + for (i = 0; i < 4; i++) {
> + if (breakinfo[i].addr == addr && breakinfo[i].enabled) {
> + idx = i;
> + break;
> + }
> + }
> + if (idx == -1)
> + return -1;
> +
> + breakinfo[idx].enabled = 0;
> + return 0;
> +}
> +
> +void kgdb_remove_all_hw_break(void)
> +{
> + int i;
> +
> + for (i = 0; i < 4; i++) {
> + if (breakinfo[i].enabled) {
> + /* Do what? */
> + ;
> + }
> + memset(&breakinfo[i], 0, sizeof(struct hw_breakpoint));
> + }
> +}
> +
> +int kgdb_set_hw_break(unsigned long addr)
> +{
> + int i, idx = -1;
> + for (i = 0; i < 4; i++) {
> + if (!breakinfo[i].enabled) {
> + idx = i;
> + break;
> + }
> + }
> + if (idx == -1)
> + return -1;
> +
> + breakinfo[idx].enabled = 1;
> + breakinfo[idx].type = 1;
> + breakinfo[idx].len = 1;
> + breakinfo[idx].addr = addr;
> + return 0;
> +}
> +
> void kgdb_printexceptioninfo(int exceptionNo, int errorcode, char *buffer)
> {
> unsigned dr6;
> @@ -292,4 +340,5 @@
>
> struct kgdb_arch arch_kgdb_ops = {
> .gdb_bpt_instr = {0xcc},
> + .flags = KGDB_HW_BREAKPOINT,
> };
prev parent reply other threads:[~2004-03-30 8:25 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-03-29 16:34 [PATCH][KGDB] Move i386 hw breakpoint bits into non-lite Tom Rini
2004-03-30 7:46 ` Amit S. Kale [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200403301316.42033.amitkale@emsyssoft.com \
--to=amitkale@emsyssoft.com \
--cc=kgdb-bugreport@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=trini@kernel.crashing.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox