x86/mm: {paging, sh}_{cmpxchg, write}_guest_entry() cannot fault

As of 2d0557c5cbeb ("x86: Fold page_info lock into type_info") we
haven't been updating guest page table entries through linear page
tables anymore. All updates have been using domain mappings since then.
Drop the use of guest/user access helpers there, and hence also the
boolean return values of the involved functions.

update_intpte(), otoh, gets its boolean return type retained for now,
as we may want to bound the CMPXCHG retry loop, indicating failure to
the caller instead when the retry threshold got exceeded.

With this {,__}cmpxchg_user() become unused, so they too get dropped.
(In fact, dropping them was the motivation of making the change.)

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Tim Deegan <tim@xen.org>

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index d1cfc8fb4af7a2573ad56a19e67f6c5d9d38f04d..8c8f054186cef677a5fef43b8d36de9d49f23794 100644 (file)
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4033,8 +4033,8 @@ long do_mmu_update(
 
                 case PGT_writable_page:
                     perfc_incr(writable_mmu_updates);
-                    if ( paging_write_guest_entry(v, va, req.val, mfn) )
-                        rc = 0;
+                    paging_write_guest_entry(v, va, req.val, mfn);
+                    rc = 0;
                     break;
                 }
                 page_unlock(page);
@@ -4044,9 +4044,9 @@ long do_mmu_update(
             else if ( get_page_type(page, PGT_writable_page) )
             {
                 perfc_incr(writable_mmu_updates);
-                if ( paging_write_guest_entry(v, va, req.val, mfn) )
-                    rc = 0;
+                paging_write_guest_entry(v, va, req.val, mfn);
                 put_page_type(page);
+                rc = 0;
             }
 
             put_page(page);

diff --git a/xen/arch/x86/mm/shadow/private.h b/xen/arch/x86/mm/shadow/private.h
index 0a8927f49ef906b067e4f3cae9c694b2f727cd82..fd72f4afb0d7cc5d7886cf09a7221482f18984c7 100644 (file)
--- a/xen/arch/x86/mm/shadow/private.h
+++ b/xen/arch/x86/mm/shadow/private.h
@@ -396,9 +396,9 @@ int shadow_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
                            unsigned int level);
 
 /* Functions that atomically write PV guest PT entries */
