[PATCH v1 0/4] Fix leaks reached by make check-qtest

[PATCH v1 0/4] Fix leaks reached by make check-qtest

[Fabiano Rosas]

../configure --enable-asan --enable-ubsan \
	     --target-list=\
	     x86_64-softmmu,i386-softmmu,\
	     aarch64-softmmu,arm-softmmu,\
	     ppc64-softmmu,\
	     s390x-softmmu,\
	     riscv64-softmmu,\
	     loongarch64-softmmu,\
	     sparc-softmmu,sparc64-softmmu

make -j$(nproc) TIMEOUT_MULTIPLIER=2 check-qtest-$TARGET

(running all targets at once triggers timeouts even with 3x
multiplier, bios-tables-test also has it's own timeout of 600s at
qtest/acpi-utils.c)

Tested on top of the migration-test fixes:
https://lore.kernel.org/r/20260311213418.16951-1-farosas@suse.de

CI run on top of master:
https://gitlab.com/farosas/qemu/-/pipelines/2383870717

Fabiano Rosas (4):
  tests/qtest: Don't dup machine name in qtest_cb_for_every_machine
    callbacks
  tests/qtest/test-hmp: Free machine options
  target/riscv: Don't leak general_user_opts
  hw/riscv: Free resources allocated at riscv_iommu_instance_init

 hw/riscv/riscv-iommu.c      | 16 +++++++++++++---
 target/riscv/cpu.c          |  3 +--
 tests/qtest/cpu-plug-test.c | 11 +++++------
 tests/qtest/qom-test.c      |  3 +--
 tests/qtest/test-hmp.c      |  6 +++---
 5 files changed, 23 insertions(+), 16 deletions(-)

-- 
2.51.0

[PATCH v1 1/4] tests/qtest: Don't dup machine name in qtest_cb_for_every_machine callbacks

[Fabiano Rosas]

The qtest_get_machines function caches the list of machines in a
static variable. Dup'ing the machine->name string only serves to leak
that memory when a single test is executed.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/cpu-plug-test.c | 11 +++++------
 tests/qtest/qom-test.c      |  3 +--
 tests/qtest/test-hmp.c      |  3 +--
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/tests/qtest/cpu-plug-test.c b/tests/qtest/cpu-plug-test.c
index 0aa4ccc5b6..e68f39cf56 100644
--- a/tests/qtest/cpu-plug-test.c
+++ b/tests/qtest/cpu-plug-test.c
@@ -14,7 +14,7 @@
 #include "qobject/qlist.h"
 
 struct PlugTestData {
-    char *machine;
+    const char *machine;
     const char *cpu_model;
     char *device_model;
     unsigned sockets;
@@ -73,7 +73,6 @@ static void test_data_free(gpointer data)
 {
     PlugTestData *pc = data;
 
-    g_free(pc->machine);
     g_free(pc->device_model);
     g_free(pc);
 }
@@ -87,7 +86,7 @@ static void add_pc_test_case(const char *mname)
         return;
     }
     data = g_new(PlugTestData, 1);
-    data->machine = g_strdup(mname);
+    data->machine = mname;
     data->cpu_model = "Haswell"; /* 1.3+ theoretically */
     data->device_model = g_strdup_printf("%s-%s-cpu", data->cpu_model,
                                          qtest_get_arch());
@@ -114,7 +113,7 @@ static void add_pseries_test_case(const char *mname)
         return;
     }
     data = g_new(PlugTestData, 1);
-    data->machine = g_strdup(mname);
+    data->machine = mname;
     data->cpu_model = "power8_v2.0";
     data->device_model = g_strdup("power8_v2.0-spapr-cpu-core");
     data->sockets = 2;
@@ -140,7 +139,7 @@ static void add_s390x_test_case(const char *mname)
     }
 
     data = g_new(PlugTestData, 1);
-    data->machine = g_strdup(mname);
+    data->machine = mname;
     data->cpu_model = "qemu";
     data->device_model = g_strdup("qemu-s390x-cpu");
     data->sockets = 1;
@@ -162,7 +161,7 @@ static void add_loongarch_test_case(const char *mname)
     PlugTestData *data;
 
     data = g_new(PlugTestData, 1);
-    data->machine = g_strdup(mname);
+    data->machine = mname;
     data->cpu_model = "la464";
     data->device_model = g_strdup("la464-loongarch-cpu");
     data->sockets = 1;
diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
index 2da9918e16..6421f2d9d9 100644
--- a/tests/qtest/qom-test.c
+++ b/tests/qtest/qom-test.c
@@ -216,7 +216,6 @@ static void test_machine(gconstpointer data)
     test_list_get_value(qts);
 
     qtest_quit(qts);
-    g_free((void *)machine);
 }
 
 static void add_machine_test_case(const char *mname)
@@ -224,7 +223,7 @@ static void add_machine_test_case(const char *mname)
     char *path;
 
     path = g_strdup_printf("qom/%s", mname);
-    qtest_add_data_func(path, g_strdup(mname), test_machine);
+    qtest_add_data_func(path, mname, test_machine);
     g_free(path);
 }
 
diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c
index 1b2e07522f..60f742e83f 100644
--- a/tests/qtest/test-hmp.c
+++ b/tests/qtest/test-hmp.c
@@ -135,7 +135,6 @@ static void test_machine(gconstpointer data)
 
     qtest_quit(qts);
     g_free(args);
-    g_free((void *)data);
 }
 
 static void add_machine_test_case(const char *mname)
@@ -143,7 +142,7 @@ static void add_machine_test_case(const char *mname)
     char *path;
 
     path = g_strdup_printf("hmp/%s", mname);
-    qtest_add_data_func(path, g_strdup(mname), test_machine);
+    qtest_add_data_func(path, mname, test_machine);
     g_free(path);
 }
 
-- 
2.51.0

[PATCH v1 2/4] tests/qtest/test-hmp: Free machine options

[Fabiano Rosas]

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/test-hmp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c
index 60f742e83f..a523dbb446 100644
--- a/tests/qtest/test-hmp.c
+++ b/tests/qtest/test-hmp.c
@@ -149,6 +149,7 @@ static void add_machine_test_case(const char *mname)
 int main(int argc, char **argv)
 {
     char *v_env = getenv("V");
+    g_autofree char *machine_opts = g_strdup("none -m 2");
 
     if (v_env && atoi(v_env) >= 2) {
         verbose = true;
@@ -159,7 +160,7 @@ int main(int argc, char **argv)
     qtest_cb_for_every_machine(add_machine_test_case, g_test_quick());
 
     /* as none machine has no memory by default, add a test case with memory */
-    qtest_add_data_func("hmp/none+2MB", g_strdup("none -m 2"), test_machine);
+    qtest_add_data_func("hmp/none+2MB", machine_opts, test_machine);
 
     return g_test_run();
 }
-- 
2.51.0

[PATCH v1 3/4] target/riscv: Don't leak general_user_opts

[Fabiano Rosas]

The global variable general_user_opts is being assigned twice, losing
the reference to the previously allocated g_hash_table.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 target/riscv/cpu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e56470a374..afccfc2935 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1099,8 +1099,6 @@ static void riscv_cpu_init(Object *obj)
                             "riscv.cpu.rnmi", RNMI_MAX);
 #endif /* CONFIG_USER_ONLY */
 
-    general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
-
     /*
      * The timer and performance counters extensions were supported
      * in QEMU before they were added as discrete extensions in the
@@ -2751,6 +2749,7 @@ static void riscv_cpu_common_class_init(ObjectClass *c, const void *data)
     cc->tcg_ops = &riscv_tcg_ops;
 #endif /* CONFIG_TCG */
 
+    general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
     device_class_set_props(dc, riscv_cpu_properties);
 }
 
-- 
2.51.0

[PATCH v1 4/4] hw/riscv: Free resources allocated at riscv_iommu_instance_init

[Fabiano Rosas]

Add a riscv_iommu_instance_finalize() routine and move there the unref
of the iot_cache and ctx_cache hash tables from riscv_iommu_unrealize.

Also start freeing the register state masks, also allocated at the
_init function.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 hw/riscv/riscv-iommu.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index 98345b1280..a9cd891039 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -2446,6 +2446,18 @@ void riscv_iommu_set_cap_igs(RISCVIOMMUState *s, riscv_iommu_igs_mode mode)
     s->cap = set_field(s->cap, RISCV_IOMMU_CAP_IGS, mode);
 }
 
