Re: Nested kvm_intel broken on pre 3.3 hosts

[Stefan Bader <stefan.bader@canonical.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3709 bytes --]

On 05.08.2012 11:18, Avi Kivity wrote:
> On 08/03/2012 01:57 PM, Stefan Bader wrote:
>>> No, you're backporting the entire feature.  All we need is to expose
>>> RDPMC intercept to the guest.
>>
>> Oh well, I thought that was the thing you asked for...
> 
> Sorry for being unclear.
> 
>>
>>> It should be sufficient to backport the bits in
>>> nested_vmx_setup_ctls_msrs() and nested_vmx_exit_handled().
>>
>> Ok, how about that? It is probably wrong again, but at least it
>> allows to load the kvm-intel module from within a nested guest
>> and not having the feature pretend to fail seems the closest
>> thing to do...
>>
>> ---
>>
>> From 0aeb99348363b7aeb2b0bd92428cb212159fa468 Mon Sep 17 00:00:00 2001
>> From: Stefan Bader <stefan.bader@canonical.com>
>> Date: Thu, 10 Nov 2011 14:57:25 +0200
>> Subject: [PATCH] KVM: VMX: Fake intercept RDPMC
>>
>> Based on commit fee84b079d5ddee2247b5c1f53162c330c622902 upstream.
>>
>>   Intercept RDPMC and forward it to the PMU emulation code.
>>
>> But drop the requirement for the feature being present and instead
>> of forwarding, cause a GP as if the call had failed.
>>
>> BugLink: http://bugs.launchpad.net/bugs/1031090
>> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
>> ---
>>  arch/x86/kvm/vmx.c |   10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 7315488..fc937f2 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -1956,6 +1956,7 @@ static __init void nested_vmx_setup_ctls_msrs(void)
>>  #endif
>>  		CPU_BASED_MOV_DR_EXITING | CPU_BASED_UNCOND_IO_EXITING |
>>  		CPU_BASED_USE_IO_BITMAPS | CPU_BASED_MONITOR_EXITING |
>> +		CPU_BASED_RDPMC_EXITING |
>>  		CPU_BASED_ACTIVATE_SECONDARY_CONTROLS;
>>  	/*
>>  	 * We can allow some features even when not supported by the
>> @@ -4613,6 +4614,14 @@ static int handle_invlpg(struct kvm_vcpu *vcpu)
>>  	return 1;
>>  }
>>  
>> +static int handle_rdpmc(struct kvm_vcpu *vcpu)
>> +{
>> +	/* Instead of implementing the feature, cause a GP */
>> +	kvm_complete_insn_gp(vcpu, 1);
>> +
>> +	return 1;
>> +}
> 
> In fact this should never be called, since we never request RDPMC
> exiting for L1.
> 
>> +
>>  static int handle_wbinvd(struct kvm_vcpu *vcpu)
>>  {
>>  	skip_emulated_instruction(vcpu);
>> @@ -5563,6 +5572,7 @@ static int (*kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>>  	[EXIT_REASON_HLT]                     = handle_halt,
>>  	[EXIT_REASON_INVD]		      = handle_invd,
>>  	[EXIT_REASON_INVLPG]		      = handle_invlpg,
>> +	[EXIT_REASON_RDPMC]                   = handle_rdpmc,
>>  	[EXIT_REASON_VMCALL]                  = handle_vmcall,
>>  	[EXIT_REASON_VMCLEAR]	              = handle_vmclear,
>>  	[EXIT_REASON_VMLAUNCH]                = handle_vmlaunch,
>>
> 
> Provided you backport the bit in nested_vmx_exit_handled().  That takes
> the L2->L1 RDPMC exit and forwards it to L1.
> 
The nested_vmx_exit_handled handled function is already there (in 3.1).

commit 644d711aa0e16111d8aba6d289caebec013e26ea
Author: Nadav Har'El <nyh@il.ibm.com>
Date:   Wed May 25 23:12:35 2011 +0300

    KVM: nVMX: Deciding if L0 or L1 should handle an L2 exit

Though I checked what happens if I drop anything beside the change in
nested_vmx_setup_ctls_msrs. If running "perf test" (which does some
RDPMC test) is sufficient to prove things are ok, then it seems to be
good enough. That test fails already on the perf_event_open syscall
(returning -ENOENT).

So this remaining (attached patch) change would be enough to allow
the kvm_intel module get loaded from newer kernels on an 3.2 host.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-KVM-VMX-Set-CPU_BASED_RDPMC_EXITING-for-nested.patch --]
[-- Type: text/x-diff; name="0001-KVM-VMX-Set-CPU_BASED_RDPMC_EXITING-for-nested.patch", Size: 1220 bytes --]

From b79a5f03b4d9a1a56949d6ef38fd4879ff1b8aee Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@canonical.com>
Date: Thu, 10 Nov 2011 14:57:25 +0200
Subject: [PATCH] KVM: VMX: Set CPU_BASED_RDPMC_EXITING for nested

Based on commit fee84b079d5ddee2247b5c1f53162c330c622902 upstream.

  Intercept RDPMC and forward it to the PMU emulation code.

Newer vmx support will only allow to load the kvm_intel module
if RDPMC_EXITING is supported. Even without the actual support
this part of the change is required on 3.2 hosts.

BugLink: http://bugs.launchpad.net/bugs/1031090
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 arch/x86/kvm/vmx.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 114fe29..94e6749 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1957,6 +1957,7 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 #endif
 		CPU_BASED_MOV_DR_EXITING | CPU_BASED_UNCOND_IO_EXITING |
 		CPU_BASED_USE_IO_BITMAPS | CPU_BASED_MONITOR_EXITING |
+		CPU_BASED_RDPMC_EXITING |
 		CPU_BASED_ACTIVATE_SECONDARY_CONTROLS;
 	/*
 	 * We can allow some features even when not supported by the
-- 
1.7.9.5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 900 bytes --]