* [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 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
* 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
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).