[PATCH] qemu-kvm: Ask kernel about supported svm features

[PATCH] qemu-kvm: Ask kernel about supported svm features

[Joerg Roedel]

This patch adds code to ask the kernel about the svm
features it supports for its guests and propagates them to
the guest. The new capability is necessary because the old
behavior of the kernel was to just return the host svm
features but every svm-feature needs emulation in the nested
svm kernel code. The new capability indicates that the
kernel is aware of that when returning svm cpuid
information.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 qemu-kvm-x86.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 748ff69..6eccd69 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -1327,8 +1327,18 @@ int kvm_arch_init_vcpu(CPUState *cenv)
     qemu_kvm_cpuid_on_env(&copy);
     limit = copy.regs[R_EAX];
 
-    for (i = 0x80000000; i <= limit; ++i)
-	do_cpuid_ent(&cpuid_ent[cpuid_nent++], i, 0, &copy);
+    for (i = 0x80000000; i <= limit; ++i) {
+        do_cpuid_ent(&cpuid_ent[cpuid_nent], i, 0, &copy);
+        switch (i) {
+            case 0x8000000a:
+                cpuid_ent[cpuid_nent].eax = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EAX);
+                cpuid_ent[cpuid_nent].ebx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EBX);
+                cpuid_ent[cpuid_nent].ebx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EBX);
+                cpuid_ent[cpuid_nent].edx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EDX);
+                break;
+        }
+        cpuid_nent += 1;
+    }
 
     kvm_setup_cpuid2(cenv, cpuid_nent, cpuid_ent);
 
-- 
1.7.0.4

Re: [PATCH] qemu-kvm: Ask kernel about supported svm features

[Avi Kivity]

On 04/22/2010 01:57 PM, Joerg Roedel wrote:
> This patch adds code to ask the kernel about the svm
> features it supports for its guests and propagates them to
> the guest. The new capability is necessary because the old
> behavior of the kernel was to just return the host svm
> features but every svm-feature needs emulation in the nested
> svm kernel code. The new capability indicates that the
> kernel is aware of that when returning svm cpuid
> information.
>    

No new capability here.

> Signed-off-by: Joerg Roedel<joerg.roedel@amd.com>
> ---
>   qemu-kvm-x86.c |   14 ++++++++++++--
>   1 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> index 748ff69..6eccd69 100644
> --- a/qemu-kvm-x86.c
> +++ b/qemu-kvm-x86.c
> @@ -1327,8 +1327,18 @@ int kvm_arch_init_vcpu(CPUState *cenv)
>       qemu_kvm_cpuid_on_env(&copy);
>       limit = copy.regs[R_EAX];
>
> -    for (i = 0x80000000; i<= limit; ++i)
> -	do_cpuid_ent(&cpuid_ent[cpuid_nent++], i, 0,&copy);
> +    for (i = 0x80000000; i<= limit; ++i) {
> +        do_cpuid_ent(&cpuid_ent[cpuid_nent], i, 0,&copy);
> +        switch (i) {
> +            case 0x8000000a:
> +                cpuid_ent[cpuid_nent].eax = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EAX);
> +                cpuid_ent[cpuid_nent].ebx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EBX);
> +                cpuid_ent[cpuid_nent].ebx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EBX);
> +                cpuid_ent[cpuid_nent].edx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EDX);
> +                break;
> +        }
> +        cpuid_nent += 1;
> +    }
>    

I don't understand why this is different compared to all other cpuid bits.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] qemu-kvm: Ask kernel about supported svm features

[Joerg Roedel]

On Thu, Apr 22, 2010 at 02:07:14PM +0300, Avi Kivity wrote:
> On 04/22/2010 01:57 PM, Joerg Roedel wrote:
> >This patch adds code to ask the kernel about the svm
> >features it supports for its guests and propagates them to
> >the guest. The new capability is necessary because the old
> >behavior of the kernel was to just return the host svm
> >features but every svm-feature needs emulation in the nested
> >svm kernel code. The new capability indicates that the
> >kernel is aware of that when returning svm cpuid
> >information.
> 
> No new capability here.

copy&paste error, sorry.

> 
> >Signed-off-by: Joerg Roedel<joerg.roedel@amd.com>
> >---
> >  qemu-kvm-x86.c |   14 ++++++++++++--
> >  1 files changed, 12 insertions(+), 2 deletions(-)
> >
> >diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> >index 748ff69..6eccd69 100644
> >--- a/qemu-kvm-x86.c
> >+++ b/qemu-kvm-x86.c
> >@@ -1327,8 +1327,18 @@ int kvm_arch_init_vcpu(CPUState *cenv)
> >      qemu_kvm_cpuid_on_env(&copy);
> >      limit = copy.regs[R_EAX];
> >
> >-    for (i = 0x80000000; i<= limit; ++i)
> >-	do_cpuid_ent(&cpuid_ent[cpuid_nent++], i, 0,&copy);
> >+    for (i = 0x80000000; i<= limit; ++i) {
> >+        do_cpuid_ent(&cpuid_ent[cpuid_nent], i, 0,&copy);
> >+        switch (i) {
> >+            case 0x8000000a:
> >+                cpuid_ent[cpuid_nent].eax = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EAX);
> >+                cpuid_ent[cpuid_nent].ebx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EBX);
> >+                cpuid_ent[cpuid_nent].ebx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EBX);
> >+                cpuid_ent[cpuid_nent].edx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EDX);
> >+                break;
> >+        }
> >+        cpuid_nent += 1;
> >+    }
> 
> I don't understand why this is different compared to all other cpuid bits.

