[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dabb2823-783a-6a3f-4f04-f3200a1086fc@c-s.fr>
Date: Tue, 17 Mar 2020 12:14:10 +0100
From: Christophe Leroy <christophe.leroy@....fr>
To: Ravi Bangoria <ravi.bangoria@...ux.ibm.com>, mpe@...erman.id.au,
mikey@...ling.org
Cc: apopple@...ux.ibm.com, paulus@...ba.org, npiggin@...il.com,
naveen.n.rao@...ux.vnet.ibm.com, peterz@...radead.org,
jolsa@...nel.org, oleg@...hat.com, fweisbec@...il.com,
mingo@...nel.org, linuxppc-dev@...ts.ozlabs.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 15/15] powerpc/watchpoint/xmon: Support 2nd dawr
Le 09/03/2020 à 09:58, Ravi Bangoria a écrit :
> Add support for 2nd DAWR in xmon. With this, we can have two
> simultaneous breakpoints from xmon.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
> ---
> arch/powerpc/xmon/xmon.c | 101 ++++++++++++++++++++++++++-------------
> 1 file changed, 69 insertions(+), 32 deletions(-)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index ac18fe3e4295..20adc83404c8 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -110,7 +110,7 @@ struct bpt {
>
> #define NBPTS 256
> static struct bpt bpts[NBPTS];
> -static struct bpt dabr;
> +static struct bpt dabr[HBP_NUM_MAX];
> static struct bpt *iabr;
> static unsigned bpinstr = 0x7fe00008; /* trap */
>
> @@ -786,10 +786,17 @@ static int xmon_sstep(struct pt_regs *regs)
>
> static int xmon_break_match(struct pt_regs *regs)
> {
> + int i;
> +
> if ((regs->msr & (MSR_IR|MSR_PR|MSR_64BIT)) != (MSR_IR|MSR_64BIT))
> return 0;
> - if (dabr.enabled == 0)
> - return 0;
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (dabr[i].enabled)
> + goto found;
> + }
> + return 0;
> +
> +found:
> xmon_core(regs, 0);
> return 1;
> }
> @@ -928,13 +935,16 @@ static void insert_bpts(void)
>
> static void insert_cpu_bpts(void)
> {
> + int i;
> struct arch_hw_breakpoint brk;
>
> - if (dabr.enabled) {
> - brk.address = dabr.address;
> - brk.type = (dabr.enabled & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL;
> - brk.len = DABR_MAX_LEN;
> - __set_breakpoint(&brk, 0);
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (dabr[i].enabled) {
> + brk.address = dabr[i].address;
> + brk.type = (dabr[i].enabled & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL;
> + brk.len = 8;
> + __set_breakpoint(&brk, i);
> + }
> }
>
> if (iabr)
> @@ -1348,6 +1358,35 @@ static long check_bp_loc(unsigned long addr)
> return 1;
> }
>
> +static int free_data_bpt(void)
This names suggests the function frees a breakpoint.
I guess it should be find_free_data_bpt()
> +{
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (!dabr[i].enabled)
> + return i;
> + }
> + printf("Couldn't find free breakpoint register\n");
> + return -1;
> +}
> +
> +static void print_data_bpts(void)
> +{
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (!dabr[i].enabled)
> + continue;
> +
> + printf(" data "REG" [", dabr[i].address);
> + if (dabr[i].enabled & 1)
> + printf("r");
> + if (dabr[i].enabled & 2)
> + printf("w");
> + printf("]\n");
> + }
> +}
> +
> static char *breakpoint_help_string =
> "Breakpoint command usage:\n"
> "b show breakpoints\n"
> @@ -1381,10 +1420,9 @@ bpt_cmds(void)
> printf("Hardware data breakpoint not supported on this cpu\n");
> break;
> }
> - if (dabr.enabled) {
> - printf("Couldn't find free breakpoint register\n");
> + i = free_data_bpt();
> + if (i < 0)
> break;
> - }
> mode = 7;
> cmd = inchar();
> if (cmd == 'r')
> @@ -1393,15 +1431,15 @@ bpt_cmds(void)
> mode = 6;
> else
> termch = cmd;
> - dabr.address = 0;
> - dabr.enabled = 0;
> - if (scanhex(&dabr.address)) {
> - if (!is_kernel_addr(dabr.address)) {
> + dabr[i].address = 0;
> + dabr[i].enabled = 0;
> + if (scanhex(&dabr[i].address)) {
> + if (!is_kernel_addr(dabr[i].address)) {
> printf(badaddr);
> break;
> }
> - dabr.address &= ~HW_BRK_TYPE_DABR;
> - dabr.enabled = mode | BP_DABR;
> + dabr[i].address &= ~HW_BRK_TYPE_DABR;
> + dabr[i].enabled = mode | BP_DABR;
> }
>
> force_enable_xmon();
> @@ -1440,7 +1478,9 @@ bpt_cmds(void)
> for (i = 0; i < NBPTS; ++i)
> bpts[i].enabled = 0;
> iabr = NULL;
> - dabr.enabled = 0;
> + for (i = 0; i < nr_wp_slots(); i++)
> + dabr[i].enabled = 0;
> +
> printf("All breakpoints cleared\n");
> break;
> }
> @@ -1474,14 +1514,7 @@ bpt_cmds(void)
> if (xmon_is_ro || !scanhex(&a)) {
> /* print all breakpoints */
> printf(" type address\n");
> - if (dabr.enabled) {
> - printf(" data "REG" [", dabr.address);
> - if (dabr.enabled & 1)
> - printf("r");
> - if (dabr.enabled & 2)
> - printf("w");
> - printf("]\n");
> - }
> + print_data_bpts();
> for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
> if (!bp->enabled)
> continue;
> @@ -1941,8 +1974,13 @@ static void dump_207_sprs(void)
>
> printf("hfscr = %.16lx dhdes = %.16lx rpr = %.16lx\n",
> mfspr(SPRN_HFSCR), mfspr(SPRN_DHDES), mfspr(SPRN_RPR));
> - printf("dawr = %.16lx dawrx = %.16lx ciabr = %.16lx\n",
> - mfspr(SPRN_DAWR0), mfspr(SPRN_DAWRX0), mfspr(SPRN_CIABR));
> + printf("dawr0 = %.16lx dawrx0 = %.16lx\n",
> + mfspr(SPRN_DAWR0), mfspr(SPRN_DAWRX0));
> + if (nr_wp_slots() > 1) {
> + printf("dawr1 = %.16lx dawrx1 = %.16lx\n",
> + mfspr(SPRN_DAWR1), mfspr(SPRN_DAWRX1));
> + }
> + printf("ciabr = %.16lx\n", mfspr(SPRN_CIABR));
> #endif
> }
>
> @@ -3862,10 +3900,9 @@ static void clear_all_bpt(void)
> bpts[i].enabled = 0;
>
> /* Clear any data or iabr breakpoints */
> - if (iabr || dabr.enabled) {
> - iabr = NULL;
> - dabr.enabled = 0;
> - }
> + iabr = NULL;
> + for (i = 0; i < nr_wp_slots(); i++)
> + dabr[i].enabled = 0;
> }
>
> #ifdef CONFIG_DEBUG_FS
>
Christophe
Powered by blists - more mailing lists