Re: [PATCH v4 2/2] target/riscv: Make PMP region count configurable
On Thu, Jun 5, 2025 at 3:34 PM Jay Chang wrote:
>
> Hi Daniel,
>
> You're absolutely right — thanks for pointing it out and providing the fix!
>
> Would you like me to send out a v5 patch incorporating your changes?
Yes please
Alistair
>
>
> Best Regards
> Jay Chang
>
>
> On Thu, Jun 5, 2025 at 2:20 AM Daniel Henrique Barboza
> wrote:
>>
>> It seems like this patch is breaking 'make check-functional':
>>
>>
>> >>> UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
>> >>> RUST_BACKTRACE=1
>> >>> LD_LIBRARY_PATH=/home/danielhb/work/qemu/build/contrib/plugins:/home/danielhb/work/qemu/build/tests/tcg/plugins
>> >>>
>> >>> MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
>> >>> QEMU_BUILD_ROOT=/home/danielhb/work/qemu/build
>> >>> QEMU_TEST_QEMU_IMG=/home/danielhb/work/qemu/build/qemu-img
>> >>> PYTHONPATH=/home/danielhb/work/qemu/python:/home/danielhb/work/qemu/tests/functional
>> >>> G_TEST_SLOW=1
>> >>> ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1
>> >>> MALLOC_PERTURB_=229 MESON_TEST_ITERATION=1
>> >>> QEMU_TEST_QEMU_BINARY=/home/danielhb/work/qemu/build/qemu-system-riscv64
>> >>> /home/danielhb/work/qemu/build/pyvenv/bin/python3
>> >>> /home/danielhb/work/qemu/tests/functional/test_riscv64_tuxrun.py
>>
>>
>> Summary of Failures:
>>
>> 11/12 qemu:func-thorough+func-riscv32-thorough+thorough /
>> func-riscv32-riscv32_tuxrun TIMEOUT 90.02s killed by signal 15
>> SIGTERM
>> 12/12 qemu:func-thorough+func-riscv64-thorough+thorough /
>> func-riscv64-riscv64_tuxrun TIMEOUT120.13s killed by signal 15
>> SIGTERM
>>
>>
>>
>> Going through the code:
>>
>>
>>
>> On 5/22/25 5:12 AM, Jay Chang wrote:
>> > Previously, the number of PMP regions was hardcoded to 16 in QEMU.
>> > This patch replaces the fixed value with a new `pmp_regions` field,
>> > allowing platforms to configure the number of PMP regions.
>> >
>> > If no specific value is provided, the default number of PMP regions
>> > remains 16 to preserve the existing behavior.
>> >
>> > A new CPU parameter num-pmp-regions has been introduced to the QEMU
>> > command line. For example:
>> >
>> > -cpu rv64, g=true, c=true, pmp=true, num-pmp-regions=8
>> >
>> > Signed-off-by: Jay Chang
>> > Reviewed-by: Frank Chang
>> > ---
>> > target/riscv/cpu.c| 54 +--
>> > target/riscv/cpu.h| 3 +-
>> > target/riscv/cpu_cfg_fields.h.inc | 1 +
>> > target/riscv/csr.c| 5 ++-
>> > target/riscv/machine.c| 3 +-
>> > target/riscv/pmp.c| 28 ++--
>> > 6 files changed, 80 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > index 629ac37501..8e32252c11 100644
>> > --- a/target/riscv/cpu.c
>> > +++ b/target/riscv/cpu.c
>> > @@ -1117,6 +1117,7 @@ static void riscv_cpu_init(Object *obj)
>> > cpu->cfg.cbom_blocksize = 64;
>> > cpu->cfg.cbop_blocksize = 64;
>> > cpu->cfg.cboz_blocksize = 64;
>> > +cpu->cfg.pmp_regions = 16;
>>
>> pmp_regions is set to 16 in the base class, which is what we want to keep
>> the default intact. But then:
>>
>> > cpu->env.vext_ver = VEXT_VERSION_1_00_0;
>> > cpu->cfg.max_satp_mode = -1;
>> >
>> > @@ -1568,6 +1569,46 @@ static const PropertyInfo prop_pmp = {
>> > .set = prop_pmp_set,
>> > };
>> >
>> > +static void prop_num_pmp_regions_set(Object *obj, Visitor *v, const char
>> > *name,
>> > + void *opaque, Error **errp)
>> > +{
>> > +RISCVCPU *cpu = RISCV_CPU(obj);
>> > +uint8_t value;
>> > +
>> > +visit_type_uint8(v, name, &value, errp);
>> > +
>> > +if (cpu->cfg.pmp_regions != value && riscv_cpu_is_vendor(obj)) {
>> > +cpu_set_prop_err(cpu, name, errp);
>> > +return;
>> > +}
>> > +
>> > +if (cpu->env.priv_ver < PRIV_VERSION_1_12_0 && value >
>> > OLD_MAX_RISCV_PMPS) {
>> > +error_setg(errp, "Number of PMP regions exceeds maximum
>> > available");
>> > +return;
>> > +} else if (value > MAX_RISCV_PMPS) {
>> > +error_setg(errp, "Number of PMP regions exceeds maximum
>> > available");
>> > +return;
>> > +}
>> > +
>> > +cpu_option_add_user_setting(name, value);
>> > +cpu->cfg.pmp_regions = value;
>> > +}
>> > +
>> > +static void prop_num_pmp_regions_get(Object *obj, Visitor *v, const char
>> > *name,
>> > + void *opaque, Error **errp)
>> > +{
>> > +uint8_t value = RISCV_CPU(obj)->cfg.pmp_regions;
>> > +
>> > +visit_type_uint8(v, name, &value, errp);
>> > +}
>> > +
>> > +static const PropertyInfo prop_num_pmp_regions = {
>> > +.type = "uint8",
>> > +.description = "num-pmp-regions",
>> > +.get = prop_num_pmp_regions_get,
>> > +.set = prop_num_pmp_regions_set,
>> > +};
>> > +
>> > static int priv_spec_from_str(const char *priv_spec_str)
>>
Re: [PATCH v4 2/2] target/riscv: Make PMP region count configurable
*Hi Daniel,* You're absolutely right — thanks for pointing it out and providing the fix! Would you like me to send out a v5 patch incorporating your changes? Best Regards Jay Chang On Thu, Jun 5, 2025 at 2:20 AM Daniel Henrique Barboza < [email protected]> wrote: > It seems like this patch is breaking 'make check-functional': > > > >>> > UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 > RUST_BACKTRACE=1 > LD_LIBRARY_PATH=/home/danielhb/work/qemu/build/contrib/plugins:/home/danielhb/work/qemu/build/tests/tcg/plugins > MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 > QEMU_BUILD_ROOT=/home/danielhb/work/qemu/build > QEMU_TEST_QEMU_IMG=/home/danielhb/work/qemu/build/qemu-img > PYTHONPATH=/home/danielhb/work/qemu/python:/home/danielhb/work/qemu/tests/functional > G_TEST_SLOW=1 ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 > MALLOC_PERTURB_=229 MESON_TEST_ITERATION=1 > QEMU_TEST_QEMU_BINARY=/home/danielhb/work/qemu/build/qemu-system-riscv64 > /home/danielhb/work/qemu/build/pyvenv/bin/python3 > /home/danielhb/work/qemu/tests/functional/test_riscv64_tuxrun.py > > > Summary of Failures: > > 11/12 qemu:func-thorough+func-riscv32-thorough+thorough / > func-riscv32-riscv32_tuxrun TIMEOUT 90.02s killed by signal 15 > SIGTERM > 12/12 qemu:func-thorough+func-riscv64-thorough+thorough / > func-riscv64-riscv64_tuxrun TIMEOUT120.13s killed by signal 15 > SIGTERM > > > > Going through the code: > > > > On 5/22/25 5:12 AM, Jay Chang wrote: > > Previously, the number of PMP regions was hardcoded to 16 in QEMU. > > This patch replaces the fixed value with a new `pmp_regions` field, > > allowing platforms to configure the number of PMP regions. > > > > If no specific value is provided, the default number of PMP regions > > remains 16 to preserve the existing behavior. > > > > A new CPU parameter num-pmp-regions has been introduced to the QEMU > > command line. For example: > > > > -cpu rv64, g=true, c=true, pmp=true, num-pmp-regions=8 > > > > Signed-off-by: Jay Chang > > Reviewed-by: Frank Chang > > --- > > target/riscv/cpu.c| 54 +-- > > target/riscv/cpu.h| 3 +- > > target/riscv/cpu_cfg_fields.h.inc | 1 + > > target/riscv/csr.c| 5 ++- > > target/riscv/machine.c| 3 +- > > target/riscv/pmp.c| 28 ++-- > > 6 files changed, 80 insertions(+), 14 deletions(-) > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > index 629ac37501..8e32252c11 100644 > > --- a/target/riscv/cpu.c > > +++ b/target/riscv/cpu.c > > @@ -1117,6 +1117,7 @@ static void riscv_cpu_init(Object *obj) > > cpu->cfg.cbom_blocksize = 64; > > cpu->cfg.cbop_blocksize = 64; > > cpu->cfg.cboz_blocksize = 64; > > +cpu->cfg.pmp_regions = 16; > > pmp_regions is set to 16 in the base class, which is what we want to keep > the default intact. But then: > > > cpu->env.vext_ver = VEXT_VERSION_1_00_0; > > cpu->cfg.max_satp_mode = -1; > > > > @@ -1568,6 +1569,46 @@ static const PropertyInfo prop_pmp = { > > .set = prop_pmp_set, > > }; > > > > +static void prop_num_pmp_regions_set(Object *obj, Visitor *v, const > char *name, > > + void *opaque, Error **errp) > > +{ > > +RISCVCPU *cpu = RISCV_CPU(obj); > > +uint8_t value; > > + > > +visit_type_uint8(v, name, &value, errp); > > + > > +if (cpu->cfg.pmp_regions != value && riscv_cpu_is_vendor(obj)) { > > +cpu_set_prop_err(cpu, name, errp); > > +return; > > +} > > + > > +if (cpu->env.priv_ver < PRIV_VERSION_1_12_0 && value > > OLD_MAX_RISCV_PMPS) { > > +error_setg(errp, "Number of PMP regions exceeds maximum > available"); > > +return; > > +} else if (value > MAX_RISCV_PMPS) { > > +error_setg(errp, "Number of PMP regions exceeds maximum > available"); > > +return; > > +} > > + > > +cpu_option_add_user_setting(name, value); > > +cpu->cfg.pmp_regions = value; > > +} > > + > > +static void prop_num_pmp_regions_get(Object *obj, Visitor *v, const > char *name, > > + void *opaque, Error **errp) > > +{ > > +uint8_t value = RISCV_CPU(obj)->cfg.pmp_regions; > > + > > +visit_type_uint8(v, name, &value, errp); > > +} > > + > > +static const PropertyInfo prop_num_pmp_regions = { > > +.type = "uint8", > > +.description = "num-pmp-regions", > > +.get = prop_num_pmp_regions_get, > > +.set = prop_num_pmp_regions_set, > > +}; > > + > > static int priv_spec_from_str(const char *priv_spec_str) > > { > > int priv_version = -1; > > @@ -2567,6 +2608,7 @@ static const Property riscv_cpu_properties[] = { > > > > {.name = "mmu", .info = &prop_mmu}, > > {.name = "pmp", .info = &prop_pmp}, > > +{.name = "num-pmp-regions", .info = &prop_num_pmp_regions}
Re: [PATCH v4 2/2] target/riscv: Make PMP region count configurable
It seems like this patch is breaking 'make check-functional':
UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
RUST_BACKTRACE=1
LD_LIBRARY_PATH=/home/danielhb/work/qemu/build/contrib/plugins:/home/danielhb/work/qemu/build/tests/tcg/plugins
MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
QEMU_BUILD_ROOT=/home/danielhb/work/qemu/build
QEMU_TEST_QEMU_IMG=/home/danielhb/work/qemu/build/qemu-img
PYTHONPATH=/home/danielhb/work/qemu/python:/home/danielhb/work/qemu/tests/functional
G_TEST_SLOW=1 ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1
MALLOC_PERTURB_=229 MESON_TEST_ITERATION=1
QEMU_TEST_QEMU_BINARY=/home/danielhb/work/qemu/build/qemu-system-riscv64
/home/danielhb/work/qemu/build/pyvenv/bin/python3
/home/danielhb/work/qemu/tests/functional/test_riscv64_tuxrun.py
Summary of Failures:
11/12 qemu:func-thorough+func-riscv32-thorough+thorough /
func-riscv32-riscv32_tuxrun TIMEOUT 90.02s killed by signal 15 SIGTERM
12/12 qemu:func-thorough+func-riscv64-thorough+thorough /
func-riscv64-riscv64_tuxrun TIMEOUT120.13s killed by signal 15 SIGTERM
Going through the code:
On 5/22/25 5:12 AM, Jay Chang wrote:
Previously, the number of PMP regions was hardcoded to 16 in QEMU.
This patch replaces the fixed value with a new `pmp_regions` field,
allowing platforms to configure the number of PMP regions.
If no specific value is provided, the default number of PMP regions
remains 16 to preserve the existing behavior.
A new CPU parameter num-pmp-regions has been introduced to the QEMU
command line. For example:
-cpu rv64, g=true, c=true, pmp=true, num-pmp-regions=8
Signed-off-by: Jay Chang
Reviewed-by: Frank Chang
---
target/riscv/cpu.c| 54 +--
target/riscv/cpu.h| 3 +-
target/riscv/cpu_cfg_fields.h.inc | 1 +
target/riscv/csr.c| 5 ++-
target/riscv/machine.c| 3 +-
target/riscv/pmp.c| 28 ++--
6 files changed, 80 insertions(+), 14 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 629ac37501..8e32252c11 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1117,6 +1117,7 @@ static void riscv_cpu_init(Object *obj)
cpu->cfg.cbom_blocksize = 64;
cpu->cfg.cbop_blocksize = 64;
cpu->cfg.cboz_blocksize = 64;
+cpu->cfg.pmp_regions = 16;
pmp_regions is set to 16 in the base class, which is what we want to keep
the default intact. But then:
cpu->env.vext_ver = VEXT_VERSION_1_00_0;
cpu->cfg.max_satp_mode = -1;
@@ -1568,6 +1569,46 @@ static const PropertyInfo prop_pmp = {
.set = prop_pmp_set,
};
+static void prop_num_pmp_regions_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPU *cpu = RISCV_CPU(obj);
+uint8_t value;
+
+visit_type_uint8(v, name, &value, errp);
+
+if (cpu->cfg.pmp_regions != value && riscv_cpu_is_vendor(obj)) {
+cpu_set_prop_err(cpu, name, errp);
+return;
+}
+
+if (cpu->env.priv_ver < PRIV_VERSION_1_12_0 && value > OLD_MAX_RISCV_PMPS)
{
+error_setg(errp, "Number of PMP regions exceeds maximum available");
+return;
+} else if (value > MAX_RISCV_PMPS) {
+error_setg(errp, "Number of PMP regions exceeds maximum available");
+return;
+}
+
+cpu_option_add_user_setting(name, value);
+cpu->cfg.pmp_regions = value;
+}
+
+static void prop_num_pmp_regions_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+uint8_t value = RISCV_CPU(obj)->cfg.pmp_regions;
+
+visit_type_uint8(v, name, &value, errp);
+}
+
+static const PropertyInfo prop_num_pmp_regions = {
+.type = "uint8",
+.description = "num-pmp-regions",
+.get = prop_num_pmp_regions_get,
+.set = prop_num_pmp_regions_set,
+};
+
static int priv_spec_from_str(const char *priv_spec_str)
{
int priv_version = -1;
@@ -2567,6 +2608,7 @@ static const Property riscv_cpu_properties[] = {
{.name = "mmu", .info = &prop_mmu},
{.name = "pmp", .info = &prop_pmp},
+{.name = "num-pmp-regions", .info = &prop_num_pmp_regions},
{.name = "priv_spec", .info = &prop_priv_spec},
{.name = "vext_spec", .info = &prop_vext_spec},
@@ -2891,6 +2933,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
DEFINE_ABSTRACT_RISCV_CPU(TYPE_RISCV_DYNAMIC_CPU, TYPE_RISCV_CPU,
.cfg.mmu = true,
.cfg.pmp = true,
+.cfg.pmp_regions = 8,
.priv_spec = PRIV_VERSION_LATEST,
),
@@ -2937,7 +2980,8 @@ static const TypeInfo riscv_cpu_type_infos[] = {
.cfg.max_satp_mode = VM_1_10_MBARE,
.cfg.ext_zifencei = true,
.cfg.ext_zicsr = true,
-.cfg.pmp = true
+.cfg.pmp = true,
+.cfg.pmp_regions =
Re: [PATCH v4 2/2] target/riscv: Make PMP region count configurable
On Thu, May 22, 2025 at 6:14 PM Jay Chang wrote:
>
> Previously, the number of PMP regions was hardcoded to 16 in QEMU.
> This patch replaces the fixed value with a new `pmp_regions` field,
> allowing platforms to configure the number of PMP regions.
>
> If no specific value is provided, the default number of PMP regions
> remains 16 to preserve the existing behavior.
>
> A new CPU parameter num-pmp-regions has been introduced to the QEMU
> command line. For example:
>
> -cpu rv64, g=true, c=true, pmp=true, num-pmp-regions=8
>
> Signed-off-by: Jay Chang
> Reviewed-by: Frank Chang
Reviewed-by: Alistair Francis
Alistair
> ---
> target/riscv/cpu.c| 54 +--
> target/riscv/cpu.h| 3 +-
> target/riscv/cpu_cfg_fields.h.inc | 1 +
> target/riscv/csr.c| 5 ++-
> target/riscv/machine.c| 3 +-
> target/riscv/pmp.c| 28 ++--
> 6 files changed, 80 insertions(+), 14 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 629ac37501..8e32252c11 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1117,6 +1117,7 @@ static void riscv_cpu_init(Object *obj)
> cpu->cfg.cbom_blocksize = 64;
> cpu->cfg.cbop_blocksize = 64;
> cpu->cfg.cboz_blocksize = 64;
> +cpu->cfg.pmp_regions = 16;
> cpu->env.vext_ver = VEXT_VERSION_1_00_0;
> cpu->cfg.max_satp_mode = -1;
>
> @@ -1568,6 +1569,46 @@ static const PropertyInfo prop_pmp = {
> .set = prop_pmp_set,
> };
>
> +static void prop_num_pmp_regions_set(Object *obj, Visitor *v, const char
> *name,
> + void *opaque, Error **errp)
> +{
> +RISCVCPU *cpu = RISCV_CPU(obj);
> +uint8_t value;
> +
> +visit_type_uint8(v, name, &value, errp);
> +
> +if (cpu->cfg.pmp_regions != value && riscv_cpu_is_vendor(obj)) {
> +cpu_set_prop_err(cpu, name, errp);
> +return;
> +}
> +
> +if (cpu->env.priv_ver < PRIV_VERSION_1_12_0 && value >
> OLD_MAX_RISCV_PMPS) {
> +error_setg(errp, "Number of PMP regions exceeds maximum available");
> +return;
> +} else if (value > MAX_RISCV_PMPS) {
> +error_setg(errp, "Number of PMP regions exceeds maximum available");
> +return;
> +}
> +
> +cpu_option_add_user_setting(name, value);
> +cpu->cfg.pmp_regions = value;
> +}
> +
> +static void prop_num_pmp_regions_get(Object *obj, Visitor *v, const char
> *name,
> + void *opaque, Error **errp)
> +{
> +uint8_t value = RISCV_CPU(obj)->cfg.pmp_regions;
> +
> +visit_type_uint8(v, name, &value, errp);
> +}
> +
> +static const PropertyInfo prop_num_pmp_regions = {
> +.type = "uint8",
> +.description = "num-pmp-regions",
> +.get = prop_num_pmp_regions_get,
> +.set = prop_num_pmp_regions_set,
> +};
> +
> static int priv_spec_from_str(const char *priv_spec_str)
> {
> int priv_version = -1;
> @@ -2567,6 +2608,7 @@ static const Property riscv_cpu_properties[] = {
>
> {.name = "mmu", .info = &prop_mmu},
> {.name = "pmp", .info = &prop_pmp},
> +{.name = "num-pmp-regions", .info = &prop_num_pmp_regions},
>
> {.name = "priv_spec", .info = &prop_priv_spec},
> {.name = "vext_spec", .info = &prop_vext_spec},
> @@ -2891,6 +2933,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> DEFINE_ABSTRACT_RISCV_CPU(TYPE_RISCV_DYNAMIC_CPU, TYPE_RISCV_CPU,
> .cfg.mmu = true,
> .cfg.pmp = true,
> +.cfg.pmp_regions = 8,
> .priv_spec = PRIV_VERSION_LATEST,
> ),
>
> @@ -2937,7 +2980,8 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> .cfg.max_satp_mode = VM_1_10_MBARE,
> .cfg.ext_zifencei = true,
> .cfg.ext_zicsr = true,
> -.cfg.pmp = true
> +.cfg.pmp = true,
> +.cfg.pmp_regions = 8
> ),
>
> DEFINE_ABSTRACT_RISCV_CPU(TYPE_RISCV_CPU_SIFIVE_U, TYPE_RISCV_VENDOR_CPU,
> @@ -2948,7 +2992,8 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> .cfg.ext_zifencei = true,
> .cfg.ext_zicsr = true,
> .cfg.mmu = true,
> -.cfg.pmp = true
> +.cfg.pmp = true,
> +.cfg.pmp_regions = 8
> ),
>
> #if defined(TARGET_RISCV32) || \
> @@ -2966,6 +3011,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> .cfg.ext_zifencei = true,
> .cfg.ext_zicsr = true,
> .cfg.pmp = true,
> +.cfg.pmp_regions = 8,
> .cfg.ext_smepmp = true,
>
> .cfg.ext_zba = true,
> @@ -3040,6 +3086,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> .cfg.ext_xtheadmempair = true,
> .cfg.ext_xtheadsync = true,
> .cfg.pmp = true,
> +.cfg.pmp_regions = 8,
>
> .cfg.mvendorid = THEAD_VENDOR_ID,
>
> @@ -3063,6 +3110,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> .cfg.rvv_ta_all_1s = true,
> .cfg.misa_w = true,
>
