Re: [PATCH v6 05/16] target-i386: Use Hypervisor level in -machine pc,accel=kvm.

2012-10-10 Thread Don Slutz 

On 10/09/12 15:11, Marcelo Tosatti wrote:

On Tue, Oct 09, 2012 at 03:09:17PM -0400, Don Slutz wrote:

On 10/09/12 14:47, Marcelo Tosatti wrote:

On Tue, Oct 09, 2012 at 03:27:26PM -0300, Marcelo Tosatti wrote:

On Tue, Oct 09, 2012 at 02:18:05PM -0300, Marcelo Tosatti wrote:

On Mon, Sep 24, 2012 at 10:32:07AM -0400, Don Slutz wrote:

Also known as Paravirtualization level.

This change is based on:

Microsoft Hypervisor CPUID Leaves:
   
http://msdn.microsoft.com/en-us/library/windows/hardware/ff542428%28v=vs.85%29.aspx

Linux kernel change starts with:
   http://fixunix.com/kernel/538707-use-cpuid-communicate-hypervisor.html
Also:
   http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html

VMware documention on CPUIDs (Mechanisms to determine if software is
running in a VMware virtual machine):
   
http://kb.vmware.com/selfservice/microsites/search.do?language=en_UScmd=displayKCexternalId=1009458

QEMU knows this is KVM_CPUID_SIGNATURE (0x4000).

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

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 895d848..8462c75 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -389,12 +389,12 @@ int kvm_arch_init_vcpu(CPUX86State *env)
  c = cpuid_data.entries[cpuid_i++];
  memset(c, 0, sizeof(*c));
  c-function = KVM_CPUID_SIGNATURE
-if (!hyperv_enabled()) {
+if (!env-cpuid_hv_level_set) {
  memcpy(signature, KVMKVMKVM\0\0\0, 12);
  c-eax = 0;
  } else {
  memcpy(signature, Microsoft Hv, 12);
-c-eax = HYPERV_CPUID_MIN;
+c-eax = env-cpuid_hv_level;

This breaks hyperv_enabled() checks.

Don, are you certain it is worthwhile to make this configurable?
Can you explain why, under your scenario, it is worthwhile?

Because these are separate problems:

- Fake VMWare hypervisor  (which seems to be your main goal).
- Make CPUID HV leafs configurable via command line.

Err, meant via properties. Point is, why have VMWare CPUID
configuration as data, if there are reasons to believe code
is a better fit (code as in current Hyper-V implementation).

Nevermind, its the right thing to do. Just separate the patchset
please:

1) Create object properties.
2) Export VMWare CPUID via properties.
3) Convert Hyper-V.

Be careful to make sure Hyper-V's current options are functional
in 3).


Did you mean 3 patch sets (or more)? Or just a different order?
-Don Slutz

Different order. Patches should be logically related (think of what
information the reviewer needs). Please write changelogs for
every patch.


Using this order causes support for Hyper-V to stop working in the 
middle of the patch set.  How about this order:

1) Create object properties.
2) Convert Hyper-V to set the new properties.
3) Change kvm.c to use the new properties.
4) Export VMWare CPUID via properties.

5) Change accel=tcg to use the new properties.

   -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

Re: [PATCH v6 05/16] target-i386: Use Hypervisor level in -machine pc,accel=kvm.

2012-10-10 Thread Marcelo Tosatti 
On Wed, Oct 10, 2012 at 09:03:20AM -0400, Don Slutz wrote:
 Did you mean 3 patch sets (or more)? Or just a different order?
 -Don Slutz
 Different order. Patches should be logically related (think of what
 information the reviewer needs). Please write changelogs for
 every patch.
 
 
 Using this order causes support for Hyper-V to stop working in the
 middle of the patch set.  How about this order:
 1) Create object properties.
 2) Convert Hyper-V to set the new properties.
 3) Change kvm.c to use the new properties.
 4) Export VMWare CPUID via properties.
 
 5) Change accel=tcg to use the new properties.
 