Because for the SVM features we report to the guest we need to ask the
kernel which of them are supported. We can't just take the host-cpuid
because most of the additional svm features need special emulation in
the kernel. Or do you think this should better be handled in
target-i386/cpuid.c?

	Joerg

Re: [PATCH] qemu-kvm: Ask kernel about supported svm features

[Avi Kivity]

On 04/22/2010 03:02 PM, Joerg Roedel wrote:
>>
>>> Signed-off-by: Joerg Roedel<joerg.roedel@amd.com>
>>> ---
>>>   qemu-kvm-x86.c |   14 ++++++++++++--
>>>   1 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
>>> index 748ff69..6eccd69 100644
>>> --- a/qemu-kvm-x86.c
>>> +++ b/qemu-kvm-x86.c
>>> @@ -1327,8 +1327,18 @@ int kvm_arch_init_vcpu(CPUState *cenv)
>>>       qemu_kvm_cpuid_on_env(&copy);
>>>       limit = copy.regs[R_EAX];
>>>
>>> -    for (i = 0x80000000; i<= limit; ++i)
>>> -	do_cpuid_ent(&cpuid_ent[cpuid_nent++], i, 0,&copy);
>>> +    for (i = 0x80000000; i<= limit; ++i) {
>>> +        do_cpuid_ent(&cpuid_ent[cpuid_nent], i, 0,&copy);
>>> +        switch (i) {
>>> +            case 0x8000000a:
>>> +                cpuid_ent[cpuid_nent].eax = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EAX);
>>> +                cpuid_ent[cpuid_nent].ebx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EBX);
>>> +                cpuid_ent[cpuid_nent].ebx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EBX);
>>> +                cpuid_ent[cpuid_nent].edx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EDX);
>>> +                break;
>>> +        }
>>> +        cpuid_nent += 1;
>>> +    }
>>>        
>> I don't understand why this is different compared to all other cpuid bits.
>>      
> Because for the SVM features we report to the guest we need to ask the
> kernel which of them are supported.

That's true for all cpuid features.

> We can't just take the host-cpuid
> because most of the additional svm features need special emulation in
> the kernel. Or do you think this should better be handled in
> target-i386/cpuid.c?
>    

Yes.  -cpu host should take KVM_GET_SUPPORTED_CPUID output and loop it 
back to the vcpu configuration, others just take the qemu configuration, 
mask it with supported bits, and pass it back (see 
check_features_against_host()).

(need feature names for the bits, too, so you can enable or disable them 
from the command line)

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] qemu-kvm: Ask kernel about supported svm features

[Joerg Roedel]

On Thu, Apr 22, 2010 at 03:13:14PM +0300, Avi Kivity wrote:
> On 04/22/2010 03:02 PM, Joerg Roedel wrote:

>> We can't just take the host-cpuid
>> because most of the additional svm features need special emulation in
>> the kernel. Or do you think this should better be handled in
>> target-i386/cpuid.c?
>>    
>
> Yes.  -cpu host should take KVM_GET_SUPPORTED_CPUID output and loop it  
> back to the vcpu configuration, others just take the qemu configuration,  
> mask it with supported bits, and pass it back (see  
> check_features_against_host()).

Hmm, the plan was to enable with -enable-nesting all kernel supported
svm features for the guest (and add switches later to remove them
individually)
If we activate nested svm with -cpu host in the future thats fine too
(closed-source hypervisors need that anyway). But we should also define
a cpu model in which we can migrate nested hypervisors between machines
were the cpu is not completly indentical.

> (need feature names for the bits, too, so you can enable or disable them  
> from the command line)

Yeah, I know. I omitted that for the first bring-up. It was planned for
a later patch.

	Joerg

Re: [PATCH] qemu-kvm: Ask kernel about supported svm features

[Avi Kivity]

On 04/22/2010 03:28 PM, Joerg Roedel wrote:
>
>> Yes.  -cpu host should take KVM_GET_SUPPORTED_CPUID output and loop it
>> back to the vcpu configuration, others just take the qemu configuration,
>> mask it with supported bits, and pass it back (see
>> check_features_against_host()).
>>      
> Hmm, the plan was to enable with -enable-nesting all kernel supported
> svm features for the guest (and add switches later to remove them
> individually)
> If we activate nested svm with -cpu host in the future thats fine too
> (closed-source hypervisors need that anyway). But we should also define
> a cpu model in which we can migrate nested hypervisors between machines
> were the cpu is not completly indentical.
>    

You can use -cpu host, or -cpu kvm64,+svm,+npt,+other_feature... just as 
with all other features.


-- 
error compiling committee.c: too many arguments to function