[PATCH] target/i386/hvf: add vmware-cpuid-freq cpu feature

yaroshchuk2000@gmail.com posted 1 patch 6 days, 19 hours ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210113205221.32308-1-yaroshchuk2000@gmail.com
Maintainers: Roman Bolshakov <r.bolshakov@yadro.com>, Paolo Bonzini <pbonzini@redhat.com>, Cameron Esfahani <dirty@apple.com>, Eduardo Habkost <ehabkost@redhat.com>, Richard Henderson <richard.henderson@linaro.org>
target/i386/hvf/hvf.c | 92 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 91 insertions(+), 1 deletion(-)

[PATCH] target/i386/hvf: add vmware-cpuid-freq cpu feature

Posted by yaroshchuk2000@gmail.com 6 days, 19 hours ago
From: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>

For `-accel hvf` cpu_x86_cpuid() is wrapped with hvf_cpu_x86_cpuid() to
add paravirtualization cpuid leaf 0x40000010
https://lkml.org/lkml/2008/10/1/246

Leaf 0x40000010, Timing Information:
EAX: (Virtual) TSC frequency in kHz.
EBX: (Virtual) Bus (local apic timer) frequency in kHz.
ECX, EDX: RESERVED (Per above, reserved fields are set to zero).

On macOS TSC and APIC Bus frequencies can be readed by sysctl call with
names `machdep.tsc.frequency` and `hw.busfrequency`

This options is required for Darwin-XNU guest to be synchronized with
host

Signed-off-by: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>
---
 target/i386/hvf/hvf.c | 92 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 91 insertions(+), 1 deletion(-)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index ed9356565c..0873e4969e 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -65,6 +65,7 @@
 
 #include <Hypervisor/hv.h>
 #include <Hypervisor/hv_vmx.h>
+#include <sys/sysctl.h>
 
 #include "exec/address-spaces.h"
 #include "hw/i386/apic_internal.h"
@@ -456,6 +457,50 @@ static void dummy_signal(int sig)
 {
 }
 
+static void init_tsc_freq(CPUX86State *env)
+{
+    size_t length;
+    uint64_t tsc_freq;
+
+    if (env->tsc_khz != 0) {
+        return;
+    }
+
+    length = sizeof(uint64_t);
+    if (sysctlbyname("machdep.tsc.frequency", &tsc_freq, &length, NULL, 0)) {
+        /* fprintf(stderr, "%s: sysctl machdep.tsc.frequency errored\n", __func__); */
+        return;
+    }
+    env->tsc_khz = tsc_freq / 1000;  /* Hz to KHz */
+}
+
+static void init_apic_bus_freq(CPUX86State *env)
+{
+    size_t length;
+    uint64_t bus_freq;
+
+    if (env->apic_bus_freq != 0) {
+        return;
+    }
+
+    length = sizeof(uint64_t);
+    if (sysctlbyname("hw.busfrequency", &bus_freq, &length, NULL, 0)) {
+        /* fprintf(stderr, "%s: sysctl hw.busfrequency errored\n", __func__); */
+        return;
+    }
+    env->apic_bus_freq = bus_freq;
+}
+
+static inline bool tsc_is_known(CPUX86State *env)
+{
+    return env->tsc_khz != 0;
+}
+
+static inline bool apic_bus_freq_is_known(CPUX86State *env)
+{
+    return env->apic_bus_freq != 0;
+}
+
 int hvf_init_vcpu(CPUState *cpu)
 {
 
@@ -480,6 +525,15 @@ int hvf_init_vcpu(CPUState *cpu)
     hvf_state->hvf_caps = g_new0(struct hvf_vcpu_caps, 1);
     env->hvf_mmio_buf = g_new(char, 4096);
 
+    if (x86cpu->vmware_cpuid_freq) {
+        init_tsc_freq(env);
+        init_apic_bus_freq(env);
+
+        if (!tsc_is_known(env) || !apic_bus_freq_is_known(env)) {
+            error_report("vmware-cpuid-freq: feature couldn't be enabled");
+        }
+    }
+
     r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
     cpu->vcpu_dirty = 1;
     assert_hvf_ok(r);
@@ -597,6 +651,42 @@ static void hvf_store_events(CPUState *cpu, uint32_t ins_len, uint64_t idtvec_in
     }
 }
 