-Don Slutz

Fine, as long as change from item A) does not leak to item B).

--
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: [PATCH v6 05/16] target-i386: Use Hypervisor level in -machine pc,accel=kvm.

2012-10-09 Thread Marcelo Tosatti 
On Mon, Sep 24, 2012 at 10:32:07AM -0400, Don Slutz wrote:
 Also known as Paravirtualization level.
 
 This change is based on:
 
 Microsoft Hypervisor CPUID Leaves:
   
 http://msdn.microsoft.com/en-us/library/windows/hardware/ff542428%28v=vs.85%29.aspx
 
 Linux kernel change starts with:
   http://fixunix.com/kernel/538707-use-cpuid-communicate-hypervisor.html
 Also:
   http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html
 
 VMware documention on CPUIDs (Mechanisms to determine if software is
 running in a VMware virtual machine):
   
 http://kb.vmware.com/selfservice/microsites/search.do?language=en_UScmd=displayKCexternalId=1009458
 
 QEMU knows this is KVM_CPUID_SIGNATURE (0x4000).
 
 Signed-off-by: Don Slutz d...@cloudswitch.com
 ---
  target-i386/kvm.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/target-i386/kvm.c b/target-i386/kvm.c
 index 895d848..8462c75 100644
 --- a/target-i386/kvm.c
 +++ b/target-i386/kvm.c
 @@ -389,12 +389,12 @@ int kvm_arch_init_vcpu(CPUX86State *env)
  c = cpuid_data.entries[cpuid_i++];
  memset(c, 0, sizeof(*c));
  c-function = KVM_CPUID_SIGNATURE
 -if (!hyperv_enabled()) {
 +if (!env-cpuid_hv_level_set) {
  memcpy(signature, KVMKVMKVM\0\0\0, 12);
  c-eax = 0;
  } else {
  memcpy(signature, Microsoft Hv, 12);
 -c-eax = HYPERV_CPUID_MIN;
 +c-eax = env-cpuid_hv_level;

This breaks hyperv_enabled() checks. 

Don, are you certain it is worthwhile to make this configurable? 
Can you explain why, under your scenario, it is worthwhile?

Because these are separate problems:

- Fake VMWare hypervisor  (which seems to be your main goal).
- Make CPUID HV leafs configurable via command line.

My point is that the CPUIDs must be carefully constructed, 
that i miss the point why making them configurable is 
desired.

--
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: [PATCH v6 05/16] target-i386: Use Hypervisor level in -machine pc,accel=kvm.

2012-10-09 Thread Marcelo Tosatti 
On Tue, Oct 09, 2012 at 02:18:05PM -0300, Marcelo Tosatti wrote:
 On Mon, Sep 24, 2012 at 10:32:07AM -0400, Don Slutz wrote:
  Also known as Paravirtualization level.
  
  This change is based on:
  
  Microsoft Hypervisor CPUID Leaves:

  http://msdn.microsoft.com/en-us/library/windows/hardware/ff542428%28v=vs.85%29.aspx
  
  Linux kernel change starts with:
http://fixunix.com/kernel/538707-use-cpuid-communicate-hypervisor.html
  Also:
http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html
  
  VMware documention on CPUIDs (Mechanisms to determine if software is
  running in a VMware virtual machine):

  http://kb.vmware.com/selfservice/microsites/search.do?language=en_UScmd=displayKCexternalId=1009458
  
  QEMU knows this is KVM_CPUID_SIGNATURE (0x4000).
  
  Signed-off-by: Don Slutz d...@cloudswitch.com
  ---
   target-i386/kvm.c |4 ++--
   1 files changed, 2 insertions(+), 2 deletions(-)
  
  diff --git a/target-i386/kvm.c b/target-i386/kvm.c
  index 895d848..8462c75 100644
  --- a/target-i386/kvm.c
  +++ b/target-i386/kvm.c
  @@ -389,12 +389,12 @@ int kvm_arch_init_vcpu(CPUX86State *env)
   c = cpuid_data.entries[cpuid_i++];
   memset(c, 0, sizeof(*c));
   c-function = KVM_CPUID_SIGNATURE
  -if (!hyperv_enabled()) {
  +if (!env-cpuid_hv_level_set) {
   memcpy(signature, KVMKVMKVM\0\0\0, 12);
   c-eax = 0;
   } else {
   memcpy(signature, Microsoft Hv, 12);
  -c-eax = HYPERV_CPUID_MIN;
  +c-eax = env-cpuid_hv_level;
 
 This breaks hyperv_enabled() checks. 
 
 Don, are you certain it is worthwhile to make this configurable? 
 Can you explain why, under your scenario, it is worthwhile?
 
 Because these are separate problems:
 
 - Fake VMWare hypervisor  (which seems to be your main goal).
 - Make CPUID HV leafs configurable via command line.

Err, meant via properties. Point is, why have VMWare CPUID
configuration as data, if there are reasons to believe code 
is a better fit (code as in current Hyper-V implementation).

 My point is that the CPUIDs must be carefully constructed, 
 that i miss the point why making them configurable is 
 desired.
 
 --
 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
--
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: [PATCH v6 05/16] target-i386: Use Hypervisor level in -machine pc,accel=kvm.

2012-10-09 Thread Marcelo Tosatti 
On Tue, Oct 09, 2012 at 03:27:26PM -0300, Marcelo Tosatti wrote:
 On Tue, Oct 09, 2012 at 02:18:05PM -0300, Marcelo Tosatti wrote:
  On Mon, Sep 24, 2012 at 10:32:07AM -0400, Don Slutz wrote:
   Also known as Paravirtualization level.
   
   This change is based on:
   
   Microsoft Hypervisor CPUID Leaves:
 
   http://msdn.microsoft.com/en-us/library/windows/hardware/ff542428%28v=vs.85%29.aspx
   
   Linux kernel change starts with:
 http://fixunix.com/kernel/538707-use-cpuid-communicate-hypervisor.html
   Also:
 http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html
   
   VMware documention on CPUIDs (Mechanisms to determine if software is
   running in a VMware virtual machine):
 
   http://kb.vmware.com/selfservice/microsites/search.do?language=en_UScmd=displayKCexternalId=1009458
   
   QEMU knows this is KVM_CPUID_SIGNATURE (0x4000).
   
   Signed-off-by: Don Slutz d...@cloudswitch.com
   ---
target-i386/kvm.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
   
   diff --git a/target-i386/kvm.c b/target-i386/kvm.c
   index 895d848..8462c75 100644
   --- a/target-i386/kvm.c
   +++ b/target-i386/kvm.c
   @@ -389,12 +389,12 @@ int kvm_arch_init_vcpu(CPUX86State *env)
c = cpuid_data.entries[cpuid_i++];
memset(c, 0, sizeof(*c));
c-function = KVM_CPUID_SIGNATURE
   -if (!hyperv_enabled()) {
   +if (!env-cpuid_hv_level_set) {
memcpy(signature, KVMKVMKVM\0\0\0, 12);
c-eax = 0;
} else {
memcpy(signature, Microsoft Hv, 12);
   -c-eax = HYPERV_CPUID_MIN;
   +c-eax = env-cpuid_hv_level;
  
  This breaks hyperv_enabled() checks. 
  
  Don, are you certain it is worthwhile to make this configurable? 
  Can you explain why, under your scenario, it is worthwhile?
  
  Because these are separate problems:
  
  - Fake VMWare hypervisor  (which seems to be your main goal).
  - Make CPUID HV leafs configurable via command line.
 
 Err, meant via properties. Point is, why have VMWare CPUID
 configuration as data, if there are reasons to believe code 
 is a better fit (code as in current Hyper-V implementation).

Nevermind, its the right thing to do. Just separate the patchset
please:

1) Create object properties.
2) Export VMWare CPUID via properties.
3) Convert Hyper-V. 

Be careful to make sure Hyper-V's current options are functional 
in 3).

--
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: [PATCH v6 05/16] target-i386: Use Hypervisor level in -machine pc,accel=kvm.

2012-10-09 Thread Don Slutz 

On 10/09/12 14:47, Marcelo Tosatti wrote:

On Tue, Oct 09, 2012 at 03:27:26PM -0300, Marcelo Tosatti wrote:

On Tue, Oct 09, 2012 at 02:18:05PM -0300, Marcelo Tosatti wrote:

On Mon, Sep 24, 2012 at 10:32:07AM -0400, Don Slutz wrote:

Also known as Paravirtualization level.

This change is based on:

Microsoft Hypervisor CPUID Leaves:
   
http://msdn.microsoft.com/en-us/library/windows/hardware/ff542428%28v=vs.85%29.aspx

Linux kernel change starts with:
   http://fixunix.com/kernel/538707-use-cpuid-communicate-hypervisor.html
Also:
   http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html

VMware documention on CPUIDs (Mechanisms to determine if software is
running in a VMware virtual machine):
   
http://kb.vmware.com/selfservice/microsites/search.do?language=en_UScmd=displayKCexternalId=1009458

QEMU knows this is KVM_CPUID_SIGNATURE (0x4000).

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

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 895d848..8462c75 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -389,12 +389,12 @@ int kvm_arch_init_vcpu(CPUX86State *env)
  c = cpuid_data.entries[cpuid_i++];
  memset(c, 0, sizeof(*c));
  c-function = KVM_CPUID_SIGNATURE
-if (!hyperv_enabled()) {
+if (!env-cpuid_hv_level_set) {
  memcpy(signature, KVMKVMKVM\0\0\0, 12);
  c-eax = 0;
  } else {
  memcpy(signature, Microsoft Hv, 12);
-c-eax = HYPERV_CPUID_MIN;
+c-eax = env-cpuid_hv_level;

This breaks hyperv_enabled() checks.

Don, are you certain it is worthwhile to make this configurable?
Can you explain why, under your scenario, it is worthwhile?

Because these are separate problems:

- Fake VMWare hypervisor  (which seems to be your main goal).
- Make CPUID HV leafs configurable via command line.

Err, meant via properties. Point is, why have VMWare CPUID
configuration as data, if there are reasons to believe code
is a better fit (code as in current Hyper-V implementation).

Nevermind, its the right thing to do. Just separate the patchset
please:

1) Create object properties.
2) Export VMWare CPUID via properties.
3) Convert Hyper-V.

Be careful to make sure Hyper-V's current options are functional
in 3).


