On 01/02/2018 04:44 PM, Michael Clark wrote:
> +#ifdef DEBUG_PMP
> +#define PMP_PRINTF(fmt, ...) \
> +do { fprintf(stderr, "pmp: " fmt, ## __VA_ARGS__); } while (0)
> +#else
> +#define PMP_PRINTF(fmt, ...) \
> +do {} while (0)
> +#endif
Debugging goes to qemu_log.
Rearrange this so that formatting is always compile-time checked.
E.g.
#define DEBUG_PMP 0
#define PMP_PRINTF(fmt, ...) \
do { \
if (DEBUG_PMP) { \
qemu_log("pmp: " fmt, ##__VA_ARGS__); \
} \
} while (0)
>
> +static target_ulong pmp_get_napot_base_and_range(target_ulong reg,
> + target_ulong *range)
> +{
> + /* construct a mask of all bits bar the top bit */
> + target_ulong mask = 0u;
> + target_ulong base = reg;
> + target_ulong numbits = (sizeof(target_ulong) * 8u) + 2u;
> + mask = (mask - 1u) >> 1;
> +
> + while (mask) {
> + if ((reg & mask) == mask) {
> + /* this is the mask to use */
> + base = reg & ~mask;
> + break;
> + }
> + mask >>= 1;
> + numbits--;
> + }
> +
> + *range = (1lu << numbits) - 1u;
> + return base;
> +}
You can compute napot with ctz64(~reg).
More useless LU suffixes.
> + if (pmp_index >= 1u) {
> + prev_addr = env->pmp_state.pmp[pmp_index].addr_reg;
pmp_index - 1
>
> + for (i = 0; i < MAX_RISCV_PMPS; i++) {
> + const uint8_t a_field =
> + pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
> + if (PMP_AMATCH_OFF != a_field) {
> + env->pmp_state.num_rules++;
> + }
> + }
Doesn't this mean that pmp_index ordering != pmp_state ordering? Which would
mean that you'd be matching rules in the wrong order for the static prioirity.
> +static int pmp_is_in_range(CPURISCVState *env, int pmp_index, target_ulong
> addr)
> +{
> + int result = 0;
> +
> + if ((addr >= env->pmp_state.addr[pmp_index].sa)
> + && (addr < env->pmp_state.addr[pmp_index].ea)) {
> + result = 1;
Given how the range is computed in pmp_update_rule, surely <= ea.
> + s = pmp_is_in_range(env, i, addr);
> + e = pmp_is_in_range(env, i, addr + size);
Surely addr + size - 1.
>
> + /* val &= 0x3ffffffffffffful; */
> +
Why is this commented out? Surely that's exactly what the spec says.
Although it's easier to compare as
val = extract64(val, 0, 54);
r~
