On Tue, Aug 20, 2024 at 2:00 PM Alvin Che-Chia Chang(張哲嘉)
<alvi...@andestech.com> wrote:
>
> Hi Alistair,
>
> > -----Original Message-----
> > From: Alvin Che-Chia Chang(張哲嘉) <alvi...@andestech.com>
> > Sent: Sunday, July 21, 2024 3:24 PM
> > To: qemu-ri...@nongnu.org; qemu-devel@nongnu.org
> > Cc: alistair.fran...@wdc.com; bin.m...@windriver.com;
> > liwei1...@gmail.com; dbarb...@ventanamicro.com;
> > zhiwei_...@linux.alibaba.com; Alvin Che-Chia Chang(張哲嘉)
> > <alvi...@andestech.com>
> > Subject: [PATCH v3 2/2] target/riscv: Add textra matching condition for the
> > triggers
> >
> > 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 <alvi...@andestech.com>
> > Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
> > ---
> > 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
> > d6b4a06144..62bb758860 100644
> > --- a/target/riscv/debug.c
> > +++ b/target/riscv/debug.c
> > @@ -364,11 +364,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();
> > + }
> > + }
>
> I have some new ideas on this part.
> Should we replace this whole if-else with just the following simple code ?
>
> case MHSELECT_MCONTEXT:
> /* Match if the low bits of mcontext/hcontext equal mhvalue. */
> if (mhvalue != env->mcontext) {
> return false;
> }
> break;
>
> Those masks on mcontext have been applied in write_mcontext().
> I think we can skip the masks here.
> What do you think ?
Yep, that would be much better
Alistair
>
>
> Regards,
> Alvin Chang
>
> > + 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
>
> 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.