+static void hvf_cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
+                              uint32_t *eax, uint32_t *ebx,
+                              uint32_t *ecx, uint32_t *edx)
+{
+    /*
+     * A wrapper extends cpu_x86_cpuid with 0x40000000 and 0x40000010 leafs
+     * Provides vmware-cpuid-freq support to hvf
+     */
+
+    uint32_t signature[3];
+
+    if (!tsc_is_known(env) || !apic_bus_freq_is_known(env)) {
+        cpu_x86_cpuid(env, index, count, eax, ebx, ecx, edx);
+        return;
+    }
+
+    switch (index) {
+        case 0x40000000:
+            memcpy(signature, "TCGTCGTCGTCG", 12); /* QEMU Signature */
+            *eax = 0x40000010;                     /* Max available cpuid leaf */
+            *ebx = signature[0];
+            *ecx = signature[1];
+            *edx = signature[2];
+            break;
+        case 0x40000010:
+            *eax = env->tsc_khz;
+            *ebx = env->apic_bus_freq / 1000; /* Hz to KHz */
+            *ecx = 0;
+            *edx = 0;
+            break;
+        default:
+            cpu_x86_cpuid(env, index, count, eax, ebx, ecx, edx);
+            break;
+    }
+}
+
 int hvf_vcpu_exec(CPUState *cpu)
 {
     X86CPU *x86_cpu = X86_CPU(cpu);
@@ -734,7 +824,7 @@ int hvf_vcpu_exec(CPUState *cpu)
             uint32_t rcx = (uint32_t)rreg(cpu->hvf_fd, HV_X86_RCX);
             uint32_t rdx = (uint32_t)rreg(cpu->hvf_fd, HV_X86_RDX);
 
-            cpu_x86_cpuid(env, rax, rcx, &rax, &rbx, &rcx, &rdx);
+            hvf_cpu_x86_cpuid(env, rax, rcx, &rax, &rbx, &rcx, &rdx);
 
             wreg(cpu->hvf_fd, HV_X86_RAX, rax);
             wreg(cpu->hvf_fd, HV_X86_RBX, rbx);
-- 
2.28.0


Re: [PATCH] target/i386/hvf: add vmware-cpuid-freq cpu feature

Posted by no-reply@patchew.org 6 days, 19 hours ago
Patchew URL: https://patchew.org/QEMU/20210113205221.32308-1-yaroshchuk2000@gmail.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210113205221.32308-1-yaroshchuk2000@gmail.com
Subject: [PATCH] target/i386/hvf: add vmware-cpuid-freq cpu feature

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
f4f6e29 target/i386/hvf: add vmware-cpuid-freq cpu feature

=== OUTPUT BEGIN ===
WARNING: line over 80 characters
#52: FILE: target/i386/hvf/hvf.c:471:
+        /* fprintf(stderr, "%s: sysctl machdep.tsc.frequency errored\n", __func__); */

ERROR: switch and case should be at the same indent
#124: FILE: target/i386/hvf/hvf.c:670:
+    switch (index) {
+        case 0x40000000:
[...]
+        case 0x40000010:
[...]
+        default:

WARNING: line over 80 characters
#127: FILE: target/i386/hvf/hvf.c:673:
+            *eax = 0x40000010;                     /* Max available cpuid leaf */

total: 1 errors, 2 warnings, 122 lines checked

Commit f4f6e29c5d8c (target/i386/hvf: add vmware-cpuid-freq cpu feature) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210113205221.32308-1-yaroshchuk2000@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

[PATCH v2] target/i386/hvf: add vmware-cpuid-freq cpu feature

Posted by yaroshchuk2000@gmail.com 5 days, 21 hours ago
From: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>

For `-accel hvf` cpu_x86_cpuid() is wrapped with hvf_cpu_x86_cpuid() to
add paravirtualization cpuid leaf 0x40000010
https://lkml.org/lkml/2008/10/1/246

Leaf 0x40000010, Timing Information:
EAX: (Virtual) TSC frequency in kHz.
EBX: (Virtual) Bus (local apic timer) frequency in kHz.
ECX, EDX: RESERVED (Per above, reserved fields are set to zero).

On macOS TSC and APIC Bus frequencies can be readed by sysctl call with
names `machdep.tsc.frequency` and `hw.busfrequency`

This options is required for Darwin-XNU guest to be synchronized with
host

Signed-off-by: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>
---
 target/i386/hvf/hvf.c | 90 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 89 insertions(+), 1 deletion(-)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index ed9356565c..a5daafe202 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -65,6 +65,7 @@
 
 #include <Hypervisor/hv.h>
 #include <Hypervisor/hv_vmx.h>
+#include <sys/sysctl.h>
 
 #include "exec/address-spaces.h"
 #include "hw/i386/apic_internal.h"
@@ -456,6 +457,48 @@ static void dummy_signal(int sig)
 {
 }
 
+static void init_tsc_freq(CPUX86State *env)
+{
+    size_t length;
+    uint64_t tsc_freq;
+
+    if (env->tsc_khz != 0) {
+        return;
+    }
+
+    length = sizeof(uint64_t);
+    if (sysctlbyname("machdep.tsc.frequency", &tsc_freq, &length, NULL, 0)) {
+        return;
+    }
+    env->tsc_khz = tsc_freq / 1000;  /* Hz to KHz */
+}
+
+static void init_apic_bus_freq(CPUX86State *env)
+{
+    size_t length;
+    uint64_t bus_freq;
+
+    if (env->apic_bus_freq != 0) {
+        return;
+    }
+
+    length = sizeof(uint64_t);
+    if (sysctlbyname("hw.busfrequency", &bus_freq, &length, NULL, 0)) {
+        return;
+    }
+    env->apic_bus_freq = bus_freq;
+}
+
+static inline bool tsc_is_known(CPUX86State *env)
+{
+    return env->tsc_khz != 0;
+}
+
+static inline bool apic_bus_freq_is_known(CPUX86State *env)
+{
+    return env->apic_bus_freq != 0;
+}
+
 int hvf_init_vcpu(CPUState *cpu)
 {
 
@@ -480,6 +523,15 @@ int hvf_init_vcpu(CPUState *cpu)
     hvf_state->hvf_caps = g_new0(struct hvf_vcpu_caps, 1);
     env->hvf_mmio_buf = g_new(char, 4096);
 
+    if (x86cpu->vmware_cpuid_freq) {
+        init_tsc_freq(env);
+        init_apic_bus_freq(env);
+
+        if (!tsc_is_known(env) || !apic_bus_freq_is_known(env)) {
+            error_report("vmware-cpuid-freq: feature couldn't be enabled");
+        }
+    }
+
     r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
     cpu->vcpu_dirty = 1;
     assert_hvf_ok(r);
@@ -597,6 +649,42 @@ static void hvf_store_events(CPUState *cpu, uint32_t ins_len, uint64_t idtvec_in
     }
 }
 
+static void hvf_cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
+                              uint32_t *eax, uint32_t *ebx,
+                              uint32_t *ecx, uint32_t *edx)
+{
+    /*
+     * A wrapper extends cpu_x86_cpuid with 0x40000000 and 0x40000010 leafs
+     * Provides vmware-cpuid-freq support to hvf
+     */
+
+    uint32_t signature[3];
+
+    if (!tsc_is_known(env) || !apic_bus_freq_is_known(env)) {
+        cpu_x86_cpuid(env, index, count, eax, ebx, ecx, edx);
+        return;
+    }
+
+    switch (index) {
+    case 0x40000000:
+        memcpy(signature, "TCGTCGTCGTCG", 12); /* QEMU Signature */
+        *eax = 0x40000010;                     /* Max available cpuid leaf */
+        *ebx = signature[0];
+        *ecx = signature[1];
+        *edx = signature[2];
+        break;
+    case 0x40000010:
+        *eax = env->tsc_khz;
+        *ebx = env->apic_bus_freq / 1000; /* Hz to KHz */
+        *ecx = 0;
+        *edx = 0;
+        break;
+    default:
+        cpu_x86_cpuid(env, index, count, eax, ebx, ecx, edx);
+        break;
+    }
+}
+
 int hvf_vcpu_exec(CPUState *cpu)
 {
     X86CPU *x86_cpu = X86_CPU(cpu);
@@ -734,7 +822,7 @@ int hvf_vcpu_exec(CPUState *cpu)
             uint32_t rcx = (uint32_t)rreg(cpu->hvf_fd, HV_X86_RCX);
             uint32_t rdx = (uint32_t)rreg(cpu->hvf_fd, HV_X86_RDX);
 
-            cpu_x86_cpuid(env, rax, rcx, &rax, &rbx, &rcx, &rdx);
+            hvf_cpu_x86_cpuid(env, rax, rcx, &rax, &rbx, &rcx, &rdx);
 
             wreg(cpu->hvf_fd, HV_X86_RAX, rax);
             wreg(cpu->hvf_fd, HV_X86_RBX, rbx);
-- 
2.28.0


Re: [PATCH v2] target/i386/hvf: add vmware-cpuid-freq cpu feature

Posted by dirty--- via 18 hours ago

> On Jan 14, 2021, at 11:47 AM, yaroshchuk2000@gmail.com wrote:
> 
> From: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>
> 
> For `-accel hvf` cpu_x86_cpuid() is wrapped with hvf_cpu_x86_cpuid() to
> add paravirtualization cpuid leaf 0x40000010
> https://lkml.org/lkml/2008/10/1/246
> 
> Leaf 0x40000010, Timing Information:
> EAX: (Virtual) TSC frequency in kHz.
> EBX: (Virtual) Bus (local apic timer) frequency in kHz.
> ECX, EDX: RESERVED (Per above, reserved fields are set to zero).
> 
> On macOS TSC and APIC Bus frequencies can be readed by sysctl call with
> names `machdep.tsc.frequency` and `hw.busfrequency`
> 
> This options is required for Darwin-XNU guest to be synchronized with
> host
> 
> Signed-off-by: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>
> ---
> target/i386/hvf/hvf.c | 90 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index ed9356565c..a5daafe202 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -65,6 +65,7 @@
> 
> #include <Hypervisor/hv.h>
> #include <Hypervisor/hv_vmx.h>
> +#include <sys/sysctl.h>
> 
> #include "exec/address-spaces.h"
> #include "hw/i386/apic_internal.h"
> @@ -456,6 +457,48 @@ static void dummy_signal(int sig)
> {
> }
> 
> +static void init_tsc_freq(CPUX86State *env)
> +{
> +    size_t length;
> +    uint64_t tsc_freq;
> +
> +    if (env->tsc_khz != 0) {
> +        return;
> +    }
> +
> +    length = sizeof(uint64_t);
> +    if (sysctlbyname("machdep.tsc.frequency", &tsc_freq, &length, NULL, 0)) {
> +        return;
> +    }
> +    env->tsc_khz = tsc_freq / 1000;  /* Hz to KHz */
> +}
> +
> +static void init_apic_bus_freq(CPUX86State *env)
> +{
> +    size_t length;
> +    uint64_t bus_freq;
> +
> +    if (env->apic_bus_freq != 0) {
> +        return;
> +    }
> +
> +    length = sizeof(uint64_t);
> +    if (sysctlbyname("hw.busfrequency", &bus_freq, &length, NULL, 0)) {
> +        return;
> +    }
> +    env->apic_bus_freq = bus_freq;
> +}
> +
> +static inline bool tsc_is_known(CPUX86State *env)
> +{
> +    return env->tsc_khz != 0;
> +}
> +
> +static inline bool apic_bus_freq_is_known(CPUX86State *env)
> +{
> +    return env->apic_bus_freq != 0;
> +}
> +
> int hvf_init_vcpu(CPUState *cpu)
> {
> 
> @@ -480,6 +523,15 @@ int hvf_init_vcpu(CPUState *cpu)
>     hvf_state->hvf_caps = g_new0(struct hvf_vcpu_caps, 1);
>     env->hvf_mmio_buf = g_new(char, 4096);
> 
> +    if (x86cpu->vmware_cpuid_freq) {
> +        init_tsc_freq(env);
> +        init_apic_bus_freq(env);
> +
> +        if (!tsc_is_known(env) || !apic_bus_freq_is_known(env)) {
> +            error_report("vmware-cpuid-freq: feature couldn't be enabled");
> +        }
> +    }
> +
>     r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
>     cpu->vcpu_dirty = 1;
>     assert_hvf_ok(r);
> @@ -597,6 +649,42 @@ static void hvf_store_events(CPUState *cpu, uint32_t ins_len, uint64_t idtvec_in
>     }
> }
> 

We already have hvf/x86_cpuid.c.  Can we put hvf_cpu_x86_cpuid() in there?

> +static void hvf_cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> +                              uint32_t *eax, uint32_t *ebx,
> +                              uint32_t *ecx, uint32_t *edx)
> +{
> +    /*
> +     * A wrapper extends cpu_x86_cpuid with 0x40000000 and 0x40000010 leafs
> +     * Provides vmware-cpuid-freq support to hvf
> +     */
> +
> +    uint32_t signature[3];
> +
> +    if (!tsc_is_known(env) || !apic_bus_freq_is_known(env)) {
> +        cpu_x86_cpuid(env, index, count, eax, ebx, ecx, edx);
> +        return;
> +    }
> +
> +    switch (index) {
> +    case 0x40000000:
> +        memcpy(signature, "TCGTCGTCGTCG", 12); /* QEMU Signature */

I agree with Roman, using "HVFHVFHVFHVF" is better.

> +        *eax = 0x40000010;                     /* Max available cpuid leaf */
> +        *ebx = signature[0];
> +        *ecx = signature[1];
> +        *edx = signature[2];
> +        break;
> +    case 0x40000010:
> +        *eax = env->tsc_khz;
> +        *ebx = env->apic_bus_freq / 1000; /* Hz to KHz */
> +        *ecx = 0;
> +        *edx = 0;
> +        break;
> +    default:
> +        cpu_x86_cpuid(env, index, count, eax, ebx, ecx, edx);
> +        break;
> +    }
> +}
> +
> int hvf_vcpu_exec(CPUState *cpu)
> {
>     X86CPU *x86_cpu = X86_CPU(cpu);
> @@ -734,7 +822,7 @@ int hvf_vcpu_exec(CPUState *cpu)
>             uint32_t rcx = (uint32_t)rreg(cpu->hvf_fd, HV_X86_RCX);
>             uint32_t rdx = (uint32_t)rreg(cpu->hvf_fd, HV_X86_RDX);
> 
> -            cpu_x86_cpuid(env, rax, rcx, &rax, &rbx, &rcx, &rdx);
> +            hvf_cpu_x86_cpuid(env, rax, rcx, &rax, &rbx, &rcx, &rdx);
> 
>             wreg(cpu->hvf_fd, HV_X86_RAX, rax);
>             wreg(cpu->hvf_fd, HV_X86_RBX, rbx);
> -- 
> 2.28.0
> 
> 

Looks good.

Cameron Esfahani
dirty@apple.com


Re: [PATCH v2] target/i386/hvf: add vmware-cpuid-freq cpu feature

Posted by Roman Bolshakov 22 hours ago
On Thu, Jan 14, 2021 at 10:47:03PM +0300, yaroshchuk2000@gmail.com wrote:
> From: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>
> 
> For `-accel hvf` cpu_x86_cpuid() is wrapped with hvf_cpu_x86_cpuid() to
> add paravirtualization cpuid leaf 0x40000010
> https://lkml.org/lkml/2008/10/1/246
> 
> Leaf 0x40000010, Timing Information:
> EAX: (Virtual) TSC frequency in kHz.
> EBX: (Virtual) Bus (local apic timer) frequency in kHz.
> ECX, EDX: RESERVED (Per above, reserved fields are set to zero).
> 
> On macOS TSC and APIC Bus frequencies can be readed by sysctl call with
> names `machdep.tsc.frequency` and `hw.busfrequency`
> 
> This options is required for Darwin-XNU guest to be synchronized with
> host
> 
> Signed-off-by: Vladislav Yaroshchuk <yaroshchuk2000@gmail.com>
> ---
>  target/i386/hvf/hvf.c | 90 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index ed9356565c..a5daafe202 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -65,6 +65,7 @@
>  
>  #include <Hypervisor/hv.h>
>  #include <Hypervisor/hv_vmx.h>
> +#include <sys/sysctl.h>
>  
>  #include "exec/address-spaces.h"
>  #include "hw/i386/apic_internal.h"
> @@ -456,6 +457,48 @@ static void dummy_signal(int sig)
>  {
>  }
>  
> +static void init_tsc_freq(CPUX86State *env)
> +{
> +    size_t length;
> +    uint64_t tsc_freq;
> +
> +    if (env->tsc_khz != 0) {
> +        return;
> +    }
> +
> +    length = sizeof(uint64_t);
> +    if (sysctlbyname("machdep.tsc.frequency", &tsc_freq, &length, NULL, 0)) {
> +        return;
> +    }
> +    env->tsc_khz = tsc_freq / 1000;  /* Hz to KHz */
> +}
> +
> +static void init_apic_bus_freq(CPUX86State *env)
> +{
> +    size_t length;
> +    uint64_t bus_freq;
> +
> +    if (env->apic_bus_freq != 0) {
> +        return;
> +    }
> +
> +    length = sizeof(uint64_t);
> +    if (sysctlbyname("hw.busfrequency", &bus_freq, &length, NULL, 0)) {
> +        return;
> +    }
> +    env->apic_bus_freq = bus_freq;
> +}
> +
> +static inline bool tsc_is_known(CPUX86State *env)
> +{
> +    return env->tsc_khz != 0;
> +}
> +
> +static inline bool apic_bus_freq_is_known(CPUX86State *env)
> +{
> +    return env->apic_bus_freq != 0;
> +}
> +
>  int hvf_init_vcpu(CPUState *cpu)
>  {
>  
> @@ -480,6 +523,15 @@ int hvf_init_vcpu(CPUState *cpu)
>      hvf_state->hvf_caps = g_new0(struct hvf_vcpu_caps, 1);
>      env->hvf_mmio_buf = g_new(char, 4096);
>  
> +    if (x86cpu->vmware_cpuid_freq) {
> +        init_tsc_freq(env);
> +        init_apic_bus_freq(env);
> +
> +        if (!tsc_is_known(env) || !apic_bus_freq_is_known(env)) {
> +            error_report("vmware-cpuid-freq: feature couldn't be enabled");
> +        }
> +    }
> +
>      r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
>      cpu->vcpu_dirty = 1;
>      assert_hvf_ok(r);
> @@ -597,6 +649,42 @@ static void hvf_store_events(CPUState *cpu, uint32_t ins_len, uint64_t idtvec_in
>      }
>  }
>  
> +static void hvf_cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> +                              uint32_t *eax, uint32_t *ebx,
> +                              uint32_t *ecx, uint32_t *edx)
> +{
> +    /*
> +     * A wrapper extends cpu_x86_cpuid with 0x40000000 and 0x40000010 leafs
> +     * Provides vmware-cpuid-freq support to hvf
> +     */
> +
> +    uint32_t signature[3];
> +
> +    if (!tsc_is_known(env) || !apic_bus_freq_is_known(env)) {
> +        cpu_x86_cpuid(env, index, count, eax, ebx, ecx, edx);
> +        return;
> +    }
> +
> +    switch (index) {
> +    case 0x40000000:
> +        memcpy(signature, "TCGTCGTCGTCG", 12); /* QEMU Signature */

Hi Vladislav,

TCG belongs to TCG accel identification, for HVF it should be
HVFHVFHVFHVF.

> +        *eax = 0x40000010;                     /* Max available cpuid leaf */
> +        *ebx = signature[0];
> +        *ecx = signature[1];
> +        *edx = signature[2];

TCG and KVM don't report their identity unless kvm or tcg-cpuid
properties are set. I wonder if we need to guard it likewise?

But as of now QEMU is not consistent in that regard. Two parameters are
needed for KVM - kvm=on,vmware-cpuid-freq=on. vmware-cpuid-freq is
sufficient for WHPX but WHPX doesn't expose itself (ebx=ecx=edx=0). TCG
doesn't seem to support vmware-cpuid-freq but reports it's name only if
tcg-cpuid property is set.

> +        break;

CPUID for not implemented hypervisor-specific leafs from 0x40000001 up
to 0x4000000f should be all zeroes but cpu_x86_cpuid() only returns zero
values for 0x40000001. Likely, you need to reset return values for the
leafs here or in cpu_x86_cpuid(). In the latter case you'll also fix a
similar bug in WHPX accel.

Otherwise, looks good.

Thanks,
Roman

> +    case 0x40000010:
> +        *eax = env->tsc_khz;
> +        *ebx = env->apic_bus_freq / 1000; /* Hz to KHz */
> +        *ecx = 0;
> +        *edx = 0;
> +        break;
> +    default:
> +        cpu_x86_cpuid(env, index, count, eax, ebx, ecx, edx);
> +        break;
> +    }
> +}
> +
>  int hvf_vcpu_exec(CPUState *cpu)
>  {
>      X86CPU *x86_cpu = X86_CPU(cpu);
> @@ -734,7 +822,7 @@ int hvf_vcpu_exec(CPUState *cpu)
>              uint32_t rcx = (uint32_t)rreg(cpu->hvf_fd, HV_X86_RCX);
>              uint32_t rdx = (uint32_t)rreg(cpu->hvf_fd, HV_X86_RDX);
>  
> -            cpu_x86_cpuid(env, rax, rcx, &rax, &rbx, &rcx, &rdx);
> +            hvf_cpu_x86_cpuid(env, rax, rcx, &rax, &rbx, &rcx, &rdx);
>  
>              wreg(cpu->hvf_fd, HV_X86_RAX, rax);
>              wreg(cpu->hvf_fd, HV_X86_RBX, rbx);
> -- 
> 2.28.0
> 

Re: [PATCH v2] target/i386/hvf: add vmware-cpuid-freq cpu feature

Posted by Paolo Bonzini 22 hours ago
On 19/01/21 19:01, Roman Bolshakov wrote:
>> +    switch (index) {
>> +    case 0x40000000:
>> +        memcpy(signature, "TCGTCGTCGTCG", 12); /* QEMU Signature */
> Hi Vladislav,
> 
> TCG belongs to TCG accel identification, for HVF it should be
> HVFHVFHVFHVF.

You can use the KVM identification just fine, as long as you comply with 
the rest of the KVM CPUID specification.

Paolo