| Message ID | 20201119103221.1665171-2-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i386: simplify Hyper-V enlightenments enablement | expand |
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 <vkuznets@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? 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; >
Claudio Fontana <cfontana@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 <vkuznets@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; >> >
On 3/10/21 12:43 PM, Vitaly Kuznetsov wrote: > Claudio Fontana <cfontana@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 <vkuznets@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. Hi Vitaly, > 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? Currently I don't, I see a lot of PROPs (hyperv_features), but I assume those are the ones you mention cannot be moved right? > Could you please > take a look at v5 > (https://mail.gnu.org/archive/html/qemu-devel/2021-03/msg00158.html)? I took a look at the series you pointed me to, and it seems it is all pretty much kvm/ work to me. Which hypervisors do you think would theoretically make sense when looking at hyperv emulation in the future? Looking at the current code and your patches, in the series you link to, I'd be tempted to suggest moving the hyperv realize function inside kvm_cpu_realizefn in kvm/kvm-cpu.c . Ciao, Claudio > > 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; >>> >> >
Claudio Fontana <cfontana@suse.de> writes: > On 3/10/21 12:43 PM, Vitaly Kuznetsov wrote: >> Claudio Fontana <cfontana@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 <vkuznets@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. > > > Hi Vitaly, > > >> 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? > > Currently I don't, > I see a lot of PROPs (hyperv_features), but I assume those are the ones you mention cannot be moved right? > Right, not without introducing a custom parser checking if KVM is in use. > >> Could you please >> take a look at v5 >> (https://mail.gnu.org/archive/html/qemu-devel/2021-03/msg00158.html)? > > > I took a look at the series you pointed me to, and it seems it is all pretty much kvm/ work to me. > > Which hypervisors do you think would theoretically make sense when looking at hyperv emulation in the future? > In theory, I don't see why we can't emulate hyper-v in TCG, I'm just not sure how practical that would be). There was a Windows Hypervisor Platform accelerator (whpx) which uses genuine Hyper-V and can use Hyper-V enlightenment option to set guest visible CPUIDs but honestly I don't know much about whpx. > Looking at the current code and your patches, in the series you link to, > I'd be tempted to suggest moving the hyperv realize function inside > kvm_cpu_realizefn in kvm/kvm-cpu.c . No problem, I wouldn't be against that. I'm waiting for some feedback on v5 and if I am to do v6 I'll include that. In case I got luck and v5 is to get merged, I can send a follow-up patch. > > Ciao, > > Claudio > > > >> >> 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; >>>> >>> >> >
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;
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 <vkuznets@redhat.com> --- target/i386/cpu.c | 23 ++++++++++++++++++++++- target/i386/cpu.h | 3 ++- target/i386/kvm.c | 25 ++++++++++--------------- 3 files changed, 34 insertions(+), 17 deletions(-)