[v6] accel: Preparatory cleanups for split-accel

[PATCH v6 04/39] system/cpus: Assert interrupt handling is done with BQL locked

Posted by Philippe Mathieu-Daudé 5 months, 2 weeks ago

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/tcg-accel-ops.c | 2 --
 system/cpus.c             | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index b24d6a75625..6116644d1c0 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -93,8 +93,6 @@ static void tcg_cpu_reset_hold(CPUState *cpu)
 /* mask must never be zero, except for A20 change call */
 void tcg_handle_interrupt(CPUState *cpu, int mask)
 {
-    g_assert(bql_locked());
-
     cpu->interrupt_request |= mask;
 
     /*
diff --git a/system/cpus.c b/system/cpus.c
index d16b0dff989..a43e0e4e796 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -265,6 +265,8 @@ static void generic_handle_interrupt(CPUState *cpu, int mask)
 
 void cpu_interrupt(CPUState *cpu, int mask)
 {
+    g_assert(bql_locked());
+
     if (cpus_accel->handle_interrupt) {
         cpus_accel->handle_interrupt(cpu, mask);
     } else {
-- 
2.49.0

Re: [PATCH v6 04/39] system/cpus: Assert interrupt handling is done with BQL locked

Posted by Alex Bennée 2 months, 1 week ago

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

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/tcg-accel-ops.c | 2 --
>  system/cpus.c             | 2 ++
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index b24d6a75625..6116644d1c0 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -93,8 +93,6 @@ static void tcg_cpu_reset_hold(CPUState *cpu)
>  /* mask must never be zero, except for A20 change call */
>  void tcg_handle_interrupt(CPUState *cpu, int mask)
>  {
> -    g_assert(bql_locked());
> -
>      cpu->interrupt_request |= mask;
>  
>      /*
> diff --git a/system/cpus.c b/system/cpus.c
> index d16b0dff989..a43e0e4e796 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -265,6 +265,8 @@ static void generic_handle_interrupt(CPUState *cpu, int mask)
>  
>  void cpu_interrupt(CPUState *cpu, int mask)
>  {
> +    g_assert(bql_locked());
> +

Is this really the case since 27e76d01010 (cpu-common: use atomic access
for interrupt_request). In fact I think tcg_handle_interrupt now uses
cpu_set_interrupt.


>      if (cpus_accel->handle_interrupt) {
>          cpus_accel->handle_interrupt(cpu, mask);
>      } else {

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v6 04/39] system/cpus: Assert interrupt handling is done with BQL locked

Posted by Zhao Liu 5 months, 2 weeks ago

On Thu, Jul 03, 2025 at 07:32:10PM +0200, Philippe Mathieu-Daudé wrote:
> Date: Thu,  3 Jul 2025 19:32:10 +0200
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH v6 04/39] system/cpus: Assert interrupt handling is done
>  with BQL locked
> X-Mailer: git-send-email 2.49.0
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/tcg-accel-ops.c | 2 --
>  system/cpus.c             | 2 ++
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH v6 04/39] system/cpus: Assert interrupt handling is done with BQL locked

Posted by Xiaoyao Li 5 months, 2 weeks ago

On 7/4/2025 1:32 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/tcg-accel-ops.c | 2 --
>   system/cpus.c             | 2 ++
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index b24d6a75625..6116644d1c0 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -93,8 +93,6 @@ static void tcg_cpu_reset_hold(CPUState *cpu)
>   /* mask must never be zero, except for A20 change call */
>   void tcg_handle_interrupt(CPUState *cpu, int mask)
>   {
> -    g_assert(bql_locked());
> -
>       cpu->interrupt_request |= mask;
>   
>       /*
> diff --git a/system/cpus.c b/system/cpus.c
> index d16b0dff989..a43e0e4e796 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -265,6 +265,8 @@ static void generic_handle_interrupt(CPUState *cpu, int mask)
>   
>   void cpu_interrupt(CPUState *cpu, int mask)
>   {
> +    g_assert(bql_locked());

The best result is it doesn't break any thing. But it can surely help 
catch the case without bql locked and get the case fixed them.

So

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

>       if (cpus_accel->handle_interrupt) {
>           cpus_accel->handle_interrupt(cpu, mask);
>       } else {