[PATCH] Fix sysenter migration issue on AMD CPUs

[PATCH] Fix sysenter migration issue on AMD CPUs

[Andre Przywara]

To enable cross-vendor migration we use VMCB external variables to hold
the full 64bit value of the SYSENTER MSRs, which get truncated to 32bit
on AMD hardware. Since we didn't intercept these MSRs, these
variables were only used in the emulation case, but were _always_ used
for migration purposes. This worked fine for cross-vendor migration in
compat mode, but did not work in pure legacy mode.
To fix this we always intercept the SYSENTER MSRs and store the values
both in the VMCB and the external variables. This works for all cases.

Signed-off-by: Andre Przywara <andre.przywara@amd.com>
---
 arch/x86/kvm/svm.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

Hi Avi,

this should fix the problem seen by Stephane and Thomas this week.
Please revert 8b2f9d194288982d654c1afef491dfdf75ec1ba9 (your proposed fix,
which broke cross-vendor migration) and apply this patch afterwards.
It worked for me with both 32on32 and 32on64 migration both cross-vendor
and between two AMD machines.
Stephane, Thomas: Can you verify this?

Thanks!

Regards,
Andre.

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index e158a2f..7853dd3 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -101,7 +101,6 @@ struct vcpu_svm {
 	unsigned long vmcb_pa;
 	struct svm_cpu_data *svm_data;
 	uint64_t asid_generation;
-	uint64_t sysenter_cs;
 	uint64_t sysenter_esp;
 	uint64_t sysenter_eip;
 
@@ -426,8 +425,6 @@ static void svm_vcpu_init_msrpm(u32 *msrpm)
 #endif
 	set_msr_interception(msrpm, MSR_K6_STAR, 1, 1);
 	set_msr_interception(msrpm, MSR_IA32_SYSENTER_CS, 1, 1);
-	set_msr_interception(msrpm, MSR_IA32_SYSENTER_ESP, 1, 1);
-	set_msr_interception(msrpm, MSR_IA32_SYSENTER_EIP, 1, 1);
 }
 
 static void svm_enable_lbrv(struct vcpu_svm *svm)
@@ -2087,7 +2084,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 *data)
 		break;
 #endif
 	case MSR_IA32_SYSENTER_CS:
-		*data = svm->sysenter_cs;
+		*data = svm->vmcb->save.sysenter_cs;
 		break;
 	case MSR_IA32_SYSENTER_EIP:
 		*data = svm->sysenter_eip;
@@ -2176,13 +2173,15 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 data)
 		break;
 #endif
 	case MSR_IA32_SYSENTER_CS:
-		svm->sysenter_cs = data;
+		svm->vmcb->save.sysenter_cs = data;
 		break;
 	case MSR_IA32_SYSENTER_EIP:
 		svm->sysenter_eip = data;
+		svm->vmcb->save.sysenter_eip = data;
 		break;
 	case MSR_IA32_SYSENTER_ESP:
 		svm->sysenter_esp = data;
