[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RFC 03/22] i386: move hyperv_vendor_id initialization to x86_
From: |
Vitaly Kuznetsov |
Subject: |
Re: [PATCH RFC 03/22] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn() |
Date: |
Tue, 22 Sep 2020 12:23:40 +0200 |
Eduardo Habkost <ehabkost@redhat.com> writes:
> On Fri, Sep 04, 2020 at 04:54:12PM +0200, Vitaly Kuznetsov wrote:
>> As a preparation to expanding Hyper-V CPU features early, move
>> hyperv_vendor_id initialization to x86_cpu_realizefn().
>>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> target/i386/cpu.c | 15 ++++++++++++++-
>> target/i386/cpu.h | 3 ++-
>> target/i386/kvm.c | 25 ++++++++++---------------
>> 3 files changed, 26 insertions(+), 17 deletions(-)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 14489def2177..07e9da9e567e 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -6625,6 +6625,19 @@ static void x86_cpu_realizefn(DeviceState *dev, Error
>> **errp)
>> }
>> }
>>
>> + if (!cpu->hyperv_vendor) {
>> + memcpy(cpu->hyperv_vendor_id, "Microsoft Hv", 12);
>> + } else {
>> + size_t len = strlen(cpu->hyperv_vendor);
>> +
>> + if (len > 12) {
>> + warn_report("hv-vendor-id truncated to 12 characters");
>> + len = 12;
>> + }
>> + memset(cpu->hyperv_vendor_id, 0, 12);
>> + memcpy(cpu->hyperv_vendor_id, cpu->hyperv_vendor, len);
>> + }
>> +
>
> The change makes sense, but considering that we'll have a lot of
> new code added to x86_cpu_realizefn(), I would prefer to create a
> separate x86_cpu_hyperv_realize() function to make
> x86_cpu_realizefn() a bit more readable.
>
Agreed.
>
>> if (cpu->ucode_rev == 0) {
>> /* The default is the same as KVM's. */
>> if (IS_AMD_CPU(env)) {
>> @@ -7313,7 +7326,7 @@ static Property x86_cpu_properties[] = {
>> DEFINE_PROP_UINT32("min-xlevel2", X86CPU, env.cpuid_min_xlevel2, 0),
>> DEFINE_PROP_UINT64("ucode-rev", X86CPU, ucode_rev, 0),
>> DEFINE_PROP_BOOL("full-cpuid-auto-level", X86CPU,
>> full_cpuid_auto_level, true),
>> - DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id),
>> + DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor),
>> DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
>> DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false),
>> DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true),
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index d3097be6a50a..903994818093 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -1654,11 +1654,12 @@ struct X86CPU {
>> uint64_t ucode_rev;
>>
>> uint32_t hyperv_spinlock_attempts;
>> - char *hyperv_vendor_id;
>> + char *hyperv_vendor;
>> bool hyperv_synic_kvm_only;
>> uint64_t hyperv_features;
>> bool hyperv_passthrough;
>> OnOffAuto hyperv_no_nonarch_cs;
>> + uint32_t hyperv_vendor_id[3];
>>
>> bool check_cpuid;
>> bool enforce_cpuid;
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 205b68bc0ce8..47779c5e1efd 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -1225,6 +1225,13 @@ static int hyperv_handle_properties(CPUState *cs,
>> memcpy(cpuid_ent, &cpuid->entries[0],
>> cpuid->nent * sizeof(cpuid->entries[0]));
>>
>> + c = cpuid_find_entry(cpuid, HV_CPUID_VENDOR_AND_MAX_FUNCTIONS, 0);
>> + if (c) {
>> + cpu->hyperv_vendor_id[0] = c->ebx;
>> + cpu->hyperv_vendor_id[1] = c->ecx;
>> + cpu->hyperv_vendor_id[2] = c->edx;
>> + }
>> +
>
> I can't find the equivalent of this code in the current tree? Is
> hyperv vendor ID broken when using hv-passthrough today?
>
> Maybe this could be done as a separate patch, as it changes
> behavior of hv-passthrough?
(this and similar changes in other patches) Actually we don't change
anything. Before this series and with hv-passthrough we just don't
reflect host's CPUIDs in our internal QEMU structures so
e.g. X86CPU->hyperv_vendor remains 'Microsoft Hv' while in reality
guest sees what kernel told us ("Linux KVM Hv" BTW). We just copy
everything we get from KVM_GET_SUPPORTED_HV_CPUID into guest's CPUIDs.
This is fine as we didn't actually need the information in QEMU but
to achieve the goal of the series we need to keep proper in-QEMU
representation.
The real change is that post-series QEMU is not enabling any Hyper-V
features which it doesn't know about while pre-series it was actually
doing this. This is arguably a good change: enabling new features may
require some additional work (e.g. enabling capabilities in KVM) and
without it just passing CPUID feature bits the guest may get confused.
>
>> c = cpuid_find_entry(cpuid, HV_CPUID_FEATURES, 0);
>> if (c) {
>> env->features[FEAT_HYPERV_EAX] = c->eax;
>> @@ -1299,23 +1306,11 @@ static int hyperv_handle_properties(CPUState *cs,
>>
>> c = &cpuid_ent[cpuid_i++];
>> c->function = HV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
>> - if (!cpu->hyperv_vendor_id) {
>> - memcpy(signature, "Microsoft Hv", 12);
>> - } else {
>> - size_t len = strlen(cpu->hyperv_vendor_id);
>> -
>> - if (len > 12) {
>> - error_report("hv-vendor-id truncated to 12 characters");
>> - len = 12;
>> - }
>> - memset(signature, 0, 12);
>> - memcpy(signature, cpu->hyperv_vendor_id, len);
>> - }
>> c->eax = hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS) ?
>> HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
>> - c->ebx = signature[0];
>> - c->ecx = signature[1];
>> - c->edx = signature[2];
>> + c->ebx = cpu->hyperv_vendor_id[0];
>> + c->ecx = cpu->hyperv_vendor_id[1];
>> + c->edx = cpu->hyperv_vendor_id[2];
>>
>> c = &cpuid_ent[cpuid_i++];
>> c->function = HV_CPUID_INTERFACE;
>> --
>> 2.25.4
>>
--
Vitaly
- [PATCH RFC 00/22] i386: KVM: expand Hyper-V features early, Vitaly Kuznetsov, 2020/09/04
- [PATCH RFC 14/22] i386: move eVMCS enablement to hyperv_init_vcpu(), Vitaly Kuznetsov, 2020/09/04
- [PATCH RFC 01/22] WIP: update linux/headers, Vitaly Kuznetsov, 2020/09/04
- [PATCH RFC 03/22] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn(), Vitaly Kuznetsov, 2020/09/04
- [PATCH RFC 04/22] i386: move hyperv_interface_id initialization to x86_cpu_realizefn(), Vitaly Kuznetsov, 2020/09/04
- [PATCH RFC 06/22] i386: move hyperv_limits initialization to x86_cpu_realizefn(), Vitaly Kuznetsov, 2020/09/04
- [PATCH RFC 07/22] i386: fill in FEAT_HYPERV_EDX from edx instead of eax, Vitaly Kuznetsov, 2020/09/04
- [PATCH RFC 11/22] i386: add reserved FEAT_HV_NESTED_EBX/ECX/EDX CPUID leaves, Vitaly Kuznetsov, 2020/09/04
- [PATCH RFC 08/22] i386: invert hyperv_spinlock_attempts setting logic with hv_passthrough, Vitaly Kuznetsov, 2020/09/04
- [PATCH RFC 02/22] i386: drop x86_cpu_get_supported_feature_word() forward declaration, Vitaly Kuznetsov, 2020/09/04
- [PATCH RFC 10/22] i386: add reserved FEAT_HV_RECOMM_ECX/FEAT_HV_RECOMM_EDX CPUID leaves, Vitaly Kuznetsov, 2020/09/04