Claudio Fontana <cfont...@suse.de> writes: > On 11/19/20 11:32 AM, Vitaly Kuznetsov wrote: >> As a preparation to expanding Hyper-V CPU features early, move >> hyperv_vendor_id initialization to x86_cpu_realizefn(). Introduce >> x86_cpu_hyperv_realize() to not not pollute x86_cpu_realizefn() >> itself. >> >> Signed-off-by: Vitaly Kuznetsov <vkuzn...@redhat.com> > > > Hi Vitaly, > > If building for TCG-only, does the x86_cpu_hyperv_realize function and other > hyperv related functions > make sense to keep in the build? > > Now that we have per-accelerator subdirs in target/i386, should the hyperv > code be moved over? >
Hi Claudio, I'm not exactly sure. On one hand, we only implement Hyper-V emulation with KVM now. On the other hand Hyper-V specific CPU options are available even without it (and as Igor feels strongly against introducing custom option parsers, I don't see how we can forbid to use them without KVM). x86_cpu_hyperv_realize() is the bare minimum which is only needed to set our internal Hyper-V related data in a sane way, e.g. set the default to 'cpu->hyperv_vendor_id'. The actual enablement code is in target/i386/kvm.c already. Do you see anything besides x86_cpu_hyperv_realize() which we could move there? Could you please take a look at v5 (https://mail.gnu.org/archive/html/qemu-devel/2021-03/msg00158.html)? Thanks! > Thanks, > > Claudio > > >> --- >> target/i386/cpu.c | 23 ++++++++++++++++++++++- >> target/i386/cpu.h | 3 ++- >> target/i386/kvm.c | 25 ++++++++++--------------- >> 3 files changed, 34 insertions(+), 17 deletions(-) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index 5a8c96072e41..2a6885753378 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -6509,6 +6509,25 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool >> verbose) >> } >> } >> >> +static void x86_cpu_hyperv_realize(X86CPU *cpu) >> +{ >> + size_t len; >> + >> + /* Hyper-V vendor id */ >> + if (!cpu->hyperv_vendor) { >> + memcpy(cpu->hyperv_vendor_id, "Microsoft Hv", 12); >> + } else { >> + 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); >> + } >> +} >> + >> static void x86_cpu_realizefn(DeviceState *dev, Error **errp) >> { >> CPUState *cs = CPU(dev); >> @@ -6680,6 +6699,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error >> **errp) >> env->cache_info_amd.l3_cache = &legacy_l3_cache; >> } >> >> + /* Process Hyper-V enlightenments */ >> + x86_cpu_hyperv_realize(cpu); >> >> cpu_exec_realizefn(cs, &local_err); >> if (local_err != NULL) { >> @@ -7198,7 +7219,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 88e8586f8fb4..be640bf45c29 100644 >> --- a/target/i386/cpu.h >> +++ b/target/i386/cpu.h >> @@ -1655,11 +1655,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 a2934dda027c..788a2cf2ec51 100644 >> --- a/target/i386/kvm.c >> +++ b/target/i386/kvm.c >> @@ -1205,6 +1205,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; >> + } >> + >> c = cpuid_find_entry(cpuid, HV_CPUID_FEATURES, 0); >> if (c) { >> env->features[FEAT_HYPERV_EAX] = c->eax; >> @@ -1279,23 +1286,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; >> > -- Vitaly
