1. Patchew
  2. QEMU
  3. [PATCH v5 0/4] RISC-V: Modularize common match conditions for trigger
  4. View patch

[PATCH v5 1/4] target/riscv: Add functions for common matching conditions of trigger

Alvin Chang via posted 4 patches 5 months, 3 weeks ago
There is a newer version of this series
[PATCH v5 1/4] target/riscv: Add functions for common matching conditions of trigger
Posted by Alvin Chang via 5 months, 3 weeks ago 
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.

[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-asciidoc

Signed-off-by: Alvin Chang <alvinga@andestech.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/debug.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index b110370ea6..05e001d041 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)
-- 
2.34.1
Re: [PATCH v5 1/4] target/riscv: Add functions for common matching conditions of trigger
Posted by Alistair Francis 5 months ago 
On Tue, Jun 4, 2024 at 2:42 PM Alvin Chang via <qemu-devel@nongnu.org> wrote:

The `From` address is mangled here. It shows it was sent from the list
instead of your actual email address.

Do you mind looking into your email setup and see if you can fix it?

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.
>
> [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-asciidoc
>
> Signed-off-by: Alvin Chang <alvinga@andestech.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/debug.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index b110370ea6..05e001d041 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)
> --
> 2.34.1
>
>
RE: [PATCH v5 1/4] target/riscv: Add functions for common matching conditions of trigger
Posted by Alvin Che-Chia Chang(張哲嘉) 5 months ago 
Hi Alistair,

> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Wednesday, June 26, 2024 2:20 PM
> 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 v5 1/4] target/riscv: Add functions for common matching
> conditions of trigger
>
> [EXTERNAL MAIL]
>
> On Tue, Jun 4, 2024 at 2:42 PM Alvin Chang via <qemu-devel@nongnu.org>
> wrote:
>
> The `From` address is mangled here. It shows it was sent from the list instead
> of your actual email address.
>
> Do you mind looking into your email setup and see if you can fix it?

I did not add "--from" when I sent the patch files.
And I've just added my name and email address by "git config sendemail.from ......".
Hope it will work next time I send the patch.
Thanks.

Alvin Chang

>
> 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.
> >
> > [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 | 70
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >
> > diff --git a/target/riscv/debug.c b/target/riscv/debug.c index
> > b110370ea6..05e001d041 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)
> > --
> > 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.
Re: [PATCH v5 1/4] target/riscv: Add functions for common matching conditions of trigger
Posted by Alistair Francis 5 months ago 
On Wed, Jun 26, 2024 at 5:27 PM Alvin Che-Chia Chang(張哲嘉)
<alvinga@andestech.com> wrote:
>
> Hi Alistair,
>
> > -----Original Message-----
> > From: Alistair Francis <alistair23@gmail.com>
> > Sent: Wednesday, June 26, 2024 2:20 PM
> > 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 v5 1/4] target/riscv: Add functions for common matching
> > conditions of trigger
> >
> > [EXTERNAL MAIL]
> >
> > On Tue, Jun 4, 2024 at 2:42 PM Alvin Chang via <qemu-devel@nongnu.org>
> > wrote:
> >
> > The `From` address is mangled here. It shows it was sent from the list instead
> > of your actual email address.
> >
> > Do you mind looking into your email setup and see if you can fix it?
>
> I did not add "--from" when I sent the patch files.
> And I've just added my name and email address by "git config sendemail.from ......".

Thanks! Let's see if that works.

This patch fails to compile though, I see this error

../target/riscv/debug.c:308:13: error: ‘trigger_common_match’ defined
but not used [-Werror=unused-function]
 308 | static bool trigger_common_match(CPURISCVState *env, trigger_type_t type,

I'm going to drop this series from my tree. Do you mind resending a
new version with the build failures fixed

Alistair

> Hope it will work next time I send the patch.
> Thanks.
>
> Alvin Chang
>
> >
> > 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.
> > >
> > > [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 | 70
> > > ++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 70 insertions(+)
> > >
> > > diff --git a/target/riscv/debug.c b/target/riscv/debug.c index
> > > b110370ea6..05e001d041 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)
> > > --
> > > 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.

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.