[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 8/9] target/riscv: debug: Return 0 if previous value written
From: |
Bin Meng |
Subject: |
Re: [PATCH 8/9] target/riscv: debug: Return 0 if previous value written to tselect >= number of triggers |
Date: |
Wed, 15 Jun 2022 21:17:03 +0800 |
On Fri, Jun 10, 2022 at 1:24 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> If the value written to tselect is greater than or equal to the number
> of supported triggers, then the following reads of tselect would return
> value 0.
Where is this behavior documented?
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> ---
> target/riscv/cpu.h | 1 +
> target/riscv/debug.c | 6 ++++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index bac5f00722..c7ee3f80e6 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -289,6 +289,7 @@ struct CPUArchState {
>
> /* trigger module */
> target_ulong trigger_cur;
> + target_ulong trigger_prev;
> target_ulong tdata1[RV_MAX_TRIGGERS];
> target_ulong tdata2[RV_MAX_TRIGGERS];
> target_ulong tdata3[RV_MAX_TRIGGERS];
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index ce9ff15d75..83b72fa1b9 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -158,6 +158,10 @@ bool tdata_available(CPURISCVState *env, int tdata_index)
>
> target_ulong tselect_csr_read(CPURISCVState *env)
> {
> + if (env->trigger_prev >= RV_MAX_TRIGGERS) {
> + return 0;
> + }
> +
> return env->trigger_cur;
> }
>
> @@ -166,6 +170,8 @@ void tselect_csr_write(CPURISCVState *env, target_ulong
> val)
> if (val < RV_MAX_TRIGGERS) {
> env->trigger_cur = val;
> }
> +
> + env->trigger_prev = val;
> }
>
> static target_ulong tdata1_validate(CPURISCVState *env, target_ulong val,
> --
The spec mentions "implementations which have 2^n triggers only need
to implement n bits of tselect", so in QEMU we can always implement
2^n triggers and have tselect implement just n bit.
In such way, writing tselect can be: env->trigger_cur = val &
(RV_MAX_TRIGGERS - 1).
and I believe you can squash this patch into patch 4 "target/riscv:
debug: Restrict the range of tselect value can be written" because in
patch 4 you changed the actual tselect range while the original
implementation allowed all bits to be set.
Regards,
Bin
- [PATCH 0/9] Improve RISC-V Debug support, frank . chang, 2022/06/10
- [PATCH 4/9] target/riscv: debug: Restrict the range of tselect value can be written, frank . chang, 2022/06/10
- [PATCH 2/9] target/riscv: debug: Introduce build_tdata1() to build tdata1 register content, frank . chang, 2022/06/10
- [PATCH 1/9] target/riscv: debug: Determine the trigger type from tdata1.type, frank . chang, 2022/06/10
- [PATCH 8/9] target/riscv: debug: Return 0 if previous value written to tselect >= number of triggers, frank . chang, 2022/06/10
- Re: [PATCH 8/9] target/riscv: debug: Return 0 if previous value written to tselect >= number of triggers,
Bin Meng <=
- [PATCH 3/9] target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs, frank . chang, 2022/06/10
- [PATCH 5/9] target/riscv: debug: Introduce tinfo CSR, frank . chang, 2022/06/10
- [PATCH 6/9] target/riscv: debug: Create common trigger actions function, frank . chang, 2022/06/10
- [PATCH 7/9] target/riscv: debug: Check VU/VS modes for type 2 trigger, frank . chang, 2022/06/10
- [PATCH 9/9] target/riscv: debug: Add initial support of type 6 trigger, frank . chang, 2022/06/10