On Fri, 20 Jun 2008 17:14:53 -0300
Luis Machado <[EMAIL PROTECTED]> wrote:
>
> Follows a re-worked patch that unifies the handling of hardware
> watchpoint structures for DABR-based and DAC-based processors.
>
> The flow is exactly the same for DABR-based processors.
>
> As for the DAC-based code, i've eliminated the "dac" field from the
> thread structure, since now we use the "dabr" field to represent DAC1 of
> the DAC-based processors. As a consequence, i removed all the
> occurrences of "dac" throughout the code, using "dabr" in those cases.
>
> The function "set_dabr" sets the correct register (DABR OR DAC) for each
> specific processor now, instead of only setting the value for DABR-based
> processors.
>
> "do_dac" was merged with "do_dabr" as they were mostly equivalent pieces
> of code. I've moved "do_dabr" to "arch/powerpc/kernel/process.c" so it
> is visible for DAC-based code as well.
>
> Tested on a Taishan 440GX with success.
>
> Is it OK to leave it as "dabr" for both DABR and DAC? What do you think
> of the patch overall?
Aside from the comment I mention below (which really does need fixing),
the overall look of the patch seems good.
> Index: linux-2.6.25-oldpatch/arch/powerpc/kernel/process.c
> ===================================================================
> --- linux-2.6.25-oldpatch.orig/arch/powerpc/kernel/process.c 2008-06-20
> 02:48:19.000000000 -0700
> +++ linux-2.6.25-oldpatch/arch/powerpc/kernel/process.c 2008-06-20
> 02:51:16.000000000 -0700
> @@ -241,6 +241,35 @@
> }
> #endif /* CONFIG_SMP */
>
> +void do_dabr(struct pt_regs *regs, unsigned long address,
> + unsigned long error_code)
> +{
> + siginfo_t info;
> +
> +#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
> + if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
> + 11, SIGSEGV) == NOTIFY_STOP)
> + return;
> +
> + if (debugger_dabr_match(regs))
> + return;
> +#else
> + /* Clear the DAC and struct entries. One shot trigger. */
> + mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
> + | DBCR0_IDM));
This doesn't look right for how it's coded. This would be the
CONFIG_4xx || CONFIG_BOOKE case, but CONFIG_4xx includes PowerPC 405.
That has a different bit layout among the DBCR registers. Namely, on
405 you would be clearing the TDE and IAC1 events because the DAC
events are in DBCR1, not DBCR0.
<snip>
> Index: linux-2.6.25-oldpatch/arch/powerpc/kernel/signal.c
> ===================================================================
> --- linux-2.6.25-oldpatch.orig/arch/powerpc/kernel/signal.c 2008-06-20
> 02:48:19.000000000 -0700
> +++ linux-2.6.25-oldpatch/arch/powerpc/kernel/signal.c 2008-06-20
> 02:51:16.000000000 -0700
> @@ -144,8 +144,12 @@
> * user space. The DABR will have been cleared if it
> * triggered inside the kernel.
> */
> - if (current->thread.dabr)
> + if (current->thread.dabr) {
> set_dabr(current->thread.dabr);
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> + mtspr(SPRN_DBCR0, current->thread.dbcr0);
> +#endif
This has the same problem I mention above, as the 44x bit layout is
stored in dbcr0 throughout the code.
> + }
>
> if (is32) {
> if (ka.sa.sa_flags & SA_SIGINFO)
> Index: linux-2.6.25-oldpatch/arch/powerpc/kernel/traps.c
> ===================================================================
> --- linux-2.6.25-oldpatch.orig/arch/powerpc/kernel/traps.c 2008-06-20
> 02:48:19.000000000 -0700
> +++ linux-2.6.25-oldpatch/arch/powerpc/kernel/traps.c 2008-06-20
> 02:54:37.000000000 -0700
> @@ -1045,6 +1045,21 @@
> return;
> }
> _exception(SIGTRAP, regs, TRAP_TRACE, 0);
> + } else if (debug_status & (DBSR_DAC1R | DBSR_DAC1W)) {
> + regs->msr &= ~MSR_DE;
> + printk("\nWatchpoint Triggered\n");
> + if (user_mode(regs)) {
> + current->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W |
> + DBCR0_IDM);
> + } else {
> + /* Disable DAC interupts. */
> + mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R |
> + DBSR_DAC1W |
> DBCR0_IDM));
> + /* Clear the DAC event. */
> + mtspr(SPRN_DBSR, (DBSR_DAC1R | DBSR_DAC1W));
And again here.
josh
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
