[PATCH 1/7] gnttab: add another compiler barrier to acquire_grant_for_copy()

Jan Beulich <[email protected]>
Newsgroups org.xenproject.lists.xen-devel
Message-ID <[email protected]>
... and mem_sharing_gref_to_gfn().

Like for the fetching of trans_{domid,gref} in the former we ought to also
fetch other fields exactly once.

Signed-off-by: Jan Beulich <[email protected]>
---
Strictly speaking mem_sharing_gref_to_gfn() would need another barriers
added on the v2 path, but it being different sub-fields of the flags looks
to make the uses okay there. Thoughts?

Note that the calculation of cache_flags in map_grant_ref() has enough of
a barrier after it already, by virtue of active_entry_release() being
called right afterwards.

The fetching of the frame in gnttab_transfer() is hopefully good enough,
by each fetch happening exactly once. The sole risk there is load tearing,
which can occur elsewhere as well.

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2606,6 +2606,7 @@ acquire_grant_for_copy(
     grant_entry_header_t *shah;
     struct active_grant_entry *act;
     grant_status_t *status;
+    unsigned int shflags;
     uint32_t old_pin;
     domid_t trans_domid;
     grant_ref_t trans_gref;
@@ -2655,8 +2656,11 @@ acquire_grant_for_copy(
         status = &status_entry(rgt, gref);
     }
 
+    shflags = shah->flags;
+    barrier();
+
     old_pin = act->pin;
-    if ( sha2 && (shah->flags & GTF_type_mask) == GTF_transitive )
+    if ( sha2 && (shflags & GTF_type_mask) == GTF_transitive )
     {
         if ( (!old_pin || (!readonly &&
                            !(old_pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)))) &&
@@ -2793,6 +2797,8 @@ acquire_grant_for_copy(
     else if ( !old_pin ||
               (!readonly && !(old_pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask))) )
     {
+        unsigned long gfn;
+
         if ( (rc = _set_status(shah, status, rd, rgt->gt_version, act,
                                readonly, 0, ldom)) != GNTST_okay )
              goto unlock_out;
@@ -2801,39 +2807,34 @@ acquire_grant_for_copy(
         trans_gref = gref;
         if ( !sha2 )
         {
-            unsigned long gfn = shared_entry_v1(rgt, gref).frame;
-
-            rc = get_paged_frame(gfn, &grant_mfn, page, readonly, rd);
-            if ( rc != GNTST_okay )
-                goto unlock_out_clear;
-            act_set_gfn(act, _gfn(gfn));
+            gfn = shared_entry_v1(rgt, gref).frame;
             is_sub_page = false;
             trans_page_off = 0;
             trans_length = PAGE_SIZE;
         }
-        else if ( !(sha2->hdr.flags & GTF_sub_page) )
+        else if ( !(shflags & GTF_sub_page) )
         {
-            rc = get_paged_frame(sha2->full_page.frame, &grant_mfn, page,
-                                 readonly, rd);
-            if ( rc != GNTST_okay )
-                goto unlock_out_clear;
-            act_set_gfn(act, _gfn(sha2->full_page.frame));
+            gfn = sha2->full_page.frame;
             is_sub_page = false;
             trans_page_off = 0;
             trans_length = PAGE_SIZE;
         }
         else
         {
-            rc = get_paged_frame(sha2->sub_page.frame, &grant_mfn, page,
-                                 readonly, rd);
-            if ( rc != GNTST_okay )
-                goto unlock_out_clear;
-            act_set_gfn(act, _gfn(sha2->sub_page.frame));
+            gfn = sha2->sub_page.frame;
             is_sub_page = true;
             trans_page_off = sha2->sub_page.page_off;
             trans_length = sha2->sub_page.length;
         }
 
+        /* Use latched copies of the shared entry fields from here on. */
+        barrier();
+
+        rc = get_paged_frame(gfn, &grant_mfn, page, readonly, rd);
+        if ( rc != GNTST_okay )
+            goto unlock_out_clear;
+        act_set_gfn(act, _gfn(gfn));
+
         if ( !act->pin )
         {
             act->domid = ldom;
@@ -4145,6 +4146,8 @@ int mem_sharing_gref_to_gfn(struct grant
            *gfn = _gfn(sha2->full_page.frame);
     }
 
+    barrier();
+
     if ( !rc && (flags & GTF_type_mask) != GTF_permit_access )
         rc = -ENXIO;
     else if ( !rc && status )
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.