+static void riscv_iommu_instance_finalize(Object *obj)
+{
+    RISCVIOMMUState *s = RISCV_IOMMU(obj);
+
+    g_hash_table_unref(s->iot_cache);
+    g_hash_table_unref(s->ctx_cache);
+
+    g_free(s->regs_rw);
+    g_free(s->regs_ro);
+    g_free(s->regs_wc);
+}
+
 static void riscv_iommu_instance_init(Object *obj)
 {
     RISCVIOMMUState *s = RISCV_IOMMU(obj);
@@ -2597,9 +2609,6 @@ static void riscv_iommu_unrealize(DeviceState *dev)
 {
     RISCVIOMMUState *s = RISCV_IOMMU(dev);
 
-    g_hash_table_unref(s->iot_cache);
-    g_hash_table_unref(s->ctx_cache);
-
     if (s->cap & RISCV_IOMMU_CAP_HPM) {
         g_hash_table_unref(s->hpm_event_ctr_map);
         timer_free(s->hpm_timer);
@@ -2675,6 +2684,7 @@ static const TypeInfo riscv_iommu_info = {
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(RISCVIOMMUState),
     .instance_init = riscv_iommu_instance_init,
+    .instance_finalize = riscv_iommu_instance_finalize,
     .class_init = riscv_iommu_class_init,
 };
 
-- 
2.51.0

Re: [PATCH v1 3/4] target/riscv: Don't leak general_user_opts

[Daniel Henrique Barboza]

On 3/13/2026 3:29 PM, Fabiano Rosas wrote:
> The global variable general_user_opts is being assigned twice, losing
> the reference to the previously allocated g_hash_table.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/cpu.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index e56470a374..afccfc2935 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1099,8 +1099,6 @@ static void riscv_cpu_init(Object *obj)
>                               "riscv.cpu.rnmi", RNMI_MAX);
>   #endif /* CONFIG_USER_ONLY */
>   
> -    general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
> -
>       /*
>        * The timer and performance counters extensions were supported
>        * in QEMU before they were added as discrete extensions in the
> @@ -2751,6 +2749,7 @@ static void riscv_cpu_common_class_init(ObjectClass *c, const void *data)
>       cc->tcg_ops = &riscv_tcg_ops;
>   #endif /* CONFIG_TCG */
>   
> +    general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
>       device_class_set_props(dc, riscv_cpu_properties);
>   }
>   

Re: [PATCH v1 4/4] hw/riscv: Free resources allocated at riscv_iommu_instance_init

[Daniel Henrique Barboza]

On 3/13/2026 3:29 PM, Fabiano Rosas wrote:
> Add a riscv_iommu_instance_finalize() routine and move there the unref
> of the iot_cache and ctx_cache hash tables from riscv_iommu_unrealize.
> 
> Also start freeing the register state masks, also allocated at the
> _init function.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   hw/riscv/riscv-iommu.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index 98345b1280..a9cd891039 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -2446,6 +2446,18 @@ void riscv_iommu_set_cap_igs(RISCVIOMMUState *s, riscv_iommu_igs_mode mode)
>       s->cap = set_field(s->cap, RISCV_IOMMU_CAP_IGS, mode);
>   }
>   
> +static void riscv_iommu_instance_finalize(Object *obj)
> +{
> +    RISCVIOMMUState *s = RISCV_IOMMU(obj);
> +
> +    g_hash_table_unref(s->iot_cache);
> +    g_hash_table_unref(s->ctx_cache);
> +
> +    g_free(s->regs_rw);
> +    g_free(s->regs_ro);
> +    g_free(s->regs_wc);
> +}
> +
>   static void riscv_iommu_instance_init(Object *obj)
>   {
>       RISCVIOMMUState *s = RISCV_IOMMU(obj);
> @@ -2597,9 +2609,6 @@ static void riscv_iommu_unrealize(DeviceState *dev)
>   {
>       RISCVIOMMUState *s = RISCV_IOMMU(dev);
>   
> -    g_hash_table_unref(s->iot_cache);
> -    g_hash_table_unref(s->ctx_cache);
> -
>       if (s->cap & RISCV_IOMMU_CAP_HPM) {
>           g_hash_table_unref(s->hpm_event_ctr_map);
>           timer_free(s->hpm_timer);
> @@ -2675,6 +2684,7 @@ static const TypeInfo riscv_iommu_info = {
>       .parent = TYPE_DEVICE,
>       .instance_size = sizeof(RISCVIOMMUState),
>       .instance_init = riscv_iommu_instance_init,
> +    .instance_finalize = riscv_iommu_instance_finalize,
>       .class_init = riscv_iommu_class_init,
>   };
>   

Re: [PATCH v1 1/4] tests/qtest: Don't dup machine name in qtest_cb_for_every_machine callbacks

[Philippe Mathieu-Daudé]

On 13/3/26 19:29, Fabiano Rosas wrote:
> The qtest_get_machines function caches the list of machines in a
> static variable. Dup'ing the machine->name string only serves to leak
> that memory when a single test is executed.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   tests/qtest/cpu-plug-test.c | 11 +++++------
>   tests/qtest/qom-test.c      |  3 +--
>   tests/qtest/test-hmp.c      |  3 +--
>   3 files changed, 7 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v1 2/4] tests/qtest/test-hmp: Free machine options

[Philippe Mathieu-Daudé]

On 13/3/26 19:29, Fabiano Rosas wrote:
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   tests/qtest/test-hmp.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v1 4/4] hw/riscv: Free resources allocated at riscv_iommu_instance_init

[Philippe Mathieu-Daudé]

On 13/3/26 19:29, Fabiano Rosas wrote:
> Add a riscv_iommu_instance_finalize() routine and move there the unref
> of the iot_cache and ctx_cache hash tables from riscv_iommu_unrealize.
> 
> Also start freeing the register state masks, also allocated at the
> _init function.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   hw/riscv/riscv-iommu.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v1 3/4] target/riscv: Don't leak general_user_opts

[Peter Maydell]

On Fri, 13 Mar 2026 at 18:30, Fabiano Rosas <farosas@suse.de> wrote:
>
> The global variable general_user_opts is being assigned twice, losing
> the reference to the previously allocated g_hash_table.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  target/riscv/cpu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index e56470a374..afccfc2935 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1099,8 +1099,6 @@ static void riscv_cpu_init(Object *obj)
>                              "riscv.cpu.rnmi", RNMI_MAX);
>  #endif /* CONFIG_USER_ONLY */
>
> -    general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
> -
>      /*
>       * The timer and performance counters extensions were supported
>       * in QEMU before they were added as discrete extensions in the
> @@ -2751,6 +2749,7 @@ static void riscv_cpu_common_class_init(ObjectClass *c, const void *data)
>      cc->tcg_ops = &riscv_tcg_ops;
>  #endif /* CONFIG_TCG */
>
> +    general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
>      device_class_set_props(dc, riscv_cpu_properties);
>  }
>

This doesn't look to me like it's sufficient. These hash
tables are used to store the values of CPU properties.
Those are per-object, not per-class, so surely the hash
table needs to be in the object, not a global ?

There are also other hash tables in the TCG riscv code
with the same problem (using a global variable to store
property info that is per-CPU, not global).

thanks
-- PMM

Re: [PATCH v1 4/4] hw/riscv: Free resources allocated at riscv_iommu_instance_init

[Peter Maydell]

On Fri, 13 Mar 2026 at 18:30, Fabiano Rosas <farosas@suse.de> wrote:
>
> Add a riscv_iommu_instance_finalize() routine and move there the unref
> of the iot_cache and ctx_cache hash tables from riscv_iommu_unrealize.
>
> Also start freeing the register state masks, also allocated at the
> _init function.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

This is the same as
https://patchew.org/QEMU/20260307125222.3656140-1-peter.maydell@linaro.org/
which I sent last week.

thanks
-- PMM

Re: [PATCH v1 3/4] target/riscv: Don't leak general_user_opts

[Daniel Henrique Barboza]

On 3/16/2026 6:56 AM, Peter Maydell wrote:
> On Fri, 13 Mar 2026 at 18:30, Fabiano Rosas <farosas@suse.de> wrote:
>>
>> The global variable general_user_opts is being assigned twice, losing
>> the reference to the previously allocated g_hash_table.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>   target/riscv/cpu.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index e56470a374..afccfc2935 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -1099,8 +1099,6 @@ static void riscv_cpu_init(Object *obj)
>>                               "riscv.cpu.rnmi", RNMI_MAX);
>>   #endif /* CONFIG_USER_ONLY */
>>
>> -    general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
>> -
>>       /*
>>        * The timer and performance counters extensions were supported
>>        * in QEMU before they were added as discrete extensions in the
>> @@ -2751,6 +2749,7 @@ static void riscv_cpu_common_class_init(ObjectClass *c, const void *data)
>>       cc->tcg_ops = &riscv_tcg_ops;
>>   #endif /* CONFIG_TCG */
>>
>> +    general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
>>       device_class_set_props(dc, riscv_cpu_properties);
>>   }
>>
> 
> This doesn't look to me like it's sufficient. These hash
> tables are used to store the values of CPU properties.
> Those are per-object, not per-class, so surely the hash
> table needs to be in the object, not a global ?


This hash is used as a hack to determine if a QEMU command line option was
user set or not (i.e. a global setting).  Back when this was introduced
(and I assume this is still the case today, need to double check) the command
line support in QEMU didn't provide this information.

Most QEMU targets don't care whether a given CPU property was user set or not,
but the RISC-V target has a lot of code that automagically disables stuff given
certain conditions (e.g. riscv_cpu_disable_priv_spec_isa_exts()).  The result is
that we were reaching situations, Gitlab bugs being opened and what have you,
where the user was doing things like this:

qemu-system..... -cpu X,extensionA=on (...)


But so it happens that extensionA isn't compatible with the existing extension set
in X.  The behavior back then was to disable extensionA during realize() and be
done with it, leaving users baffled and frustrated with their cmd line settings
being ignored.  Now we're able to error out in these cases.

All this said, most likely there's a better/cleaner way of storing this command
line information that has global effect.  Just point me in the right direction and
we can make it happen for the next release.


> 
> There are also other hash tables in the TCG riscv code
> with the same problem (using a global variable to store
> property info that is per-CPU, not global).


Not sure which hashes are you talking about but there's a high chance that they
have to do with tracking QEMU command line options that were set or not and we
want to know the difference.

In hindsight I should've sent a RFC detailing this situation we have in RISC-V
and maybe we could have discussed changes in the QEMU cmd line handling to provide
this info.  I'm willing to do that route if that means less hash table stuff to
deal with in RISC-V code.


Thanks,
Daniel


> 
> thanks
> -- PMM

Re: [PATCH v1 3/4] target/riscv: Don't leak general_user_opts

[Peter Maydell]

On Mon, 16 Mar 2026 at 10:42, Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
> On 3/16/2026 6:56 AM, Peter Maydell wrote:
> > This doesn't look to me like it's sufficient. These hash
> > tables are used to store the values of CPU properties.
> > Those are per-object, not per-class, so surely the hash
> > table needs to be in the object, not a global ?
>
>
> This hash is used as a hack to determine if a QEMU command line option was
> user set or not (i.e. a global setting).  Back when this was introduced
> (and I assume this is still the case today, need to double check) the command
> line support in QEMU didn't provide this information.
>
> Most QEMU targets don't care whether a given CPU property was user set or not,
> but the RISC-V target has a lot of code that automagically disables stuff given
> certain conditions (e.g. riscv_cpu_disable_priv_spec_isa_exts()).  The result is
> that we were reaching situations, Gitlab bugs being opened and what have you,
> where the user was doing things like this:
>
> qemu-system..... -cpu X,extensionA=on (...)
>
>
> But so it happens that extensionA isn't compatible with the existing extension set
> in X.  The behavior back then was to disable extensionA during realize() and be
> done with it, leaving users baffled and frustrated with their cmd line settings
> being ignored.  Now we're able to error out in these cases.

OK, but why does this require the hash table to be global? This sounds
like it's a per-CPU thing: if I have two riscv CPUs and one is:
 cpu 0: has extension X
 cpu 1: has extension A, not extension X

that's fine -- A conflicts with X but we don't have both of them
in the same CPU. With a global hash table we have no way to distinguish
this from "one CPU with both A and X".

I would have thought we only want to error out if the user sets up
a single CPU with a conflicting set of extensions.

I'm also not sure why you need to care whether the property is set
by the user on the command line or by the board code with C code.
If a board or SoC model tries to create a CPU that has both extensions
X and A, shouldn't that also be an error that we want to bail out on?

> > There are also other hash tables in the TCG riscv code
> > with the same problem (using a global variable to store
> > property info that is per-CPU, not global).
>
>
> Not sure which hashes are you talking about but there's a high chance that they
> have to do with tracking QEMU command line options that were set or not and we
> want to know the difference.

riscv_tcg_cpu_instance_init() calls g_hash_table_new() for
misa_ext_user_opts and multi_ext_user_opts unconditionally.

thanks
-- PMM

Re: [PATCH v1 3/4] target/riscv: Don't leak general_user_opts

[Daniel Henrique Barboza]

