[Xen-devel] [PATCH 2/2] x86/pv: Code improvements to do_update_descriptor()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

 * Add "uint64_t raw" to seg_desc_t to remove the opencoded uint64_t casting
   in this function.
 * Rename the 'pa' parameter to 'gaddr', because it lives in GFN space rather
   than physical address space.
 * Use gfn_t and mfn_t rather than unsigned longs.
 * Check the alignment and proposed new descriptor before taking a page
   reference.
 * Reuse the out label for all exit paths.
 * Use the more flexible ACCESS_ONCE() accessor in preference to
   write_atomic()

No expected change in behaviour.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/x86/pv/descriptor-tables.c | 41 +++++++++++++++++--------------------
 xen/include/asm-x86/desc.h          |  7 +++++--
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/xen/arch/x86/pv/descriptor-tables.c 
b/xen/arch/x86/pv/descriptor-tables.c
index caa62eb..efcfbab 100644
--- a/xen/arch/x86/pv/descriptor-tables.c
+++ b/xen/arch/x86/pv/descriptor-tables.c
@@ -206,30 +206,26 @@ int compat_set_gdt(XEN_GUEST_HANDLE_PARAM(uint) 
frame_list,
     return ret;
 }
 
-long do_update_descriptor(uint64_t pa, uint64_t desc)
+long do_update_descriptor(uint64_t gaddr, uint64_t desc)
 {
     struct domain *currd = current->domain;
-    unsigned long gmfn = pa >> PAGE_SHIFT;
-    unsigned long mfn;
-    unsigned int  offset;
-    seg_desc_t *gdt_pent, d;
+    gfn_t gfn = gaddr_to_gfn(gaddr);
+    mfn_t mfn;
+    seg_desc_t *entry, d;
     struct page_info *page;
     long ret = -EINVAL;
 
-    offset = ((unsigned int)pa & ~PAGE_MASK) / sizeof(seg_desc_t);
+    d.raw = desc;
 
-    *(uint64_t *)&d = desc;
+    /* gaddr must be aligned, or it will corrupt adjacent descriptors. */
+    if ( !IS_ALIGNED(gaddr, sizeof(d)) || !check_descriptor(currd, &d) )
+        goto out;
 
-    page = get_page_from_gfn(currd, gmfn, NULL, P2M_ALLOC);
-    if ( (((unsigned int)pa % sizeof(seg_desc_t)) != 0) ||
-         !page ||
-         !check_descriptor(currd, &d) )
-    {
-        if ( page )
-            put_page(page);
-        return -EINVAL;
-    }
-    mfn = mfn_x(page_to_mfn(page));
+    page = get_page_from_gfn(currd, gfn_x(gfn), NULL, P2M_ALLOC);
+    if ( !page )
+        goto out;
+
+    mfn = page_to_mfn(page);
 
     /* Check if the given frame is in use in an unsafe context. */
     switch ( page->u.inuse.type_info & PGT_type_mask )
@@ -244,19 +240,20 @@ long do_update_descriptor(uint64_t pa, uint64_t desc)
         break;
     }
 
-    paging_mark_dirty(currd, _mfn(mfn));
+    paging_mark_dirty(currd, mfn);
 
     /* All is good so make the update. */
-    gdt_pent = map_domain_page(_mfn(mfn));
-    write_atomic((uint64_t *)&gdt_pent[offset], *(uint64_t *)&d);
-    unmap_domain_page(gdt_pent);
+    entry = map_domain_page(mfn) + (gaddr & ~PAGE_MASK);
+    ACCESS_ONCE(entry->raw) = d.raw;
+    unmap_domain_page(entry);
 
     put_page_type(page);
 
     ret = 0; /* success */
 
  out:
-    put_page(page);
+    if ( page )
+        put_page(page);
 
     return ret;
 }
diff --git a/xen/include/asm-x86/desc.h b/xen/include/asm-x86/desc.h
index 5a8afb6..85e83bc 100644
--- a/xen/include/asm-x86/desc.h
+++ b/xen/include/asm-x86/desc.h
@@ -102,8 +102,11 @@
 #define SYS_DESC_irq_gate     14
 #define SYS_DESC_trap_gate    15
 
-typedef struct {
-    uint32_t a, b;
+typedef union {
+    struct {
+        uint32_t a, b;
+    };
+    uint64_t raw;
 } seg_desc_t;
 
 typedef union {
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel