WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] do_grant_table_op()'s input array size limitation

To: "Keir Fraser" <keir.fraser@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] do_grant_table_op()'s input array size limitation
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Thu, 02 Jul 2009 13:59:26 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Thu, 02 Jul 2009 06:00:50 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C6722238.EB64%keir.fraser@xxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4A4C7CA9020000780000887C@xxxxxxxxxxxxxxxxxx> <C6722238.EB64%keir.fraser@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>>> Keir Fraser <keir.fraser@xxxxxxxxxxxxx> 02.07.09 09:37 >>>
>On 02/07/2009 08:23, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:
>
>> Am I right in understanding that the restriction on 512 entries to be passed
>> in is solely to cap the time that's being spent in the hypervisor without
>> being
>> able to service softirq requests as well as while the domain lock is being
>> held?
>> If that's the case, eliminating that restriction could be fairly strait
>> forward by
>> putting the majority of the function body in a loop or, perhaps even better,
>> calling hypercall_preempt_check() every so many entries and using the
>> continuation mechanism.
>
>Yes, true.

Would you have time to look over this (so far only compile tested) patch,
to see if there's anything obviously wrong (I admit I feel uncertain
whenever it comes to touching the grant table code)?

Thanks, Jan

--- 2009-06-10.orig/xen/common/compat/grant_table.c     2008-08-01 
08:48:42.000000000 +0200
+++ 2009-06-10/xen/common/compat/grant_table.c  2009-07-02 14:42:05.000000000 
+0200
@@ -35,7 +35,9 @@ int compat_grant_table_op(unsigned int c
 {
     int rc = 0;
     unsigned int i;
+    XEN_GUEST_HANDLE(void) cnt_uop;
 
+    set_xen_guest_handle(cnt_uop, NULL);
     switch ( cmd )
     {
 #define CASE(name) \
@@ -79,7 +81,7 @@ int compat_grant_table_op(unsigned int c
         return do_grant_table_op(cmd, cmp_uop, count);
     }
 
-    if ( count > 512 )
+    if ( (int)count < 0 )
         rc = -EINVAL;
 
     for ( i = 0; i < count && rc == 0; )
@@ -128,6 +130,7 @@ int compat_grant_table_op(unsigned int c
                     rc = gnttab_setup_table(guest_handle_cast(nat.uop, 
gnttab_setup_table_t), 1);
                 }
             }
+            ASSERT(rc <= 0);
             if ( rc == 0 )
             {
 #define XLAT_gnttab_setup_table_HNDL_frame_list(_d_, _s_) \
@@ -163,12 +166,19 @@ int compat_grant_table_op(unsigned int c
             }
             if ( rc == 0 )
                 rc = gnttab_transfer(guest_handle_cast(nat.uop, 
gnttab_transfer_t), n);
-            if ( rc == 0 )
+            if ( rc > 0 )
+            {
+                ASSERT(rc < n);
+                i -= n - rc;
+                n = rc;
+            }
+            if ( rc >= 0 )
             {
                 XEN_GUEST_HANDLE(gnttab_transfer_compat_t) xfer;
 
                 xfer = guest_handle_cast(cmp_uop, gnttab_transfer_compat_t);
                 guest_handle_add_offset(xfer, i);
+                cnt_uop = guest_handle_cast(xfer, void);
                 while ( n-- )
                 {
                     guest_handle_add_offset(xfer, -1);
@@ -201,12 +211,19 @@ int compat_grant_table_op(unsigned int c
             }
             if ( rc == 0 )
                 rc = gnttab_copy(guest_handle_cast(nat.uop, gnttab_copy_t), n);
-            if ( rc == 0 )
+            if ( rc > 0 )
+            {
+                ASSERT(rc < n);
+                i -= n - rc;
+                n = rc;
+            }
+            if ( rc >= 0 )
             {
                 XEN_GUEST_HANDLE(gnttab_copy_compat_t) copy;
 
                 copy = guest_handle_cast(cmp_uop, gnttab_copy_compat_t);
                 guest_handle_add_offset(copy, i);
+                cnt_uop = guest_handle_cast(copy, void);
                 while ( n-- )
                 {
                     guest_handle_add_offset(copy, -1);
@@ -222,6 +239,14 @@ int compat_grant_table_op(unsigned int c
         }
     }
 
+    if ( rc > 0 )
+    {
+        ASSERT(i < count);
+        ASSERT(!guest_handle_is_null(cnt_uop));
+        rc = hypercall_create_continuation(__HYPERVISOR_grant_table_op,
+                                           "ihi", cmd, cnt_uop, count - i);
+    }
+
     return rc;
 }
 
--- 2009-06-10.orig/xen/common/grant_table.c    2009-06-03 12:22:45.000000000 
+0200
+++ 2009-06-10/xen/common/grant_table.c 2009-07-02 14:42:33.000000000 +0200
@@ -29,6 +29,7 @@
 #include <xen/lib.h>
 #include <xen/sched.h>
 #include <xen/mm.h>
+#include <xen/event.h>
 #include <xen/trace.h>
 #include <xen/guest_access.h>
 #include <xen/domain_page.h>
@@ -465,6 +466,8 @@ gnttab_map_grant_ref(
 
     for ( i = 0; i < count; i++ )
     {
+        if (i && hypercall_preempt_check())
+            return i;
         if ( unlikely(__copy_from_guest_offset(&op, uop, i, 1)) )
             return -EFAULT;
         __gnttab_map_grant_ref(&op);
@@ -722,6 +725,9 @@ gnttab_unmap_grant_ref(
 
         count -= c;
         done += c;
+
+        if (count && hypercall_preempt_check())
+            return done;
     }
      
     return 0;
@@ -781,6 +787,9 @@ gnttab_unmap_and_replace(
 
         count -= c;
         done += c;
+
+        if (count && hypercall_preempt_check())
+            return done;
     }
 
     return 0;
@@ -1086,6 +1095,9 @@ gnttab_transfer(
 
     for ( i = 0; i < count; i++ )
     {
+        if (i && hypercall_preempt_check())
+            return i;
+
         /* Read from caller address space. */
         if ( unlikely(__copy_from_guest_offset(&gop, uop, i, 1)) )
         {
@@ -1478,6 +1490,8 @@ gnttab_copy(
 
     for ( i = 0; i < count; i++ )
     {
+        if (i && hypercall_preempt_check())
+            return i;
         if ( unlikely(__copy_from_guest_offset(&op, uop, i, 1)) )
             return -EFAULT;
         __gnttab_copy(&op);
@@ -1494,7 +1508,7 @@ do_grant_table_op(
     long rc;
     struct domain *d = current->domain;
     
-    if ( count > 512 )
+    if ( (int)count < 0 )
         return -EINVAL;
     
     domain_lock(d);
@@ -1509,6 +1523,11 @@ do_grant_table_op(
         if ( unlikely(!guest_handle_okay(map, count)) )
             goto out;
         rc = gnttab_map_grant_ref(map, count);
+        if ( rc > 0 )
+        {
+            guest_handle_add_offset(map, rc);
+            uop = guest_handle_cast(map, void);
+        }
         break;
     }
     case GNTTABOP_unmap_grant_ref:
@@ -1518,6 +1537,11 @@ do_grant_table_op(
         if ( unlikely(!guest_handle_okay(unmap, count)) )
             goto out;
         rc = gnttab_unmap_grant_ref(unmap, count);
+        if ( rc > 0 )
+        {
+            guest_handle_add_offset(unmap, rc);
+            uop = guest_handle_cast(unmap, void);
+        }
         break;
     }
     case GNTTABOP_unmap_and_replace:
@@ -1530,12 +1554,18 @@ do_grant_table_op(
         if ( unlikely(!replace_grant_supported()) )
             goto out;
         rc = gnttab_unmap_and_replace(unmap, count);
+        if ( rc > 0 )
+        {
+            guest_handle_add_offset(unmap, rc);
+            uop = guest_handle_cast(unmap, void);
+        }
         break;
     }
     case GNTTABOP_setup_table:
     {
         rc = gnttab_setup_table(
             guest_handle_cast(uop, gnttab_setup_table_t), count);
+        ASSERT(rc <= 0);
         break;
     }
     case GNTTABOP_transfer:
@@ -1545,6 +1575,11 @@ do_grant_table_op(
         if ( unlikely(!guest_handle_okay(transfer, count)) )
             goto out;
         rc = gnttab_transfer(transfer, count);
+        if ( rc > 0 )
+        {
+            guest_handle_add_offset(transfer, rc);
+            uop = guest_handle_cast(transfer, void);
+        }
         break;
     }
     case GNTTABOP_copy:
@@ -1554,12 +1589,18 @@ do_grant_table_op(
         if ( unlikely(!guest_handle_okay(copy, count)) )
             goto out;
         rc = gnttab_copy(copy, count);
+        if ( rc > 0 )
+        {
+            guest_handle_add_offset(copy, rc);
+            uop = guest_handle_cast(copy, void);
+        }
         break;
     }
     case GNTTABOP_query_size:
     {
         rc = gnttab_query_size(
             guest_handle_cast(uop, gnttab_query_size_t), count);
+        ASSERT(rc <= 0);
         break;
     }
     default:
@@ -1569,6 +1610,13 @@ do_grant_table_op(
     
   out:
     domain_unlock(d);
+
+    if ( rc > 0 )
+    {
+        ASSERT(rc < count);
+        rc = hypercall_create_continuation(__HYPERVISOR_grant_table_op,
+                                           "ihi", cmd, uop, count - rc);
+    }
     
     return rc;
 }


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