[PATCH] HVM guests are allowed to use the entire virtual address space for

[PATCH] HVM guests are allowed to use the entire virtual address space for

[Tim Deegan]

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

whatever they want, without any Xen hole.  __addr_ok() is therefore
always true for such guests.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>



[-- Attachment #2: xen-unstable.hg.patch --]
[-- Type: text/x-patch, Size: 2916 bytes --]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1274798621 -3600
# Node ID 0b3b2b31e962561390cac4896a4a70841990f0e6
# Parent  fef1e7dc74e64cf25d817991dc2594579abf5bd0
HVM guests are allowed to use the entire virtual address space for
whatever they want, without any Xen hole.  __addr_ok() is therefore
always true for such guests.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r fef1e7dc74e6 -r 0b3b2b31e962 xen/arch/x86/usercopy.c
--- a/xen/arch/x86/usercopy.c	Tue May 25 15:36:25 2010 +0100
+++ b/xen/arch/x86/usercopy.c	Tue May 25 15:43:41 2010 +0100
@@ -7,6 +7,7 @@
  */
 
 #include <xen/config.h>
+#include <xen/sched.h>
 #include <xen/lib.h>
 #include <asm/uaccess.h>
 
diff -r fef1e7dc74e6 -r 0b3b2b31e962 xen/include/asm-x86/x86_32/uaccess.h
--- a/xen/include/asm-x86/x86_32/uaccess.h	Tue May 25 15:36:25 2010 +0100
+++ b/xen/include/asm-x86/x86_32/uaccess.h	Tue May 25 15:43:41 2010 +0100
@@ -15,7 +15,8 @@
 		:"1" (addr),"g" ((int)(size)),"r" (HYPERVISOR_VIRT_START)); \
 	flag; })
 
-#define access_ok(addr,size) (likely(__range_not_ok(addr,size) == 0))
+#define access_ok(addr,size) \
+    (is_hvm_vcpu(current) || likely(__range_not_ok(addr,size) == 0))
 
 #define array_access_ok(addr,count,size) \
     (likely(count < (~0UL/size)) && access_ok(addr,count*size))
diff -r fef1e7dc74e6 -r 0b3b2b31e962 xen/include/asm-x86/x86_64/uaccess.h
--- a/xen/include/asm-x86/x86_64/uaccess.h	Tue May 25 15:36:25 2010 +0100
+++ b/xen/include/asm-x86/x86_64/uaccess.h	Tue May 25 15:43:41 2010 +0100
@@ -14,13 +14,15 @@
 })
 
 /*
- * Valid if in +ve half of 48-bit address space, or above Xen-reserved area.
+ * Valid if in +ve half of 48-bit address space, or above Xen-reserved
+ * area or anywhere in HVM guests (which have no Xen-reserved area).
  * This is also valid for range checks (addr, addr+size). As long as the
  * start address is outside the Xen-reserved area then we will access a
  * non-canonical address (and thus fault) before ever reaching VIRT_START.
  */
-#define __addr_ok(addr) \
-    (((unsigned long)(addr) < (1UL<<48)) || \
+#define __addr_ok(addr)                                 \
+    (is_hvm_vcpu(current) ||                            \
+     ((unsigned long)(addr) < (1UL<<48)) ||             \
      ((unsigned long)(addr) >= HYPERVISOR_VIRT_END))
 
 #define access_ok(addr, size) \
@@ -32,8 +34,9 @@
 #define __compat_addr_ok(d, addr) \
     ((unsigned long)(addr) < HYPERVISOR_COMPAT_VIRT_START(d))
 
-#define __compat_access_ok(d, addr, size) \
-    __compat_addr_ok(d, (unsigned long)(addr) + ((size) ? (size) - 1 : 0))
+#define __compat_access_ok(d, addr, size)                               \
+    (is_hvm_vcpu(current) ||                                            \
+     __compat_addr_ok(d, (unsigned long)(addr) + ((size) ? (size) - 1 : 0)))
 
 #define compat_access_ok(addr, size) \
     __compat_access_ok(current->domain, addr, size)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] HVM guests are allowed to use the entire virtual address space for

[Keir Fraser]

I think this is already handled correctly by guest_access.h, which is what
really matters. Nowadays copy_{to,from}_user (but not __copy_{to,from}_user)
are mainly for private usage only by the guest-access macros, and just a few
other arch-dependent PV_specific bits of code. Of course the __ versions are
still generally useful as 'copy but catch any fault' functions.

 -- Keir

On 25/05/2010 15:49, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

> whatever they want, without any Xen hole.  __addr_ok() is therefore
> always true for such guests.
> 
> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
> 
> 

Re: [PATCH] HVM guests are allowed to use the entire virtual address space for

[Keir Fraser]

Put another way, I'd rather ASSERT(!is_hvm_vcpu(current)) than further
reinforce the current confusion around these macros by supporting an
obsolete usage.

 -- Keir

On 25/05/2010 16:04, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:

