* [PATCH] KVM: Fix EPT identity IOCTL on 32pae
@ 2009-08-03 6:17 Sheng Yang
2009-08-03 11:48 ` Jan Kiszka
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sheng Yang @ 2009-08-03 6:17 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, Sheng Yang
Copy u64 from guest result in chaos.
Also fix a mistake of still using old macro rather than new variable().
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
arch/x86/kvm/vmx.c | 2 +-
arch/x86/kvm/x86.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 7931c72..c5aaa1b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2199,7 +2199,7 @@ static int alloc_identity_pagetable(struct kvm *kvm)
goto out;
kvm->arch.ept_identity_pagetable = gfn_to_page(kvm,
- VMX_EPT_IDENTITY_PAGETABLE_ADDR >> PAGE_SHIFT);
+ kvm->arch.ept_identity_map_addr >> PAGE_SHIFT);
out:
up_write(&kvm->slots_lock);
return r;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2539e9a..977b705 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2178,7 +2178,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
goto out;
break;
case KVM_SET_IDENTITY_MAP_ADDR: {
- u64 ident_addr;
+ unsigned long ident_addr;
r = -EFAULT;
if (copy_from_user(&ident_addr, argp, sizeof ident_addr))
--
1.5.4.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae
2009-08-03 6:17 [PATCH] KVM: Fix EPT identity IOCTL on 32pae Sheng Yang
@ 2009-08-03 11:48 ` Jan Kiszka
2009-08-03 12:25 ` Avi Kivity
2009-08-03 16:47 ` Marcelo Tosatti
2 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2009-08-03 11:48 UTC (permalink / raw)
To: Sheng Yang; +Cc: Avi Kivity, Marcelo Tosatti, kvm
Sheng Yang wrote:
> Copy u64 from guest result in chaos.
Not doing so, while the ABI as been defined as u64, will surely result
in chaos as well - just different one. I'm recalling the problems Alex
recently ran into on big-endian PowerPC and sizeof(userland_long) !=
sizeof(kernel_long).
>
> Also fix a mistake of still using old macro rather than new variable().
Two patches then?
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
> arch/x86/kvm/vmx.c | 2 +-
> arch/x86/kvm/x86.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 7931c72..c5aaa1b 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2199,7 +2199,7 @@ static int alloc_identity_pagetable(struct kvm *kvm)
> goto out;
>
> kvm->arch.ept_identity_pagetable = gfn_to_page(kvm,
> - VMX_EPT_IDENTITY_PAGETABLE_ADDR >> PAGE_SHIFT);
> + kvm->arch.ept_identity_map_addr >> PAGE_SHIFT);
> out:
> up_write(&kvm->slots_lock);
> return r;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2539e9a..977b705 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2178,7 +2178,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
> goto out;
> break;
> case KVM_SET_IDENTITY_MAP_ADDR: {
> - u64 ident_addr;
> + unsigned long ident_addr;
>
> r = -EFAULT;
> if (copy_from_user(&ident_addr, argp, sizeof ident_addr))
I don't know what as to be fixed here, but you must stick to the ABI.
Maybe you just want to convert u64 -> ulong afterwards?
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae
2009-08-03 6:17 [PATCH] KVM: Fix EPT identity IOCTL on 32pae Sheng Yang
2009-08-03 11:48 ` Jan Kiszka
@ 2009-08-03 12:25 ` Avi Kivity
2009-08-03 13:38 ` Sheng Yang
2009-08-03 16:47 ` Marcelo Tosatti
2 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2009-08-03 12:25 UTC (permalink / raw)
To: Sheng Yang; +Cc: Marcelo Tosatti, kvm
On 08/03/2009 09:17 AM, Sheng Yang wrote:
> Copy u64 from guest result in chaos.
>
> Also fix a mistake of still using old macro rather than new variable().
>
> Signed-off-by: Sheng Yang<sheng@linux.intel.com>
> ---
> arch/x86/kvm/vmx.c | 2 +-
> arch/x86/kvm/x86.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 7931c72..c5aaa1b 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2199,7 +2199,7 @@ static int alloc_identity_pagetable(struct kvm *kvm)
> goto out;
>
> kvm->arch.ept_identity_pagetable = gfn_to_page(kvm,
> - VMX_EPT_IDENTITY_PAGETABLE_ADDR>> PAGE_SHIFT);
> + kvm->arch.ept_identity_map_addr>> PAGE_SHIFT);
>
This is a fix, but as Jan says, separate patch.
> out:
> up_write(&kvm->slots_lock);
> return r;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2539e9a..977b705 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2178,7 +2178,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
> goto out;
> break;
> case KVM_SET_IDENTITY_MAP_ADDR: {
> - u64 ident_addr;
> + unsigned long ident_addr;
>
> r = -EFAULT;
> if (copy_from_user(&ident_addr, argp, sizeof ident_addr))
>
This doesn't look right. Consider 32-bit userspace running on a 32-bit
kernel and the same userspace running on a 64-bit kernel. We need to
copy the same size in both cases.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae
2009-08-03 12:25 ` Avi Kivity
@ 2009-08-03 13:38 ` Sheng Yang
0 siblings, 0 replies; 6+ messages in thread
From: Sheng Yang @ 2009-08-03 13:38 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm
On Monday 03 August 2009 20:25:43 Avi Kivity wrote:
> On 08/03/2009 09:17 AM, Sheng Yang wrote:
> > Copy u64 from guest result in chaos.
> >
> > Also fix a mistake of still using old macro rather than new variable().
> >
> > Signed-off-by: Sheng Yang<sheng@linux.intel.com>
> > ---
> > arch/x86/kvm/vmx.c | 2 +-
> > arch/x86/kvm/x86.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 7931c72..c5aaa1b 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -2199,7 +2199,7 @@ static int alloc_identity_pagetable(struct kvm
> > *kvm) goto out;
> >
> > kvm->arch.ept_identity_pagetable = gfn_to_page(kvm,
> > - VMX_EPT_IDENTITY_PAGETABLE_ADDR>> PAGE_SHIFT);
> > + kvm->arch.ept_identity_map_addr>> PAGE_SHIFT);
>
> This is a fix, but as Jan says, separate patch.
Sure. (and thanks Jan's comment :) )
>
> > out:
> > up_write(&kvm->slots_lock);
> > return r;
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 2539e9a..977b705 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -2178,7 +2178,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
> > goto out;
> > break;
> > case KVM_SET_IDENTITY_MAP_ADDR: {
> > - u64 ident_addr;
> > + unsigned long ident_addr;
> >
> > r = -EFAULT;
> > if (copy_from_user(&ident_addr, argp, sizeof ident_addr))
>
> This doesn't look right. Consider 32-bit userspace running on a 32-bit
> kernel and the same userspace running on a 64-bit kernel. We need to
> copy the same size in both cases.
Yeah... Then I think I should fix the userspace. Would update the patch.
--
regards
Yang, Sheng
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae
2009-08-03 6:17 [PATCH] KVM: Fix EPT identity IOCTL on 32pae Sheng Yang
2009-08-03 11:48 ` Jan Kiszka
2009-08-03 12:25 ` Avi Kivity
@ 2009-08-03 16:47 ` Marcelo Tosatti
2009-08-03 16:50 ` Sheng Yang
2 siblings, 1 reply; 6+ messages in thread
From: Marcelo Tosatti @ 2009-08-03 16:47 UTC (permalink / raw)
To: Sheng Yang; +Cc: Avi Kivity, kvm
On Mon, Aug 03, 2009 at 02:17:27PM +0800, Sheng Yang wrote:
> Copy u64 from guest result in chaos.
Why?
>
> Also fix a mistake of still using old macro rather than new variable().
>
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
> arch/x86/kvm/vmx.c | 2 +-
> arch/x86/kvm/x86.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 7931c72..c5aaa1b 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2199,7 +2199,7 @@ static int alloc_identity_pagetable(struct kvm *kvm)
> goto out;
>
> kvm->arch.ept_identity_pagetable = gfn_to_page(kvm,
> - VMX_EPT_IDENTITY_PAGETABLE_ADDR >> PAGE_SHIFT);
> + kvm->arch.ept_identity_map_addr >> PAGE_SHIFT);
> out:
> up_write(&kvm->slots_lock);
> return r;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2539e9a..977b705 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2178,7 +2178,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
> goto out;
> break;
> case KVM_SET_IDENTITY_MAP_ADDR: {
> - u64 ident_addr;
> + unsigned long ident_addr;
>
> r = -EFAULT;
> if (copy_from_user(&ident_addr, argp, sizeof ident_addr))
> --
> 1.5.4.5
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae
2009-08-03 16:47 ` Marcelo Tosatti
@ 2009-08-03 16:50 ` Sheng Yang
0 siblings, 0 replies; 6+ messages in thread
From: Sheng Yang @ 2009-08-03 16:50 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, kvm
On Tuesday 04 August 2009 00:47:16 Marcelo Tosatti wrote:
> On Mon, Aug 03, 2009 at 02:17:27PM +0800, Sheng Yang wrote:
> > Copy u64 from guest result in chaos.
>
> Why?
My fault in userspace...
--
regards
Yang, Sheng
>
> > Also fix a mistake of still using old macro rather than new variable().
> >
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> > arch/x86/kvm/vmx.c | 2 +-
> > arch/x86/kvm/x86.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 7931c72..c5aaa1b 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -2199,7 +2199,7 @@ static int alloc_identity_pagetable(struct kvm
> > *kvm) goto out;
> >
> > kvm->arch.ept_identity_pagetable = gfn_to_page(kvm,
> > - VMX_EPT_IDENTITY_PAGETABLE_ADDR >> PAGE_SHIFT);
> > + kvm->arch.ept_identity_map_addr >> PAGE_SHIFT);
> > out:
> > up_write(&kvm->slots_lock);
> > return r;
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 2539e9a..977b705 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -2178,7 +2178,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
> > goto out;
> > break;
> > case KVM_SET_IDENTITY_MAP_ADDR: {
> > - u64 ident_addr;
> > + unsigned long ident_addr;
> >
> > r = -EFAULT;
> > if (copy_from_user(&ident_addr, argp, sizeof ident_addr))
> > --
> > 1.5.4.5
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-08-03 16:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-03 6:17 [PATCH] KVM: Fix EPT identity IOCTL on 32pae Sheng Yang
2009-08-03 11:48 ` Jan Kiszka
2009-08-03 12:25 ` Avi Kivity
2009-08-03 13:38 ` Sheng Yang
2009-08-03 16:47 ` Marcelo Tosatti
2009-08-03 16:50 ` Sheng Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox