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.
Reported-by: Peter Maydell <[email protected]> Signed-off-by: Frédéric Pétrot <[email protected]> --- target/riscv/tcg/csr.c | 66 ++++++++++++++++++++++++++++++++---------- target/riscv/tcg/csr.h | 3 ++ target/riscv/tcg/pmp.c | 38 ++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 16 deletions(-) diff --git a/target/riscv/tcg/csr.c b/target/riscv/tcg/csr.c index 063590f56a..fabb95c171 100644 --- a/target/riscv/tcg/csr.c +++ b/target/riscv/tcg/csr.c @@ -5598,6 +5598,24 @@ static RISCVException write_pmpcfg(CPURISCVState *env, int csrno, return RISCV_EXCP_NONE; } +static RISCVException read_pmpcfg_i128(CPURISCVState *env, int csrno, + Int128 *val) +{ + uint32_t reg_index = csrno - CSR_PMPCFG0; + + *val = pmpcfg_csr_read_i128(env, reg_index); + return RISCV_EXCP_NONE; +} + +static RISCVException write_pmpcfg_i128(CPURISCVState *env, int csrno, + Int128 val) +{ + uint32_t reg_index = csrno - CSR_PMPCFG0; + + pmpcfg_csr_write_i128(env, reg_index, val); + return RISCV_EXCP_NONE; +} + static RISCVException read_pmpaddr(CPURISCVState *env, int csrno, target_ulong *val) { @@ -6472,33 +6490,49 @@ 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_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_PMPCFG0] = { "pmpcfg0", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128 }, + [CSR_PMPCFG1] = { "pmpcfg1", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128 }, + [CSR_PMPCFG2] = { "pmpcfg2", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128 }, + [CSR_PMPCFG3] = { "pmpcfg3", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128 }, + [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, + [CSR_PMPCFG5] = { "pmpcfg5", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_PMPCFG6] = { "pmpcfg6", pmp, read_pmpcfg, write_pmpcfg, + [CSR_PMPCFG6] = { "pmpcfg6", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_PMPCFG7] = { "pmpcfg7", pmp, read_pmpcfg, write_pmpcfg, + [CSR_PMPCFG7] = { "pmpcfg7", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .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, + [CSR_PMPCFG9] = { "pmpcfg9", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_PMPCFG10] = { "pmpcfg10", pmp, read_pmpcfg, write_pmpcfg, + [CSR_PMPCFG10] = { "pmpcfg10", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_PMPCFG11] = { "pmpcfg11", pmp, read_pmpcfg, write_pmpcfg, + [CSR_PMPCFG11] = { "pmpcfg11", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .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, + [CSR_PMPCFG13] = { "pmpcfg13", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_PMPCFG14] = { "pmpcfg14", pmp, read_pmpcfg, write_pmpcfg, + [CSR_PMPCFG14] = { "pmpcfg14", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .min_priv_ver = PRIV_VERSION_1_12_0 }, - [CSR_PMPCFG15] = { "pmpcfg15", pmp, read_pmpcfg, write_pmpcfg, + [CSR_PMPCFG15] = { "pmpcfg15", pmp, read_pmpcfg, write_pmpcfg, NULL, + read_pmpcfg_i128, write_pmpcfg_i128, .min_priv_ver = PRIV_VERSION_1_12_0 }, [CSR_PMPADDR0] = { "pmpaddr0", pmp, read_pmpaddr, write_pmpaddr }, [CSR_PMPADDR1] = { "pmpaddr1", pmp, read_pmpaddr, write_pmpaddr }, 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 41b55519a8..6fda5192ec 100644 --- a/target/riscv/tcg/pmp.c +++ b/target/riscv/tcg/pmp.c @@ -556,6 +556,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 @@ -576,6 +598,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