+		svm->vmcb->save.sysenter_esp = data;
 		break;
 	case MSR_IA32_DEBUGCTLMSR:
 		if (!svm_has(SVM_FEATURE_LBRV)) {
-- 
1.6.1.3

Re: [PATCH] Fix sysenter migration issue on AMD CPUs

[Avi Kivity]

On 08/21/2009 02:58 PM, Andre Przywara wrote:
> To enable cross-vendor migration we use VMCB external variables to hold
> the full 64bit value of the SYSENTER MSRs, which get truncated to 32bit
> on AMD hardware. Since we didn't intercept these MSRs, these
> variables were only used in the emulation case, but were _always_ used
> for migration purposes. This worked fine for cross-vendor migration in
> compat mode, but did not work in pure legacy mode.
> To fix this we always intercept the SYSENTER MSRs and store the values
> both in the VMCB and the external variables. This works for all cases.
>
> Signed-off-by: Andre Przywara<andre.przywara@amd.com>
> ---
>   arch/x86/kvm/svm.c |    9 ++++-----
>   1 files changed, 4 insertions(+), 5 deletions(-)
>
> Hi Avi,
>
> this should fix the problem seen by Stephane and Thomas this week.
> Please revert 8b2f9d194288982d654c1afef491dfdf75ec1ba9 (your proposed fix,
> which broke cross-vendor migration) and apply this patch afterwards.
> It worked for me with both 32on32 and 32on64 migration both cross-vendor
> and between two AMD machines.
>    

Reverted and applied; thanks.

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

Re: [PATCH] Fix sysenter migration issue on AMD CPUs

[Thomas Besser]

Andre Przywara wrote:
> Stephane, Thomas: Can you verify this?

I'm not very familiar with compiling kvm-mod from git sources. And your
patch does not apply to svm.c shipped with kernel 2.6.30.5

So at the moment I have no clue, how to verify. Is there any short howto out
there, how to get kvm module from git source?

Regards
Thomas

Re: [PATCH] Fix sysenter migration issue on AMD CPUs

[Andre Przywara]

[-- Attachment #1: Type: text/plain, Size: 1419 bytes --]

Thomas Besser wrote:
> Andre Przywara wrote:
>> Stephane, Thomas: Can you verify this?
> 
> I'm not very familiar with compiling kvm-mod from git sources. And your
> patch does not apply to svm.c shipped with kernel 2.6.30.5
You shouldn't have seen any problems with 2.6.30.5, since the code in 
question (sysenter/syscall emulation) is not in here.

> 
> So at the moment I have no clue, how to verify. Is there any short howto out
> there, how to get kvm module from git source?
You can use the attached patch, which applies against 
kvm-kmod-devel-88.tar.gz

If that does not help, tell me with what tree or tarball you are usually 
generating the KVM kernel modules. I use Avi's latest git tree, which is 
regularly synced with 2.6.31.rc<x>. The patch from Friday should apply 
against this one.
$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/avi/kvm.git
You will have to build your whole kernel with this tree, if you build 
only the modules from here they will certainly mismatch your running kernel.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: fix_sysenter_migration.patch --]
[-- Type: text/x-patch, Size: 1435 bytes --]

diff --git a/x86/svm.c b/x86/svm.c
index fb29061..75d18bf 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -117,7 +117,6 @@ struct vcpu_svm {
 	unsigned long vmcb_pa;
 	struct svm_cpu_data *svm_data;
 	uint64_t asid_generation;
-	uint64_t sysenter_cs;
 	uint64_t sysenter_esp;
 	uint64_t sysenter_eip;
 
@@ -436,8 +435,6 @@ static void svm_vcpu_init_msrpm(u32 *msrpm)
 #endif
 	set_msr_interception(msrpm, MSR_K6_STAR, 1, 1);
 	set_msr_interception(msrpm, MSR_IA32_SYSENTER_CS, 1, 1);
-	set_msr_interception(msrpm, MSR_IA32_SYSENTER_ESP, 1, 1);
-	set_msr_interception(msrpm, MSR_IA32_SYSENTER_EIP, 1, 1);
 }
 
 static void svm_enable_lbrv(struct vcpu_svm *svm)
@@ -2062,7 +2059,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 *data)
 		break;
 #endif
 	case MSR_IA32_SYSENTER_CS:
-		*data = svm->sysenter_cs;
+		*data = svm->vmcb->save.sysenter_cs;
 		break;
 	case MSR_IA32_SYSENTER_EIP:
 		*data = svm->sysenter_eip;
@@ -2151,13 +2148,15 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 data)
 		break;
 #endif
 	case MSR_IA32_SYSENTER_CS:
-		svm->sysenter_cs = data;
+		svm->vmcb->save.sysenter_cs = data;
 		break;
 	case MSR_IA32_SYSENTER_EIP:
 		svm->sysenter_eip = data;
+		svm->vmcb->save.sysenter_eip = data;
 		break;
 	case MSR_IA32_SYSENTER_ESP:
 		svm->sysenter_esp = data;
+		svm->vmcb->save.sysenter_esp = data;
 		break;
 	case MSR_IA32_DEBUGCTLMSR:
 		if (!svm_has(SVM_FEATURE_LBRV)) {

Re: [PATCH] Fix sysenter migration issue on AMD CPUs

[Thomas Besser]

Andre Przywara wrote:

> Thomas Besser wrote:
>> Andre Przywara wrote:
>>> Stephane, Thomas: Can you verify this?
>> 
>> I'm not very familiar with compiling kvm-mod from git sources. And your
>> patch does not apply to svm.c shipped with kernel 2.6.30.5
> You shouldn't have seen any problems with 2.6.30.5, since the code in
> question (sysenter/syscall emulation) is not in here.

Both hosts running 2.6.30.5 with kvm as module from kernel source. So I
should have another problem with live migration and qemu-kvm (0.10.6). 

Probably this http://article.gmane.org/gmane.comp.emulators.kvm.devel/39185

Thanx
Thomas