Re: [Qemu-devel] [PATCH v3 05/17] target-i386: Add x86_set_hyperv.

2012-09-20 Thread Eduardo Habkost 
On Wed, Sep 19, 2012 at 05:26:01PM -0400, Don Slutz wrote:
 On 09/19/12 15:32, Eduardo Habkost wrote:
 On Mon, Sep 17, 2012 at 10:00:55AM -0400, Don Slutz wrote:
 This is used to set the cpu object's hypervisor level to the default for 
 Microsoft's Hypervisor.
 
 Signed-off-by: Don Slutz d...@cloudswitch.com
 ---
   target-i386/cpu.c |   10 ++
   1 files changed, 10 insertions(+), 0 deletions(-)
 
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index 0e4a18d..4120393 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -1192,6 +1192,13 @@ static void x86_cpuid_set_hv_level(Object *obj, 
 Visitor *v, void *opaque,
   }
 
   #if !defined(CONFIG_USER_ONLY)
 +static void x86_set_hyperv(Object *obj, Error **errp)
 +{
 +X86CPU *cpu = X86_CPU(obj);
 +
 +cpu-env.cpuid_hv_level = HYPERV_CPUID_MIN;
 HYPERV_CPUID_MIN is defined on linux-headers/asm-x86/hyperv.h, that is
 included only if build host is linux-x86 and CONFIG_KVM is set.
 
 Will fix.  I see 3 options:
 
 1) Use the numbers like 0x4005

If we're going to use the number directly, it's better to have a define
for it (so #2 is better).

 
 2) Use QEMU defines like:
  #define CPUID_HV_LEVEL_HYPERV  0x4005
 
 3) Use condtional define:
  #ifndef HYPERV_CPUID_MIN
  #define CPUID_HV_LEVEL_HYPERV  0x4005
  #else
  #define CPUID_HV_LEVEL_HYPERV  HYPERV_CPUID_MIN
  #endif
 

If QEMU is going to contain a #define CPUID_HV_LEVEL_HYPERV 0x4005
in the code, I don't see a reason to try to use the definition from
asm-x86/hyperv.h if available.

So, #2 looks like the best option.

 I lean to #2 of #3 and both over #1.  This is because if in the
 future if HYPERV_CPUID_MIN ever changes then a patch to QEMU needs to
 be done and considered before that change can be seen by a guest.
 Note: since hypervisor-level=0x4006 can be specified, this might
 be enough to do some testing.

I don't think HYPERV_CPUID_MIN will ever change, but the meaning of the
constant is not clear to me. Anyway, if it ever changes, that's another
reason for QEMU to have its own constant. The resulting CPUID bits
exposed to the guest should be a function of the machine-type and
command-line/config parameters, and nothing else (otherwise the CPUID
bits would change under the guest's feet when live-migrating).

-- 
Eduardo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [PATCH v3 05/17] target-i386: Add x86_set_hyperv.

2012-09-19 Thread Eduardo Habkost 
On Mon, Sep 17, 2012 at 10:00:55AM -0400, Don Slutz wrote:
 This is used to set the cpu object's hypervisor level to the default for 
 Microsoft's Hypervisor.
 
 Signed-off-by: Don Slutz d...@cloudswitch.com
 ---
  target-i386/cpu.c |   10 ++
  1 files changed, 10 insertions(+), 0 deletions(-)
 
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index 0e4a18d..4120393 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -1192,6 +1192,13 @@ static void x86_cpuid_set_hv_level(Object *obj, 
 Visitor *v, void *opaque,
  }
  
  #if !defined(CONFIG_USER_ONLY)
 +static void x86_set_hyperv(Object *obj, Error **errp)
 +{
 +X86CPU *cpu = X86_CPU(obj);
 +
 +cpu-env.cpuid_hv_level = HYPERV_CPUID_MIN;

HYPERV_CPUID_MIN is defined on linux-headers/asm-x86/hyperv.h, that is
included only if build host is linux-x86 and CONFIG_KVM is set.


 +}
 +
  static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
   const char *name, Error **errp)
  {
 @@ -1214,6 +1221,7 @@ static void x86_set_hv_spinlocks(Object *obj, Visitor 
 *v, void *opaque,
  return;
  }
  hyperv_set_spinlock_retries(value);
 +x86_set_hyperv(obj, errp);
  }
  
  static void x86_get_hv_relaxed(Object *obj, Visitor *v, void *opaque,
 @@ -1234,6 +1242,7 @@ static void x86_set_hv_relaxed(Object *obj, Visitor *v, 
 void *opaque,
  return;
  }
  hyperv_enable_relaxed_timing(value);
 +x86_set_hyperv(obj, errp);
  }
  
  static void x86_get_hv_vapic(Object *obj, Visitor *v, void *opaque,
 @@ -1254,6 +1263,7 @@ static void x86_set_hv_vapic(Object *obj, Visitor *v, 
 void *opaque,
  return;
  }
  hyperv_enable_vapic_recommended(value);
 +x86_set_hyperv(obj, errp);
  }
  #endif
  
 -- 
 1.7.1
 
 