-bool sh_write_guest_entry(struct vcpu *v, intpte_t *p, intpte_t new,
+void sh_write_guest_entry(struct vcpu *v, intpte_t *p, intpte_t new,
                           mfn_t gmfn);
-bool sh_cmpxchg_guest_entry(struct vcpu *v, intpte_t *p, intpte_t *old,
+void sh_cmpxchg_guest_entry(struct vcpu *v, intpte_t *p, intpte_t *old,
                             intpte_t new, mfn_t gmfn);
 
 /* Update all the things that are derived from the guest's CR0/CR3/CR4.

diff --git a/xen/arch/x86/mm/shadow/pv.c b/xen/arch/x86/mm/shadow/pv.c
index 25dca1eb25b95f772850e1d4f8abe3f8d448fee6..ac779afce13700ba4b9eaaceac0711e6878ef20c 100644 (file)
--- a/xen/arch/x86/mm/shadow/pv.c
+++ b/xen/arch/x86/mm/shadow/pv.c
@@ -26,43 +26,35 @@
 
 /*
  * Write a new value into the guest pagetable, and update the shadows
- * appropriately.  Returns false if we page-faulted, true for success.
+ * appropriately.
  */
-bool
+void
 sh_write_guest_entry(struct vcpu *v, intpte_t *p, intpte_t new, mfn_t gmfn)
 {
-    unsigned int failed;
-
     paging_lock(v->domain);
-    failed = __copy_to_user(p, &new, sizeof(new));
-    if ( failed != sizeof(new) )
-        sh_validate_guest_entry(v, gmfn, p, sizeof(new));
+    write_atomic(p, new);
+    sh_validate_guest_entry(v, gmfn, p, sizeof(new));
     paging_unlock(v->domain);
-
-    return !failed;
 }
 
 /*
  * Cmpxchg a new value into the guest pagetable, and update the shadows
- * appropriately. Returns false if we page-faulted, true if not.
+ * appropriately.
  * N.B. caller should check the value of "old" to see if the cmpxchg itself
  * was successful.
  */
-bool
+void
 sh_cmpxchg_guest_entry(struct vcpu *v, intpte_t *p, intpte_t *old,
                        intpte_t new, mfn_t gmfn)
 {
-    bool failed;
-    intpte_t t = *old;
+    intpte_t t;
 
     paging_lock(v->domain);
-    failed = cmpxchg_user(p, t, new);
+    t = cmpxchg(p, *old, new);
     if ( t == *old )
         sh_validate_guest_entry(v, gmfn, p, sizeof(new));
     *old = t;
     paging_unlock(v->domain);
-
-    return !failed;
 }
 
 /*

diff --git a/xen/arch/x86/pv/mm.h b/xen/arch/x86/pv/mm.h
index a1bd473b2918530e906ceeaa166467509c6b7c0b..f537e58a8dc35cfe0ba22e99ccee0cd1c7adabd6 100644 (file)
--- a/xen/arch/x86/pv/mm.h
+++ b/xen/arch/x86/pv/mm.h
@@ -43,9 +43,7 @@ static inline bool update_intpte(intpte_t *p, intpte_t old, intpte_t new,
 
 #ifndef PTE_UPDATE_WITH_CMPXCHG
     if ( !preserve_ad )
-    {
-        rv = paging_write_guest_entry(v, p, new, mfn);
-    }
+        paging_write_guest_entry(v, p, new, mfn);
     else
 #endif
     {
@@ -58,14 +56,7 @@ static inline bool update_intpte(intpte_t *p, intpte_t old, intpte_t new,
             if ( preserve_ad )
                 _new |= old & (_PAGE_ACCESSED | _PAGE_DIRTY);
 
-            rv = paging_cmpxchg_guest_entry(v, p, &t, _new, mfn);
-            if ( unlikely(rv == 0) )
-            {
-                gdprintk(XENLOG_WARNING,
-                         "Failed to update %" PRIpte " -> %" PRIpte
-                         ": saw %" PRIpte "\n", old, _new, t);
-                break;
-            }
+            paging_cmpxchg_guest_entry(v, p, &t, _new, mfn);
 
             if ( t == old )
                 break;

diff --git a/xen/arch/x86/pv/ro-page-fault.c b/xen/arch/x86/pv/ro-page-fault.c
index 556ee7ca116d2043a23064caa67b0580be8b77a7..8468f9e9bbacf04ee75b9a7f993e10d7d60dc0d8 100644 (file)
--- a/xen/arch/x86/pv/ro-page-fault.c
+++ b/xen/arch/x86/pv/ro-page-fault.c
@@ -168,10 +168,9 @@ static int ptwr_emulated_update(unsigned long addr, intpte_t *p_old,
     if ( p_old )
     {
         ol1e = l1e_from_intpte(old);
-        if ( !paging_cmpxchg_guest_entry(v, &l1e_get_intpte(*pl1e),
-                                         &old, l1e_get_intpte(nl1e), mfn) )
-            ret = X86EMUL_UNHANDLEABLE;
-        else if ( l1e_get_intpte(ol1e) == old )
+        paging_cmpxchg_guest_entry(v, &l1e_get_intpte(*pl1e), &old,
+                                   l1e_get_intpte(nl1e), mfn);
+        if ( l1e_get_intpte(ol1e) == old )
             ret = X86EMUL_OKAY;
         else
         {

diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index 87ce135b8c72c5e565d8bab20765da056745281b..4a26f30c81313a2cc2949c4a11ec1e7501bfbfc3 100644 (file)
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -100,9 +100,9 @@ struct shadow_paging_mode {
 #ifdef CONFIG_SHADOW_PAGING
     void          (*detach_old_tables     )(struct vcpu *v);
 #ifdef CONFIG_PV
-    bool          (*write_guest_entry     )(struct vcpu *v, intpte_t *p,
+    void          (*write_guest_entry     )(struct vcpu *v, intpte_t *p,
                                             intpte_t new, mfn_t gmfn);
-    bool          (*cmpxchg_guest_entry   )(struct vcpu *v, intpte_t *p,
+    void          (*cmpxchg_guest_entry   )(struct vcpu *v, intpte_t *p,
                                             intpte_t *old, intpte_t new,
                                             mfn_t gmfn);
 #endif
@@ -333,15 +333,15 @@ static inline void paging_update_paging_modes(struct vcpu *v)
  * paging-assistance state appropriately.  Returns false if we page-faulted,
  * true for success.
  */
-static inline bool paging_write_guest_entry(
+static inline void paging_write_guest_entry(
     struct vcpu *v, intpte_t *p, intpte_t new, mfn_t gmfn)
 {
 #ifdef CONFIG_SHADOW_PAGING
     if ( unlikely(paging_mode_shadow(v->domain)) && paging_get_hostmode(v) )
-        return paging_get_hostmode(v)->shadow.write_guest_entry(v, p, new,
-                                                                gmfn);
+        paging_get_hostmode(v)->shadow.write_guest_entry(v, p, new, gmfn);
+    else
 #endif
-    return !__copy_to_user(p, &new, sizeof(new));
+        write_atomic(p, new);
 }
 
 
@@ -351,15 +351,16 @@ static inline bool paging_write_guest_entry(
  * true if not.  N.B. caller should check the value of "old" to see if the
  * cmpxchg itself was successful.
  */
-static inline bool paging_cmpxchg_guest_entry(
+static inline void paging_cmpxchg_guest_entry(
     struct vcpu *v, intpte_t *p, intpte_t *old, intpte_t new, mfn_t gmfn)
 {
 #ifdef CONFIG_SHADOW_PAGING
     if ( unlikely(paging_mode_shadow(v->domain)) && paging_get_hostmode(v) )
-        return paging_get_hostmode(v)->shadow.cmpxchg_guest_entry(v, p, old,
-                                                                  new, gmfn);
+        paging_get_hostmode(v)->shadow.cmpxchg_guest_entry(v, p, old,
+                                                           new, gmfn);
+    else
 #endif
-    return !cmpxchg_user(p, *old, new);
+        *old = cmpxchg(p, *old, new);
 }
 
 #endif /* CONFIG_PV */

diff --git a/xen/include/asm-x86/x86_64/system.h b/xen/include/asm-x86/x86_64/system.h
index f471859c19cc73d014aef2097534c1898693ce03..e94371cf2038a58e5c201bef1612ea3c96feae2a 100644 (file)
--- a/xen/include/asm-x86/x86_64/system.h
+++ b/xen/include/asm-x86/x86_64/system.h
@@ -59,47 +59,4 @@ static always_inline __uint128_t cmpxchg16b_local_(
     __cmpxchg16b(_p, (void *)(o), (void *)(n));            \
 })
 
-/*
- * This function causes value _o to be changed to _n at location _p.
- * If this access causes a fault then we return 1, otherwise we return 0.
- * If no fault occurs then _o is updated to the value we saw at _p. If this
- * is the same as the initial value of _o then _n is written to location _p.
- */
-#define __cmpxchg_user(_p, _o, _n, _oppre, _regtype)                    \
-    stac();                                                             \
-    asm volatile (                                                      \
-        "1: lock cmpxchg %"_oppre"[new], %[ptr]\n"                      \
-        "2:\n"                                                          \
-        ".section .fixup,\"ax\"\n"                                      \
-        "3:     movl $1, %[rc]\n"                                       \
-        "       jmp 2b\n"                                               \
-        ".previous\n"                                                   \
-        _ASM_EXTABLE(1b, 3b)                                            \
-        : "+a" (_o), [rc] "=r" (_rc),                                   \
-          [ptr] "+m" (*(volatile typeof(*(_p)) *)(_p))                  \
-        : [new] _regtype (_n), "[rc]" (0)                               \
-        : "memory");                                                    \
-    clac()
-
-#define cmpxchg_user(_p, _o, _n)                                        \
-({                                                                      \
-    int _rc;                                                            \
-    switch ( sizeof(*(_p)) )                                            \
-    {                                                                   \
-    case 1:                                                             \
-        __cmpxchg_user(_p, _o, _n, "b", "q");                           \
-        break;                                                          \
-    case 2:                                                             \
-        __cmpxchg_user(_p, _o, _n, "w", "r");                           \
-        break;                                                          \
-    case 4:                                                             \
-        __cmpxchg_user(_p, _o, _n, "k", "r");                           \
-        break;                                                          \
-    case 8:                                                             \
-        __cmpxchg_user(_p, _o, _n, "q", "r");                           \
-        break;                                                          \
-    }                                                                   \
-    _rc;                                                                \
-})
-
 #endif /* __X86_64_SYSTEM_H__ */