[Xen-devel] [PATCH v2 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.  Change the parameter to be of type seg_desc_t.
 * 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>
Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>

v2:
 * Change parameter to be seg_desc_t.
---
 xen/arch/x86/pv/descriptor-tables.c | 43 +++++++++++++++++--------------------
 xen/include/asm-x86/desc.h          |  7 ++++--
 xen/include/asm-x86/hypercall.h     |  3 +--
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/pv/descriptor-tables.c 
b/xen/arch/x86/pv/descriptor-tables.c
index caa62eb..940804b 100644
--- a/xen/arch/x86/pv/descriptor-tables.c
+++ b/xen/arch/x86/pv/descriptor-tables.c
@@ -206,30 +206,24 @@ 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, seg_desc_t d)
 {
     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;
     struct page_info *page;
     long ret = -EINVAL;
 
-    offset = ((unsigned int)pa & ~PAGE_MASK) / sizeof(seg_desc_t);
-
-    *(uint64_t *)&d = desc;
+    /* gaddr must be aligned, or it will corrupt adjacent descriptors. */
+    if ( !IS_ALIGNED(gaddr, sizeof(d)) || !check_descriptor(currd, &d) )
+        return -EINVAL;
 
-    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);
+    page = get_page_from_gfn(currd, gfn_x(gfn), NULL, P2M_ALLOC);
+    if ( !page )
         return -EINVAL;
-    }
-    mfn = mfn_x(page_to_mfn(page));
+
+    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,12 +238,12 @@ 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);
 
@@ -264,8 +258,11 @@ long do_update_descriptor(uint64_t pa, uint64_t desc)
 int compat_update_descriptor(uint32_t pa_lo, uint32_t pa_hi,
                              uint32_t desc_lo, uint32_t desc_hi)
 {
-    return do_update_descriptor(pa_lo | ((uint64_t)pa_hi << 32),
-                                desc_lo | ((uint64_t)desc_hi << 32));
+    seg_desc_t d;
+
+    d.raw = ((uint64_t)desc_hi << 32) | desc_lo;
+
+    return do_update_descriptor(pa_lo | ((uint64_t)pa_hi << 32), d);
 }
 
 /*
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 {
diff --git a/xen/include/asm-x86/hypercall.h b/xen/include/asm-x86/hypercall.h
index 7f302ec..49eb5f1 100644
--- a/xen/include/asm-x86/hypercall.h
+++ b/xen/include/asm-x86/hypercall.h
@@ -88,8 +88,7 @@ do_get_debugreg(
 
 extern long
 do_update_descriptor(
-    u64 pa,
-    u64 desc);
+    uint64_t gaddr, seg_desc_t desc);
 
 extern long
 do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc);
-- 
2.1.4


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