-- 
Eduardo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [PATCH v3 05/17] target-i386: Add x86_set_hyperv.

2012-09-19 Thread Don Slutz 

On 09/19/12 15:32, Eduardo Habkost wrote:

On Mon, Sep 17, 2012 at 10:00:55AM -0400, Don Slutz wrote:

This is used to set the cpu object's hypervisor level to the default for 
Microsoft's Hypervisor.

Signed-off-by: Don Slutz d...@cloudswitch.com
---
  target-i386/cpu.c |   10 ++
  1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0e4a18d..4120393 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1192,6 +1192,13 @@ static void x86_cpuid_set_hv_level(Object *obj, Visitor 
*v, void *opaque,
  }

  #if !defined(CONFIG_USER_ONLY)
+static void x86_set_hyperv(Object *obj, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+
+cpu-env.cpuid_hv_level = HYPERV_CPUID_MIN;

HYPERV_CPUID_MIN is defined on linux-headers/asm-x86/hyperv.h, that is
included only if build host is linux-x86 and CONFIG_KVM is set.


Will fix.  I see 3 options:

1) Use the numbers like 0x4005

2) Use QEMU defines like:
 #define CPUID_HV_LEVEL_HYPERV  0x4005

3) Use condtional define:
 #ifndef HYPERV_CPUID_MIN
 #define CPUID_HV_LEVEL_HYPERV  0x4005
 #else
 #define CPUID_HV_LEVEL_HYPERV  HYPERV_CPUID_MIN
 #endif

I lean to #2 of #3 and both over #1.  This is because if in the future 
if HYPERV_CPUID_MIN ever changes then a patch to QEMU needs to be done 
and considered before that change can be seen by a guest.  Note: since 
hypervisor-level=0x4006 can be specified, this might be enough to do 
some testing.


Please advise.


+}
+
  static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
   const char *name, Error **errp)
  {
@@ -1214,6 +1221,7 @@ static void x86_set_hv_spinlocks(Object *obj, Visitor *v, 
void *opaque,
  return;
  }
  hyperv_set_spinlock_retries(value);
+x86_set_hyperv(obj, errp);
  }

  static void x86_get_hv_relaxed(Object *obj, Visitor *v, void *opaque,
@@ -1234,6 +1242,7 @@ static void x86_set_hv_relaxed(Object *obj, Visitor *v, 
void *opaque,
  return;
  }
  hyperv_enable_relaxed_timing(value);
+x86_set_hyperv(obj, errp);
  }

  static void x86_get_hv_vapic(Object *obj, Visitor *v, void *opaque,
@@ -1254,6 +1263,7 @@ static void x86_set_hv_vapic(Object *obj, Visitor *v, 
void *opaque,
  return;
  }
  hyperv_enable_vapic_recommended(value);
+x86_set_hyperv(obj, errp);
  }
  #endif

--
1.7.1



   -Don Slutz
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html