Re: [PATCH v2] target/riscv: Support Smpmpmt extension

2026-03-04 Thread Chao Liu
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  wrote:
> 
> > Hi,
> >
> > Gentle ping on this patch.
> >
> > Thanks,
> > Jay Chang
> >
> > On Fri, Nov 7, 2025 at 10:31 AM Jay Chang  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 
> >> Reviewed-by: Daniel Henrique Barboza 
> >> Reviewed-by: Frank Chang 
> >>
> >> ---
> >> v2:
> >> Place smpmpmt in the correct riscv,isa order.
> >>
> >> Signed-off-by: Jay Chang 
> >> ---
> >>  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
> >> 

Re: [PATCH v2] target/riscv: Support Smpmpmt extension

2026-03-03 Thread Jay Chang
Hi,

Gentle ping on this patch.

Thanks,
Jay Chang

On Mon, Dec 1, 2025 at 1:11 PM Jay Chang  wrote:

> Hi,
>
> Gentle ping on this patch.
>
> Thanks,
> Jay Chang
>
> On Fri, Nov 7, 2025 at 10:31 AM Jay Chang  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 
>> Reviewed-by: Daniel Henrique Barboza 
>> Reviewed-by: Frank Chang 
>>
>> ---
>> v2:
>> Place smpmpmt in the correct riscv,isa order.
>>
>> Signed-off-by: Jay Chang 
>> ---
>>  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),
>>  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)
>>  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);
>> +}
>>  /*
>>   * 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),
>>  PMP_LOCK  = 1 << 7
>>  } pmp_priv_t;
>>
>> --
>> 2.48.1
>>
>>


Re: [PATCH v2] target/riscv: Support Smpmpmt extension

2025-11-30 Thread Jay Chang
Hi,

Gentle ping on this patch.

Thanks,
Jay Chang

On Fri, Nov 7, 2025 at 10:31 AM Jay Chang  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 
> Reviewed-by: Daniel Henrique Barboza 
> Reviewed-by: Frank Chang 
>
> ---
> v2:
> Place smpmpmt in the correct riscv,isa order.
>
> Signed-off-by: Jay Chang 
> ---
>  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),
>  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)
>  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);
> +}
>  /*
>   * 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),
>  PMP_LOCK  = 1 << 7
>  } pmp_priv_t;
>
> --
> 2.48.1
>
>