Re: [libvirt] [PATCH] Enable support for nested SVM

2010-10-13 Thread Daniel Veillard 
On Tue, Oct 12, 2010 at 08:23:21AM -0600, Eric Blake wrote:
 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.

  Yup, that's why I'm always a bit suspicious when a function returns
a boolean, that said for an internal API it's less of a problem

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Enable support for nested SVM (v4)

2010-10-13 Thread Daniel P. Berrange 
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

In v4: changed bool to int to allow error code propagation

* 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/qemuhelptest.c: Add nesting flag where required
---
 src/cpu/cpu.c|   24 
 src/cpu/cpu.h|   11 +++
 src/cpu/cpu_x86.c|   30 ++
 src/libvirt_private.syms |1 +
 src/qemu/qemu_conf.c |   24 ++--
 src/qemu/qemu_conf.h |1 +
 tests/qemuhelptest.c |   12 
 7 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index def6974..62db3fe 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -424,3 +424,27 @@ cpuUpdate(virCPUDefPtr guest,
 
 return driver-update(guest, host);
 }
+
+int
+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(data, feature);
+}
+
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index a745917..76d0e8e 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -82,6 +82,10 @@ typedef int
 (*cpuArchUpdate)(virCPUDefPtr guest,
  const virCPUDefPtr host);
 
+typedef int
+(*cpuArchHasFeature) (const union cpuData *data,
+  const char *feature);
+
 
 struct cpuArchDriver {
 const char *name;
@@ -95,6 +99,7 @@ struct cpuArchDriver {
 cpuArchGuestDataguestData;
 cpuArchBaseline baseline;
 cpuArchUpdate   update;
+cpuArchHasFeaturehasFeature;
 };
 
 
@@ -151,4 +156,10 @@ extern int
 cpuUpdate   (virCPUDefPtr guest,
  const virCPUDefPtr host);
 
+extern int
+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..813b499 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1754,6 +1754,35 @@ cleanup:
 return ret;
 }
 
+static int x86HasFeature(const union cpuData *data,
+ const char *name)
+{
+struct x86_map *map;
+struct x86_feature *feature;
+int ret = -1;
+int i;
+
+if (!(map = x86LoadMap()))
+return -1;
+
+if (!(feature = x86FeatureFind(map, name)))
+goto cleanup;
+
+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 = 1;
+goto cleanup;
+}
+}
+ret = 0;
+
+cleanup:
+x86MapFree(map);
+return ret;
+}
 
 struct cpuArchDriver cpuDriverX86 = {
 .name = x86,
@@ -1771,4 +1800,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 1d8ea95..966289b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -96,6 +96,7 @@ cpuEncode;
 cpuGuestData;
 cpuNodeData;
 cpuUpdate;
+cpuHasFeature;
 
 
 # cpu_conf.h
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 16145b0..de14f92 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1211,6 +1211,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;
 if (strstr(help, -fsdev))
@@ -3577,7 +3579,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver,
const char *emulator,
unsigned long long qemuCmdFlags,

Re: [libvirt] [PATCH] Enable support for nested SVM (v4)

2010-10-13 Thread Daniel Veillard 
On Wed, Oct 13, 2010 at 04:04:00PM +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', 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
 
 In v4: changed bool to int to allow error code propagation

ACK,

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Enable support for nested SVM

2010-10-12 Thread Daniel P. Berrange 
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

* 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/qemuhelptest.c: Add nesting flag where required
---
 src/cpu/cpu.c|   24 
 src/cpu/cpu.h|   11 +++
 src/cpu/cpu_x86.c|   29 +
 src/libvirt_private.syms |1 +
 src/qemu/qemu_conf.c |   21 +++--
 src/qemu/qemu_conf.h |1 +
 tests/qemuhelptest.c |   12 
 7 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index def6974..f509e1c 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(data, feature);
+}
+
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);
+
 
 struct cpuArchDriver {
 const char *name;
@@ -95,6 +99,7 @@ struct cpuArchDriver {
 cpuArchGuestDataguestData;
 cpuArchBaseline baseline;
 cpuArchUpdate   update;
+cpuArchHasFeaturehasFeature;
 };
 
 
@@ -151,4 +156,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..42349f0 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1754,6 +1754,34 @@ cleanup:
 return ret;
 }
 
+static bool x86HasFeature(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  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 +1799,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 301b0ef..0189183 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 737b255..429c399 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;
 
@@ -3503,7 +3505,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver,
const char *emulator,
unsigned long long qemuCmdFlags,
const struct utsname *ut,
-   char **opt)
+   char **opt,

Re: [libvirt] [PATCH] Enable support for nested SVM

2010-10-12 Thread Eric Blake 

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   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Enable support for nested SVM

2010-09-22 Thread Daniel P. Berrange 
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/qemu/qemu_conf.c |   39 +--
 src/qemu/qemu_conf.h |1 +
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 7a37c70..a2aab95 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;
 
@@ -3494,13 +3496,36 @@ error:
 }
 
 
+#define CPUID_FUNCTION_SVM 0x8001
+#define CPUID_ECX_SVM  0x0004
+
+#define CPUID_FUNCTION_VMX 0x0001
+#define CPUID_ECX_VMX  0x0020
+
+static bool qemuCpuHasHwVirt(union cpuData *data)
+{
+int i;
+for (i = 0 ; i  data-x86.basic_len ; i++) {
+if (data-x86.basic[i].function == CPUID_FUNCTION_VMX 
+(data-x86.basic[i].ecx  CPUID_ECX_VMX))
+return true;
+}
+for (i = 0 ; i  data-x86.extended_len ; i++) {
+if (data-x86.extended[i].function == CPUID_FUNCTION_SVM 
+(data-x86.extended[i].ecx  CPUID_ECX_SVM))
+return true;
+}
+return false;
+}
+
 static int
 qemuBuildCpuArgStr(const struct qemud_driver *driver,
const virDomainDefPtr def,
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 +3536,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 +3579,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver,
 if (cpuDecode(guest, data, cpus, ncpus, preferred)  0)
 goto cleanup;
 
+*hasHwVirt = qemuCpuHasHwVirt(data);
+
 virBufferVSprintf(buf, %s, guest-model);
 for (i = 0; i  guest-nfeatures; i++) {
 char sign;
@@ -3678,6 +3707,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
 char *cpu;
 char *smp;
 int last_good_net = -1;
+bool hasHwVirt = false;
 
 uname_normalize(ut);
 
@@ -3871,13 +3901,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

[libvirt] [PATCH] Enable support for nested SVM

2010-09-22 Thread Daniel P. Berrange 
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 {
 cpuArchGuestDataguestData;
 cpuArchBaseline baseline;
 cpuArchUpdate   update;
+cpuArchHasFeaturehasFeature;
 };
 
 
@@ -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

Re: [libvirt] [PATCH] Enable support for nested SVM

2010-09-22 Thread Eric Blake 

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   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Enable support for nested SVM

2010-09-22 Thread Dave Allan 
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 {
  cpuArchGuestDataguestData;
  cpuArchBaseline baseline;
  cpuArchUpdate   update;
 +cpuArchHasFeaturehasFeature;
  };
  
  
 @@ -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 = {

Re: [libvirt] [PATCH] Enable support for nested SVM

2010-09-22 Thread Daniel P. Berrange 
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Enable support for nested SVM

2010-09-22 Thread Daniel P. Berrange 
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list