On Wed, Mar 04, 2026 at 01:30:18PM +0800, Jay Chang wrote:
> Hi,
>
> Gentle ping on this patch.
>
> Thanks,
> Jay Chang
>
> On Mon, Dec 1, 2025 at 1:11 PM Jay Chang <[email protected]> wrote:
>
> > Hi,
> >
> > Gentle ping on this patch.
> >
> > Thanks,
> > Jay Chang
> >
> > On Fri, Nov 7, 2025 at 10:31 AM Jay Chang <[email protected]> wrote:
> >
> >> The Smpmpmt extension provides a mechanism to control memory attributes
> >> at the granularity of PMP (Physical Memory Protection) registers, similar
> >> to how Svpbmt controls memory attributes at the page level.
> >>
> >> Version 0.6
> >>
> >> https://github.com/riscv/riscv-isa-manual/blob/smpmpmt/src/smpmpmt.adoc#svpbmt
> >>
> >> Signed-off-by: Jay Chang <[email protected]>
> >> Reviewed-by: Daniel Henrique Barboza <[email protected]>
> >> Reviewed-by: Frank Chang <[email protected]>
> >>
> >> ---
> >> v2:
> >> Place smpmpmt in the correct riscv,isa order.
> >>
> >> Signed-off-by: Jay Chang <[email protected]>
> >> ---
> >> target/riscv/cpu.c | 2 ++
> >> target/riscv/cpu_cfg_fields.h.inc | 1 +
> >> target/riscv/pmp.c | 12 ++++++++++++
> >> target/riscv/pmp.h | 1 +
> >> 4 files changed, 16 insertions(+)
> >>
> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >> index ae8b721e55..0760ee7d9e 100644
> >> --- a/target/riscv/cpu.c
> >> +++ b/target/riscv/cpu.c
> >> @@ -203,6 +203,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> >> ISA_EXT_DATA_ENTRY(smcsrind, PRIV_VERSION_1_13_0, ext_smcsrind),
> >> ISA_EXT_DATA_ENTRY(smdbltrp, PRIV_VERSION_1_13_0, ext_smdbltrp),
> >> ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
> >> + ISA_EXT_DATA_ENTRY(smpmpmt, PRIV_VERSION_1_12_0, ext_smpmpmt),
> >> ISA_EXT_DATA_ENTRY(smrnmi, PRIV_VERSION_1_12_0, ext_smrnmi),
> >> ISA_EXT_DATA_ENTRY(smmpm, PRIV_VERSION_1_13_0, ext_smmpm),
> >> ISA_EXT_DATA_ENTRY(smnpm, PRIV_VERSION_1_13_0, ext_smnpm),
> >> @@ -1262,6 +1263,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[]
> >> = {
> >> MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
> >> MULTI_EXT_CFG_BOOL("svnapot", ext_svnapot, false),
> >> MULTI_EXT_CFG_BOOL("svpbmt", ext_svpbmt, false),
> >> + MULTI_EXT_CFG_BOOL("smpmpmt", ext_smpmpmt, false),
smpmpmt is an sm* extension but it's placed here among the
sv* entries (between svpbmt and svrsw60t59b). It should be
grouped with other sm* extensions around smepmp/smrnmi.
This seems like a copy-paste from v1 that wasn't updated.
> >> MULTI_EXT_CFG_BOOL("svrsw60t59b", ext_svrsw60t59b, false),
> >> MULTI_EXT_CFG_BOOL("svvptc", ext_svvptc, true),
> >>
> >> diff --git a/target/riscv/cpu_cfg_fields.h.inc
> >> b/target/riscv/cpu_cfg_fields.h.inc
> >> index a154ecdc79..b1096da664 100644
> >> --- a/target/riscv/cpu_cfg_fields.h.inc
> >> +++ b/target/riscv/cpu_cfg_fields.h.inc
> >> @@ -57,6 +57,7 @@ BOOL_FIELD(ext_svadu)
> >> BOOL_FIELD(ext_svinval)
> >> BOOL_FIELD(ext_svnapot)
> >> BOOL_FIELD(ext_svpbmt)
> >> +BOOL_FIELD(ext_smpmpmt)
Same ordering concern here — ext_smpmpmt is placed among
the sv* fields (after ext_svpbmt). It would be more
consistent to group it with the sm* fields.
> >> BOOL_FIELD(ext_svrsw60t59b)
> >> BOOL_FIELD(ext_svvptc)
> >> BOOL_FIELD(ext_svukte)
> >> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> >> index 3ef62d26ad..52a7677683 100644
> >> --- a/target/riscv/pmp.c
> >> +++ b/target/riscv/pmp.c
> >> @@ -165,6 +165,18 @@ static bool pmp_write_cfg(CPURISCVState *env,
> >> uint32_t pmp_index, uint8_t val)
> >> "ignoring pmpcfg write - invalid\n");
> >> } else {
> >> uint8_t a_field = pmp_get_a_field(val);
> >> +
> >> + if (!riscv_cpu_cfg(env)->ext_smpmpmt) {
> >> + /* If smpmpmt not supported, clear the MTMATCH bit */
> >> + val &= ~PMP_MTMATCH;
> >> + } else if ((val & PMP_MTMATCH) == PMP_MTMATCH) {
> >> + /*
> >> + * If trying to set reserved value (0x3) for MT field,
> >> + * preserve the original MT field from current config.
> >> + */
> >> + val = (val & ~PMP_MTMATCH) |
> >> + (env->pmp_state.pmp[pmp_index].cfg_reg &
> >> PMP_MTMATCH);
> >> + }
The WARL write-filtering logic looks correct to me. Nice
handling of the reserved value case.
PS: this patch implements the CSR interface for the
MT field but doesn't change PMP access-check behavior based
on the memory type.
For QEMU emulation this is fine since cache attributes have no
functional impact, but it might be worth a brief comment noting
that the MT field is stored but not acted upon during access
checks, so future readers don't wonder if it's missing.
> >> /*
> >> * When granularity g >= 1 (i.e., granularity > 4 bytes),
> >> * the NA4 (Naturally Aligned 4-byte) mode is not selectable
> >> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> >> index 271cf24169..467fb6b4b1 100644
> >> --- a/target/riscv/pmp.h
> >> +++ b/target/riscv/pmp.h
> >> @@ -29,6 +29,7 @@ typedef enum {
> >> PMP_WRITE = 1 << 1,
> >> PMP_EXEC = 1 << 2,
> >> PMP_AMATCH = (3 << 3),
> >> + PMP_MTMATCH = (3 << 5),
Looks good — bits [6:5] for the MT field per the spec.
Reviewed-by: Chao Liu <[email protected]>
Best regards,
Chao Liu
> >> PMP_LOCK = 1 << 7
> >> } pmp_priv_t;
> >>
> >> --
> >> 2.48.1
> >>
> >>
