kvmvapic: Fix TB invalidation after instruction patching

PATCH Superseded Jan Kiszka 3 participants 4 messages Nov 4, 2012 – Nov 10, 2012

lore

Share & tools

lore

Jan Kiszka

author

2012-11-04 06:52 UTC #0 lore raw

PATCH 1/1 kvmvapic: Fix TB invalidation after instruction patching

From: Jan Kiszka <[email protected]>

Since 0b57e287, cpu_memory_rw_debug already triggers a TB invalidation. As it doesn't (and cannot) set is_cpu_write_access=1 but "consumes" the currently executed TB, the tb_invalidate_phys_page_range call from patch_instruction didn't work anymore.

Fix this by open-coding the required bits to restore the CPU state from the current TB position before patching and resume execution on the patched instruction afterward.

Signed-off-by Jan Kiszka <[email protected]>

---

I see no better way ATM.

 hw/kvmvapic.c |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

hw/kvmvapic.c +16 -4

index dc111ee..e7ab3cc 100644
@@ -384,10 +384,13 @@ static void patch_call(VAPICROMState *s, CPUX86State *env, target_ulong ip,
 
 static void patch_instruction(VAPICROMState *s, CPUX86State *env, target_ulong ip)
 {
-    hwaddr paddr;
     VAPICHandlers *handlers;
     uint8_t opcode[2];
     uint32_t imm32;
+    TranslationBlock *current_tb;
+    target_ulong current_pc;
+    target_ulong current_cs_base;
+    int current_flags;
 
     if (smp_cpus == 1) {
         handlers = &s->rom_state.up;

@@ -395,6 +398,13 @@ static void patch_instruction(VAPICROMState *s, CPUX86State *env, target_ulong i
         handlers = &s->rom_state.mp;
     }
 
+    if (!kvm_enabled()) {
+        current_tb = tb_find_pc(env->mem_io_pc);
+        cpu_restore_state(current_tb, env, env->mem_io_pc);
+        cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
+                             &current_flags);
+    }
+
     pause_all_vcpus();
 
     cpu_memory_rw_debug(env, ip, opcode, sizeof(opcode), 0);

@@ -430,9 +440,11 @@ static void patch_instruction(VAPICROMState *s, CPUX86State *env, target_ulong i
 
     resume_all_vcpus();
 
-    paddr = cpu_get_phys_page_debug(env, ip);
-    paddr += ip & ~TARGET_PAGE_MASK;
-    tb_invalidate_phys_page_range(paddr, paddr + 1, 1);
+    if (!kvm_enabled()) {
+        env->current_tb = NULL;
+        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
+        cpu_resume_from_signal(env, NULL);
+    }
 }
 
 void vapic_report_tpr_access(DeviceState *dev, void *cpu, target_ulong ip,

Signature · 1 line

1.7.3.4

Hervé Poussineau

Tested-by on v1 1/1

2012-11-04 07:51 UTC #1 lore raw

Quoted 7 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>
Since 0b57e287, cpu_memory_rw_debug already triggers a TB invalidation. As it doesn't (and cannot) set is_cpu_write_access=1 but "consumes" the currently executed TB, the tb_invalidate_phys_page_range call from patch_instruction didn't work anymore.

Quoted 7 lines

Fix this by open-coding the required bits to restore the CPU state from the current TB position before patching and resume execution on the patched instruction afterward.
Signed-off-by Jan Kiszka <[email protected]>
---

Tested-by Hervé Poussineau <[email protected]>

However, I had to initialize current_pc, current_cs_base and current_flags to 0 to prevent uninitialized warning. (GCC 4.7.1, KVM disabled by configure)

Regards,

Hervé

Jan Kiszka

author

2012-11-04 08:17 UTC #2 lore raw

PATCH v2 1/1 kvmvapic: Fix TB invalidation after instruction patching

From: Jan Kiszka <[email protected]>

Fix this by open-coding the required bits to restore the CPU state from the current TB position before patching and resume execution on the patched instruction afterward.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- make the compiler happier by initializing local variables to 0

 hw/kvmvapic.c |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

hw/kvmvapic.c +16 -4

index dc111ee..09c6433 100644
@@ -384,10 +384,13 @@ static void patch_call(VAPICROMState *s, CPUX86State *env, target_ulong ip,
 
 static void patch_instruction(VAPICROMState *s, CPUX86State *env, target_ulong ip)
 {
-    hwaddr paddr;
     VAPICHandlers *handlers;
     uint8_t opcode[2];
     uint32_t imm32;
+    TranslationBlock *current_tb;
+    target_ulong current_pc = 0;
+    target_ulong current_cs_base = 0;
+    int current_flags = 0;
 
     if (smp_cpus == 1) {
         handlers = &s->rom_state.up;

@@ -395,6 +398,13 @@ static void patch_instruction(VAPICROMState *s, CPUX86State *env, target_ulong i
         handlers = &s->rom_state.mp;
     }
 
+    if (!kvm_enabled()) {
+        current_tb = tb_find_pc(env->mem_io_pc);
+        cpu_restore_state(current_tb, env, env->mem_io_pc);
+        cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
+                             &current_flags);
+    }
+
     pause_all_vcpus();
 
     cpu_memory_rw_debug(env, ip, opcode, sizeof(opcode), 0);

@@ -430,9 +440,11 @@ static void patch_instruction(VAPICROMState *s, CPUX86State *env, target_ulong i
 
     resume_all_vcpus();
 
-    paddr = cpu_get_phys_page_debug(env, ip);
-    paddr += ip & ~TARGET_PAGE_MASK;
-    tb_invalidate_phys_page_range(paddr, paddr + 1, 1);
+    if (!kvm_enabled()) {
+        env->current_tb = NULL;
+        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
+        cpu_resume_from_signal(env, NULL);
+    }
 }
 
 void vapic_report_tpr_access(DeviceState *dev, void *cpu, target_ulong ip,

Signature · 1 line

1.7.3.4

Blue Swirl

2012-11-10 19:27 UTC #3 lore raw

Thanks, applied. Fixes also Win2k boot.

Quoted 70 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

Fix this by open-coding the required bits to restore the CPU state from the current TB position before patching and resume execution on the patched instruction afterward.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- make the compiler happier by initializing local variables to 0

 hw/kvmvapic.c |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

hw/kvmvapic.c +16 -5

index dc111ee..09c6433 100644
@@ -384,10 +384,13 @@ static void patch_call(VAPICROMState *s, CPUX86State *env, target_ulong ip,
 
 static void patch_instruction(VAPICROMState *s, CPUX86State *env, target_ulong ip)
 {
-    hwaddr paddr;
     VAPICHandlers *handlers;
     uint8_t opcode[2];
     uint32_t imm32;
+    TranslationBlock *current_tb;
+    target_ulong current_pc = 0;
+    target_ulong current_cs_base = 0;
+    int current_flags = 0;
 
     if (smp_cpus == 1) {
         handlers = &s->rom_state.up;

@@ -395,6 +398,13 @@ static void patch_instruction(VAPICROMState *s, CPUX86State *env, target_ulong i
         handlers = &s->rom_state.mp;
     }
 
+    if (!kvm_enabled()) {
+        current_tb = tb_find_pc(env->mem_io_pc);
+        cpu_restore_state(current_tb, env, env->mem_io_pc);
+        cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
+                             &current_flags);
+    }
+
     pause_all_vcpus();
 
     cpu_memory_rw_debug(env, ip, opcode, sizeof(opcode), 0);

@@ -430,9 +440,11 @@ static void patch_instruction(VAPICROMState *s, CPUX86State *env, target_ulong i
 
     resume_all_vcpus();
 
-    paddr = cpu_get_phys_page_debug(env, ip);
-    paddr += ip & ~TARGET_PAGE_MASK;
-    tb_invalidate_phys_page_range(paddr, paddr + 1, 1);
+    if (!kvm_enabled()) {
+        env->current_tb = NULL;
+        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
+        cpu_resume_from_signal(env, NULL);
+    }
 }
 
 void vapic_report_tpr_access(DeviceState *dev, void *cpu, target_ulong ip,
--

1.7.3.4