Re: [Xen-devel] [PATCH v4 09/31] x86/mm: rename and move update_intpte

[Jan Beulich]

>>> On 17.08.17 at 16:44,  wrote:
> That function is only used by PV guests supporting code, add pv_
> prefix.

Is any code outside of x86/pv/ going to need access to it? I hope not,
in which case it shouldn't be exposed via include/asm-x86/pv/mm.h,
but via a private header in x86/pv/. That non-private header should
have only declarations of things needed by non-PV/HVM-specific code.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 09/31] x86/mm: rename and move update_intpte

[Wei Liu]

That function is only used by PV guests supporting code, add pv_
prefix.

Export it via pv/mm.h. Move UPDATE_ENTRY as well.

Signed-off-by: Wei Liu 
---
Now it is no longer an inline function, but I don't think that matters
much.
---
 xen/arch/x86/mm.c   | 65 -
 xen/arch/x86/pv/mm.c| 54 +
 xen/include/asm-x86/pv/mm.h | 16 +++
 3 files changed, 70 insertions(+), 65 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index d25d314673..dc4ac5592a 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -133,14 +133,6 @@
 l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE)
 l1_fixmap[L1_PAGETABLE_ENTRIES];
 
-/*
- * PTE updates can be done with ordinary writes except:
- *  1. Debug builds get extra checking by using CMPXCHG[8B].
- */
-#if !defined(NDEBUG)
-#define PTE_UPDATE_WITH_CMPXCHG
-#endif
-
 paddr_t __read_mostly mem_hotplug;
 
 /* Private domain structs for DOMID_XEN and DOMID_IO. */
@@ -1812,63 +1804,6 @@ void page_unlock(struct page_info *page)
 } while ( (y = cmpxchg(>u.inuse.type_info, x, nx)) != x );
 }
 
-/*
- * How to write an entry to the guest pagetables.
- * Returns false for failure (pointer not valid), true for success.
- */
-static inline bool update_intpte(
-intpte_t *p, intpte_t old, intpte_t new, unsigned long mfn,
-struct vcpu *v, int preserve_ad)
-{
-bool rv = true;
-
-#ifndef PTE_UPDATE_WITH_CMPXCHG
-if ( !preserve_ad )
-{
-rv = paging_write_guest_entry(v, p, new, _mfn(mfn));
-}
-else
-#endif
-{
-intpte_t t = old;
-
-for ( ; ; )
-{
-intpte_t _new = new;
-
-if ( preserve_ad )
-_new |= old & (_PAGE_ACCESSED | _PAGE_DIRTY);
-
-rv = paging_cmpxchg_guest_entry(v, p, , _new, _mfn(mfn));
-if ( unlikely(rv == 0) )
-{
-gdprintk(XENLOG_WARNING,
- "Failed to update %" PRIpte " -> %" PRIpte
- ": saw %" PRIpte "\n", old, _new, t);
-break;
-}
-
-if ( t == old )
-break;
-
-/* Allowed to change in Accessed/Dirty flags only. */
-BUG_ON((t ^ old) & ~(intpte_t)(_PAGE_ACCESSED|_PAGE_DIRTY));
-
-old = t;
-}
-}
-return rv;
-}
-
-/*
- * Macro that wraps the appropriate type-changes around update_intpte().
- * Arguments are: type, ptr, old, new, mfn, vcpu
- */
-#define UPDATE_ENTRY(_t,_p,_o,_n,_m,_v,_ad) \
-update_intpte(&_t ## e_get_intpte(*(_p)),   \
-  _t ## e_get_intpte(_o), _t ## e_get_intpte(_n),   \
-  (_m), (_v), (_ad))
-
 /*
  * PTE flags that a guest may change without re-validating the PTE.
  * All other bits affect translation, caching, or Xen's safety.
diff --git a/xen/arch/x86/pv/mm.c b/xen/arch/x86/pv/mm.c
index aa2ce34145..2cb5995e62 100644
--- a/xen/arch/x86/pv/mm.c
+++ b/xen/arch/x86/pv/mm.c
@@ -24,6 +24,13 @@
 
 #include 
 
+/*
+ * PTE updates can be done with ordinary writes except:
+ *  1. Debug builds get extra checking by using CMPXCHG[8B].
+ */
+#if !defined(NDEBUG)
+#define PTE_UPDATE_WITH_CMPXCHG
+#endif
 
 /* Read a PV guest's l1e that maps this virtual address. */
 void pv_get_guest_eff_l1e(unsigned long addr, l1_pgentry_t *eff_l1e)
@@ -56,6 +63,53 @@ void pv_get_guest_eff_kern_l1e(struct vcpu *v, unsigned long 
addr,
 toggle_guest_mode(v);
 }
 
+/*
+ * How to write an entry to the guest pagetables.
+ * Returns false for failure (pointer not valid), true for success.
+ */
+bool pv_update_intpte(intpte_t *p, intpte_t old, intpte_t new,
+  unsigned long mfn, struct vcpu *v, int preserve_ad)
+{
+bool rv = true;
+
+#ifndef PTE_UPDATE_WITH_CMPXCHG
+if ( !preserve_ad )
+{
+rv = paging_write_guest_entry(v, p, new, _mfn(mfn));
+}
+else
+#endif
+{
+intpte_t t = old;
+
+for ( ; ; )
+{
+intpte_t _new = new;
+
+if ( preserve_ad )
+_new |= old & (_PAGE_ACCESSED | _PAGE_DIRTY);
+
+rv = paging_cmpxchg_guest_entry(v, p, , _new, _mfn(mfn));
+if ( unlikely(rv == 0) )
+{
+gdprintk(XENLOG_WARNING,
+ "Failed to update %" PRIpte " -> %" PRIpte
+ ": saw %" PRIpte "\n", old, _new, t);
+break;
+}
+
+if ( t == old )
+break;
+
+/* Allowed to change in Accessed/Dirty flags only. */
+BUG_ON((t ^ old) & ~(intpte_t)(_PAGE_ACCESSED|_PAGE_DIRTY));
+
+old = t;
+}
+}
+return rv;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-x86/pv/mm.h b/xen/include/asm-x86/pv/mm.h
index 19dbc3b66c..72c04c684f 100644
---