[Xen-devel] [PATCH 2/2] x86: miscellaneous mm.c cleanup

["Jan Beulich" <JBeulich@xxxxxxxx>]

This simply streamlines code in a few places, where room for
improvement was noticed during the earlier here and the patches in
the XSA-45 series.

This also drops the bogus use of the domain lock in the CR3 write
emulation (which protected against nothing).

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Acked-by: Tim Deegan <tim@xxxxxxx>

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2075,19 +2075,17 @@ static int alloc_page_type(struct page_i
 
     /* No need for atomic update of type_info here: noone else updates it. */
     wmb();
-    if ( rc == -EAGAIN )
-    {
-        get_page_light(page);
-        page->u.inuse.type_info |= PGT_partial;
-    }
-    else if ( rc == -EINTR )
+    switch ( rc )
     {
+    case 0:
+        page->u.inuse.type_info |= PGT_validated;
+        break;
+    case -EINTR:
         ASSERT((page->u.inuse.type_info &
                 (PGT_count_mask|PGT_validated|PGT_partial)) == 1);
         page->u.inuse.type_info &= ~PGT_count_mask;
-    }
-    else if ( rc )
-    {
+        break;
+    default:
         ASSERT(rc < 0);
         MEM_LOG("Error while validating mfn %lx (pfn %lx) for type %"
                 PRtype_info ": caf=%08lx taf=%" PRtype_info,
@@ -2099,13 +2097,11 @@ static int alloc_page_type(struct page_i
         {
             ASSERT((page->u.inuse.type_info &
                     (PGT_count_mask | PGT_validated)) == 1);
+    case -EAGAIN:
             get_page_light(page);
             page->u.inuse.type_info |= PGT_partial;
         }
-    }
-    else
-    {
-        page->u.inuse.type_info |= PGT_validated;
+        break;
     }
 
     return rc;
@@ -2884,7 +2880,7 @@ long do_mmuext_op(
 {
     struct mmuext_op op;
     unsigned long type;
-    unsigned int i = 0, done = 0;
+    unsigned int i, done = 0;
     struct vcpu *curr = current;
     struct domain *d = curr->domain;
     struct domain *pg_owner;
@@ -2917,22 +2913,16 @@ long do_mmuext_op(
         perfc_incr(calls_to_mmuext_op);
 
     if ( unlikely(!guest_handle_okay(uops, count)) )
-    {
-        rc = -EFAULT;
-        goto out;
-    }
+        return -EFAULT;
 
     if ( (pg_owner = get_pg_owner(foreigndom)) == NULL )
-    {
-        rc = -ESRCH;
-        goto out;
-    }
+        return -ESRCH;
 
     rc = xsm_mmuext_op(XSM_TARGET, d, pg_owner);
     if ( rc )
     {
-        rcu_unlock_domain(pg_owner);
-        goto out;
+        put_pg_owner(pg_owner);
+        return rc;
     }
 
     for ( i = 0; i < count; i++ )
@@ -3404,7 +3394,6 @@ long do_mmuext_op(
 
     perfc_add(num_mmuext_ops, i);
 
- out:
     /* Add incremental work we have done to the @done output parameter. */
     if ( unlikely(!guest_handle_is_null(pdone)) )
     {
@@ -3460,22 +3449,17 @@ long do_mmu_update(
         perfc_incr(calls_to_mmu_update);
 
     if ( unlikely(!guest_handle_okay(ureqs, count)) )
-    {
-        rc = -EFAULT;
-        goto out;
-    }
+        return -EFAULT;
 
     if ( (pt_dom = foreigndom >> 16) != 0 )
     {
         /* Pagetables belong to a foreign domain (PFD). */
         if ( (pt_owner = rcu_lock_domain_by_id(pt_dom - 1)) == NULL )
-        {
-            rc = -EINVAL;
-            goto out;
-        }
+            return -EINVAL;
+
         if ( pt_owner == d )
             rcu_unlock_domain(pt_owner);
-        if ( (v = pt_owner->vcpu ? pt_owner->vcpu[0] : NULL) == NULL )
+        else if ( !pt_owner->vcpu || (v = pt_owner->vcpu[0]) == NULL )
         {
             rc = -EINVAL;
             goto out;
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2318,7 +2318,7 @@ static int emulate_privileged_op(struct 
         case 3: {/* Write CR3 */
             unsigned long gfn;
             struct page_info *page;
-            domain_lock(v->domain);
+
             gfn = !is_pv_32on64_vcpu(v)
                 ? xen_cr3_to_pfn(*reg) : compat_cr3_to_pfn(*reg);
             page = get_page_from_gfn(v->domain, gfn, NULL, P2M_ALLOC);
@@ -2329,7 +2329,7 @@ static int emulate_privileged_op(struct 
             }
             else
                 rc = -EINVAL;
-            domain_unlock(v->domain);
+
             switch ( rc )
             {
             case 0:
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -273,8 +273,6 @@ extern unsigned char boot_edid_info[128]
 
 #endif
 
-#define PGT_base_page_table     PGT_l4_page_table
-
 #define __HYPERVISOR_CS64 0xe008
 #define __HYPERVISOR_CS32 0xe038
 #define __HYPERVISOR_CS   __HYPERVISOR_CS64

Attachment: x86-preemptible-cleanup-2.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel