qtest_set_command_cb passed to g_once should match GThreadFunc,
which it does not.  But using g_once is actually unnecessary,
because the function is called by riscv_harts_realize() under
the Big QEMU Lock.

Reported-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/riscv/riscv_hart.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index a55d1566687..bb9104bae0b 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -104,8 +104,11 @@ static bool csr_qtest_callback(CharBackend *chr, gchar **words)
 
 static void riscv_cpu_register_csr_qtest_callback(void)
 {
-    static GOnce once;
-    g_once(&once, (GThreadFunc)qtest_set_command_cb, csr_qtest_callback);
+    static bool first = true;
+    if (first) {
+        first = false;
+        qtest_set_command_cb(csr_qtest_callback);
+    }
 }
 #endif
 
-- 
2.49.0

On Fri, Apr 11, 2025 at 2:19 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> qtest_set_command_cb passed to g_once should match GThreadFunc,
> which it does not.  But using g_once is actually unnecessary,
> because the function is called by riscv_harts_realize() under
> the Big QEMU Lock.
>
> Reported-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  hw/riscv/riscv_hart.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index a55d1566687..bb9104bae0b 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -104,8 +104,11 @@ static bool csr_qtest_callback(CharBackend *chr, gchar **words)
>
>  static void riscv_cpu_register_csr_qtest_callback(void)
>  {
> -    static GOnce once;
> -    g_once(&once, (GThreadFunc)qtest_set_command_cb, csr_qtest_callback);
> +    static bool first = true;
> +    if (first) {
> +        first = false;
> +        qtest_set_command_cb(csr_qtest_callback);
> +    }
>  }
>  #endif
>
> --
> 2.49.0
>
>

On Fri, Apr 11, 2025 at 2:19 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> qtest_set_command_cb passed to g_once should match GThreadFunc,
> which it does not.  But using g_once is actually unnecessary,
> because the function is called by riscv_harts_realize() under
> the Big QEMU Lock.
>
> Reported-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/riscv_hart.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index a55d1566687..bb9104bae0b 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -104,8 +104,11 @@ static bool csr_qtest_callback(CharBackend *chr, gchar **words)
>
>  static void riscv_cpu_register_csr_qtest_callback(void)
>  {
> -    static GOnce once;
> -    g_once(&once, (GThreadFunc)qtest_set_command_cb, csr_qtest_callback);
> +    static bool first = true;
> +    if (first) {
> +        first = false;
> +        qtest_set_command_cb(csr_qtest_callback);
> +    }
>  }
>  #endif
>
> --
> 2.49.0
>
>

On 10/4/25 18:17, Paolo Bonzini wrote:
> qtest_set_command_cb passed to g_once should match GThreadFunc,
> which it does not.  But using g_once is actually unnecessary,
> because the function is called by riscv_harts_realize() under
> the Big QEMU Lock.
> 
> Reported-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/riscv/riscv_hart.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index a55d1566687..bb9104bae0b 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -104,8 +104,11 @@ static bool csr_qtest_callback(CharBackend *chr, gchar **words)
>   
>   static void riscv_cpu_register_csr_qtest_callback(void)
>   {
> -    static GOnce once;
> -    g_once(&once, (GThreadFunc)qtest_set_command_cb, csr_qtest_callback);
> +    static bool first = true;

Preferably using 'qtest_cb_registered' boolean name,

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

> +    if (first) {
> +        first = false;
> +        qtest_set_command_cb(csr_qtest_callback);
> +    }
>   }
>   #endif
>   

Hi Paolo,
I appreciate you addressing this issue and submitting the fix.

Reviewed-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>