Hi Philippe, Thank you for the suggestion. QEMU_ALIGN_UP() is not suitable for NAPOT because the logic is different:
- QEMU_ALIGN_UP() performs alignment (rounds up to the nearest multiple) - NAPOT requires setting lower bits to 1 (not rounding up) For example, with g=3 and this_addr=0x1005: - Current code: 0x1005 | 0x3 = 0x1007 (sets lower 2 bits) - QEMU_ALIGN_UP: QEMU_ALIGN_UP(0x1005, 4) = 0x1008 (rounds up) However, you're right that the TOR case can be improved using ROUND_DOWN() since it clears lower bits (which is alignment) I'll update the patch with this improvement. On Sun, Dec 28, 2025 at 1:03 AM Philippe Mathieu-Daudé <[email protected]> wrote: > Hi, > > On 23/12/25 11:25, Jay Chang wrote: > > When configuring pmpcfg (TOR, NA4, or NAPOT) and pmpaddr, if the > > value is smaller than the PMP granularity, it automatically aligned > > to the PMP granularity. > > > > Signed-off-by: Jay Chang <[email protected]> > > Reviewed-by: Frank Chang <[email protected]> > > --- > > target/riscv/pmp.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > > index 3ef62d26ad..01b337f529 100644 > > --- a/target/riscv/pmp.c > > +++ b/target/riscv/pmp.c > > @@ -167,11 +167,12 @@ static bool pmp_write_cfg(CPURISCVState *env, > uint32_t pmp_index, uint8_t val) > > uint8_t a_field = pmp_get_a_field(val); > > /* > > * When granularity g >= 1 (i.e., granularity > 4 bytes), > > - * the NA4 (Naturally Aligned 4-byte) mode is not selectable > > + * the NA4 (Naturally Aligned 4-byte) mode is not > selectable. > > + * In this case, an NA4 setting is reinterpreted as a NAPOT > mode. > > */ > > if ((riscv_cpu_cfg(env)->pmp_granularity > > > MIN_RISCV_PMP_GRANULARITY) && (a_field == > PMP_AMATCH_NA4)) { > > - return false; > > + val |= PMP_AMATCH; > > } > > env->pmp_state.pmp[pmp_index].cfg_reg = val; > > pmp_update_rule_addr(env, pmp_index); > > @@ -251,6 +252,11 @@ void pmp_update_rule_addr(CPURISCVState *env, > uint32_t pmp_index) > > break; > > > > case PMP_AMATCH_NAPOT: > > + /* Align to pmp_granularity */ > > + if (g >= 2) { > > + this_addr |= ((1ULL << (g - 1ULL)) - 1ULL); > > Could we use QEMU_ALIGN_UP() here? > > > + } > > + > > pmp_decode_napot(this_addr, &sa, &ea); > > break; > > > >
