| Message ID | 1348497138-2516-6-git-send-email-Don@CloudSwitch.com |
|---|---|
| State | New |
| Headers | show |
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_US&cmd=displayKC&externalId=1009458 > > QEMU knows this is KVM_CPUID_SIGNATURE (0x40000000). > > Signed-off-by: Don Slutz <Don@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.
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_US&cmd=displayKC&externalId=1009458 > > > > QEMU knows this is KVM_CPUID_SIGNATURE (0x40000000). > > > > Signed-off-by: Don Slutz <Don@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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
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_US&cmd=displayKC&externalId=1009458 > > > > > > QEMU knows this is KVM_CPUID_SIGNATURE (0x40000000). > > > > > > Signed-off-by: Don Slutz <Don@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).
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_US&cmd=displayKC&externalId=1009458 >>>> >>>> QEMU knows this is KVM_CPUID_SIGNATURE (0x40000000). >>>> >>>> Signed-off-by: Don Slutz <Don@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
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_US&cmd=displayKC&externalId=1009458 > >>>> > >>>>QEMU knows this is KVM_CPUID_SIGNATURE (0x40000000). > >>>> > >>>>Signed-off-by: Don Slutz <Don@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.
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_US&cmd=displayKC&externalId=1009458 >>>>>> >>>>>> QEMU knows this is KVM_CPUID_SIGNATURE (0x40000000). >>>>>> >>>>>> Signed-off-by: Don Slutz <Don@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
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).
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];
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_US&cmd=displayKC&externalId=1009458 QEMU knows this is KVM_CPUID_SIGNATURE (0x40000000). Signed-off-by: Don Slutz <Don@CloudSwitch.com> --- target-i386/kvm.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)