[Qemu-devel] [PATCH 0/8] WHPX updates based on the 17095 insider sdk.

[Qemu-devel] [PATCH 0/8] WHPX updates based on the 17095 insider sdk.

[Justin Terry (VM)]

This change set includes fixes for two breaking changes that were introduced
in the Windows Insider SDK 17095. First, the casing of the headers/libs changed
such that a direct reference out of Windows Kits will fail during compile on
case sensitive file systems. Second, a few API's were modified to improve
performance and changes are required to QEMU to maintain ability to build on
such platforms.

The rest of the changes are around improvements/fixes to the accelerator to
increase performance and correctness.

Thanks,
Justin Terry

Justin Terry (VM) (8):
  Fixing WHPX casing to match SDK
  Resolves WHPX breaking changes in SDK 17095
  Remove unnecessary WHPX __debugbreak();
  Fix WHPX additional lock acquisition
  Fix WHPX typo in 'mmio'
  Fix WHPX issue leaking tpr values
  WHXP Removes the use of WHvGetExitContextSize
  WHPX improve interrupt notification registration

 configure              | 10 ++++----
 target/i386/whpx-all.c | 70 +++++++++++++++++---------------------------------
 2 files changed, 28 insertions(+), 52 deletions(-)

-- 
2.13.6

[Qemu-devel] [PATCH 1/8] Fixing WHPX casing to match SDK

[Justin Terry (VM)]

Fixes an issue where the SDK that was releases had a different casing for the
*.h and *.lib files causing a build break if linked directly from Windows Kits.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
---
 configure              | 10 +++++-----
 target/i386/whpx-all.c |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index 39f3a43001..220b9ce52b 100755
--- a/configure
+++ b/configure
@@ -2475,20 +2475,20 @@ fi
 if test "$whpx" != "no" ; then
     cat > $TMPC << EOF
 #include <windows.h>
-#include <winhvplatform.h>
-#include <winhvemulation.h>
+#include <WinHvPlatform.h>
+#include <WinHvEmulation.h>
 int main(void) {
     WHV_CAPABILITY whpx_cap;
     WHvGetCapability(WHvCapabilityCodeFeatures, &whpx_cap, sizeof(whpx_cap));
     return 0;
 }
 EOF
-    if compile_prog "" "-lwinhvplatform -lwinhvemulation" ; then
-        libs_softmmu="$libs_softmmu -lwinhvplatform -lwinhvemulation"
+    if compile_prog "" "-lWinHvPlatform -lWinHvEmulation" ; then
+        libs_softmmu="$libs_softmmu -lWinHvPlatform -lWinHvEmulation"
         whpx="yes"
     else
         if test "$whpx" = "yes"; then
-            feature_not_found "winhvplatform" "winhvemulation is not installed"
+            feature_not_found "WinHvPlatform" "WinHvEmulation is not installed"
         fi
         whpx="no"
     fi
diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 0015b27509..eeee43e187 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -26,8 +26,8 @@
 #include "qapi/error.h"
 #include "migration/blocker.h"
 
