KVM: Fix wrong KVM_GET_LAPIC

PATCH Applied Yang, Sheng 3 participants 5 messages Aug 18, 2008 – Aug 20, 2008

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Yang, Sheng

author

2008-08-18 03:13 UTC #0 lore raw

PATCH 1/1 KVM: Fix wrong KVM_GET_LAPIC

From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Mon, 18 Aug 2008 11:04:22 +0800
Subject: [PATCH] KVM: Fix wrong KVM_GET_LAPIC

Which caused migration fail in recent commits.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/kvm/x86.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/kvm/x86.c +1 -1

index ee005a6..4a03375 100644
@@ -1555,7 +1555,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		if (r)
 			goto out;
 		r = -EFAULT;
-		if (copy_to_user(argp, &lapic, sizeof lapic))
+		if (copy_to_user(argp, lapic, sizeof(struct kvm_lapic_state)))
 			goto out;
 		r = 0;
 		break;

--
1.5.4.5

From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Mon, 18 Aug 2008 11:04:22 +0800
Subject: [PATCH] KVM: Fix wrong KVM_GET_LAPIC

Which caused migration fail in recent commits.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/kvm/x86.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/kvm/x86.c +1 -1

index ee005a6..4a03375 100644
@@ -1555,7 +1555,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		if (r)
 			goto out;
 		r = -EFAULT;
-		if (copy_to_user(argp, &lapic, sizeof lapic))
+		if (copy_to_user(argp, lapic, sizeof(struct kvm_lapic_state)))
 			goto out;
 		r = 0;
 		break;

Signature · 1 line

1.5.4.5

Avi Kivity

Applied

2008-08-18 10:08 UTC #1 lore raw

Quoted 8 lines · Sheng

From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Mon, 18 Aug 2008 11:04:22 +0800
Subject: [PATCH] KVM: Fix wrong KVM_GET_LAPIC
Which caused migration fail in recent commits.

Applied, thanks.

Signature · 1 line

error compiling committee.c: too many arguments to function

Amit Shah

2008-08-20 06:43 UTC #2 lore raw

In reply to Yang, Sheng

Quoted 17 lines · * On Monday 18 Aug 2008 08:45:40 Yang, Sheng wrote:

From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Mon, 18 Aug 2008 11:04:22 +0800
Subject: [PATCH] KVM: Fix wrong KVM_GET_LAPIC
Which caused migration fail in recent commits.
Signed-off-by Sheng Yang <[email protected]>
---
 arch/x86/kvm/x86.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/x86/kvm/x86.c +0 -0
index ee005a6..4a03375 100644

arch/x86/kvm/x86.c long kvm_arch_vcpu_ioctl(struct file *filp, · line 1558

Quoted 5 lines

arch/x86/kvm/x86.c +1 -1

@@ -1555,7 +1555,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		if (r)
 			goto out;
 		r = -EFAULT;
-		if (copy_to_user(argp, &lapic, sizeof lapic))
+		if (copy_to_user(argp, lapic, sizeof(struct kvm_lapic_state)))

We're passing the structure instead of the pointer and this leads to build break with the tip on kvm.git.

What's the point of this change anyway?

Avi Kivity

2008-08-20 07:10 UTC #3 lore raw

Quoted 14 lines · Amit Shah

Quoted 12 lines · * On Monday 18 Aug 2008 08:45:40 Yang, Sheng wrote:
From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Mon, 18 Aug 2008 11:04:22 +0800
Subject: [PATCH] KVM: Fix wrong KVM_GET_LAPIC
Which caused migration fail in recent commits.
Signed-off-by Sheng Yang <[email protected]>
---
 arch/x86/kvm/x86.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Quoted 13 lines

arch/x86/kvm/x86.c long kvm_arch_vcpu_ioctl(struct file *filp, · line 1559
Quoted 11 lines
arch/x86/kvm/x86.c +1 -1
index ee005a6..4a03375 100644
@@ -1555,7 +1555,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		if (r)
 			goto out;
 		r = -EFAULT;
-		if (copy_to_user(argp, &lapic, sizeof lapic))
+		if (copy_to_user(argp, lapic, sizeof(struct kvm_lapic_state)))
 
We're passing the structure instead of the pointer

'lapic' is now a pointer to a struct kvm_lapic_state.

Quoted 4 lines

and this leads to build
break with the tip on kvm.git.

Builds for me. Please double check.

Quoted 2 lines

What's the point of this change anyway?

Fixing save/restore, which got broken by the stack usage reduction patches.

Signature · 1 line

Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Amit Shah

2008-08-20 09:18 UTC #4 lore raw

Quoted 27 lines · * On Wednesday 20 Aug 2008 12:40:57 Avi Kivity wrote:

Quoted 13 lines · Amit Shah
Quoted 12 lines · * On Monday 18 Aug 2008 08:45:40 Yang, Sheng wrote:
From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Mon, 18 Aug 2008 11:04:22 +0800
Subject: [PATCH] KVM: Fix wrong KVM_GET_LAPIC
Which caused migration fail in recent commits.
Signed-off-by Sheng Yang <[email protected]>
---
 arch/x86/kvm/x86.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Quoted 12 lines
arch/x86/kvm/x86.c long kvm_arch_vcpu_ioctl(struct file *filp, · line 1558
Quoted 10 lines
arch/x86/kvm/x86.c +1 -1
index ee005a6..4a03375 100644
@@ -1555,7 +1555,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		if (r)
 			goto out;
 		r = -EFAULT;
-		if (copy_to_user(argp, &lapic, sizeof lapic))
+		if (copy_to_user(argp, lapic, sizeof(struct kvm_lapic_state)))
We're passing the structure instead of the pointer

Quoted 1 line

'lapic' is now a pointer to a struct kvm_lapic_state.

Looks like some git problem; git pull from your repo into my branch says everything is uptodate (git merge also says the same thing). However, my branch still has the older struct (where lapic is not a pointer). git log shows the relevant commit. git checkout -f doesn't help either. I'm still to figure out how to resolve this.

Quoted 8 lines

Quoted 2 lines
and this leads to build
break with the tip on kvm.git.
Builds for me. Please double check.
Quoted 1 line
What's the point of this change anyway?
Fixing save/restore, which got broken by the stack usage reduction patches.

Thanks; without the stack reduction patch, this just looked to me like adding a build break and not changing anything else.

Amit.