diff mbox series

[PULL,20/38] accel/whpx: Use accel-specific per-vcpu @dirty field

Message ID 20240426194200.43723-21-philmd@linaro.org
State Accepted
Commit 9ad49538c7b7c0672110994d81d687ed42bf3ef4
Headers show
Series None | expand

Commit Message

Philippe Mathieu-Daudé April 26, 2024, 7:41 p.m. UTC 
WHPX has a specific use of the CPUState::vcpu_dirty field
(CPUState::vcpu_dirty is not used by common code).
To make this field accel-specific, add and use a new
@dirty variable in the AccelCPUState structure.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240424174506.326-2-philmd@linaro.org>
---
 target/i386/whpx/whpx-all.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Comments

Volker Rümelin April 28, 2024, 8:12 p.m. UTC | #1 
Am 26.04.24 um 21:41 schrieb Philippe Mathieu-Daudé:
> WHPX has a specific use of the CPUState::vcpu_dirty field
> (CPUState::vcpu_dirty is not used by common code).
> To make this field accel-specific, add and use a new
> @dirty variable in the AccelCPUState structure.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20240424174506.326-2-philmd@linaro.org>
> ---
>  target/i386/whpx/whpx-all.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
> index 31eec7048c..b08e644517 100644
> --- a/target/i386/whpx/whpx-all.c
> +++ b/target/i386/whpx/whpx-all.c

> @@ -2235,7 +2236,7 @@ int whpx_init_vcpu(CPUState *cpu)
>      }
>  
>      vcpu->interruptable = true;
> -    cpu->vcpu_dirty = true;

Hi Philippe,

cpu->accel is NULL here. You probably wanted to write

+    vcpu->dirty = true;

instead of

+    cpu->accel->dirty = true;

I think your patch for nvmm_init_vcpu() in target/i386/nvmm/nvmm-all.c
has the same issue.

With best regards,
Volker

> +    cpu->accel->dirty = true;
>      cpu->accel = vcpu;
>      max_vcpu_index = max(max_vcpu_index, cpu->cpu_index);
>      qemu_add_vm_change_state_handler(whpx_cpu_update_state, env);
Philippe Mathieu-Daudé April 29, 2024, 8:22 a.m. UTC | #2 
On 28/4/24 22:12, Volker Rümelin wrote:
> Am 26.04.24 um 21:41 schrieb Philippe Mathieu-Daudé:
>> WHPX has a specific use of the CPUState::vcpu_dirty field
>> (CPUState::vcpu_dirty is not used by common code).
>> To make this field accel-specific, add and use a new
>> @dirty variable in the AccelCPUState structure.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Message-Id: <20240424174506.326-2-philmd@linaro.org>
>> ---
>>   target/i386/whpx/whpx-all.c | 23 ++++++++++++-----------
>>   1 file changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
>> index 31eec7048c..b08e644517 100644
>> --- a/target/i386/whpx/whpx-all.c
>> +++ b/target/i386/whpx/whpx-all.c
> 
>> @@ -2235,7 +2236,7 @@ int whpx_init_vcpu(CPUState *cpu)
>>       }
>>   
>>       vcpu->interruptable = true;
>> -    cpu->vcpu_dirty = true;
> 
> Hi Philippe,
> 
> cpu->accel is NULL here. You probably wanted to write
> 
> +    vcpu->dirty = true;
> 
> instead of
> 
> +    cpu->accel->dirty = true;
> 
> I think your patch for nvmm_init_vcpu() in target/i386/nvmm/nvmm-all.c
> has the same issue.

Doh, sorry I missed that :/

I'll post fixes, thanks Volker!

Phil.
diff mbox series

Patch

diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
index 31eec7048c..b08e644517 100644
--- a/target/i386/whpx/whpx-all.c
+++ b/target/i386/whpx/whpx-all.c
@@ -237,6 +237,7 @@  struct AccelCPUState {
     uint64_t tpr;
     uint64_t apic_base;
     bool interruption_pending;
+    bool dirty;
 
     /* Must be the last field as it may have a tail */
     WHV_RUN_VP_EXIT_CONTEXT exit_ctx;
@@ -839,7 +840,7 @@  static HRESULT CALLBACK whpx_emu_setreg_callback(
      * The emulator just successfully wrote the register state. We clear the
      * dirty state so we avoid the double write on resume of the VP.
      */
-    cpu->vcpu_dirty = false;
+    cpu->accel->dirty = false;
 
     return hr;
 }
@@ -1394,7 +1395,7 @@  static int whpx_last_vcpu_stopping(CPUState *cpu)
 /* Returns the address of the next instruction that is about to be executed. */
 static vaddr whpx_vcpu_get_pc(CPUState *cpu, bool exit_context_valid)
 {
-    if (cpu->vcpu_dirty) {
+    if (cpu->accel->dirty) {
         /* The CPU registers have been modified by other parts of QEMU. */
         return cpu_env(cpu)->eip;
     } else if (exit_context_valid) {
@@ -1713,9 +1714,9 @@  static int whpx_vcpu_run(CPUState *cpu)
     }
 
     do {
-        if (cpu->vcpu_dirty) {
+        if (cpu->accel->dirty) {
             whpx_set_registers(cpu, WHPX_SET_RUNTIME_STATE);
-            cpu->vcpu_dirty = false;
+            cpu->accel->dirty = false;
         }
 
         if (exclusive_step_mode == WHPX_STEP_NONE) {
@@ -2063,9 +2064,9 @@  static int whpx_vcpu_run(CPUState *cpu)
 
 static void do_whpx_cpu_synchronize_state(CPUState *cpu, run_on_cpu_data arg)
 {
-    if (!cpu->vcpu_dirty) {
+    if (!cpu->accel->dirty) {
         whpx_get_registers(cpu);
-        cpu->vcpu_dirty = true;
+        cpu->accel->dirty = true;
     }
 }
 
@@ -2073,20 +2074,20 @@  static void do_whpx_cpu_synchronize_post_reset(CPUState *cpu,
                                                run_on_cpu_data arg)
 {
     whpx_set_registers(cpu, WHPX_SET_RESET_STATE);
-    cpu->vcpu_dirty = false;
+    cpu->accel->dirty = false;
 }
 
 static void do_whpx_cpu_synchronize_post_init(CPUState *cpu,
                                               run_on_cpu_data arg)
 {
     whpx_set_registers(cpu, WHPX_SET_FULL_STATE);
-    cpu->vcpu_dirty = false;
+    cpu->accel->dirty = false;
 }
 
 static void do_whpx_cpu_synchronize_pre_loadvm(CPUState *cpu,
                                                run_on_cpu_data arg)
 {
-    cpu->vcpu_dirty = true;
+    cpu->accel->dirty = true;
 }
 
 /*
@@ -2095,7 +2096,7 @@  static void do_whpx_cpu_synchronize_pre_loadvm(CPUState *cpu,
 
 void whpx_cpu_synchronize_state(CPUState *cpu)
 {
-    if (!cpu->vcpu_dirty) {
+    if (!cpu->accel->dirty) {
         run_on_cpu(cpu, do_whpx_cpu_synchronize_state, RUN_ON_CPU_NULL);
     }
 }
@@ -2235,7 +2236,7 @@  int whpx_init_vcpu(CPUState *cpu)
     }
 
     vcpu->interruptable = true;
-    cpu->vcpu_dirty = true;
+    cpu->accel->dirty = true;
     cpu->accel = vcpu;
     max_vcpu_index = max(max_vcpu_index, cpu->cpu_index);
     qemu_add_vm_change_state_handler(whpx_cpu_update_state, env);