Did you mean 3 patch sets (or more)? Or just a different order?
   -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

Re: [PATCH v6 05/16] target-i386: Use Hypervisor level in -machine pc,accel=kvm.

2012-10-09 Thread Marcelo Tosatti 
On Tue, Oct 09, 2012 at 03:09:17PM -0400, Don Slutz wrote:
 On 10/09/12 14:47, Marcelo Tosatti wrote:
 On Tue, Oct 09, 2012 at 03:27:26PM -0300, Marcelo Tosatti wrote:
 On Tue, Oct 09, 2012 at 02:18:05PM -0300, Marcelo Tosatti wrote:
 On Mon, Sep 24, 2012 at 10:32:07AM -0400, Don Slutz wrote:
 Also known as Paravirtualization level.
 
 This change is based on:
 
 Microsoft Hypervisor CPUID Leaves:

  http://msdn.microsoft.com/en-us/library/windows/hardware/ff542428%28v=vs.85%29.aspx
 
 Linux kernel change starts with:
http://fixunix.com/kernel/538707-use-cpuid-communicate-hypervisor.html
 Also:
http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html
 
 VMware documention on CPUIDs (Mechanisms to determine if software is
 running in a VMware virtual machine):

  http://kb.vmware.com/selfservice/microsites/search.do?language=en_UScmd=displayKCexternalId=1009458
 
 QEMU knows this is KVM_CPUID_SIGNATURE (0x4000).
 
 Signed-off-by: Don Slutz d...@cloudswitch.com
 ---
   target-i386/kvm.c |4 ++--
   1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/target-i386/kvm.c b/target-i386/kvm.c
 index 895d848..8462c75 100644
 --- a/target-i386/kvm.c
 +++ b/target-i386/kvm.c
 @@ -389,12 +389,12 @@ int kvm_arch_init_vcpu(CPUX86State *env)
   c = cpuid_data.entries[cpuid_i++];
   memset(c, 0, sizeof(*c));
   c-function = KVM_CPUID_SIGNATURE
 -if (!hyperv_enabled()) {
 +if (!env-cpuid_hv_level_set) {
   memcpy(signature, KVMKVMKVM\0\0\0, 12);
   c-eax = 0;
   } else {
   memcpy(signature, Microsoft Hv, 12);
 -c-eax = HYPERV_CPUID_MIN;
 +c-eax = env-cpuid_hv_level;
 This breaks hyperv_enabled() checks.
 
 Don, are you certain it is worthwhile to make this configurable?
 Can you explain why, under your scenario, it is worthwhile?
 
 Because these are separate problems:
 
 - Fake VMWare hypervisor  (which seems to be your main goal).
 - Make CPUID HV leafs configurable via command line.
 Err, meant via properties. Point is, why have VMWare CPUID
 configuration as data, if there are reasons to believe code
 is a better fit (code as in current Hyper-V implementation).
 Nevermind, its the right thing to do. Just separate the patchset
 please:
 
 1) Create object properties.
 2) Export VMWare CPUID via properties.
 3) Convert Hyper-V.
 
 Be careful to make sure Hyper-V's current options are functional
 in 3).
 
 Did you mean 3 patch sets (or more)? Or just a different order?
