Re: [PATCH 2/2] target/riscv: Add textra matching condition for the triggers

[Alistair Francis <alistair23@gmail.com>]

On Thu, Jul 4, 2024 at 2:03 PM Alvin Chang via <qemu-devel@nongnu.org> wrote:
>
> According to RISC-V Debug specification, the optional textra32 and
> textra64 trigger CSRs can be used to configure additional matching
> conditions for the triggers. For example, if the textra.MHSELECT field
> is set to 4 (mcontext), this trigger will only match or fire if the low
> bits of mcontext/hcontext equal textra.MHVALUE field.
>
> This commit adds the aforementioned matching condition as common trigger
> matching conditions. Currently, the only legal values of textra.MHSELECT
> are 0 (ignore) and 4 (mcontext). When textra.MHSELECT is 0, we pass the
> checking. When textra.MHSELECT is 4, we compare textra.MHVALUE with
> mcontext CSR. The remaining fields, such as textra.SBYTEMASK,
> textra.SVALUE, and textra.SSELECT, are hardwired to zero for now. Thus,
> we skip checking them here.
>
> Signed-off-by: Alvin Chang <alvinga@andestech.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/debug.c | 63 +++++++++++++++++++++++++++++++++++++++++++-
>  target/riscv/debug.h |  3 +++
>  2 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index f7d8f5e320..58dd2f25ae 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -367,11 +367,72 @@ static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
>      return false;
>  }
>
> +static bool trigger_textra_match(CPURISCVState *env, trigger_type_t type,
> +                                 int trigger_index)
> +{
> +    target_ulong textra = env->tdata3[trigger_index];
> +    target_ulong mhvalue, mhselect;
> +
> +    if (type < TRIGGER_TYPE_AD_MATCH || type > TRIGGER_TYPE_AD_MATCH6) {
> +        /* textra checking is only applicable when type is 2, 3, 4, 5, or 6 */
> +        return true;
> +    }
> +
> +    switch (riscv_cpu_mxl(env)) {
> +    case MXL_RV32:
> +        mhvalue  = get_field(textra, TEXTRA32_MHVALUE);
> +        mhselect = get_field(textra, TEXTRA32_MHSELECT);
> +        break;
> +    case MXL_RV64:
> +    case MXL_RV128:
> +        mhvalue  = get_field(textra, TEXTRA64_MHVALUE);
> +        mhselect = get_field(textra, TEXTRA64_MHSELECT);
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    /* Check mhvalue and mhselect. */
> +    switch (mhselect) {
> +    case MHSELECT_IGNORE:
> +        break;
> +    case MHSELECT_MCONTEXT:
> +        /* Match or fire if the low bits of mcontext/hcontext equal mhvalue. */
> +        if (riscv_has_ext(env, RVH)) {
> +            if (mhvalue != env->mcontext) {
> +                return false;
> +            }
> +        } else {
> +            switch (riscv_cpu_mxl(env)) {
> +            case MXL_RV32:
> +                if (mhvalue != (env->mcontext & MCONTEXT32)) {
> +                    return false;
> +                }
> +                break;
> +            case MXL_RV64:
> +            case MXL_RV128:
> +                if (mhvalue != (env->mcontext & MCONTEXT64)) {
> +                    return false;
> +                }
> +                break;
> +            default:
> +                g_assert_not_reached();
> +            }
> +        }
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    return true;
> +}
> +
>  /* 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);
> +    return trigger_priv_match(env, type, trigger_index) &&
> +           trigger_textra_match(env, type, trigger_index);
>  }
>
>  /* type 2 trigger */
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index c347863578..f76b8f944a 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -131,6 +131,9 @@ enum {
>  #define ITRIGGER_VU           BIT(25)
>  #define ITRIGGER_VS           BIT(26)
>
> +#define MHSELECT_IGNORE       0
> +#define MHSELECT_MCONTEXT     4
> +
>  bool tdata_available(CPURISCVState *env, int tdata_index);
>
>  target_ulong tselect_csr_read(CPURISCVState *env);
> --
> 2.34.1
>
>