[PATCH 0/2][RFC] Nested Paging support for Nested SVM (userspace part)

[PATCH 0/2][RFC] Nested Paging support for Nested SVM (userspace part)

[Joerg Roedel]

Hi,

this is the small userspace part. They are necessary for the l1 guest to see
the npt feature bit with cpuid.
Please review and give feedback :-)

Thanks,

	Joerg

Diffstat:

 kvm/include/linux/kvm.h |    2 ++
 qemu-kvm-x86.c          |   15 +++++++++++++--
 target-i386/cpu.h       |    2 ++
 target-i386/helper.c    |    3 ++-
 4 files changed, 19 insertions(+), 3 deletions(-)

Shortlog:

Joerg Roedel (2):
      QEMU-KVM: Fix ext3_feature propagation
      QEMU-KVM: Ask kernel about supported svm features

[PATCH 1/2] QEMU-KVM: Fix ext3_feature propagation

[Joerg Roedel]

This patch fixes the propagation of the ext3_features from
the qemu cpu-model to kvm. This is required for the guest to
see the svm flag.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 target-i386/helper.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index e595a3e..73d8389 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -759,6 +759,7 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
     env->pat = 0x0007040600070406ULL;
     env->cpuid_ext_features = def->ext_features;
     env->cpuid_ext2_features = def->ext2_features;
+    env->cpuid_ext3_features = def->ext3_features;
     env->cpuid_xlevel = def->xlevel;
     env->cpuid_kvm_features = def->kvm_features;
     {
-- 
1.7.0

[PATCH 2/2] 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>
---
 kvm/include/linux/kvm.h |    2 ++
 qemu-kvm-x86.c          |   15 +++++++++++++--
 target-i386/cpu.h       |    2 ++
 target-i386/helper.c    |    2 +-
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/kvm/include/linux/kvm.h b/kvm/include/linux/kvm.h
index 6485981..aeb2c9b 100644
--- a/kvm/include/linux/kvm.h
+++ b/kvm/include/linux/kvm.h
@@ -500,6 +500,8 @@ struct kvm_ioeventfd {
 
 #define KVM_CAP_PCI_SEGMENT 47
 
+#define KVM_CAP_SVM_CPUID_FIXED 52
+
 #ifdef KVM_CAP_IRQ_ROUTING
 
 struct kvm_irq_routing_irqchip {
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 7a5925a..60e6d26 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -1291,8 +1291,19 @@ 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:
+		if (!kvm_check_extension(kvm_state, KVM_CAP_SVM_CPUID_FIXED))
+			break;
+		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].edx = kvm_arch_get_supported_cpuid(cenv, 0x8000000a, R_EDX);
+		break;
+	}
+	cpuid_nent += 1;
+    }
 
     kvm_setup_cpuid2(cenv, cpuid_nent, cpuid_ent);
 
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index b64bd02..adcc19f 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -421,6 +421,8 @@
 #define CPUID_EXT3_IBS     (1 << 10)
 #define CPUID_EXT3_SKINIT  (1 << 12)
 
+#define CPUID_SVM_NPT      (1 << 0)
+
 #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */
 #define CPUID_VENDOR_INTEL_2 0x49656e69 /* "ineI" */
 #define CPUID_VENDOR_INTEL_3 0x6c65746e /* "ntel" */
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 73d8389..109f656 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -2220,7 +2220,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *eax = 0x00000001; /* SVM Revision */
         *ebx = 0x00000010; /* nr of ASIDs */
         *ecx = 0;
-        *edx = 0; /* optional features */
+        *edx = 0;
         break;
     default:
         /* reserved values: zero */
-- 
1.7.0

Re: [PATCH 2/2] QEMU-KVM: Ask kernel about supported svm features

[Alexander Graf]

Am 03.03.2010 um 20:15 schrieb Joerg Roedel <joerg.roedel@amd.com>:

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

Do we really need that complexity? By default the kernel masks out  
unsupported cpuid features anyway. So if we don't have npt guest  
support (enabled), the kernel module should just mask it out.

IOW, always passing npt should work. No capability should make it get  
masked out.


Alex
>

Re: [PATCH 2/2] QEMU-KVM: Ask kernel about supported svm features

[Joerg Roedel]

On Wed, Mar 03, 2010 at 11:58:49PM +0100, Alexander Graf wrote:
> 
> Am 03.03.2010 um 20:15 schrieb Joerg Roedel <joerg.roedel@amd.com>:
> 
> >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.
> 
> Do we really need that complexity?

Yes :-)

> By default the kernel masks out unsupported cpuid features anyway. So
> if we don't have npt guest support (enabled), the kernel module should
> just mask it out.

The kernel does not mask out unsupported features. I also don't think
this would be a good idea because userspace won't be aware of that
change.
Fact it, we need a way to report the npt-emulation feature to userspace
because old kvm versions don't support it. So we can't pass the npt bit
unconditionally. The get_supported_cpuid ioctl is the way of choice
here.
But the current way get_supported_cpuid works for function 0x8000000a is
broken because it reports the host features. This was the reason to
introduce the new capability.
 
> IOW, always passing npt should work. No capability should make it
> get masked out.

No, as stated above always passing npt-bit into the kernel and letting
it mask out there isn't a good way to go (not only because this will
break if you use new qem-kvm on old kernel-space).

	Joerg

Re: [PATCH 2/2] QEMU-KVM: Ask kernel about supported svm features

[Alexander Graf]

Joerg Roedel wrote:
> On Wed, Mar 03, 2010 at 11:58:49PM +0100, Alexander Graf wrote:
>   
>> Am 03.03.2010 um 20:15 schrieb Joerg Roedel <joerg.roedel@amd.com>:
>>
>>     
>>> 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.
>>>       
>> Do we really need that complexity?
>>     
>
> Yes :-)
>
>   
>> By default the kernel masks out unsupported cpuid features anyway. So
>> if we don't have npt guest support (enabled), the kernel module should
>> just mask it out.
>>     
>
> The kernel does not mask out unsupported features. I also don't think
> this would be a good idea because userspace won't be aware of that
> change.
> Fact it, we need a way to report the npt-emulation feature to userspace
> because old kvm versions don't support it. So we can't pass the npt bit
> unconditionally. The get_supported_cpuid ioctl is the way of choice
> here.
> But the current way get_supported_cpuid works for function 0x8000000a is
> broken because it reports the host features. This was the reason to
> introduce the new capability.
>   

That's what I mean by masking. It used to happen implicitly, but has
been changed to directly asking the kernel for its capabilities apparently.

>> IOW, always passing npt should work. No capability should make it
>> get masked out.
>>     
>
> No, as stated above always passing npt-bit into the kernel and letting
> it mask out there isn't a good way to go (not only because this will
> break if you use new qem-kvm on old kernel-space).
>   

Ah, so we did have a bug in old KVM kernel modules. Sigh.


Alex