RE: [PATCH v6 1/3] target/riscv: Add functions for common matching conditions of trigger

[張哲嘉]

> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Thursday, June 27, 2024 10:18 AM
> To: Alvin Che-Chia Chang(張哲嘉) <alvinga@andestech.com>
> Cc: qemu-riscv@nongnu.org; qemu-devel@nongnu.org;
> alistair.francis@wdc.com; bin.meng@windriver.com; liwei1518@gmail.com;
> dbarboza@ventanamicro.com; zhiwei_liu@linux.alibaba.com
> Subject: Re: [PATCH v6 1/3] target/riscv: Add functions for common matching
> conditions of trigger
>
> [EXTERNAL MAIL]
>
> On Wed, Jun 26, 2024 at 11:25 PM Alvin Chang via <qemu-devel@nongnu.org>
> wrote:
> >
> > From: Alvin Chang via <qemu-devel@nongnu.org>
>
> Something is still strange with your From

Hmm... It doesn't work, even if I input "git send-email --from 
alvinga@andestech.com"
I am checking it with my teammates.

Alvin

>
> Alistair
>
> >
> > According to RISC-V Debug specification version 0.13 [1] (also applied
> > to version 1.0 [2] but it has not been ratified yet), there are
> > several common matching conditions before firing a trigger, including
> > the enabled privilege levels of the trigger.
> >
> > This commit adds trigger_common_match() to prepare the common
> matching
> > conditions for the type 2/3/6 triggers. For now, we just implement
> > trigger_priv_match() to check if the enabled privilege levels of the
> > trigger match CPU's current privilege level.
> >
> > Remove the related code in riscv_cpu_debug_check_breakpoint() and
> > invoke
> > trigger_common_match() to check the privilege levels of the type 2 and
> > type 6 triggers for the breakpoints.
> >
> > This commit also changes the behavior of looping the triggers. In
> > previous implementation, if we have a type 2 trigger and
> > env->virt_enabled is true, we directly return false to stop the loop.
> > Now we keep looping all the triggers until we find a matched trigger.
> >
> > Only the execution bit and the executed PC should be futher checked in
> > riscv_cpu_debug_check_breakpoint().
> >
> > [1]:
> > https://github.com/riscv/riscv-debug-spec/releases/tag/task_group_vote
> > [2]:
> > https://github.com/riscv/riscv-debug-spec/releases/tag/1.0.0-rc1-ascii
> > doc
> >
> > Signed-off-by: Alvin Chang <alvinga@andestech.com>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/debug.c | 101
> > +++++++++++++++++++++++++++++++++----------
> >  1 file changed, 78 insertions(+), 23 deletions(-)
> >
> > diff --git a/target/riscv/debug.c b/target/riscv/debug.c index
> > b110370e..11125f33 100644
> > --- a/target/riscv/debug.c
> > +++ b/target/riscv/debug.c
> > @@ -241,6 +241,76 @@ static void do_trigger_action(CPURISCVState *env,
> target_ulong trigger_index)
> >      }
> >  }
> >
> > +/*
> > + * Check the privilege level of specific trigger matches CPU's
> > +current privilege
> > + * level.
> > + */
> > +static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
> > +                               int trigger_index) {
> > +    target_ulong ctrl = env->tdata1[trigger_index];
> > +
> > +    switch (type) {
> > +    case TRIGGER_TYPE_AD_MATCH:
> > +        /* type 2 trigger cannot be fired in VU/VS mode */
> > +        if (env->virt_enabled) {
> > +            return false;
> > +        }
> > +        /* check U/S/M bit against current privilege level */
> > +        if ((ctrl >> 3) & BIT(env->priv)) {
> > +            return true;
> > +        }
> > +        break;
> > +    case TRIGGER_TYPE_AD_MATCH6:
> > +        if (env->virt_enabled) {
> > +            /* check VU/VS bit against current privilege level */
> > +            if ((ctrl >> 23) & BIT(env->priv)) {
> > +                return true;
> > +            }
> > +        } else {
> > +            /* check U/S/M bit against current privilege level */
> > +            if ((ctrl >> 3) & BIT(env->priv)) {
> > +                return true;
> > +            }
> > +        }
> > +        break;
> > +    case TRIGGER_TYPE_INST_CNT:
> > +        if (env->virt_enabled) {
> > +            /* check VU/VS bit against current privilege level */
> > +            if ((ctrl >> 25) & BIT(env->priv)) {
> > +                return true;
> > +            }
> > +        } else {
> > +            /* check U/S/M bit against current privilege level */
> > +            if ((ctrl >> 6) & BIT(env->priv)) {
> > +                return true;
> > +            }
> > +        }
> > +        break;
> > +    case TRIGGER_TYPE_INT:
> > +    case TRIGGER_TYPE_EXCP:
> > +    case TRIGGER_TYPE_EXT_SRC:
> > +        qemu_log_mask(LOG_UNIMP, "trigger type: %d is not
> supported\n", type);
> > +        break;
> > +    case TRIGGER_TYPE_NO_EXIST:
> > +    case TRIGGER_TYPE_UNAVAIL:
> > +        qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not
> exist\n",
> > +                      type);
> > +        break;
> > +    default:
> > +        g_assert_not_reached();
> > +    }
> > +
> > +    return false;
> > +}
> > +
> > +/* Common matching conditions for all types of the triggers. */
> > +static bool trigger_common_match(CPURISCVState *env, trigger_type_t
> type,
> > +                                 int trigger_index) {
> > +    return trigger_priv_match(env, type, trigger_index); }
> > +
> >  /* type 2 trigger */
> >
> >  static uint32_t type2_breakpoint_size(CPURISCVState *env,
> > target_ulong ctrl) @@ -785,22 +855,18 @@ bool
> riscv_cpu_debug_check_breakpoint(CPUState *cs)
> >          for (i = 0; i < RV_MAX_TRIGGERS; i++) {
> >              trigger_type = get_trigger_type(env, i);
> >
> > +            if (!trigger_common_match(env, trigger_type, i)) {
> > +                continue;
> > +            }
> > +
> >              switch (trigger_type) {
> >              case TRIGGER_TYPE_AD_MATCH:
> > -                /* type 2 trigger cannot be fired in VU/VS mode */
> > -                if (env->virt_enabled) {
> > -                    return false;
> > -                }
> > -
> >                  ctrl = env->tdata1[i];
> >                  pc = env->tdata2[i];
> >
> >                  if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
> > -                    /* check U/S/M bit against current privilege level
> */
> > -                    if ((ctrl >> 3) & BIT(env->priv)) {
> > -                        env->badaddr = pc;
> > -                        return true;
> > -                    }
> > +                    env->badaddr = pc;
> > +                    return true;
> >                  }
> >                  break;
> >              case TRIGGER_TYPE_AD_MATCH6:
> > @@ -808,19 +874,8 @@ bool riscv_cpu_debug_check_breakpoint(CPUState
> *cs)
> >                  pc = env->tdata2[i];
> >
> >                  if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
> > -                    if (env->virt_enabled) {
> > -                        /* check VU/VS bit against current privilege
> level */
> > -                        if ((ctrl >> 23) & BIT(env->priv)) {
> > -                            env->badaddr = pc;
> > -                            return true;
> > -                        }
> > -                    } else {
> > -                        /* check U/S/M bit against current privilege
> level */
> > -                        if ((ctrl >> 3) & BIT(env->priv)) {
> > -                            env->badaddr = pc;
> > -                            return true;
> > -                        }
> > -                    }
> > +                    env->badaddr = pc;
> > +                    return true;
> >                  }
> >                  break;
> >              default:
> > --
> > 2.34.1
> >
> >
CONFIDENTIALITY NOTICE:

This e-mail (and its attachments) may contain confidential and legally 
privileged information or information protected from disclosure. If you are not 
the intended recipient, you are hereby notified that any disclosure, copying, 
distribution, or use of the information contained herein is strictly 
prohibited. In this case, please immediately notify the sender by return 
e-mail, delete the message (and any accompanying documents) and destroy all 
printed hard copies. Thank you for your cooperation.

Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.