[PATCH] KVM: Fix wrong KVM_GET_LAPIC

[PATCH] KVM: Fix wrong KVM_GET_LAPIC

[Yang, Sheng]

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

From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng.yang@intel.com>
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 <sheng.yang@intel.com>
---
 arch/x86/kvm/x86.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ee005a6..4a03375 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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


[-- Attachment #2: 0001-KVM-Fix-wrong-KVM_GET_LAPIC.patch --]
[-- Type: text/x-diff, Size: 807 bytes --]

From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng.yang@intel.com>
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 <sheng.yang@intel.com>
---
 arch/x86/kvm/x86.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ee005a6..4a03375 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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

Re: [PATCH] KVM: Fix wrong KVM_GET_LAPIC

[Avi Kivity]

Yang, Sheng wrote:
> From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
> From: Sheng Yang <sheng.yang@intel.com>
> 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.

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

Re: [PATCH] KVM: Fix wrong KVM_GET_LAPIC

[Amit Shah]

* On Monday 18 Aug 2008 08:45:40 Yang, Sheng wrote:
> From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
> From: Sheng Yang <sheng.yang@intel.com>
> 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 <sheng.yang@intel.com>
> ---
>  arch/x86/kvm/x86.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ee005a6..4a03375 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -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?

Re: [PATCH] KVM: Fix wrong KVM_GET_LAPIC

[Avi Kivity]

Amit Shah wrote:
> * On Monday 18 Aug 2008 08:45:40 Yang, Sheng wrote:
>   
>> From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
>> From: Sheng Yang <sheng.yang@intel.com>
>> 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 <sheng.yang@intel.com>
>> ---
>>  arch/x86/kvm/x86.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index ee005a6..4a03375 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -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.

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

Builds for me.  Please double check.

> What's the point of this change anyway?
>   

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

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

Re: [PATCH] KVM: Fix wrong KVM_GET_LAPIC

[Amit Shah]

* On Wednesday 20 Aug 2008 12:40:57 Avi Kivity wrote:
> Amit Shah wrote:
> > * On Monday 18 Aug 2008 08:45:40 Yang, Sheng wrote:
> >> From a8ca7dd8f5fe0125e7b7d0a21f5caddacd754911 Mon Sep 17 00:00:00 2001
> >> From: Sheng Yang <sheng.yang@intel.com>
> >> 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 <sheng.yang@intel.com>
> >> ---
> >>  arch/x86/kvm/x86.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> index ee005a6..4a03375 100644
> >> --- a/arch/x86/kvm/x86.c
> >> +++ b/arch/x86/kvm/x86.c
> >> @@ -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.

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.

> > and this leads to build
> > break with the tip on kvm.git.
>
> Builds for me.  Please double check.
>
> > 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.