* [PATCH v3 1/2] target/riscv: FIX xATP_MODE validation
@ 2024-01-09 14:59 Irina Ryapolova
2024-01-09 14:59 ` [PATCH v3 2/2] target/riscv: UPDATE xATP write CSR Irina Ryapolova
2024-02-15 3:43 ` [PATCH v3 1/2] target/riscv: FIX xATP_MODE validation Alistair Francis
0 siblings, 2 replies; 5+ messages in thread
From: Irina Ryapolova @ 2024-01-09 14:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Irina Ryapolova
The SATP register is an SXLEN-bit read/write WARL register. It means that CSR fields are only defined
for a subset of bit encodings, but allow any value to be written while guaranteeing to return a legal
value whenever read (See riscv-privileged-20211203, SATP CSR).
For example on rv64 we are trying to write to SATP CSR val = 0x1000000000000000 (SATP_MODE = 1 - Reserved for standard use)
and after that we are trying to read SATP_CSR. We read from the SATP CSR value = 0x1000000000000000, which is not a correct
operation (return illegal value).
Signed-off-by: Irina Ryapolova <irina.ryapolova@syntacore.com>
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
Changes for v2:
-used satp_mode.map instead of satp_mode.supported
Changes for v3:
-patch formatting corrected
---
target/riscv/csr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fde7ce1a53..735fb27be7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1278,8 +1278,8 @@ static RISCVException read_mstatus(CPURISCVState *env, int csrno,
static bool validate_vm(CPURISCVState *env, target_ulong vm)
{
- return (vm & 0xf) <=
- satp_mode_max_from_map(riscv_cpu_cfg(env)->satp_mode.map);
+ uint64_t mode_supported = riscv_cpu_cfg(env)->satp_mode.map;
+ return get_field(mode_supported, (1 << vm));
}
static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] target/riscv: UPDATE xATP write CSR
2024-01-09 14:59 [PATCH v3 1/2] target/riscv: FIX xATP_MODE validation Irina Ryapolova
@ 2024-01-09 14:59 ` Irina Ryapolova
2024-02-15 3:45 ` Alistair Francis
2024-02-16 0:33 ` Alistair Francis
2024-02-15 3:43 ` [PATCH v3 1/2] target/riscv: FIX xATP_MODE validation Alistair Francis
1 sibling, 2 replies; 5+ messages in thread
From: Irina Ryapolova @ 2024-01-09 14:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Irina Ryapolova
Added xATP_MODE validation for vsatp/hgatp CSRs.
The xATP register is an SXLEN-bit read/write WARL register, so
the legal value must be returned (See riscv-privileged-20211203, SATP/VSATP/HGATP CSRs).
Signed-off-by: Irina Ryapolova <irina.ryapolova@syntacore.com>
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/csr.c | 52 ++++++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 23 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 735fb27be7..6d7a3dd9aa 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1282,6 +1282,32 @@ static bool validate_vm(CPURISCVState *env, target_ulong vm)
return get_field(mode_supported, (1 << vm));
}
+static target_ulong legalize_xatp(CPURISCVState *env, target_ulong old_xatp,
+ target_ulong val)
+{
+ target_ulong mask;
+ bool vm;
+ if (riscv_cpu_mxl(env) == MXL_RV32) {
+ vm = validate_vm(env, get_field(val, SATP32_MODE));
+ mask = (val ^ old_xatp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN);
+ } else {
+ vm = validate_vm(env, get_field(val, SATP64_MODE));
+ mask = (val ^ old_xatp) & (SATP64_MODE | SATP64_ASID | SATP64_PPN);
+ }
+
+ if (vm && mask) {
+ /*
+ * The ISA defines SATP.MODE=Bare as "no translation", but we still
+ * pass these through QEMU's TLB emulation as it improves
+ * performance. Flushing the TLB on SATP writes with paging
+ * enabled avoids leaking those invalid cached mappings.
+ */
+ tlb_flush(env_cpu(env));
+ return val;
+ }
+ return old_xatp;
+}
+
static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
target_ulong val)
{
@@ -2997,31 +3023,11 @@ static RISCVException read_satp(CPURISCVState *env, int csrno,
static RISCVException write_satp(CPURISCVState *env, int csrno,
target_ulong val)
{
- target_ulong mask;
- bool vm;
-
if (!riscv_cpu_cfg(env)->mmu) {
return RISCV_EXCP_NONE;
}
- if (riscv_cpu_mxl(env) == MXL_RV32) {
- vm = validate_vm(env, get_field(val, SATP32_MODE));
- mask = (val ^ env->satp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN);
- } else {
- vm = validate_vm(env, get_field(val, SATP64_MODE));
- mask = (val ^ env->satp) & (SATP64_MODE | SATP64_ASID | SATP64_PPN);
- }
-
- if (vm && mask) {
- /*
- * The ISA defines SATP.MODE=Bare as "no translation", but we still
- * pass these through QEMU's TLB emulation as it improves
- * performance. Flushing the TLB on SATP writes with paging
- * enabled avoids leaking those invalid cached mappings.
- */
- tlb_flush(env_cpu(env));
- env->satp = val;
- }
+ env->satp = legalize_xatp(env, env->satp, val);
return RISCV_EXCP_NONE;
}
@@ -3506,7 +3512,7 @@ static RISCVException read_hgatp(CPURISCVState *env, int csrno,
static RISCVException write_hgatp(CPURISCVState *env, int csrno,
target_ulong val)
{
- env->hgatp = val;
+ env->hgatp = legalize_xatp(env, env->hgatp, val);
return RISCV_EXCP_NONE;
}
@@ -3772,7 +3778,7 @@ static RISCVException read_vsatp(CPURISCVState *env, int csrno,
static RISCVException write_vsatp(CPURISCVState *env, int csrno,
target_ulong val)
{
- env->vsatp = val;
+ env->vsatp = legalize_xatp(env, env->vsatp, val);
return RISCV_EXCP_NONE;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] target/riscv: FIX xATP_MODE validation
2024-01-09 14:59 [PATCH v3 1/2] target/riscv: FIX xATP_MODE validation Irina Ryapolova
2024-01-09 14:59 ` [PATCH v3 2/2] target/riscv: UPDATE xATP write CSR Irina Ryapolova
@ 2024-02-15 3:43 ` Alistair Francis
1 sibling, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2024-02-15 3:43 UTC (permalink / raw)
To: Irina Ryapolova
Cc: qemu-devel, qemu-riscv, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu
On Wed, Jan 10, 2024 at 1:00 AM Irina Ryapolova
<irina.ryapolova@syntacore.com> wrote:
>
> The SATP register is an SXLEN-bit read/write WARL register. It means that CSR fields are only defined
> for a subset of bit encodings, but allow any value to be written while guaranteeing to return a legal
> value whenever read (See riscv-privileged-20211203, SATP CSR).
>
> For example on rv64 we are trying to write to SATP CSR val = 0x1000000000000000 (SATP_MODE = 1 - Reserved for standard use)
> and after that we are trying to read SATP_CSR. We read from the SATP CSR value = 0x1000000000000000, which is not a correct
> operation (return illegal value).
>
> Signed-off-by: Irina Ryapolova <irina.ryapolova@syntacore.com>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> Changes for v2:
> -used satp_mode.map instead of satp_mode.supported
> Changes for v3:
> -patch formatting corrected
> ---
> target/riscv/csr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index fde7ce1a53..735fb27be7 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1278,8 +1278,8 @@ static RISCVException read_mstatus(CPURISCVState *env, int csrno,
>
> static bool validate_vm(CPURISCVState *env, target_ulong vm)
> {
> - return (vm & 0xf) <=
> - satp_mode_max_from_map(riscv_cpu_cfg(env)->satp_mode.map);
> + uint64_t mode_supported = riscv_cpu_cfg(env)->satp_mode.map;
> + return get_field(mode_supported, (1 << vm));
> }
>
> static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] target/riscv: UPDATE xATP write CSR
2024-01-09 14:59 ` [PATCH v3 2/2] target/riscv: UPDATE xATP write CSR Irina Ryapolova
@ 2024-02-15 3:45 ` Alistair Francis
2024-02-16 0:33 ` Alistair Francis
1 sibling, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2024-02-15 3:45 UTC (permalink / raw)
To: Irina Ryapolova
Cc: qemu-devel, qemu-riscv, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu
On Wed, Jan 10, 2024 at 2:07 AM Irina Ryapolova
<irina.ryapolova@syntacore.com> wrote:
>
> Added xATP_MODE validation for vsatp/hgatp CSRs.
> The xATP register is an SXLEN-bit read/write WARL register, so
> the legal value must be returned (See riscv-privileged-20211203, SATP/VSATP/HGATP CSRs).
>
> Signed-off-by: Irina Ryapolova <irina.ryapolova@syntacore.com>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/csr.c | 52 ++++++++++++++++++++++++++--------------------
> 1 file changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 735fb27be7..6d7a3dd9aa 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1282,6 +1282,32 @@ static bool validate_vm(CPURISCVState *env, target_ulong vm)
> return get_field(mode_supported, (1 << vm));
> }
>
> +static target_ulong legalize_xatp(CPURISCVState *env, target_ulong old_xatp,
> + target_ulong val)
> +{
> + target_ulong mask;
> + bool vm;
> + if (riscv_cpu_mxl(env) == MXL_RV32) {
> + vm = validate_vm(env, get_field(val, SATP32_MODE));
> + mask = (val ^ old_xatp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN);
> + } else {
> + vm = validate_vm(env, get_field(val, SATP64_MODE));
> + mask = (val ^ old_xatp) & (SATP64_MODE | SATP64_ASID | SATP64_PPN);
> + }
> +
> + if (vm && mask) {
> + /*
> + * The ISA defines SATP.MODE=Bare as "no translation", but we still
> + * pass these through QEMU's TLB emulation as it improves
> + * performance. Flushing the TLB on SATP writes with paging
> + * enabled avoids leaking those invalid cached mappings.
> + */
> + tlb_flush(env_cpu(env));
> + return val;
> + }
> + return old_xatp;
> +}
> +
> static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
> target_ulong val)
> {
> @@ -2997,31 +3023,11 @@ static RISCVException read_satp(CPURISCVState *env, int csrno,
> static RISCVException write_satp(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> - target_ulong mask;
> - bool vm;
> -
> if (!riscv_cpu_cfg(env)->mmu) {
> return RISCV_EXCP_NONE;
> }
>
> - if (riscv_cpu_mxl(env) == MXL_RV32) {
> - vm = validate_vm(env, get_field(val, SATP32_MODE));
> - mask = (val ^ env->satp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN);
> - } else {
> - vm = validate_vm(env, get_field(val, SATP64_MODE));
> - mask = (val ^ env->satp) & (SATP64_MODE | SATP64_ASID | SATP64_PPN);
> - }
> -
> - if (vm && mask) {
> - /*
> - * The ISA defines SATP.MODE=Bare as "no translation", but we still
> - * pass these through QEMU's TLB emulation as it improves
> - * performance. Flushing the TLB on SATP writes with paging
> - * enabled avoids leaking those invalid cached mappings.
> - */
> - tlb_flush(env_cpu(env));
> - env->satp = val;
> - }
> + env->satp = legalize_xatp(env, env->satp, val);
> return RISCV_EXCP_NONE;
> }
>
> @@ -3506,7 +3512,7 @@ static RISCVException read_hgatp(CPURISCVState *env, int csrno,
> static RISCVException write_hgatp(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> - env->hgatp = val;
> + env->hgatp = legalize_xatp(env, env->hgatp, val);
> return RISCV_EXCP_NONE;
> }
>
> @@ -3772,7 +3778,7 @@ static RISCVException read_vsatp(CPURISCVState *env, int csrno,
> static RISCVException write_vsatp(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> - env->vsatp = val;
> + env->vsatp = legalize_xatp(env, env->vsatp, val);
> return RISCV_EXCP_NONE;
> }
>
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] target/riscv: UPDATE xATP write CSR
2024-01-09 14:59 ` [PATCH v3 2/2] target/riscv: UPDATE xATP write CSR Irina Ryapolova
2024-02-15 3:45 ` Alistair Francis
@ 2024-02-16 0:33 ` Alistair Francis
1 sibling, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2024-02-16 0:33 UTC (permalink / raw)
To: Irina Ryapolova
Cc: qemu-devel, qemu-riscv, palmer, alistair.francis, bin.meng,
liwei1518, dbarboza, zhiwei_liu
On Wed, Jan 10, 2024 at 2:07 AM Irina Ryapolova
<irina.ryapolova@syntacore.com> wrote:
>
> Added xATP_MODE validation for vsatp/hgatp CSRs.
> The xATP register is an SXLEN-bit read/write WARL register, so
> the legal value must be returned (See riscv-privileged-20211203, SATP/VSATP/HGATP CSRs).
>
> Signed-off-by: Irina Ryapolova <irina.ryapolova@syntacore.com>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Thanks for the patches.
In the future can you please send a multi-patch series with a cover
letter. It makes
the QEMU automation a lot easier to work with.
I have applied these two patches to the riscv-to-apply.next branch
Alistair
> ---
> target/riscv/csr.c | 52 ++++++++++++++++++++++++++--------------------
> 1 file changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 735fb27be7..6d7a3dd9aa 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1282,6 +1282,32 @@ static bool validate_vm(CPURISCVState *env, target_ulong vm)
> return get_field(mode_supported, (1 << vm));
> }
>
> +static target_ulong legalize_xatp(CPURISCVState *env, target_ulong old_xatp,
> + target_ulong val)
> +{
> + target_ulong mask;
> + bool vm;
> + if (riscv_cpu_mxl(env) == MXL_RV32) {
> + vm = validate_vm(env, get_field(val, SATP32_MODE));
> + mask = (val ^ old_xatp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN);
> + } else {
> + vm = validate_vm(env, get_field(val, SATP64_MODE));
> + mask = (val ^ old_xatp) & (SATP64_MODE | SATP64_ASID | SATP64_PPN);
> + }
> +
> + if (vm && mask) {
> + /*
> + * The ISA defines SATP.MODE=Bare as "no translation", but we still
> + * pass these through QEMU's TLB emulation as it improves
> + * performance. Flushing the TLB on SATP writes with paging
> + * enabled avoids leaking those invalid cached mappings.
> + */
> + tlb_flush(env_cpu(env));
> + return val;
> + }
> + return old_xatp;
> +}
> +
> static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
> target_ulong val)
> {
> @@ -2997,31 +3023,11 @@ static RISCVException read_satp(CPURISCVState *env, int csrno,
> static RISCVException write_satp(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> - target_ulong mask;
> - bool vm;
> -
> if (!riscv_cpu_cfg(env)->mmu) {
> return RISCV_EXCP_NONE;
> }
>
> - if (riscv_cpu_mxl(env) == MXL_RV32) {
> - vm = validate_vm(env, get_field(val, SATP32_MODE));
> - mask = (val ^ env->satp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN);
> - } else {
> - vm = validate_vm(env, get_field(val, SATP64_MODE));
> - mask = (val ^ env->satp) & (SATP64_MODE | SATP64_ASID | SATP64_PPN);
> - }
> -
> - if (vm && mask) {
> - /*
> - * The ISA defines SATP.MODE=Bare as "no translation", but we still
> - * pass these through QEMU's TLB emulation as it improves
> - * performance. Flushing the TLB on SATP writes with paging
> - * enabled avoids leaking those invalid cached mappings.
> - */
> - tlb_flush(env_cpu(env));
> - env->satp = val;
> - }
> + env->satp = legalize_xatp(env, env->satp, val);
> return RISCV_EXCP_NONE;
> }
>
> @@ -3506,7 +3512,7 @@ static RISCVException read_hgatp(CPURISCVState *env, int csrno,
> static RISCVException write_hgatp(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> - env->hgatp = val;
> + env->hgatp = legalize_xatp(env, env->hgatp, val);
> return RISCV_EXCP_NONE;
> }
>
> @@ -3772,7 +3778,7 @@ static RISCVException read_vsatp(CPURISCVState *env, int csrno,
> static RISCVException write_vsatp(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> - env->vsatp = val;
> + env->vsatp = legalize_xatp(env, env->vsatp, val);
> return RISCV_EXCP_NONE;
> }
>
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-16 0:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-09 14:59 [PATCH v3 1/2] target/riscv: FIX xATP_MODE validation Irina Ryapolova
2024-01-09 14:59 ` [PATCH v3 2/2] target/riscv: UPDATE xATP write CSR Irina Ryapolova
2024-02-15 3:45 ` Alistair Francis
2024-02-16 0:33 ` Alistair Francis
2024-02-15 3:43 ` [PATCH v3 1/2] target/riscv: FIX xATP_MODE validation Alistair Francis
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).