-#include <winhvplatform.h>
-#include <winhvemulation.h>
+#include <WinHvPlatform.h>
+#include <WinHvEmulation.h>
 
 struct whpx_state {
     uint64_t mem_quota;
-- 
2.13.6

[Qemu-devel] [PATCH 2/8] Resolves WHPX breaking changes in SDK 17095

[Justin Terry (VM)]

1. Fixes the changes required to the WHvTryMmioEmulation, WHvTryIoEmulation, and
WHvEmulatorCreateEmulator based on the new VpContext forwarding.
2. Removes the WHvRunVpExitReasonAlerted case.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
---
 target/i386/whpx-all.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index eeee43e187..969c2f5f93 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -613,6 +613,7 @@ static HRESULT CALLBACK whpx_emu_translate_callback(
 }
 
 static const WHV_EMULATOR_CALLBACKS whpx_emu_callbacks = {
+    .Size = sizeof(WHV_EMULATOR_CALLBACKS),
     .WHvEmulatorIoPortCallback = whpx_emu_ioport_callback,
     .WHvEmulatorMemoryCallback = whpx_emu_memio_callback,
     .WHvEmulatorGetVirtualProcessorRegisters = whpx_emu_getreg_callback,
@@ -626,7 +627,9 @@ static int whpx_handle_mmio(CPUState *cpu, WHV_MEMORY_ACCESS_CONTEXT *ctx)
     struct whpx_vcpu *vcpu = get_whpx_vcpu(cpu);
     WHV_EMULATOR_STATUS emu_status;
 
-    hr = WHvEmulatorTryMmioEmulation(vcpu->emulator, cpu, ctx, &emu_status);
+    hr = WHvEmulatorTryMmioEmulation(vcpu->emulator, cpu,
+                                     &vcpu->exit_ctx.VpContext, ctx,
+                                     &emu_status);
     if (FAILED(hr)) {
         __debugbreak();
         error_report("WHPX: Failed to parse MMIO access, hr=%08lx", hr);
@@ -649,7 +652,9 @@ static int whpx_handle_portio(CPUState *cpu,
     struct whpx_vcpu *vcpu = get_whpx_vcpu(cpu);
     WHV_EMULATOR_STATUS emu_status;
 
-    hr = WHvEmulatorTryIoEmulation(vcpu->emulator, cpu, ctx, &emu_status);
+    hr = WHvEmulatorTryIoEmulation(vcpu->emulator, cpu,
+                                   &vcpu->exit_ctx.VpContext, ctx,
+                                   &emu_status);
     if (FAILED(hr)) {
         __debugbreak();
         error_report("WHPX: Failed to parse PortIO access, hr=%08lx", hr);
@@ -905,18 +910,8 @@ static int whpx_vcpu_run(CPUState *cpu)
             whpx_vcpu_kick(cpu);
         }
 
-        for (;;) {
-            hr = WHvRunVirtualProcessor(whpx->partition, cpu->cpu_index,
-                                        &vcpu->exit_ctx, whpx->exit_ctx_size);
-
-            if (SUCCEEDED(hr) && (vcpu->exit_ctx.ExitReason ==
-                                  WHvRunVpExitReasonAlerted)) {
-                WHvCancelRunVirtualProcessor(whpx->partition, cpu->cpu_index,
-                                             0);
-            } else {
-                break;
-            }
-        }
+        hr = WHvRunVirtualProcessor(whpx->partition, cpu->cpu_index,
+                                    &vcpu->exit_ctx, whpx->exit_ctx_size);
 
         if (FAILED(hr)) {
             error_report("WHPX: Failed to exec a virtual processor,"
@@ -956,7 +951,6 @@ static int whpx_vcpu_run(CPUState *cpu)
         case WHvRunVpExitReasonX64MsrAccess:
         case WHvRunVpExitReasonX64Cpuid:
         case WHvRunVpExitReasonException:
-        case WHvRunVpExitReasonAlerted:
         default:
             error_report("WHPX: Unexpected VP exit code %d",
                          vcpu->exit_ctx.ExitReason);
@@ -1068,7 +1062,7 @@ int whpx_init_vcpu(CPUState *cpu)
         return -ENOMEM;
     }
 
-    hr = WHvEmulatorCreateEmulator(whpx_emu_callbacks, &vcpu->emulator);
+    hr = WHvEmulatorCreateEmulator(&whpx_emu_callbacks, &vcpu->emulator);
     if (FAILED(hr)) {
         error_report("WHPX: Failed to setup instruction completion support,"
                      " hr=%08lx", hr);
-- 
2.13.6

[Qemu-devel] [PATCH 3/8] Remove unnecessary WHPX __debugbreak();

[Justin Terry (VM)]

Minor code cleanup. The calls to __debugbreak() are not required and should
no longer be used to prevent unnecessary breaks.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
---
 target/i386/whpx-all.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 969c2f5f93..14ea732ec6 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -364,7 +364,6 @@ static void whpx_set_registers(CPUState *cpu)
     if (FAILED(hr)) {
         error_report("WHPX: Failed to set virtual processor context, hr=%08lx",
                      hr);
-        __debugbreak();
     }
 
     return;
@@ -391,7 +390,6 @@ static void whpx_get_registers(CPUState *cpu)
     if (FAILED(hr)) {
         error_report("WHPX: Failed to get virtual processor context, hr=%08lx",
                      hr);
-        __debugbreak();
     }
 
     /* Indexes for first 16 registers match between HV and QEMU definitions */
@@ -554,7 +552,6 @@ static HRESULT CALLBACK whpx_emu_getreg_callback(
     if (FAILED(hr)) {
         error_report("WHPX: Failed to get virtual processor registers,"
                      " hr=%08lx", hr);
-        __debugbreak();
     }
 
     return hr;
@@ -576,7 +573,6 @@ static HRESULT CALLBACK whpx_emu_setreg_callback(
     if (FAILED(hr)) {
         error_report("WHPX: Failed to set virtual processor registers,"
                      " hr=%08lx", hr);
-        __debugbreak();
     }
 
     /*
@@ -604,7 +600,6 @@ static HRESULT CALLBACK whpx_emu_translate_callback(
                          Gva, TranslateFlags, &res, Gpa);
     if (FAILED(hr)) {
         error_report("WHPX: Failed to translate GVA, hr=%08lx", hr);
-        __debugbreak();
     } else {
         *TranslationResult = res.ResultCode;
     }
@@ -631,13 +626,11 @@ static int whpx_handle_mmio(CPUState *cpu, WHV_MEMORY_ACCESS_CONTEXT *ctx)
                                      &vcpu->exit_ctx.VpContext, ctx,
                                      &emu_status);
     if (FAILED(hr)) {
-        __debugbreak();
         error_report("WHPX: Failed to parse MMIO access, hr=%08lx", hr);
         return -1;
     }
 
     if (!emu_status.EmulationSuccessful) {
-        __debugbreak();
         error_report("WHPX: Failed to emulate MMIO access");
         return -1;
     }
@@ -656,13 +649,11 @@ static int whpx_handle_portio(CPUState *cpu,
                                    &vcpu->exit_ctx.VpContext, ctx,
                                    &emu_status);
     if (FAILED(hr)) {
-        __debugbreak();
         error_report("WHPX: Failed to parse PortIO access, hr=%08lx", hr);
         return -1;
     }
 
     if (!emu_status.EmulationSuccessful) {
-        __debugbreak();
         error_report("WHPX: Failed to emulate PortMMIO access");
         return -1;
     }
@@ -716,7 +707,6 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
         if (cpu->interrupt_request & CPU_INTERRUPT_SMI) {
             qemu_mutex_lock_iothread();
             cpu->interrupt_request &= ~CPU_INTERRUPT_SMI;
-            __debugbreak();
             qemu_mutex_unlock_iothread();
         }
     }
@@ -785,7 +775,6 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
         if (FAILED(hr)) {
             error_report("WHPX: Failed to set interrupt state registers,"
                          " hr=%08lx", hr);
-            __debugbreak();
         }
     }
 
@@ -812,7 +801,6 @@ static void whpx_vcpu_post_run(CPUState *cpu)
     if (FAILED(hr)) {
         error_report("WHPX: Failed to get interrupt state regusters,"
                      " hr=%08lx", hr);
-        __debugbreak();
         vcpu->interruptable = false;
         return;
     }
-- 
2.13.6

[Qemu-devel] [PATCH 4/8] Fix WHPX additional lock acquisition

[Justin Terry (VM)]

The code already is holding the qemu_mutex for the IO thread. We do not need
to additionally take the lock again in this case.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
---
 target/i386/whpx-all.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 14ea732ec6..74a8f4d599 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -705,9 +705,7 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
             new_int.InterruptionVector = 2;
         }
         if (cpu->interrupt_request & CPU_INTERRUPT_SMI) {
-            qemu_mutex_lock_iothread();
             cpu->interrupt_request &= ~CPU_INTERRUPT_SMI;
-            qemu_mutex_unlock_iothread();
         }
     }
 
-- 
2.13.6

[Qemu-devel] [PATCH 5/8] Fix WHPX typo in 'mmio'

[Justin Terry (VM)]

Renames the usage of 'memio' to 'mmio' in the emulator callbacks.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
---
 target/i386/whpx-all.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 74a8f4d599..7e58d5f68c 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -527,7 +527,7 @@ static HRESULT CALLBACK whpx_emu_ioport_callback(
     return S_OK;
 }
 
-static HRESULT CALLBACK whpx_emu_memio_callback(
+static HRESULT CALLBACK whpx_emu_mmio_callback(
     void *ctx,
     WHV_EMULATOR_MEMORY_ACCESS_INFO *ma)
 {
@@ -610,7 +610,7 @@ static HRESULT CALLBACK whpx_emu_translate_callback(
 static const WHV_EMULATOR_CALLBACKS whpx_emu_callbacks = {
     .Size = sizeof(WHV_EMULATOR_CALLBACKS),
     .WHvEmulatorIoPortCallback = whpx_emu_ioport_callback,
-    .WHvEmulatorMemoryCallback = whpx_emu_memio_callback,
+    .WHvEmulatorMemoryCallback = whpx_emu_mmio_callback,
     .WHvEmulatorGetVirtualProcessorRegisters = whpx_emu_getreg_callback,
     .WHvEmulatorSetVirtualProcessorRegisters = whpx_emu_setreg_callback,
     .WHvEmulatorTranslateGvaPage = whpx_emu_translate_callback,
-- 
2.13.6

[Qemu-devel] [PATCH 6/8] Fix WHPX issue leaking tpr values

[Justin Terry (VM)]

Fixes an issue where if the tpr is assigned to the array but not a different
value from what is already expected on the vp the code will skip incrementing
the reg_count. In this case its possible that we set an invalid memory section
of the next call for DeliverabilityNotifications that was not expected.

The fix is to use a local variable to store the temporary tpr and only update
the array if the local tpr value is different than the vp context.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
---
 target/i386/whpx-all.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 7e58d5f68c..47a6935288 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -687,6 +687,7 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
     struct CPUX86State *env = (CPUArchState *)(cpu->env_ptr);
     X86CPU *x86_cpu = X86_CPU(cpu);
     int irq;
+    uint8_t tpr;
     WHV_X64_PENDING_INTERRUPTION_REGISTER new_int = {0};
     UINT32 reg_count = 0;
     WHV_REGISTER_VALUE reg_values[3] = {0};
@@ -746,9 +747,10 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
     }
 
     /* Sync the TPR to the CR8 if was modified during the intercept */
-    reg_values[reg_count].Reg64 = cpu_get_apic_tpr(x86_cpu->apic_state);
-    if (reg_values[reg_count].Reg64 != vcpu->tpr) {
-        vcpu->tpr = reg_values[reg_count].Reg64;
+    tpr = cpu_get_apic_tpr(x86_cpu->apic_state);
+    if (tpr != vcpu->tpr) {
+        vcpu->tpr = tpr;
+        reg_values[reg_count].Reg64 = tpr;
         cpu->exit_request = 1;
         reg_names[reg_count] = WHvX64RegisterCr8;
         reg_count += 1;
-- 
2.13.6

[Qemu-devel] [PATCH 7/8] WHXP Removes the use of WHvGetExitContextSize

[Justin Terry (VM)]

The use of WHvGetExitContextSize will break ABI compatibility if the platform
changes the context size while a qemu compiled executable does not recompile.
To avoid this we now use sizeof and let the platform determine which version
of the struction was passed for ABI compatibility.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
---
 target/i386/whpx-all.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 47a6935288..24387bebad 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -32,7 +32,6 @@
 struct whpx_state {
     uint64_t mem_quota;
     WHV_PARTITION_HANDLE partition;
-    uint32_t exit_ctx_size;
 };
 
 static const WHV_REGISTER_NAME whpx_register_names[] = {
@@ -899,7 +898,7 @@ static int whpx_vcpu_run(CPUState *cpu)
         }
 
         hr = WHvRunVirtualProcessor(whpx->partition, cpu->cpu_index,
-                                    &vcpu->exit_ctx, whpx->exit_ctx_size);
+                                    &vcpu->exit_ctx, sizeof(vcpu->exit_ctx));
 
         if (FAILED(hr)) {
             error_report("WHPX: Failed to exec a virtual processor,"
@@ -1042,8 +1041,7 @@ int whpx_init_vcpu(CPUState *cpu)
         }
     }
 
-    vcpu = g_malloc0(FIELD_OFFSET(struct whpx_vcpu, exit_ctx) +
-                     whpx->exit_ctx_size);
+    vcpu = g_malloc0(sizeof(struct whpx_vcpu));
 
     if (!vcpu) {
         error_report("WHPX: Failed to allocte VCPU context.");
@@ -1300,9 +1298,6 @@ static int whpx_accel_init(MachineState *ms)
         goto error;
     }
 
-    whpx->exit_ctx_size = WHvGetRunExitContextSize();
-    assert(whpx->exit_ctx_size);
-
     whpx_memory_init();
 
     cpu_interrupt_handler = whpx_handle_interrupt;
-- 
2.13.6

[Qemu-devel] [PATCH 8/8] WHPX improve interrupt notification registration

[Justin Terry (VM)]

Improves the usage of the InterruptNotification registration by skipping the
additional call to WHvSetVirtualProcessorRegisters if we have already
registered for the window exit.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
---
 target/i386/whpx-all.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 24387bebad..940bbe590d 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -756,12 +756,11 @@ static void whpx_vcpu_pre_run(CPUState *cpu)
     }
 
     /* Update the state of the interrupt delivery notification */
-    if (cpu->interrupt_request & CPU_INTERRUPT_HARD) {
+    if (!vcpu->window_registered &&
+        cpu->interrupt_request & CPU_INTERRUPT_HARD) {
         reg_values[reg_count].DeliverabilityNotifications.InterruptNotification
             = 1;
-        if (vcpu->window_registered != 1) {
-            vcpu->window_registered = 1;
-        }
+        vcpu->window_registered = 1;
         reg_names[reg_count] = WHvX64RegisterDeliverabilityNotifications;
         reg_count += 1;
     }
-- 
2.13.6

Re: [Qemu-devel] [PATCH 0/8] WHPX updates based on the 17095 insider sdk.

[Paolo Bonzini]

On 26/02/2018 18:13, Justin Terry (VM) wrote:
> This change set includes fixes for two breaking changes that were introduced
> in the Windows Insider SDK 17095. First, the casing of the headers/libs changed
> such that a direct reference out of Windows Kits will fail during compile on
> case sensitive file systems. Second, a few API's were modified to improve
> performance and changes are required to QEMU to maintain ability to build on
> such platforms.
> 
> The rest of the changes are around improvements/fixes to the accelerator to
> increase performance and correctness.
> 
> Thanks,
> Justin Terry
> 
> Justin Terry (VM) (8):
>   Fixing WHPX casing to match SDK
>   Resolves WHPX breaking changes in SDK 17095
>   Remove unnecessary WHPX __debugbreak();
>   Fix WHPX additional lock acquisition
>   Fix WHPX typo in 'mmio'
>   Fix WHPX issue leaking tpr values
>   WHXP Removes the use of WHvGetExitContextSize
>   WHPX improve interrupt notification registration
> 
>  configure              | 10 ++++----
>  target/i386/whpx-all.c | 70 +++++++++++++++++---------------------------------
>  2 files changed, 28 insertions(+), 52 deletions(-)
> 

Queued, thanks!

Paolo