[Xen-devel] [RFC PATCH 05/16] x86/xen: add feature support in xenhost_t

[Ankur Arora <ankur.a.arora@xxxxxxxxxx>]

With nested xenhosts, both the xenhosts could have different supported
xen_features. Add support for probing both.

In addition, validate that features are compatible across xenhosts.

For runtime feature checking, the code uses xen_feature() with the
default xenhost. This should be good enough because we do feature
validation early which guarantees that the features of interest are
compatible. Features not of interest, are related to MMU, clock, pirq, etc where
the interface to L0-Xen should not matter.

Signed-off-by: Ankur Arora <ankur.a.arora@xxxxxxxxxx>
---
 arch/x86/xen/enlighten_hvm.c | 15 +++++++++++----
 arch/x86/xen/enlighten_pv.c  | 14 ++++++++++----
 drivers/xen/features.c       | 33 +++++++++++++++++++++++++++------
 include/xen/features.h       | 17 ++++++++++++++---
 include/xen/xenhost.h        | 10 ++++++++++
 5 files changed, 72 insertions(+), 17 deletions(-)

diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index f84941d6944e..a118b61a1a8a 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -119,17 +119,24 @@ static void __init init_hvm_pv_info(void)
 
        xen_domain_type = XEN_HVM_DOMAIN;
 
-       /* PVH set up hypercall page in xen_prepare_pvh(). */
        if (xen_pvh_domain())
                pv_info.name = "Xen PVH";
-       else {
+       else
                pv_info.name = "Xen HVM";
 
-               for_each_xenhost(xh)
+       for_each_xenhost(xh) {
+               /* PVH set up hypercall page in xen_prepare_pvh(). */
+               if (!xen_pvh_domain())
                        xenhost_setup_hypercall_page(*xh);
+               xen_setup_features(*xh);
        }
 
-       xen_setup_features();
+       /*
+        * Check if features are compatible across L1-Xen and L0-Xen;
+        * If not, get rid of xenhost_r2.
+        */
+       if (xen_validate_features() == false)
+               __xenhost_unregister(xenhost_r2);
 
        cpuid(base + 4, &eax, &ebx, &ecx, &edx);
        if (eax & XEN_HVM_CPUID_VCPU_ID_PRESENT)
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index a2c07cc71498..484968ff16a4 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1236,13 +1236,19 @@ asmlinkage __visible void __init xen_start_kernel(void)
        if (xen_driver_domain() && xen_nested())
                xenhost_register(xenhost_r2, &xh_pv_nested_ops);
 
-       for_each_xenhost(xh)
-               xenhost_setup_hypercall_page(*xh);
-
        xen_domain_type = XEN_PV_DOMAIN;
        xen_start_flags = xen_start_info->flags;
 
-       xen_setup_features();
+       for_each_xenhost(xh) {
+               xenhost_setup_hypercall_page(*xh);
+               xen_setup_features(*xh);
+       }
+       /*
+        * Check if features are compatible across L1-Xen and L0-Xen;
+        * If not, get rid of xenhost_r2.
+        */
+       if (xen_validate_features() == false)
+               __xenhost_unregister(xenhost_r2);
 
        /* Install Xen paravirt ops */
        pv_info = xen_info;
diff --git a/drivers/xen/features.c b/drivers/xen/features.c
index d7d34fdfc993..b4fba808ebae 100644
--- a/drivers/xen/features.c
+++ b/drivers/xen/features.c
@@ -15,19 +15,40 @@
 #include <xen/interface/version.h>
 #include <xen/features.h>
 
-u8 xen_features[XENFEAT_NR_SUBMAPS * 32] __read_mostly;
-EXPORT_SYMBOL_GPL(xen_features);
-
-void xen_setup_features(void)
+void xen_setup_features(xenhost_t *xh)
 {
        struct xen_feature_info fi;
        int i, j;
 
        for (i = 0; i < XENFEAT_NR_SUBMAPS; i++) {
                fi.submap_idx = i;
-               if (HYPERVISOR_xen_version(XENVER_get_features, &fi) < 0)
+               if (hypervisor_xen_version(xh, XENVER_get_features, &fi) < 0)
                        break;
                for (j = 0; j < 32; j++)
-                       xen_features[i * 32 + j] = !!(fi.submap & 1<<j);
+                       xh->features[i * 32 + j] = !!(fi.submap & 1<<j);
        }
 }
+
+bool xen_validate_features(void)
+{
+       int fail = 0;
+
+       if (xh_default && xh_remote) {
+               /*
+                * Check xh_default->features and xh_remote->features for
+                * compatibility. Relevant features should be compatible
+                * or we are asking for trouble.
+                */
+               fail += __xen_feature(xh_default, 
XENFEAT_auto_translated_physmap) !=
+                       __xen_feature(xh_remote, 
XENFEAT_auto_translated_physmap);
+
+               /* We would like callbacks via hvm_callback_vector. */
+               fail += __xen_feature(xh_default, XENFEAT_hvm_callback_vector) 
== 0;
+               fail += __xen_feature(xh_remote, XENFEAT_hvm_callback_vector) 
== 0;
+
+               if (fail)
+                       return false;
+       }
+
+       return fail ? false : true;
+}
diff --git a/include/xen/features.h b/include/xen/features.h
index e4cb464386a9..63e6735ed6a3 100644
--- a/include/xen/features.h
+++ b/include/xen/features.h
@@ -11,14 +11,25 @@
 #define __XEN_FEATURES_H__
 
 #include <xen/interface/features.h>
+#include <xen/xenhost.h>
 
-void xen_setup_features(void);
+void xen_setup_features(xenhost_t *xh);
 
-extern u8 xen_features[XENFEAT_NR_SUBMAPS * 32];
+bool xen_validate_features(void);
 
+static inline int __xen_feature(xenhost_t *xh, int flag)
+{
+       return xh->features[flag];
+}
+
+/*
+ * We've validated the features that need to be common for both xenhost_r1 and
+ * xenhost_r2 (XENFEAT_hvm_callback_vector, XENFEAT_auto_translated_physmap.)
+ * Most of the other features should be only needed for the default xenhost.
+ */
 static inline int xen_feature(int flag)
 {
-       return xen_features[flag];
+       return __xen_feature(xh_default, flag);
 }
 
 #endif /* __ASM_XEN_FEATURES_H__ */
diff --git a/include/xen/xenhost.h b/include/xen/xenhost.h
index d9bc1fb6cce4..dd1e2b64f50d 100644
--- a/include/xen/xenhost.h
+++ b/include/xen/xenhost.h
@@ -4,6 +4,7 @@
 #include <xen/interface/features.h>
 #include <xen/interface/xen.h>
 #include <asm/xen/hypervisor.h>
+
 /*
  * Xenhost abstracts out the Xen interface. It co-exists with the PV/HVM/PVH
  * abstractions (x86_init, hypervisor_x86, pv_ops etc) and is meant to
@@ -72,6 +73,15 @@ typedef struct {
        struct xenhost_ops *ops;
 
        struct hypercall_entry *hypercall_page;
+
+       /*
+        * Not clear if we need to draw features from two different
+        * hypervisors. There is one feature that seems might be necessary:
+        * XENFEAT_hvm_callback_vector.
+        * Ensuring support in both L1-Xen and L0-Xen means that L0-Xen can
+        * bounce callbacks via L1-Xen.
+        */
+       u8 features[XENFEAT_NR_SUBMAPS * 32];
 } xenhost_t;
 
 typedef struct xenhost_ops {
-- 
2.20.1


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