From: "Alvin Che-Chia Chang(張哲嘉)" <alvinga@andestech.com>
To: Daniel Henrique Barboza <dbarboza@ventanamicro.com>,
"qemu-riscv@nongnu.org" <qemu-riscv@nongnu.org>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Cc: "alistair.francis@wdc.com" <alistair.francis@wdc.com>,
"bin.meng@windriver.com" <bin.meng@windriver.com>,
"liwei1518@gmail.com" <liwei1518@gmail.com>,
"zhiwei_liu@linux.alibaba.com" <zhiwei_liu@linux.alibaba.com>
Subject: RE: [PATCH 4/4] target/riscv: Apply modularized matching conditions for icount trigger
Date: Thu, 22 Feb 2024 02:05:46 +0000 [thread overview]
Message-ID: <SEYPR03MB6700369300FD61FE0854B60FA8562@SEYPR03MB6700.apcprd03.prod.outlook.com> (raw)
In-Reply-To: <094d7589-09f6-4515-ae8d-72acbd3ade58@ventanamicro.com>
Hi Daniel,
> -----Original Message-----
> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Sent: Thursday, February 22, 2024 2:06 AM
> To: Alvin Che-Chia Chang(張哲嘉) <alvinga@andestech.com>;
> qemu-riscv@nongnu.org; qemu-devel@nongnu.org
> Cc: alistair.francis@wdc.com; bin.meng@windriver.com;
> liwei1518@gmail.com; zhiwei_liu@linux.alibaba.com
> Subject: Re: [PATCH 4/4] target/riscv: Apply modularized matching conditions
> for icount trigger
>
>
>
> On 2/19/24 00:25, Alvin Chang wrote:
> > We have implemented trigger_common_match(), which checks if the
> > enabled privilege levels of the trigger match CPU's current privilege
> > level. We can invoke trigger_common_match() to check the privilege
> > levels of the type 3 triggers.
> >
> > Signed-off-by: Alvin Chang <alvinga@andestech.com>
> > ---
> > target/riscv/debug.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/debug.c b/target/riscv/debug.c index
> > 67ba19c966..de996a393c 100644
> > --- a/target/riscv/debug.c
> > +++ b/target/riscv/debug.c
> > @@ -624,7 +624,7 @@ void helper_itrigger_match(CPURISCVState *env)
> > if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
> > continue;
> > }
> > - if (check_itrigger_priv(env, i)) {
> > + if (!trigger_common_match(env, TRIGGER_TYPE_INST_CNT, i)) {
> > continue;
> > }
>
>
> Looks good. Shouldn't we also change riscv_itrigger_enabled() to also use
> trigger_common_match()? riscv_itrigger_enabled() is remarkably similar to
> helper_itrigger_match() so I believe we can also use the new function there.
I think we might not want to apply trigger_common_match() into riscv_itrigger_enabled().
The trigger_common_match() is used to check if the trigger can be matched in current privilege level.
It will check many conditions: trigger privilege levels, textra, tcontrol, etc.
The riscv_itrigger_enabled() is used to check if any icount trigger is enabled by checking vs/vu/count/s/u fields of tdata1 only.
In fact, we found the comparisons between tdata1 bit-fields and env->priv in check_itrigger_priv() are bugs.
And we have a patch to fix that:
bool riscv_itrigger_enabled(CPURISCVState *env)
{
int count;
for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
continue;
}
- if (check_itrigger_priv(env, i)) {
+ if ((env->tdata1[i] & ITRIGGER_VS) == 0 &&
+ (env->tdata1[i] & ITRIGGER_VU) == 0 &&
+ (env->tdata1[i] & ITRIGGER_U) == 0 &&
+ (env->tdata1[i] & ITRIGGER_S) == 0 &&
+ (env->tdata1[i] & ITRIGGER_M) == 0 ) {
continue;
}
count = itrigger_get_count(env, i);
if (!count) {
continue;
}
return true;
}
return false;
}
Sincerely,
Alvin Chang
>
>
> Thanks,
>
> Daniel
>
>
>
>
>
> > count = itrigger_get_count(env, i);
prev parent reply other threads:[~2024-02-22 2:07 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-19 3:25 [PATCH 0/4] RISC-V: Modularize common match conditions for trigger Alvin Chang via
2024-02-19 3:25 ` [PATCH 1/4] target/riscv: Add functions for common matching conditions of trigger Alvin Chang via
2024-02-21 17:09 ` Daniel Henrique Barboza
2024-02-19 3:25 ` [PATCH 2/4] target/riscv: Apply modularized matching conditions for breakpoint Alvin Chang via
2024-02-21 17:25 ` Daniel Henrique Barboza
2024-02-22 1:46 ` Alvin Che-Chia Chang(張哲嘉)
2024-02-22 12:37 ` Daniel Henrique Barboza
2024-02-19 3:25 ` [PATCH 3/4] target/riscv: Apply modularized matching conditions for watchpoint Alvin Chang via
2024-02-21 17:31 ` Daniel Henrique Barboza
2024-02-19 3:25 ` [PATCH 4/4] target/riscv: Apply modularized matching conditions for icount trigger Alvin Chang via
2024-02-21 18:05 ` Daniel Henrique Barboza
2024-02-22 2:05 ` Alvin Che-Chia Chang(張哲嘉) [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SEYPR03MB6700369300FD61FE0854B60FA8562@SEYPR03MB6700.apcprd03.prod.outlook.com \
--to=alvinga@andestech.com \
--cc=alistair.francis@wdc.com \
--cc=bin.meng@windriver.com \
--cc=dbarboza@ventanamicro.com \
--cc=liwei1518@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).