> I think this is already handled correctly by guest_access.h, which is what
> really matters. Nowadays copy_{to,from}_user (but not __copy_{to,from}_user)
> are mainly for private usage only by the guest-access macros, and just a few
> other arch-dependent PV_specific bits of code. Of course the __ versions are
> still generally useful as 'copy but catch any fault' functions.
> 
>  -- Keir
> 
> On 25/05/2010 15:49, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:
> 
>> whatever they want, without any Xen hole.  __addr_ok() is therefore
>> always true for such guests.
>> 
>> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
>> 
>> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] HVM guests are allowed to use the entire virtual address space for

[Tim Deegan]

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

At 16:04 +0100 on 25 May (1274803495), Keir Fraser wrote:
> I think this is already handled correctly by guest_access.h, which is what
> really matters. Nowadays copy_{to,from}_user (but not __copy_{to,from}_user)
> are mainly for private usage only by the guest-access macros, and just a few
> other arch-dependent PV_specific bits of code. Of course the __ versions are
> still generally useful as 'copy but catch any fault' functions.

OK, can I suggest the attached change instead?  It brings the 
compat guest-handle macros in line with the normal ones.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

There are a few more callers of access_ok() around but at first glance
they seem all to be PV-only. 

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[-- Attachment #2: compat-handle --]
[-- Type: text/plain, Size: 695 bytes --]

diff -r 10ad9b50b4ca xen/include/xen/compat.h
--- a/xen/include/xen/compat.h	Tue May 25 11:28:58 2010 +0100
+++ b/xen/include/xen/compat.h	Tue May 25 16:13:54 2010 +0100
@@ -90,8 +90,9 @@
  * Allows use of faster __copy_* functions.
  */
 #define compat_handle_okay(hnd, nr)                                  \
+    (paging_mode_external(current->domain) ||                        \
     compat_array_access_ok((void *)(full_ptr_t)(hnd).c, (nr),        \
-                           sizeof(**(hnd)._))
+                           sizeof(**(hnd)._)))
 
 #define __copy_to_compat_offset(hnd, off, ptr, nr) ({                \
     const typeof(*(ptr)) *_s = (ptr);                                \

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] HVM guests are allowed to use the entire virtual address space for

[Keir Fraser]

On 25/05/2010 16:14, "Tim Deegan" <Tim.Deegan@eu.citrix.com> wrote:

> At 16:04 +0100 on 25 May (1274803495), Keir Fraser wrote:
>> I think this is already handled correctly by guest_access.h, which is what
>> really matters. Nowadays copy_{to,from}_user (but not __copy_{to,from}_user)
>> are mainly for private usage only by the guest-access macros, and just a few
>> other arch-dependent PV_specific bits of code. Of course the __ versions are
>> still generally useful as 'copy but catch any fault' functions.
> 
> OK, can I suggest the attached change instead?  It brings the
> compat guest-handle macros in line with the normal ones.

Yeah, that's a good bug fix.

 Thanks,
 K.

Re: [PATCH] HVM guests are allowed to use the entire virtual address space for

[Paolo Bonzini]

On 05/25/2010 04:49 PM, Tim Deegan wrote:
> whatever they want, without any Xen hole.  __addr_ok() is therefore
> always true for such guests.
>
> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

Sorry for picking up yet another old patch after two months. :)

Do you have a testcase for this or the other patch in the thread?

Paolo

Re: [PATCH] HVM guests are allowed to use the entire virtual address space for

[Tim Deegan]

At 10:52 +0100 on 26 Jul (1280141550), Paolo Bonzini wrote:
> On 05/25/2010 04:49 PM, Tim Deegan wrote:
> > whatever they want, without any Xen hole.  __addr_ok() is therefore
> > always true for such guests.
> >
> > Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
> 
> Sorry for picking up yet another old patch after two months. :)
> 
> Do you have a testcase for this or the other patch in the thread?

I'm afraid not - this was a very old patch that had been gathering dust
at the bottom of the XenServer code base.  Calling
e.g. GNTTABOP_setup_table from inside a HVM guest with the frame_list
argument somewhere in the Xen virtual address space should trigger it,
though.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: Re: [PATCH] HVM guests are allowed to use the entire virtual address space for

[Keir Fraser]

On 26/07/2010 11:00, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

> At 10:52 +0100 on 26 Jul (1280141550), Paolo Bonzini wrote:
>> On 05/25/2010 04:49 PM, Tim Deegan wrote:
>>> whatever they want, without any Xen hole.  __addr_ok() is therefore
>>> always true for such guests.
>>> 
>>> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
>> 
>> Sorry for picking up yet another old patch after two months. :)
>> 
>> Do you have a testcase for this or the other patch in the thread?
> 
> I'm afraid not - this was a very old patch that had been gathering dust
> at the bottom of the XenServer code base.  Calling
> e.g. GNTTABOP_setup_table from inside a HVM guest with the frame_list
> argument somewhere in the Xen virtual address space should trigger it,
> though.

The patch never got applied (to xen-unstable) by the way. We agreed on
xen-unstable:21456, which was a more targeted bug fix.

 -- Keir

> Cheers,
> 
> Tim.