* [PATCH v3] target/riscv: raise an exception when CSRRS/CSRRC writes a read-only CSR
@ 2024-04-03 7:08 Yu-Ming Chang via
2024-04-29 3:58 ` Alistair Francis
2024-04-29 4:01 ` Alistair Francis
0 siblings, 2 replies; 4+ messages in thread
From: Yu-Ming Chang via @ 2024-04-03 7:08 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: palmer, alistair.francis, bin.meng, liwei1518, dbarboza,
zhiwei_liu, yumin686
Both CSRRS and CSRRC always read the addressed CSR and cause any read side
effects regardless of rs1 and rd fields. Note that if rs1 specifies a register
holding a zero value other than x0, the instruction will still attempt to write
the unmodified value back to the CSR and will cause any attendant side effects.
So if CSRRS or CSRRC tries to write a read-only CSR with rs1 which specifies
a register holding a zero value, an illegal instruction exception should be
raised.
Signed-off-by: Yu-Ming Chang <yumin686@andestech.com>
---
Hi maintainers,
Do I need to make any further improvements to this patch?
Best regards,
Yuming
target/riscv/cpu.h | 4 ++++
target/riscv/csr.c | 51 ++++++++++++++++++++++++++++++++++++----
target/riscv/op_helper.c | 6 ++---
3 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3b1a02b944..99006bdb45 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -710,6 +710,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
void riscv_cpu_update_mask(CPURISCVState *env);
bool riscv_cpu_is_32bit(RISCVCPU *cpu);
+RISCVException riscv_csrr(CPURISCVState *env, int csrno,
+ target_ulong *ret_value);
RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
target_ulong *ret_value,
target_ulong new_value, target_ulong write_mask);
@@ -742,6 +744,8 @@ typedef RISCVException (*riscv_csr_op_fn)(CPURISCVState *env, int csrno,
target_ulong new_value,
target_ulong write_mask);
+RISCVException riscv_csrr_i128(CPURISCVState *env, int csrno,
+ Int128 *ret_value);
RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno,
Int128 *ret_value,
Int128 new_value, Int128 write_mask);
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 726096444f..35662e1777 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -4312,7 +4312,7 @@ static RISCVException rmw_seed(CPURISCVState *env, int csrno,
static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
int csrno,
- bool write_mask)
+ bool write)
{
/* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */
bool read_only = get_field(csrno, 0xC00) == 3;
@@ -4334,7 +4334,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
}
/* read / write check */
- if (write_mask && read_only) {
+ if (write && read_only) {
return RISCV_EXCP_ILLEGAL_INST;
}
@@ -4421,11 +4421,22 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
+RISCVException riscv_csrr(CPURISCVState *env, int csrno,
+ target_ulong *ret_value)
+{
+ RISCVException ret = riscv_csrrw_check(env, csrno, false);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ return riscv_csrrw_do64(env, csrno, ret_value, 0, 0);
+}
+
RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
target_ulong *ret_value,
target_ulong new_value, target_ulong write_mask)
{
- RISCVException ret = riscv_csrrw_check(env, csrno, write_mask);
+ RISCVException ret = riscv_csrrw_check(env, csrno, true);
if (ret != RISCV_EXCP_NONE) {
return ret;
}
@@ -4473,13 +4484,45 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
+RISCVException riscv_csrr_i128(CPURISCVState *env, int csrno,
+ Int128 *ret_value)
+{
+ RISCVException ret;
+
+ ret = riscv_csrrw_check(env, csrno, false);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ if (csr_ops[csrno].read128) {
+ return riscv_csrrw_do128(env, csrno, ret_value,
+ int128_zero(), int128_zero());
+ }
+
+ /*
+ * Fall back to 64-bit version for now, if the 128-bit alternative isn't
+ * at all defined.
+ * Note, some CSRs don't need to extend to MXLEN (64 upper bits non
+ * significant), for those, this fallback is correctly handling the
+ * accesses
+ */
+ target_ulong old_value;
+ ret = riscv_csrrw_do64(env, csrno, &old_value,
+ (target_ulong)0,
+ (target_ulong)0);
+ if (ret == RISCV_EXCP_NONE && ret_value) {
+ *ret_value = int128_make64(old_value);
+ }
+ return ret;
+}
+
RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno,
Int128 *ret_value,
Int128 new_value, Int128 write_mask)
{
RISCVException ret;
- ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask));
+ ret = riscv_csrrw_check(env, csrno, true);
if (ret != RISCV_EXCP_NONE) {
return ret;
}
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index f414aaebdb..b95d47e9ac 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -51,7 +51,7 @@ target_ulong helper_csrr(CPURISCVState *env, int csr)
}
target_ulong val = 0;
- RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0);
+ RISCVException ret = riscv_csrr(env, csr, &val);
if (ret != RISCV_EXCP_NONE) {
riscv_raise_exception(env, ret, GETPC());
@@ -84,9 +84,7 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr,
target_ulong helper_csrr_i128(CPURISCVState *env, int csr)
{
Int128 rv = int128_zero();
- RISCVException ret = riscv_csrrw_i128(env, csr, &rv,
- int128_zero(),
- int128_zero());
+ RISCVException ret = riscv_csrr_i128(env, csr, &rv);
if (ret != RISCV_EXCP_NONE) {
riscv_raise_exception(env, ret, GETPC());
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] target/riscv: raise an exception when CSRRS/CSRRC writes a read-only CSR
2024-04-03 7:08 [PATCH v3] target/riscv: raise an exception when CSRRS/CSRRC writes a read-only CSR Yu-Ming Chang via
@ 2024-04-29 3:58 ` Alistair Francis
2024-04-29 4:01 ` Alistair Francis
1 sibling, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2024-04-29 3:58 UTC (permalink / raw)
To: Yu-Ming Chang
Cc: qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu
On Wed, Apr 3, 2024 at 5:10 PM Yu-Ming Chang via <qemu-devel@nongnu.org> wrote:
>
> Both CSRRS and CSRRC always read the addressed CSR and cause any read side
> effects regardless of rs1 and rd fields. Note that if rs1 specifies a register
> holding a zero value other than x0, the instruction will still attempt to write
> the unmodified value back to the CSR and will cause any attendant side effects.
>
> So if CSRRS or CSRRC tries to write a read-only CSR with rs1 which specifies
> a register holding a zero value, an illegal instruction exception should be
> raised.
>
> Signed-off-by: Yu-Ming Chang <yumin686@andestech.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> Hi maintainers,
> Do I need to make any further improvements to this patch?
>
> Best regards,
> Yuming
>
> target/riscv/cpu.h | 4 ++++
> target/riscv/csr.c | 51 ++++++++++++++++++++++++++++++++++++----
> target/riscv/op_helper.c | 6 ++---
> 3 files changed, 53 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3b1a02b944..99006bdb45 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -710,6 +710,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
> void riscv_cpu_update_mask(CPURISCVState *env);
> bool riscv_cpu_is_32bit(RISCVCPU *cpu);
>
> +RISCVException riscv_csrr(CPURISCVState *env, int csrno,
> + target_ulong *ret_value);
> RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
> target_ulong *ret_value,
> target_ulong new_value, target_ulong write_mask);
> @@ -742,6 +744,8 @@ typedef RISCVException (*riscv_csr_op_fn)(CPURISCVState *env, int csrno,
> target_ulong new_value,
> target_ulong write_mask);
>
> +RISCVException riscv_csrr_i128(CPURISCVState *env, int csrno,
> + Int128 *ret_value);
> RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno,
> Int128 *ret_value,
> Int128 new_value, Int128 write_mask);
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 726096444f..35662e1777 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -4312,7 +4312,7 @@ static RISCVException rmw_seed(CPURISCVState *env, int csrno,
>
> static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> int csrno,
> - bool write_mask)
> + bool write)
> {
> /* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */
> bool read_only = get_field(csrno, 0xC00) == 3;
> @@ -4334,7 +4334,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> }
>
> /* read / write check */
> - if (write_mask && read_only) {
> + if (write && read_only) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> @@ -4421,11 +4421,22 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +RISCVException riscv_csrr(CPURISCVState *env, int csrno,
> + target_ulong *ret_value)
> +{
> + RISCVException ret = riscv_csrrw_check(env, csrno, false);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> + return riscv_csrrw_do64(env, csrno, ret_value, 0, 0);
> +}
> +
> RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
> target_ulong *ret_value,
> target_ulong new_value, target_ulong write_mask)
> {
> - RISCVException ret = riscv_csrrw_check(env, csrno, write_mask);
> + RISCVException ret = riscv_csrrw_check(env, csrno, true);
> if (ret != RISCV_EXCP_NONE) {
> return ret;
> }
> @@ -4473,13 +4484,45 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +RISCVException riscv_csrr_i128(CPURISCVState *env, int csrno,
> + Int128 *ret_value)
> +{
> + RISCVException ret;
> +
> + ret = riscv_csrrw_check(env, csrno, false);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> + if (csr_ops[csrno].read128) {
> + return riscv_csrrw_do128(env, csrno, ret_value,
> + int128_zero(), int128_zero());
> + }
> +
> + /*
> + * Fall back to 64-bit version for now, if the 128-bit alternative isn't
> + * at all defined.
> + * Note, some CSRs don't need to extend to MXLEN (64 upper bits non
> + * significant), for those, this fallback is correctly handling the
> + * accesses
> + */
> + target_ulong old_value;
> + ret = riscv_csrrw_do64(env, csrno, &old_value,
> + (target_ulong)0,
> + (target_ulong)0);
> + if (ret == RISCV_EXCP_NONE && ret_value) {
> + *ret_value = int128_make64(old_value);
> + }
> + return ret;
> +}
> +
> RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno,
> Int128 *ret_value,
> Int128 new_value, Int128 write_mask)
> {
> RISCVException ret;
>
> - ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask));
> + ret = riscv_csrrw_check(env, csrno, true);
> if (ret != RISCV_EXCP_NONE) {
> return ret;
> }
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index f414aaebdb..b95d47e9ac 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -51,7 +51,7 @@ target_ulong helper_csrr(CPURISCVState *env, int csr)
> }
>
> target_ulong val = 0;
> - RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0);
> + RISCVException ret = riscv_csrr(env, csr, &val);
>
> if (ret != RISCV_EXCP_NONE) {
> riscv_raise_exception(env, ret, GETPC());
> @@ -84,9 +84,7 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr,
> target_ulong helper_csrr_i128(CPURISCVState *env, int csr)
> {
> Int128 rv = int128_zero();
> - RISCVException ret = riscv_csrrw_i128(env, csr, &rv,
> - int128_zero(),
> - int128_zero());
> + RISCVException ret = riscv_csrr_i128(env, csr, &rv);
>
> if (ret != RISCV_EXCP_NONE) {
> riscv_raise_exception(env, ret, GETPC());
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] target/riscv: raise an exception when CSRRS/CSRRC writes a read-only CSR
2024-04-03 7:08 [PATCH v3] target/riscv: raise an exception when CSRRS/CSRRC writes a read-only CSR Yu-Ming Chang via
2024-04-29 3:58 ` Alistair Francis
@ 2024-04-29 4:01 ` Alistair Francis
1 sibling, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2024-04-29 4:01 UTC (permalink / raw)
To: Yu-Ming Chang
Cc: qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu
On Wed, Apr 3, 2024 at 5:10 PM Yu-Ming Chang via <qemu-devel@nongnu.org> wrote:
>
> Both CSRRS and CSRRC always read the addressed CSR and cause any read side
> effects regardless of rs1 and rd fields. Note that if rs1 specifies a register
> holding a zero value other than x0, the instruction will still attempt to write
> the unmodified value back to the CSR and will cause any attendant side effects.
>
> So if CSRRS or CSRRC tries to write a read-only CSR with rs1 which specifies
> a register holding a zero value, an illegal instruction exception should be
> raised.
>
> Signed-off-by: Yu-Ming Chang <yumin686@andestech.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> Hi maintainers,
> Do I need to make any further improvements to this patch?
>
> Best regards,
> Yuming
>
> target/riscv/cpu.h | 4 ++++
> target/riscv/csr.c | 51 ++++++++++++++++++++++++++++++++++++----
> target/riscv/op_helper.c | 6 ++---
> 3 files changed, 53 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3b1a02b944..99006bdb45 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -710,6 +710,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
> void riscv_cpu_update_mask(CPURISCVState *env);
> bool riscv_cpu_is_32bit(RISCVCPU *cpu);
>
> +RISCVException riscv_csrr(CPURISCVState *env, int csrno,
> + target_ulong *ret_value);
> RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
> target_ulong *ret_value,
> target_ulong new_value, target_ulong write_mask);
> @@ -742,6 +744,8 @@ typedef RISCVException (*riscv_csr_op_fn)(CPURISCVState *env, int csrno,
> target_ulong new_value,
> target_ulong write_mask);
>
> +RISCVException riscv_csrr_i128(CPURISCVState *env, int csrno,
> + Int128 *ret_value);
> RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno,
> Int128 *ret_value,
> Int128 new_value, Int128 write_mask);
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 726096444f..35662e1777 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -4312,7 +4312,7 @@ static RISCVException rmw_seed(CPURISCVState *env, int csrno,
>
> static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> int csrno,
> - bool write_mask)
> + bool write)
> {
> /* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */
> bool read_only = get_field(csrno, 0xC00) == 3;
> @@ -4334,7 +4334,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> }
>
> /* read / write check */
> - if (write_mask && read_only) {
> + if (write && read_only) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> @@ -4421,11 +4421,22 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +RISCVException riscv_csrr(CPURISCVState *env, int csrno,
> + target_ulong *ret_value)
> +{
> + RISCVException ret = riscv_csrrw_check(env, csrno, false);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> + return riscv_csrrw_do64(env, csrno, ret_value, 0, 0);
> +}
> +
> RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
> target_ulong *ret_value,
> target_ulong new_value, target_ulong write_mask)
> {
> - RISCVException ret = riscv_csrrw_check(env, csrno, write_mask);
> + RISCVException ret = riscv_csrrw_check(env, csrno, true);
> if (ret != RISCV_EXCP_NONE) {
> return ret;
> }
> @@ -4473,13 +4484,45 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +RISCVException riscv_csrr_i128(CPURISCVState *env, int csrno,
> + Int128 *ret_value)
> +{
> + RISCVException ret;
> +
> + ret = riscv_csrrw_check(env, csrno, false);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> + if (csr_ops[csrno].read128) {
> + return riscv_csrrw_do128(env, csrno, ret_value,
> + int128_zero(), int128_zero());
> + }
> +
> + /*
> + * Fall back to 64-bit version for now, if the 128-bit alternative isn't
> + * at all defined.
> + * Note, some CSRs don't need to extend to MXLEN (64 upper bits non
> + * significant), for those, this fallback is correctly handling the
> + * accesses
> + */
> + target_ulong old_value;
> + ret = riscv_csrrw_do64(env, csrno, &old_value,
> + (target_ulong)0,
> + (target_ulong)0);
> + if (ret == RISCV_EXCP_NONE && ret_value) {
> + *ret_value = int128_make64(old_value);
> + }
> + return ret;
> +}
> +
> RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno,
> Int128 *ret_value,
> Int128 new_value, Int128 write_mask)
> {
> RISCVException ret;
>
> - ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask));
> + ret = riscv_csrrw_check(env, csrno, true);
> if (ret != RISCV_EXCP_NONE) {
> return ret;
> }
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index f414aaebdb..b95d47e9ac 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -51,7 +51,7 @@ target_ulong helper_csrr(CPURISCVState *env, int csr)
> }
>
> target_ulong val = 0;
> - RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0);
> + RISCVException ret = riscv_csrr(env, csr, &val);
>
> if (ret != RISCV_EXCP_NONE) {
> riscv_raise_exception(env, ret, GETPC());
> @@ -84,9 +84,7 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr,
> target_ulong helper_csrr_i128(CPURISCVState *env, int csr)
> {
> Int128 rv = int128_zero();
> - RISCVException ret = riscv_csrrw_i128(env, csr, &rv,
> - int128_zero(),
> - int128_zero());
> + RISCVException ret = riscv_csrr_i128(env, csr, &rv);
>
> if (ret != RISCV_EXCP_NONE) {
> riscv_raise_exception(env, ret, GETPC());
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3] target/riscv: raise an exception when CSRRS/CSRRC writes a read-only CSR
@ 2024-03-13 1:50 Yu-Ming Chang via
0 siblings, 0 replies; 4+ messages in thread
From: Yu-Ming Chang via @ 2024-03-13 1:50 UTC (permalink / raw)
To: palmer, alistair.francis, bin.meng, liwei1518, dbarboza, zhiwei_liu
Cc: qemu-riscv, qemu-devel, Yu-Ming Chang
Both CSRRS and CSRRC always read the addressed CSR and cause any read side
effects regardless of rs1 and rd fields. Note that if rs1 specifies a register
holding a zero value other than x0, the instruction will still attempt to write
the unmodified value back to the CSR and will cause any attendant side effects.
So if CSRRS or CSRRC tries to write a read-only CSR with rs1 which specifies
a register holding a zero value, an illegal instruction exception should be
raised.
Signed-off-by: Yu-Ming Chang <yumin686@andestech.com>
---
This version solves similar bug in 128-bit CSR operations.
Thank you for the comment, Zhiwei.
target/riscv/cpu.h | 4 ++++
target/riscv/csr.c | 51 ++++++++++++++++++++++++++++++++++++----
target/riscv/op_helper.c | 6 ++---
3 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 5d291a7092..09e42e2ffa 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -710,6 +710,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
void riscv_cpu_update_mask(CPURISCVState *env);
bool riscv_cpu_is_32bit(RISCVCPU *cpu);
+RISCVException riscv_csrr(CPURISCVState *env, int csrno,
+ target_ulong *ret_value);
RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
target_ulong *ret_value,
target_ulong new_value, target_ulong write_mask);
@@ -742,6 +744,8 @@ typedef RISCVException (*riscv_csr_op_fn)(CPURISCVState *env, int csrno,
target_ulong new_value,
target_ulong write_mask);
+RISCVException riscv_csrr_i128(CPURISCVState *env, int csrno,
+ Int128 *ret_value);
RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno,
Int128 *ret_value,
Int128 new_value, Int128 write_mask);
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d4e8ac13b9..3e9cc64bf2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -4306,7 +4306,7 @@ static RISCVException rmw_seed(CPURISCVState *env, int csrno,
static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
int csrno,
- bool write_mask)
+ bool write)
{
/* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */
bool read_only = get_field(csrno, 0xC00) == 3;
@@ -4328,7 +4328,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
}
/* read / write check */
- if (write_mask && read_only) {
+ if (write && read_only) {
return RISCV_EXCP_ILLEGAL_INST;
}
@@ -4415,11 +4415,22 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
+RISCVException riscv_csrr(CPURISCVState *env, int csrno,
+ target_ulong *ret_value)
+{
+ RISCVException ret = riscv_csrrw_check(env, csrno, false);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ return riscv_csrrw_do64(env, csrno, ret_value, 0, 0);
+}
+
RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
target_ulong *ret_value,
target_ulong new_value, target_ulong write_mask)
{
- RISCVException ret = riscv_csrrw_check(env, csrno, write_mask);
+ RISCVException ret = riscv_csrrw_check(env, csrno, true);
if (ret != RISCV_EXCP_NONE) {
return ret;
}
@@ -4467,13 +4478,45 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
+RISCVException riscv_csrr_i128(CPURISCVState *env, int csrno,
+ Int128 *ret_value)
+{
+ RISCVException ret;
+
+ ret = riscv_csrrw_check(env, csrno, false);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ if (csr_ops[csrno].read128) {
+ return riscv_csrrw_do128(env, csrno, ret_value,
+ int128_zero(), int128_zero());
+ }
+
+ /*
+ * Fall back to 64-bit version for now, if the 128-bit alternative isn't
+ * at all defined.
+ * Note, some CSRs don't need to extend to MXLEN (64 upper bits non
+ * significant), for those, this fallback is correctly handling the
+ * accesses
+ */
+ target_ulong old_value;
+ ret = riscv_csrrw_do64(env, csrno, &old_value,
+ (target_ulong)0,
+ (target_ulong)0);
+ if (ret == RISCV_EXCP_NONE && ret_value) {
+ *ret_value = int128_make64(old_value);
+ }
+ return ret;
+}
+
RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno,
Int128 *ret_value,
Int128 new_value, Int128 write_mask)
{
RISCVException ret;
- ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask));
+ ret = riscv_csrrw_check(env, csrno, true);
if (ret != RISCV_EXCP_NONE) {
return ret;
}
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index f414aaebdb..b95d47e9ac 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -51,7 +51,7 @@ target_ulong helper_csrr(CPURISCVState *env, int csr)
}
target_ulong val = 0;
- RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0);
+ RISCVException ret = riscv_csrr(env, csr, &val);
if (ret != RISCV_EXCP_NONE) {
riscv_raise_exception(env, ret, GETPC());
@@ -84,9 +84,7 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr,
target_ulong helper_csrr_i128(CPURISCVState *env, int csr)
{
Int128 rv = int128_zero();
- RISCVException ret = riscv_csrrw_i128(env, csr, &rv,
- int128_zero(),
- int128_zero());
+ RISCVException ret = riscv_csrr_i128(env, csr, &rv);
if (ret != RISCV_EXCP_NONE) {
riscv_raise_exception(env, ret, GETPC());
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-29 4:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-03 7:08 [PATCH v3] target/riscv: raise an exception when CSRRS/CSRRC writes a read-only CSR Yu-Ming Chang via
2024-04-29 3:58 ` Alistair Francis
2024-04-29 4:01 ` Alistair Francis
-- strict thread matches above, loose matches on Subject: below --
2024-03-13 1:50 Yu-Ming Chang via
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).