From: Frédéric Pétrot <[email protected]> UndefinedBehaviorSanitizer reports an undefined behavior due to shifts larger than type in the handling of the pmp configuration registers for the x-rv128 cpu. Adding an "i128" version for the functions accessing the pmpcfg registers to properly handle the 128-bit case, and make sure only legal accesses to these pmp registers can take place.
Reported-by: Peter Maydell <[email protected]> Signed-off-by: Frédéric Pétrot <[email protected]> --- target/riscv/tcg/csr.c | 16 +++++++++++----- target/riscv/tcg/csr.h | 3 +++ target/riscv/tcg/pmp.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/target/riscv/tcg/csr.c b/target/riscv/tcg/csr.c index af444e3e6f..c3ef16cf1a 100644 --- a/target/riscv/tcg/csr.c +++ b/target/riscv/tcg/csr.c @@ -775,10 +775,12 @@ static RISCVException pmp(CPURISCVState *env, int csrno) if (csrno <= max_pmpcfg) { uint32_t reg_index = csrno - CSR_PMPCFG0; - /* TODO: RV128 restriction check */ if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) { return RISCV_EXCP_ILLEGAL_INST; } + if ((reg_index & 3) && (riscv_cpu_mxl(env) == MXL_RV128)) { + return RISCV_EXCP_ILLEGAL_INST; + } } return RISCV_EXCP_NONE; @@ -6508,11 +6510,13 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { /* Physical Memory Protection */ [CSR_MSECCFG] = { "mseccfg", have_mseccfg, read_mseccfg, write_mseccfg, .min_priv_ver = PRIV_VERSION_1_11_0 }, - [CSR_PMPCFG0] = { "pmpcfg0", pmp, read_pmpcfg, write_pmpcfg }, + [CSR_PMPCFG0] = { "pmpcfg0", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128 }, [CSR_PMPCFG1] = { "pmpcfg1", pmp, read_pmpcfg, write_pmpcfg }, [CSR_PMPCFG2] = { "pmpcfg2", pmp, read_pmpcfg, write_pmpcfg }, [CSR_PMPCFG3] = { "pmpcfg3", pmp, read_pmpcfg, write_pmpcfg }, - [CSR_PMPCFG4] = { "pmpcfg4", pmp, read_pmpcfg, write_pmpcfg, + [CSR_PMPCFG4] = { "pmpcfg4", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .min_priv_ver = PRIV_VERSION_1_12_0 }, [CSR_PMPCFG5] = { "pmpcfg5", pmp, read_pmpcfg, write_pmpcfg, .min_priv_ver = PRIV_VERSION_1_12_0 }, @@ -6520,7 +6524,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { .min_priv_ver = PRIV_VERSION_1_12_0 }, [CSR_PMPCFG7] = { "pmpcfg7", pmp, read_pmpcfg, write_pmpcfg, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_PMPCFG8] = { "pmpcfg8", pmp, read_pmpcfg, write_pmpcfg, + [CSR_PMPCFG8] = { "pmpcfg8", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .min_priv_ver = PRIV_VERSION_1_12_0 }, [CSR_PMPCFG9] = { "pmpcfg9", pmp, read_pmpcfg, write_pmpcfg, .min_priv_ver = PRIV_VERSION_1_12_0 }, @@ -6528,7 +6533,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { .min_priv_ver = PRIV_VERSION_1_12_0 }, [CSR_PMPCFG11] = { "pmpcfg11", pmp, read_pmpcfg, write_pmpcfg, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_PMPCFG12] = { "pmpcfg12", pmp, read_pmpcfg, write_pmpcfg, + [CSR_PMPCFG12] = { "pmpcfg12", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .min_priv_ver = PRIV_VERSION_1_12_0 }, [CSR_PMPCFG13] = { "pmpcfg13", pmp, read_pmpcfg, write_pmpcfg, .min_priv_ver = PRIV_VERSION_1_12_0 }, diff --git a/target/riscv/tcg/csr.h b/target/riscv/tcg/csr.h index 53ba3b4a4d..40396bafdb 100644 --- a/target/riscv/tcg/csr.h +++ b/target/riscv/tcg/csr.h @@ -91,6 +91,9 @@ extern const RISCVCSR mips_csr_list[]; void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index, target_ulong val); target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index); +void pmpcfg_csr_write_i128(CPURISCVState *env, uint32_t reg_index, + Int128 val); +Int128 pmpcfg_csr_read_i128(CPURISCVState *env, uint32_t reg_index); void mseccfg_csr_write(CPURISCVState *env, uint64_t val); uint64_t mseccfg_csr_read(CPURISCVState *env); diff --git a/target/riscv/tcg/pmp.c b/target/riscv/tcg/pmp.c index 04df53c234..7ca60bf854 100644 --- a/target/riscv/tcg/pmp.c +++ b/target/riscv/tcg/pmp.c @@ -564,6 +564,28 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index, } } +void pmpcfg_csr_write_i128(CPURISCVState *env, uint32_t reg_index, + Int128 val) +{ + int i; + uint8_t cfg_val; + int pmpcfg_nums = 2 << riscv_cpu_mxl(env); + bool modified = false; + + trace_pmpcfg_csr_write(env->mhartid, reg_index, int128_getlo(val)); + trace_pmpcfg_csr_write(env->mhartid, reg_index, int128_gethi(val)); + + for (i = 0; i < pmpcfg_nums; i++) { + cfg_val = int128_getlo(int128_rshift(val, 8 * i)) & 0xff; + modified |= pmp_write_cfg(env, (reg_index * 4) + i, cfg_val); + } + + /* If PMP permission of any addr has been changed, flush TLB pages. */ + if (modified) { + pmp_update_rule_nums(env); + tlb_flush(env_cpu(env)); + } +} /* * Handle a read from a pmpcfg CSR @@ -584,6 +606,22 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index) return cfg_val; } +Int128 pmpcfg_csr_read_i128(CPURISCVState *env, uint32_t reg_index) +{ + int i; + Int128 cfg_val = int128_zero(); + target_ulong val = 0; + int pmpcfg_nums = 2 << riscv_cpu_mxl(env); + + for (i = 0; i < pmpcfg_nums; i++) { + val = pmp_read_cfg(env, (reg_index * 4) + i); + cfg_val = int128_or(cfg_val, int128_lshift(int128_make64(val), i * 8)); + } + trace_pmpcfg_csr_read(env->mhartid, reg_index, int128_getlo(cfg_val)); + trace_pmpcfg_csr_read(env->mhartid, reg_index, int128_gethi(cfg_val)); + + return cfg_val; +} /* * Handle a write to a pmpaddr CSR -- 2.43.0
