| Message ID | 1382094469-7971-4-git-send-email-vijay.kilari@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 18, 2013 at 12:07:48PM +0100, vijay.kilari@gmail.com wrote: > From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> > > Add KGDB software step debugging support for EL1 debug > in AArch64 mode. > > KGDB registers step debug handler with debug monitor. > On receiving 'step' command from GDB tool, target enables > software step debugging and step address is updated. > If no step address is received from GDB tool, target assumes > next step address is current PC. > > Software Step debugging is disabled when 'continue' command > is received Reviewed-by: Will Deacon <will.deacon@arm.com> Will
Hi, On 10/18/2013 08:07 PM, vijay.kilari@gmail.com wrote: > From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> > > Add KGDB software step debugging support for EL1 debug > in AArch64 mode. > > diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c > index 50cef79..2b0b987 100644 > --- a/arch/arm64/kernel/kgdb.c > +++ b/arch/arm64/kernel/kgdb.c > ... > int kgdb_arch_handle_exception(int exception_vector, int signo, > int err_code, char *remcom_in_buffer, > char *remcom_out_buffer, > struct pt_regs *linux_regs) > { > ... > + case 's': > + /* > + * Update step address value with address passed > + * with step packet. > + * On debug exception return PC is copied to ELR > + * So just update PC. > + * If no step address is passed, resume from the address > + * pointed by PC. Do not update PC > + */ > + kgdb_arch_update_addr(linux_regs, remcom_in_buffer); > > + /* > + * Enable single step handling > + */ > + if (!kernel_active_single_step()) > + kernel_enable_single_step(linux_regs); > err = 0; > break; > default: Other architectures handle the state, by modifying kgdb_single_step or kgdb_cpu_doing_single_step, when entering into or exiting from single step mode. Do you have a good reason that you don't need to do so on arm64? (I'm just asking because my own kgdb patch follows the way that x86 does.) -Takahiro AKASHI
On Thu, Oct 31, 2013 at 8:05 AM, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > Hi, > > > On 10/18/2013 08:07 PM, vijay.kilari@gmail.com wrote: >> >> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com> >> >> Add KGDB software step debugging support for EL1 debug >> in AArch64 mode. >> >> diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c >> index 50cef79..2b0b987 100644 >> --- a/arch/arm64/kernel/kgdb.c >> +++ b/arch/arm64/kernel/kgdb.c > >> ... > >> int kgdb_arch_handle_exception(int exception_vector, int signo, >> int err_code, char *remcom_in_buffer, >> char *remcom_out_buffer, >> struct pt_regs *linux_regs) >> { > >> ... > >> + case 's': >> + /* >> + * Update step address value with address passed >> + * with step packet. >> + * On debug exception return PC is copied to ELR >> + * So just update PC. >> + * If no step address is passed, resume from the address >> + * pointed by PC. Do not update PC >> + */ >> + kgdb_arch_update_addr(linux_regs, remcom_in_buffer); >> >> + /* >> + * Enable single step handling >> + */ >> + if (!kernel_active_single_step()) >> + kernel_enable_single_step(linux_regs); >> err = 0; >> break; >> default: > > > Other architectures handle the state, by modifying kgdb_single_step or > kgdb_cpu_doing_single_step, when entering into or exiting from single > step mode. > > Do you have a good reason that you don't need to do so on arm64? > (I'm just asking because my own kgdb patch follows the way that x86 does.) > The 'kgdb_cpu_doing_single_step' is set with the cpu on which the step command is issued for that pid. So when an debug exception occurs, the debug handler (kdgb_cpu_enter) checks if cpu on which step debug was enabled acquires master lock and step debugging is performed on the same cpu for the pid In case of arm64, the step exception is always taken on the cpu on which the step debugging is enabled and step debugging is disabled on continue command. In case of kgdb_single_step, if this is not set, the slave locks (cpu's which has not taken debug exception) are released for every step command and acquired on next step exception and slave cpu's are again set into tight loop by sending smp request/nmi. When set, the slave locks are not released untill step debugging ends. So it avoids acquiring slave lock for every step debug exception This improvement can be taken for arm64. I will re-send new version of patches But I don't see this is taken care in armv7 > -Takahiro AKASHI
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index 50cef79..2b0b987 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -137,13 +137,26 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) static int compiled_break; +static void kgdb_arch_update_addr(struct pt_regs *regs, + char *remcom_in_buffer) +{ + unsigned long addr; + char *ptr; + + ptr = &remcom_in_buffer[1]; + if (kgdb_hex2long(&ptr, &addr)) + kgdb_arch_set_pc(regs, addr); + else if (compiled_break == 1) + kgdb_arch_set_pc(regs, regs->pc + 4); + + compiled_break = 0; +} + int kgdb_arch_handle_exception(int exception_vector, int signo, int err_code, char *remcom_in_buffer, char *remcom_out_buffer, struct pt_regs *linux_regs) { - unsigned long addr; - char *ptr; int err; switch (remcom_in_buffer[0]) { @@ -158,14 +171,31 @@ int kgdb_arch_handle_exception(int exception_vector, int signo, * to the next instruction or we will just breakpoint * over and over again. */ - ptr = &remcom_in_buffer[1]; - if (kgdb_hex2long(&ptr, &addr)) - kgdb_arch_set_pc(linux_regs, addr); - else if (compiled_break == 1) - kgdb_arch_set_pc(linux_regs, linux_regs->pc + 4); + kgdb_arch_update_addr(linux_regs, remcom_in_buffer); - compiled_break = 0; + /* + * Received continue command, disable single step + */ + if (kernel_active_single_step()) + kernel_disable_single_step(); + err = 0; + break; + case 's': + /* + * Update step address value with address passed + * with step packet. + * On debug exception return PC is copied to ELR + * So just update PC. + * If no step address is passed, resume from the address + * pointed by PC. Do not update PC + */ + kgdb_arch_update_addr(linux_regs, remcom_in_buffer); + /* + * Enable single step handling + */ + if (!kernel_active_single_step()) + kernel_enable_single_step(linux_regs); err = 0; break; default: @@ -188,6 +218,12 @@ static int kgdb_compiled_brk_fn(struct pt_regs *regs, unsigned int esr) return 0; } +static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr) +{ + kgdb_handle_exception(1, SIGTRAP, 0, regs); + return 0; +} + static struct break_hook kgdb_brkpt_hook = { .esr_mask = 0xffffffff, .esr_val = DBG_ESR_VAL_BRK(KGDB_DYN_DGB_BRK_IMM), @@ -200,6 +236,10 @@ static struct break_hook kgdb_compiled_brkpt_hook = { .fn = kgdb_compiled_brk_fn }; +static struct step_hook kgdb_step_hook = { + .fn = kgdb_step_brk_fn +}; + static void kgdb_call_nmi_hook(void *ignored) { kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); @@ -253,6 +293,7 @@ int kgdb_arch_init(void) register_break_hook(&kgdb_brkpt_hook); register_break_hook(&kgdb_compiled_brkpt_hook); + register_step_hook(&kgdb_step_hook); return 0; } @@ -266,6 +307,7 @@ void kgdb_arch_exit(void) { unregister_break_hook(&kgdb_brkpt_hook); unregister_break_hook(&kgdb_compiled_brkpt_hook); + unregister_step_hook(&kgdb_step_hook); unregister_die_notifier(&kgdb_notifier); }