Message ID | 20230708091055.38505-3-reaperlu@hust.edu.cn |
---|---|
State | New |
Headers | show |
Series | target/riscv: improve code accuracy and | expand |
08.07.2023 12:10, Ruibo Lu пишет: > These two values represents whether start/end address is in pmp_range. > However, the type and name of them is ambiguous. This commit change the > name and type of them to improve code readability and accuracy. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1735 > Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Signed-off-by: Ruibo Lu <reaperlu@hust.edu.cn> > --- > target/riscv/pmp.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index 1a9279ba88..ea3d29217a 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -203,16 +203,16 @@ void pmp_update_rule_nums(CPURISCVState *env) > } > } > > -static int pmp_is_in_range(CPURISCVState *env, int pmp_index, > - target_ulong addr) > +static bool pmp_is_in_range(CPURISCVState *env, int pmp_index, > + target_ulong addr) > { > - int result = 0; > + bool result = false; > > if ((addr >= env->pmp_state.addr[pmp_index].sa) && > (addr <= env->pmp_state.addr[pmp_index].ea)) { > - result = 1; > + result = true; > } else { > - result = 0; > + result = false; > } > > return result; And the initial assignment of result isn't needed. How about this: return (addr >= env->pmp_state.addr[pmp_index].sa) && (addr <= env->pmp_state.addr[pmp_index].ea); instead? :) > @@ -287,8 +287,8 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr, > { > int i = 0; > int pmp_size = 0; > - target_ulong s = 0; > - target_ulong e = 0; > + bool sa_in = false; > + bool ea_in = false; > > /* Short cut if no rules */ > if (0 == pmp_get_num_rules(env)) { > @@ -314,11 +314,11 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr, > * from low to high > */ > for (i = 0; i < MAX_RISCV_PMPS; i++) { > - s = pmp_is_in_range(env, i, addr); > - e = pmp_is_in_range(env, i, addr + pmp_size - 1); > + sa_in = pmp_is_in_range(env, i, addr); > + ea_in = pmp_is_in_range(env, i, addr + pmp_size - 1); > > /* partially inside */ > - if ((s + e) == 1) { > + if (sa_in ^ ea_in) { Dunno how for others, but to me this is a bit difficult to read, as an exclusive or isn't a commonly used operation. Maybe sa_in != ea_in ? Thanks, /mjt > qemu_log_mask(LOG_GUEST_ERROR, > "pmp violation - access is partially inside\n"); > *allowed_privs = 0; > @@ -339,7 +339,7 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr, > (env->pmp_state.pmp[i].cfg_reg & PMP_WRITE) | > ((env->pmp_state.pmp[i].cfg_reg & PMP_EXEC) >> 2); > > - if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) { > + if (sa_in && ea_in && (PMP_AMATCH_OFF != a_field)) { > /* > * If the PMP entry is not off and the address is in range, > * do the priv check
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c index 1a9279ba88..ea3d29217a 100644 --- a/target/riscv/pmp.c +++ b/target/riscv/pmp.c @@ -203,16 +203,16 @@ void pmp_update_rule_nums(CPURISCVState *env) } } -static int pmp_is_in_range(CPURISCVState *env, int pmp_index, - target_ulong addr) +static bool pmp_is_in_range(CPURISCVState *env, int pmp_index, + target_ulong addr) { - int result = 0; + bool result = false; if ((addr >= env->pmp_state.addr[pmp_index].sa) && (addr <= env->pmp_state.addr[pmp_index].ea)) { - result = 1; + result = true; } else { - result = 0; + result = false; } return result; @@ -287,8 +287,8 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr, { int i = 0; int pmp_size = 0; - target_ulong s = 0; - target_ulong e = 0; + bool sa_in = false; + bool ea_in = false; /* Short cut if no rules */ if (0 == pmp_get_num_rules(env)) { @@ -314,11 +314,11 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr, * from low to high */ for (i = 0; i < MAX_RISCV_PMPS; i++) { - s = pmp_is_in_range(env, i, addr); - e = pmp_is_in_range(env, i, addr + pmp_size - 1); + sa_in = pmp_is_in_range(env, i, addr); + ea_in = pmp_is_in_range(env, i, addr + pmp_size - 1); /* partially inside */ - if ((s + e) == 1) { + if (sa_in ^ ea_in) { qemu_log_mask(LOG_GUEST_ERROR, "pmp violation - access is partially inside\n"); *allowed_privs = 0; @@ -339,7 +339,7 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr, (env->pmp_state.pmp[i].cfg_reg & PMP_WRITE) | ((env->pmp_state.pmp[i].cfg_reg & PMP_EXEC) >> 2); - if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) { + if (sa_in && ea_in && (PMP_AMATCH_OFF != a_field)) { /* * If the PMP entry is not off and the address is in range, * do the priv check