[PATCH v5 05/31] sysemu: Introduce AccelOpsClass::has_work()

Philippe Mathieu-Daudé posted 31 patches 4 years, 4 months ago
Maintainers: Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Thomas Huth <thuth@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
There is a newer version of this series
[PATCH v5 05/31] sysemu: Introduce AccelOpsClass::has_work()
Posted by Philippe Mathieu-Daudé 4 years, 4 months ago
Introduce an accelerator-specific has_work() handler.
Eventually call it from cpu_has_work().

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/sysemu/accel-ops.h | 5 +++++
 softmmu/cpus.c             | 9 +++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index 032f6979d76..de83f095f20 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -31,6 +31,11 @@ struct AccelOpsClass {
     void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
     void (*kick_vcpu_thread)(CPUState *cpu);
 
+    /**
+     * @has_work: Callback for checking if there is work to do.
+     */
+    bool (*has_work)(CPUState *cpu);
+
     void (*synchronize_post_reset)(CPUState *cpu);
     void (*synchronize_post_init)(CPUState *cpu);
     void (*synchronize_state)(CPUState *cpu);
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 7e2cb2c571b..e59046ce39c 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -255,8 +255,13 @@ bool cpu_has_work(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
-    g_assert(cc->has_work);
-    return cc->has_work(cpu);
+    if (cc->has_work) {
+        return cc->has_work(cpu);
+    }
+    if (cpus_accel->has_work) {
+        return cpus_accel->has_work(cpu);
+    }
+    g_assert_not_reached();
 }
 
 static int do_vm_stop(RunState state, bool send_stop)
-- 
2.31.1

Re: [PATCH v5 05/31] sysemu: Introduce AccelOpsClass::has_work()
Posted by Richard Henderson 4 years, 4 months ago
On 9/20/21 2:44 PM, Philippe Mathieu-Daudé wrote:
> -    g_assert(cc->has_work);
> -    return cc->has_work(cpu);
> +    if (cc->has_work) {
> +        return cc->has_work(cpu);
> +    }
> +    if (cpus_accel->has_work) {
> +        return cpus_accel->has_work(cpu);
> +    }
> +    g_assert_not_reached();

This might be close to the end result, but it isn't what we begin with in cpu_thread_is_idle.

You'd want

     if (cc->has_work && cc->has_work(cpu)) {
         return true;
     }
     if (cpus_accel->has_work && cpus_accel->has_work(cpu)) {
         return true;
     }
     return false;

to start.  After the cpus_accel hook is filled in you can assert and return from 
cpus_accel->has_work.  And of course after cc->has_work is removed, that clause is gone.


r~

Re: [PATCH v5 05/31] sysemu: Introduce AccelOpsClass::has_work()
Posted by Philippe Mathieu-Daudé 4 years, 4 months ago
On 9/20/21 23:58, Richard Henderson wrote:
> On 9/20/21 2:44 PM, Philippe Mathieu-Daudé wrote:
>> -    g_assert(cc->has_work);
>> -    return cc->has_work(cpu);
>> +    if (cc->has_work) {
>> +        return cc->has_work(cpu);
>> +    }
>> +    if (cpus_accel->has_work) {
>> +        return cpus_accel->has_work(cpu);
>> +    }
>> +    g_assert_not_reached();
> 
> This might be close to the end result, but it isn't what we begin with 
> in cpu_thread_is_idle.
> 
> You'd want
> 
>      if (cc->has_work && cc->has_work(cpu)) {
>          return true;
>      }
>      if (cpus_accel->has_work && cpus_accel->has_work(cpu)) {
>          return true;
>      }
>      return false;
> 
> to start.  After the cpus_accel hook is filled in you can assert and 
> return from cpus_accel->has_work.  And of course after cc->has_work is 
> removed, that clause is gone.

Much cleaner, thank you for the hints :)