[libvirt] [PATCH] Enable support for nested SVM
This enables support for nested SVM using the regular CPU model/features block. If the CPU model or features include 'svm' or 'vmx', then the '-enable-nesting' flag will be added to the QEMU command line. Several of the models already include svm support, but QEMU was just masking out the svm bit silently. So this will enable SVM on such models * src/qemu/qemu_conf.h: flag for -enable-nesting * src/qemu/qemu_conf.c: Use -enable-nesting if VMX or SVM are in the CPUID * src/cpu/cpu.h, src/cpu/cpu.c: API to check for a named feature * src/cpu/cpu_x86.c: x86 impl of feature check * src/libvirt_private.syms: Add cpuHasFeature --- src/cpu/cpu.c | 24 ++++++++++++++++++++++ src/cpu/cpu.h | 12 +++++++++++ src/cpu/cpu_x86.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 19 +++++++++++++++- src/qemu/qemu_conf.h | 1 + 6 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index def6974..c7a282e 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -424,3 +424,27 @@ cpuUpdate(virCPUDefPtr guest, return driver->update(guest, host); } + +bool +cpuHasFeature(const char *arch, + const union cpuData *data, + const char *feature) +{ + struct cpuArchDriver *driver; + + VIR_DEBUG("arch=%s, data=%p, feature=%s", + arch, data, feature); + + if ((driver = cpuGetSubDriver(arch)) == NULL) + return -1; + + if (driver->hasFeature == NULL) { + virCPUReportError(VIR_ERR_NO_SUPPORT, + _("cannot check guest CPU data for %s architecture"), + arch); + return -1; + } + + return driver->hasFeature(arch, data, feature); +} + diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index a745917..405af48 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -82,6 +82,11 @@ typedef int (*cpuArchUpdate) (virCPUDefPtr guest, const virCPUDefPtr host); +typedef bool +(*cpuArchHasFeature) (const char *arch, + const union cpuData *data, + const char *feature); + struct cpuArchDriver { const char *name; @@ -95,6 +100,7 @@ struct cpuArchDriver { cpuArchGuestData guestData; cpuArchBaseline baseline; cpuArchUpdate update; + cpuArchHasFeature hasFeature; }; @@ -151,4 +157,10 @@ extern int cpuUpdate (virCPUDefPtr guest, const virCPUDefPtr host); +extern bool +cpuHasFeature(const char *arch, + const union cpuData *data, + const char *feature); + + #endif /* __VIR_CPU_H__ */ diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 1937901..cc82d58 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1754,6 +1754,55 @@ cleanup: return ret; } +static bool x86HasFeature(const char *arch ATTRIBUTE_UNUSED, + const union cpuData *data, + const char *name) +{ + struct x86_map *map; + struct x86_feature *feature; + bool ret = false; + int i; + + if (!(map = x86LoadMap())) + return false; + + if (!(feature = x86FeatureFind(map, name))) + goto cleanup; + + for (i = 0 ; i < data->x86.basic_len ; i++) { + if (data->x86.basic[i].function == feature->cpuid->function && + ((data->x86.basic[i].eax & feature->cpuid->eax) + == feature->cpuid->eax) && + ((data->x86.basic[i].ebx & feature->cpuid->ebx) + == feature->cpuid->ebx) && + ((data->x86.basic[i].ecx & feature->cpuid->ecx) + == feature->cpuid->ecx) && + ((data->x86.basic[i].edx & feature->cpuid->edx) + == feature->cpuid->edx)) { + ret = true; + goto cleanup; + } + } + + for (i = 0 ; i < data->x86.extended_len ; i++) { + if (data->x86.extended[i].function == feature->cpuid->function && + ((data->x86.extended[i].eax & feature->cpuid->eax) + == feature->cpuid->eax) && + ((data->x86.extended[i].ebx & feature->cpuid->ebx) + == feature->cpuid->ebx) && + ((data->x86.extended[i].ecx & feature->cpuid->ecx) + == feature->cpuid->ecx) && + ((data->x86.extended[i].edx & feature->cpuid->edx) + == feature->cpuid->edx)) { + ret = true; + goto cleanup; + } + } + +cleanup: + x86MapFree(map); + return ret; +} struct cpuArchDriver cpuDriverX86 = { .name = "x86", @@ -1771,4 +1820,5 @@ struct cpuArchDriver cpuDriverX86 = { .guestData = x86GuestData, .baseline = x86Baseline, .update = x86Update, + .hasFeature = x86HasFeature, }; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c2905ba..428b887 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -90,6 +90,7 @@ cpuEncode; cpuGuestData; cpuNodeData; cpuUpdate; +cpuHasFeature; # cpu_conf.h diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 7a37c70..88dbf9b 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1210,6 +1210,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_NO_KVM_PIT; if (strstr(help, "-tdf")) flags |= QEMUD_CMD_FLAG_TDF; + if (strstr(help, "-enable-nesting")) + flags |= QEMUD_CMD_FLAG_NESTING; if (strstr(help, ",menu=on")) flags |= QEMUD_CMD_FLAG_BOOT_MENU; @@ -3500,7 +3502,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, const char *emulator, unsigned long long qemuCmdFlags, const struct utsname *ut, - char **opt) + char **opt, + bool *hasHwVirt) { const virCPUDefPtr host = driver->caps->host.cpu; virCPUDefPtr guest = NULL; @@ -3511,6 +3514,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, virBuffer buf = VIR_BUFFER_INITIALIZER; int i; + *hasHwVirt = false; + if (def->cpu && def->cpu->model) { if (qemudProbeCPUModels(emulator, qemuCmdFlags, ut->machine, &ncpus, &cpus) < 0) @@ -3552,6 +3557,10 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, if (cpuDecode(guest, data, cpus, ncpus, preferred) < 0) goto cleanup; + *hasHwVirt = + cpuHasFeature(guest->arch, data, "svm") || + cpuHasFeature(guest->arch, data, "vmx"); + virBufferVSprintf(&buf, "%s", guest->model); for (i = 0; i < guest->nfeatures; i++) { char sign; @@ -3678,6 +3687,7 @@ int qemudBuildCommandLine(virConnectPtr conn, char *cpu; char *smp; int last_good_net = -1; + bool hasHwVirt = false; uname_normalize(&ut); @@ -3871,13 +3881,18 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT(def->os.machine); } - if (qemuBuildCpuArgStr(driver, def, emulator, qemuCmdFlags, &ut, &cpu) < 0) + if (qemuBuildCpuArgStr(driver, def, emulator, qemuCmdFlags, + &ut, &cpu, &hasHwVirt) < 0) goto error; if (cpu) { ADD_ARG_LIT("-cpu"); ADD_ARG_LIT(cpu); VIR_FREE(cpu); + + if ((qemuCmdFlags & QEMUD_CMD_FLAG_NESTING) && + hasHwVirt) + ADD_ARG_LIT("-enable-nesting"); } if (disableKQEMU) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 2c9e608..16f72f5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -93,6 +93,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_NODEFCONFIG = (1LL << 37), /* -nodefconfig */ QEMUD_CMD_FLAG_BOOT_MENU = (1LL << 38), /* -boot menu=on support */ QEMUD_CMD_FLAG_ENABLE_KQEMU = (1LL << 39), /* -enable-kqemu flag */ + QEMUD_CMD_FLAG_NESTING = (1LL << 40), /* -enable-nesting (SVM/VMX) */ }; /* Main driver state */ -- 1.7.2.3
On 09/22/2010 10:47 AM, Daniel P. Berrange wrote: Marking patches as v2 makes it easier for reviewers to note that this is fixing comments from an earlier review :)
This enables support for nested SVM using the regular CPU model/features block. If the CPU model or features include 'svm' or 'vmx', then the '-enable-nesting' flag will be added to the QEMU command line. Several of the models already include svm support, but QEMU was just masking out the svm bit silently. So this will enable SVM on such models
ACK to this patch, but...
+++ b/src/qemu/qemu_conf.h @@ -93,6 +93,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_NODEFCONFIG = (1LL<< 37), /* -nodefconfig */ QEMUD_CMD_FLAG_BOOT_MENU = (1LL<< 38), /* -boot menu=on support */ QEMUD_CMD_FLAG_ENABLE_KQEMU = (1LL<< 39), /* -enable-kqemu flag */ + QEMUD_CMD_FLAG_NESTING = (1LL<< 40), /* -enable-nesting (SVM/VMX) */
...as a followup, should we add to tests/qemuhelpdata an example of qemu output that supports -enable-nesting, to be sure the parser works correctly? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Wed, Sep 22, 2010 at 11:13:06AM -0600, Eric Blake wrote:
On 09/22/2010 10:47 AM, Daniel P. Berrange wrote:
Marking patches as v2 makes it easier for reviewers to note that this is fixing comments from an earlier review :)
This enables support for nested SVM using the regular CPU model/features block. If the CPU model or features include 'svm' or 'vmx', then the '-enable-nesting' flag will be added to the QEMU command line. Several of the models already include svm support, but QEMU was just masking out the svm bit silently. So this will enable SVM on such models
ACK to this patch, but...
+++ b/src/qemu/qemu_conf.h @@ -93,6 +93,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_NODEFCONFIG = (1LL<< 37), /* -nodefconfig */ QEMUD_CMD_FLAG_BOOT_MENU = (1LL<< 38), /* -boot menu=on support */ QEMUD_CMD_FLAG_ENABLE_KQEMU = (1LL<< 39), /* -enable-kqemu flag */ + QEMUD_CMD_FLAG_NESTING = (1LL<< 40), /* -enable-nesting (SVM/VMX) */
...as a followup, should we add to tests/qemuhelpdata an example of qemu output that supports -enable-nesting, to be sure the parser works correctly?
Yep, need that already or the test case is broken, so I will add that. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Wed, Sep 22, 2010 at 05:47:42PM +0100, Daniel P. Berrange wrote:
This enables support for nested SVM using the regular CPU model/features block. If the CPU model or features include 'svm' or 'vmx', then the '-enable-nesting' flag will be added to the QEMU command line. Several of the models already include svm support, but QEMU was just masking out the svm bit silently. So this will enable SVM on such models
I'm very glad to see this patch, as I've wanted to run nested VMs on my development VM for a while. Can you give a little more information on what's required to run a nested VM? Dave
* src/qemu/qemu_conf.h: flag for -enable-nesting * src/qemu/qemu_conf.c: Use -enable-nesting if VMX or SVM are in the CPUID * src/cpu/cpu.h, src/cpu/cpu.c: API to check for a named feature * src/cpu/cpu_x86.c: x86 impl of feature check * src/libvirt_private.syms: Add cpuHasFeature --- src/cpu/cpu.c | 24 ++++++++++++++++++++++ src/cpu/cpu.h | 12 +++++++++++ src/cpu/cpu_x86.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 19 +++++++++++++++- src/qemu/qemu_conf.h | 1 + 6 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index def6974..c7a282e 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -424,3 +424,27 @@ cpuUpdate(virCPUDefPtr guest,
return driver->update(guest, host); } + +bool +cpuHasFeature(const char *arch, + const union cpuData *data, + const char *feature) +{ + struct cpuArchDriver *driver; + + VIR_DEBUG("arch=%s, data=%p, feature=%s", + arch, data, feature); + + if ((driver = cpuGetSubDriver(arch)) == NULL) + return -1; + + if (driver->hasFeature == NULL) { + virCPUReportError(VIR_ERR_NO_SUPPORT, + _("cannot check guest CPU data for %s architecture"), + arch); + return -1; + } + + return driver->hasFeature(arch, data, feature); +} + diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index a745917..405af48 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -82,6 +82,11 @@ typedef int (*cpuArchUpdate) (virCPUDefPtr guest, const virCPUDefPtr host);
+typedef bool +(*cpuArchHasFeature) (const char *arch, + const union cpuData *data, + const char *feature); +
struct cpuArchDriver { const char *name; @@ -95,6 +100,7 @@ struct cpuArchDriver { cpuArchGuestData guestData; cpuArchBaseline baseline; cpuArchUpdate update; + cpuArchHasFeature hasFeature; };
@@ -151,4 +157,10 @@ extern int cpuUpdate (virCPUDefPtr guest, const virCPUDefPtr host);
+extern bool +cpuHasFeature(const char *arch, + const union cpuData *data, + const char *feature); + + #endif /* __VIR_CPU_H__ */ diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 1937901..cc82d58 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1754,6 +1754,55 @@ cleanup: return ret; }
+static bool x86HasFeature(const char *arch ATTRIBUTE_UNUSED, + const union cpuData *data, + const char *name) +{ + struct x86_map *map; + struct x86_feature *feature; + bool ret = false; + int i; + + if (!(map = x86LoadMap())) + return false; + + if (!(feature = x86FeatureFind(map, name))) + goto cleanup; + + for (i = 0 ; i < data->x86.basic_len ; i++) { + if (data->x86.basic[i].function == feature->cpuid->function && + ((data->x86.basic[i].eax & feature->cpuid->eax) + == feature->cpuid->eax) && + ((data->x86.basic[i].ebx & feature->cpuid->ebx) + == feature->cpuid->ebx) && + ((data->x86.basic[i].ecx & feature->cpuid->ecx) + == feature->cpuid->ecx) && + ((data->x86.basic[i].edx & feature->cpuid->edx) + == feature->cpuid->edx)) { + ret = true; + goto cleanup; + } + } + + for (i = 0 ; i < data->x86.extended_len ; i++) { + if (data->x86.extended[i].function == feature->cpuid->function && + ((data->x86.extended[i].eax & feature->cpuid->eax) + == feature->cpuid->eax) && + ((data->x86.extended[i].ebx & feature->cpuid->ebx) + == feature->cpuid->ebx) && + ((data->x86.extended[i].ecx & feature->cpuid->ecx) + == feature->cpuid->ecx) && + ((data->x86.extended[i].edx & feature->cpuid->edx) + == feature->cpuid->edx)) { + ret = true; + goto cleanup; + } + } + +cleanup: + x86MapFree(map); + return ret; +}
struct cpuArchDriver cpuDriverX86 = { .name = "x86", @@ -1771,4 +1820,5 @@ struct cpuArchDriver cpuDriverX86 = { .guestData = x86GuestData, .baseline = x86Baseline, .update = x86Update, + .hasFeature = x86HasFeature, }; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c2905ba..428b887 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -90,6 +90,7 @@ cpuEncode; cpuGuestData; cpuNodeData; cpuUpdate; +cpuHasFeature;
# cpu_conf.h diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 7a37c70..88dbf9b 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1210,6 +1210,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_NO_KVM_PIT; if (strstr(help, "-tdf")) flags |= QEMUD_CMD_FLAG_TDF; + if (strstr(help, "-enable-nesting")) + flags |= QEMUD_CMD_FLAG_NESTING; if (strstr(help, ",menu=on")) flags |= QEMUD_CMD_FLAG_BOOT_MENU;
@@ -3500,7 +3502,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, const char *emulator, unsigned long long qemuCmdFlags, const struct utsname *ut, - char **opt) + char **opt, + bool *hasHwVirt) { const virCPUDefPtr host = driver->caps->host.cpu; virCPUDefPtr guest = NULL; @@ -3511,6 +3514,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, virBuffer buf = VIR_BUFFER_INITIALIZER; int i;
+ *hasHwVirt = false; + if (def->cpu && def->cpu->model) { if (qemudProbeCPUModels(emulator, qemuCmdFlags, ut->machine, &ncpus, &cpus) < 0) @@ -3552,6 +3557,10 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, if (cpuDecode(guest, data, cpus, ncpus, preferred) < 0) goto cleanup;
+ *hasHwVirt = + cpuHasFeature(guest->arch, data, "svm") || + cpuHasFeature(guest->arch, data, "vmx"); + virBufferVSprintf(&buf, "%s", guest->model); for (i = 0; i < guest->nfeatures; i++) { char sign; @@ -3678,6 +3687,7 @@ int qemudBuildCommandLine(virConnectPtr conn, char *cpu; char *smp; int last_good_net = -1; + bool hasHwVirt = false;
uname_normalize(&ut);
@@ -3871,13 +3881,18 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT(def->os.machine); }
- if (qemuBuildCpuArgStr(driver, def, emulator, qemuCmdFlags, &ut, &cpu) < 0) + if (qemuBuildCpuArgStr(driver, def, emulator, qemuCmdFlags, + &ut, &cpu, &hasHwVirt) < 0) goto error;
if (cpu) { ADD_ARG_LIT("-cpu"); ADD_ARG_LIT(cpu); VIR_FREE(cpu); + + if ((qemuCmdFlags & QEMUD_CMD_FLAG_NESTING) && + hasHwVirt) + ADD_ARG_LIT("-enable-nesting"); }
if (disableKQEMU) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 2c9e608..16f72f5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -93,6 +93,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_NODEFCONFIG = (1LL << 37), /* -nodefconfig */ QEMUD_CMD_FLAG_BOOT_MENU = (1LL << 38), /* -boot menu=on support */ QEMUD_CMD_FLAG_ENABLE_KQEMU = (1LL << 39), /* -enable-kqemu flag */ + QEMUD_CMD_FLAG_NESTING = (1LL << 40), /* -enable-nesting (SVM/VMX) */ };
/* Main driver state */ -- 1.7.2.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Sep 22, 2010 at 02:19:06PM -0400, Dave Allan wrote:
On Wed, Sep 22, 2010 at 05:47:42PM +0100, Daniel P. Berrange wrote:
This enables support for nested SVM using the regular CPU model/features block. If the CPU model or features include 'svm' or 'vmx', then the '-enable-nesting' flag will be added to the QEMU command line. Several of the models already include svm support, but QEMU was just masking out the svm bit silently. So this will enable SVM on such models
I'm very glad to see this patch, as I've wanted to run nested VMs on my development VM for a while. Can you give a little more information on what's required to run a nested VM?
On a host with SVM, simply copy the <cpu> block from the virsh capabilities output into your guest config. This should already include the SVM feature flag, either explicitly, or as part of the model (eg Phenom includes svm). You need KVM > 0.10.5. You're out of luck with VMX for now. There are patches on kvm mailing list, but they're still being discussed with some people questioning whether it should be done at all :-( Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index def6974..c7a282e 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -424,3 +424,27 @@ cpuUpdate(virCPUDefPtr guest,
return driver->update(guest, host); } + +bool +cpuHasFeature(const char *arch, + const union cpuData *data, + const char *feature) +{ + struct cpuArchDriver *driver; + + VIR_DEBUG("arch=%s, data=%p, feature=%s", + arch, data, feature); + + if ((driver = cpuGetSubDriver(arch)) == NULL) + return -1; + + if (driver->hasFeature == NULL) { + virCPUReportError(VIR_ERR_NO_SUPPORT, + _("cannot check guest CPU data for %s architecture"), + arch); + return -1; + } + + return driver->hasFeature(arch, data, feature);
No need to pass arch down here.
+} + diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index a745917..405af48 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -82,6 +82,11 @@ typedef int (*cpuArchUpdate) (virCPUDefPtr guest, const virCPUDefPtr host);
+typedef bool +(*cpuArchHasFeature) (const char *arch, + const union cpuData *data, + const char *feature); +
The 'arch' argument is not needed here since cpuHasFeature already selected appropriate implementation according to CPU architecture.
struct cpuArchDriver { const char *name; @@ -95,6 +100,7 @@ struct cpuArchDriver { cpuArchGuestData guestData; cpuArchBaseline baseline; cpuArchUpdate update; + cpuArchHasFeature hasFeature; };
@@ -151,4 +157,10 @@ extern int cpuUpdate (virCPUDefPtr guest, const virCPUDefPtr host);
+extern bool +cpuHasFeature(const char *arch, + const union cpuData *data, + const char *feature); + + #endif /* __VIR_CPU_H__ */ diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 1937901..cc82d58 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1754,6 +1754,55 @@ cleanup: return ret; }
+static bool x86HasFeature(const char *arch ATTRIBUTE_UNUSED, + const union cpuData *data, + const char *name)
No arch argument needed here as well.
+{ + struct x86_map *map; + struct x86_feature *feature; + bool ret = false; + int i; + + if (!(map = x86LoadMap())) + return false; + + if (!(feature = x86FeatureFind(map, name))) + goto cleanup; + + for (i = 0 ; i < data->x86.basic_len ; i++) { + if (data->x86.basic[i].function == feature->cpuid->function && + ((data->x86.basic[i].eax & feature->cpuid->eax) + == feature->cpuid->eax) && + ((data->x86.basic[i].ebx & feature->cpuid->ebx) + == feature->cpuid->ebx) && + ((data->x86.basic[i].ecx & feature->cpuid->ecx) + == feature->cpuid->ecx) && + ((data->x86.basic[i].edx & feature->cpuid->edx) + == feature->cpuid->edx)) { + ret = true; + goto cleanup; + } + } + + for (i = 0 ; i < data->x86.extended_len ; i++) { + if (data->x86.extended[i].function == feature->cpuid->function && + ((data->x86.extended[i].eax & feature->cpuid->eax) + == feature->cpuid->eax) && + ((data->x86.extended[i].ebx & feature->cpuid->ebx) + == feature->cpuid->ebx) && + ((data->x86.extended[i].ecx & feature->cpuid->ecx) + == feature->cpuid->ecx) && + ((data->x86.extended[i].edx & feature->cpuid->edx) + == feature->cpuid->edx)) { + ret = true; + goto cleanup; + } + }
The two for loops should be replaced by the following single loop (which in practise will be walked through only once): for (i = 0; i < feature->ncpuid; i++) { struct cpuX86cpuid *cpuid; cpuid = x86DataCpuid(data, feature->cpuid[i].function); if (cpuid && x86cpuidMatchMasked(cpuid, feature->cpuid + i)) { ret = true; goto cleanup; } }
+ +cleanup: + x86MapFree(map); + return ret; +}
struct cpuArchDriver cpuDriverX86 = { .name = "x86", @@ -1771,4 +1820,5 @@ struct cpuArchDriver cpuDriverX86 = { .guestData = x86GuestData, .baseline = x86Baseline, .update = x86Update, + .hasFeature = x86HasFeature, };
The current code would actually work so this is not a show stopper and the changes could be done as part of a bigger cleanup patch for cpu_x86.c that I have in my git tree. However I won't send the cleanup patch before I finish unit tests for all this to check the cleanup doesn't break anything. The rest looks fine. Jirka
participants (4)
-
Daniel P. Berrange -
Dave Allan -
Eric Blake -
Jiri Denemark