[Xen-devel] [PATCH 3/5] x86/paging: Enforce PG_external == PG_translate == PG_refcounts

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

Setting PG_refcounts but not PG_translate is not useful.

While adjusting this, make a few other improvements.

 * Have paging_enable() unilaterally reject any unknown modes.
 * Correct the function description.  paging_enable() is also used to enable
   logdirty during runtime.
 * Drop the or'ing of PG_{HAP,SH}_enable.  The underlying functions already do
   this.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
---
 xen/arch/x86/mm/paging.c        | 19 +++++++++++++------
 xen/arch/x86/mm/shadow/common.c |  3 +--
 xen/include/asm-x86/paging.h    |  9 +++++++--
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index 853a035..9548222 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -842,23 +842,30 @@ void paging_final_teardown(struct domain *d)
     p2m_final_teardown(d);
 }
 
-/* Enable an arbitrary paging-assistance mode.  Call once at domain
- * creation. */
+/*
+ * Enable an arbitrary paging-assistance mode.  Call once at domain
+ * creation, and during runtime for logdirty mode.
+ */
 int paging_enable(struct domain *d, u32 mode)
 {
-    switch ( mode & (PG_external | PG_translate) )
+    /* Unrecognised paging mode? */
+    if ( mode & ~PG_MASK )
+        return -EINVAL;
+
+    /* All of external|translate|refcounts, or none. */
+    switch ( mode & (PG_external | PG_translate | PG_refcounts) )
     {
     case 0:
-    case PG_external | PG_translate:
+    case PG_external | PG_translate | PG_refcounts:
         break;
     default:
         return -EINVAL;
     }
 
     if ( hap_enabled(d) )
-        return hap_enable(d, mode | PG_HAP_enable);
+        return hap_enable(d, mode);
     else
-        return shadow_enable(d, mode | PG_SH_enable);
+        return shadow_enable(d, mode);
 }
 
 /* Called from the guest to indicate that a process is being torn down
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 39be564..84a87f3 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -3144,8 +3144,7 @@ int shadow_enable(struct domain *d, u32 mode)
     domain_pause(d);
 
     /* Sanity check the arguments */
-    if ( shadow_mode_enabled(d) ||
-         ((mode & PG_translate) && !(mode & PG_refcounts)) )
+    if ( shadow_mode_enabled(d) )
     {
         rv = -EINVAL;
         goto out_unlocked;
diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index f83ed8b..1535659 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -57,6 +57,9 @@
  * requires VT or similar mechanisms */
 #define PG_external    (XEN_DOMCTL_SHADOW_ENABLE_EXTERNAL << PG_mode_shift)
 
+/* All paging modes. */
+#define PG_MASK (PG_refcounts | PG_log_dirty | PG_translate | PG_external)
+
 #define paging_mode_enabled(_d)   (!!(_d)->arch.paging.mode)
 #define paging_mode_shadow(_d)    (!!((_d)->arch.paging.mode & PG_SH_enable))
 #define paging_mode_hap(_d)       (!!((_d)->arch.paging.mode & PG_HAP_enable))
@@ -212,8 +215,10 @@ int paging_teardown(struct domain *d);
 /* Call once all of the references to the domain have gone away */
 void paging_final_teardown(struct domain *d);
 
-/* Enable an arbitrary paging-assistance mode.  Call once at domain
- * creation. */
+/*
+ * Enable an arbitrary paging-assistance mode.  Call once at domain
+ * creation, and during runtime for logdirty mode.
+ */
 int paging_enable(struct domain *d, u32 mode);
 
 #define paging_get_hostmode(v)         ((v)->arch.paging.mode)
-- 
2.1.4


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