On 3/16/2026 7:49 AM, Peter Maydell wrote:
> On Mon, 16 Mar 2026 at 10:42, Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>> On 3/16/2026 6:56 AM, Peter Maydell wrote:
>>> This doesn't look to me like it's sufficient. These hash
>>> tables are used to store the values of CPU properties.
>>> Those are per-object, not per-class, so surely the hash
>>> table needs to be in the object, not a global ?
>>
>>
>> This hash is used as a hack to determine if a QEMU command line option was
>> user set or not (i.e. a global setting).  Back when this was introduced
>> (and I assume this is still the case today, need to double check) the command
>> line support in QEMU didn't provide this information.
>>
>> Most QEMU targets don't care whether a given CPU property was user set or not,
>> but the RISC-V target has a lot of code that automagically disables stuff given
>> certain conditions (e.g. riscv_cpu_disable_priv_spec_isa_exts()).  The result is
>> that we were reaching situations, Gitlab bugs being opened and what have you,
>> where the user was doing things like this:
>>
>> qemu-system..... -cpu X,extensionA=on (...)
>>
>>
>> But so it happens that extensionA isn't compatible with the existing extension set
>> in X.  The behavior back then was to disable extensionA during realize() and be
>> done with it, leaving users baffled and frustrated with their cmd line settings
>> being ignored.  Now we're able to error out in these cases.
> 
> OK, but why does this require the hash table to be global? This sounds
> like it's a per-CPU thing: if I have two riscv CPUs and one is:
>   cpu 0: has extension X
>   cpu 1: has extension A, not extension X
> 
> that's fine -- A conflicts with X but we don't have both of them
> in the same CPU. With a global hash table we have no way to distinguish
> this from "one CPU with both A and X".
> 
> I would have thought we only want to error out if the user sets up
> a single CPU with a conflicting set of extensions.


In light of Phil's efforts w.r.t single binary QEMU, hybrid targets and etc, I
agree that this hash table should be an object instance so each CPU can have
their own extension set.  Back then the scenario you described wasn't something
we were concerning with, thus create multiple instance hashes with the same content
(since all CPUs would always be the same) seemed too much.


> 
> I'm also not sure why you need to care whether the property is set
> by the user on the command line or by the board code with C code.
> If a board or SoC model tries to create a CPU that has both extensions
> X and A, shouldn't that also be an error that we want to bail out on?

We have too much code that enable/disable extensions and chained dependencies
under the hood.

Let's say you have "-cpu rv64,extA=off,extX=on", i.e. disable A and enable X.
But X has a dependency of Y, which has a dependency of Z, and Z requires A
enabled.

So now we're on realize(), see extA=off, we disable A.  Then we go to X, figure
out that we need to enable Y because of it, but then we need to enable Z, oh and we
need to enable A as well.

In the end we ended up enabling A because it's a 3 hop dependency of Z, and now the
user is confused because he demanded A disabled.  And why wouldn't we enable A?
The default state of extensions in TCG is off, thus there was nothing to tell us if
that particular extA=off was something that the user demanded or not.

And this is how the hash table to track user opts business started. Keep in mind that
we have 148 extensions (and counting!) to deal with.  Users aren't able to track these
dependencies and get frustrated when QEMU doesn't do what it is told, and they were
going to Gitlab letting us all know about it.

Maybe the right course of action would be to claim that the user command line was bogus,
QEMU did nothing wrong and call it a day.  And maybe, now that we have profile CPUs and
people care mostly about them, we could remove all this stuff from the code and let
users fall in their faces with bogus cmd lines.  I have plans to simplify some things
(mostly the distinction between CPUs types) and might touch on this area too.  But
for now this is where we are.


Thanks,
Daniel





> 
>>> There are also other hash tables in the TCG riscv code
>>> with the same problem (using a global variable to store
>>> property info that is per-CPU, not global).
>>
>>
>> Not sure which hashes are you talking about but there's a high chance that they
>> have to do with tracking QEMU command line options that were set or not and we
>> want to know the difference.
> 
> riscv_tcg_cpu_instance_init() calls g_hash_table_new() for
> misa_ext_user_opts and multi_ext_user_opts unconditionally.
> 
> thanks
> -- PMM

Re: [PATCH v1 4/4] hw/riscv: Free resources allocated at riscv_iommu_instance_init

[Fabiano Rosas]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 13 Mar 2026 at 18:30, Fabiano Rosas <farosas@suse.de> wrote:
>>
>> Add a riscv_iommu_instance_finalize() routine and move there the unref
>> of the iot_cache and ctx_cache hash tables from riscv_iommu_unrealize.
>>
>> Also start freeing the register state masks, also allocated at the
>> _init function.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> This is the same as
> https://patchew.org/QEMU/20260307125222.3656140-1-peter.maydell@linaro.org/
> which I sent last week.
>

Ah ok, I missed it. Let's drop this one then.

> thanks
> -- PMM