-Don Slutz

Different order. Patches should be logically related (think of what
information the reviewer needs). Please write changelogs for
every patch.


--
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

[PATCH v6 05/16] target-i386: Use Hypervisor level in -machine pc,accel=kvm.

2012-09-24 Thread Don Slutz 
Also known as Paravirtualization level.

This change is based on:

Microsoft Hypervisor CPUID Leaves:
  
http://msdn.microsoft.com/en-us/library/windows/hardware/ff542428%28v=vs.85%29.aspx

Linux kernel change starts with:
  http://fixunix.com/kernel/538707-use-cpuid-communicate-hypervisor.html
Also:
  http://lkml.indiana.edu/hypermail/linux/kernel/1205.0/00100.html

VMware documention on CPUIDs (Mechanisms to determine if software is
running in a VMware virtual machine):
  
http://kb.vmware.com/selfservice/microsites/search.do?language=en_UScmd=displayKCexternalId=1009458

QEMU knows this is KVM_CPUID_SIGNATURE (0x4000).

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

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 895d848..8462c75 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -389,12 +389,12 @@ int kvm_arch_init_vcpu(CPUX86State *env)
 c = cpuid_data.entries[cpuid_i++];
 memset(c, 0, sizeof(*c));
 c-function = KVM_CPUID_SIGNATURE;
-if (!hyperv_enabled()) {
+if (!env-cpuid_hv_level_set) {
 memcpy(signature, KVMKVMKVM\0\0\0, 12);
 c-eax = 0;
 } else {
 memcpy(signature, Microsoft Hv, 12);
-c-eax = HYPERV_CPUID_MIN;
+c-eax = env-cpuid_hv_level;
 }
 c-ebx = signature[0];
 c-ecx = signature[1];
-- 
1.7.1

--
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