Re: [libvirt] [PATCH] Enable support for nested SVM
On 10/12/2010 04:46 AM, 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', then the '-enable-nesting' flag will be added to the
QEMU command line. Latest out of tree patches for nested
'vmx', no longer require the '-enable-nesting' flag. They
instead just look at the cpu features. 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
+
+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;
Ouch. -1 promotes to true.
+
+ if (driver->hasFeature == NULL) {
+ virCPUReportError(VIR_ERR_NO_SUPPORT,
+ _("cannot check guest CPU data for %s architecture"),
+ arch);
+ return -1;
Likewise.
+ }
+
+ return driver->hasFeature(data, feature);
+}
You either need to change the return type to int and take a bool*
parameter (return -1 on failure, 0 on success when *param was written
to), or return an int value rather than a bool; since the caller needs
to distinguish between feature-not-present and error-message-reported.
+
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index a745917..3a7b996 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -82,6 +82,10 @@ typedef int
(*cpuArchUpdate) (virCPUDefPtr guest,
const virCPUDefPtr host);
+typedef bool
+(*cpuArchHasFeature) (const union cpuData *data,
+ const char *feature);
But this typedef is okay.
...
+ 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;
I probably would have used 'break' rather than 'goto cleanup', since
it's shorter, but since you already have to have the label due to
earlier code in the method, either way is fine.
+ }
+ }
+
+cleanup:
+ x86MapFree(map);
+ return ret;
+}
+++ 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;
Any reason you didn't put the new feature at the end of the list, in
enum order?
@@ -3555,6 +3560,12 @@ qemuBuildCpuArgStr(const struct qemud_driver
*driver,
if (cpuDecode(guest, data, cpus, ncpus, preferred)< 0)
goto cleanup;
+ /* Only 'svm' requires --enable-nesting. The out-of-tree
+ * 'vmx' patches now simply hook off the CPU features
This comment will be out-of-date once the vmx patches are no longer out
of tree. Should we just say:
"Nested vmx support in qemu simply hooks off the CPU features"
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org