[Xen-devel] [PATCH v2 3/5] xen/domain: Stricter configuration checking

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

Currently, a number of options passed for domain creation are ignored, or have
implicit fallback behaviour.  This is bad for forwards compatibility, and for
end users to be certain that they got the configuration they asked for.

With this change:
 * ARM now strictly requires that XEN_DOMCTL_CDF_hap is passed.  Previously,
   only XEN_DOMCTL_CDF_hvm_guest was checked.
 * For x86, requesting HAP without HVM is now prohibited, as the combination
   makes no sense.
 * For x86, requesting HAP on a non-HAP capable system will fail, rather than
   silently fall back to Shadow.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>

Semi RFC because this may cause a user-visible change in behaviour.  However,
if the user has gone to the effort of specifying hap=1, silently falling back
to shadow is unexpected, and IMO, a bug.

Alternatively, if this proves to be controversial, it can be dropped from the
series to avoid blocking the main bugfix.

v2:
 * New
---
 xen/arch/arm/domain.c |  7 +++++++
 xen/arch/x86/domain.c | 40 ++++++++++++++++++++++++++++++++++++++++
 xen/common/domain.c   | 34 +++-------------------------------
 3 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index c24ace6..08ba412 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -601,6 +601,13 @@ void vcpu_switch_to_aarch64_mode(struct vcpu *v)
 
 int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
 {
+    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) ||
+         !(config->flags & XEN_DOMCTL_CDF_hap) )
+    {
+        dprintk(XENLOG_INFO, "Unsupported configuration %#x\n", config->flags);
+        return -EINVAL;
+    }
+
     /* Fill in the native GIC version, passed back to the toolstack. */
     if ( config->arch.gic_version == XEN_DOMCTL_CONFIG_GIC_NATIVE )
     {
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 28a145a..f47ad04 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -420,6 +420,46 @@ void arch_vcpu_destroy(struct vcpu *v)
 
 int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
 {
+    bool hvm;
+
+    if ( !IS_ENABLED(CONFIG_PV) && !(config->flags & XEN_DOMCTL_CDF_hvm_guest) 
)
+    {
+        dprintk(XENLOG_INFO, "PV support not available\n");
+        return -EINVAL;
+    }
+
+    if ( !hvm_enabled && (config->flags & XEN_DOMCTL_CDF_hvm_guest) )
+    {
+        dprintk(XENLOG_INFO, "HVM support not available\n");
+        return -EINVAL;
+    }
+
+    hvm = config->flags & XEN_DOMCTL_CDF_hvm_guest;
+
+    if ( !hvm )
+    {
+        if ( config->flags & XEN_DOMCTL_CDF_hap )
+        {
+            dprintk(XENLOG_INFO, "HAP inapplicable for PV guests\n");
+            return -EINVAL;
+        }
+    }
+    else
+    {
+        if ( !IS_ENABLED(CONFIG_SHADOW_PAGING) &&
+             !(config->flags & XEN_DOMCTL_CDF_hap) )
+        {
+            dprintk(XENLOG_INFO, "SHADOW support not available\n");
+            return -EINVAL;
+        }
+
+        if ( !hvm_hap_supported() && (config->flags & XEN_DOMCTL_CDF_hap) )
+        {
+            dprintk(XENLOG_INFO, "HAP support not available\n");
+            return -EINVAL;
+        }
+    }
+
     return 0;
 }
 
diff --git a/xen/common/domain.c b/xen/common/domain.c
index ddaf74a..f69f405 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -339,37 +339,9 @@ struct domain *domain_create(domid_t domid,
         hardware_domain = d;
     }
 
-    /* Sort out our idea of is_{pv,hvm}_domain(). */
-    if ( config )
-    {
-        if ( config->flags & XEN_DOMCTL_CDF_hvm_guest )
-        {
-#ifdef CONFIG_HVM
-            d->guest_type = guest_type_hvm;
-#else
-            err = -EINVAL;
-            goto fail;
-#endif
-        }
-        else
-        {
-#ifdef CONFIG_PV
-            d->guest_type = guest_type_pv;
-#else
-            err = -EINVAL;
-            goto fail;
-#endif
-        }
-    }
-    else
-    {
-        /*
-         * At least the idle domain should be treated as PV domain
-         * because it uses PV context switch functions. To err on the
-         * safe side, leave all system domains to be guest_type_pv.
-         */
-        d->guest_type = guest_type_pv;
-    }
+    /* Sort out our idea of is_{pv,hvm}_domain().  All system domains are PV. 
*/
+    d->guest_type = ((config && (config->flags & XEN_DOMCTL_CDF_hvm_guest))
+                     ? guest_type_hvm : guest_type_pv);
 
     TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
 
-- 
2.1.4


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