[PATCH] KVM: Fix EPT identity IOCTL on 32pae

2009-08-03 Thread 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 sh...@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

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae

2009-08-03 Thread Jan Kiszka 
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 sh...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae

2009-08-03 Thread Avi Kivity 

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 Yangsh...@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

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae

2009-08-03 Thread Sheng Yang 
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 Yangsh...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: Fix EPT identity IOCTL on 32pae

2009-08-03 Thread Sheng Yang 
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 sh...@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


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html