[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH v2] tools/ocaml: Fix xenctrl ABI and introduce build-time checks



c/s f089fddd941 broke the Ocaml ABI by renumering XEN_SYSCTL_PHYSCAP_directio
without adjusting the Ocaml physinfo_cap_flag enumeration.  Fix this by
inserting CAP_PV between CAP_HVM and CAP_DirectIO.

Factor out the bitmap-to-list conversion logic into a helper, to avoid an
opencoded truncation of the bitmap.  To cover this, add BUILD_BUG_ON()'s at
the caller for each constant in the C-to-Ocaml conversion, and for the the
applicable max/all constant.

This will result in a compile time failure whenever constants get
renumbered/added without a compatible adjustment to the Ocaml ABI.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Christian Lindig <christian.lindig@xxxxxxxxxx>
CC: Rob Hoes <Rob.Hoes@xxxxxxxxxx>
CC: Edwin Török <edvin.torok@xxxxxxxxxx>

This needs backporting to Xen 4.12

v2:
 * Base this patch correctly.
---
 tools/ocaml/libs/xc/xenctrl.ml      |  1 +
 tools/ocaml/libs/xc/xenctrl.mli     |  5 ++-
 tools/ocaml/libs/xc/xenctrl_stubs.c | 77 +++++++++++++++++++++++++++----------
 xen/include/public/sysctl.h         |  4 ++
 4 files changed, 66 insertions(+), 21 deletions(-)

diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 35958b94d5..cd7e95a6fa 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -99,6 +99,7 @@ type sched_control =
 
 type physinfo_cap_flag =
        | CAP_HVM
+       | CAP_PV
        | CAP_DirectIO
 
 type physinfo =
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 6c4268d453..0bd06ed920 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -82,7 +82,10 @@ type domaininfo = {
   arch_config : arch_domainconfig;
 }
 type sched_control = { weight : int; cap : int; }
-type physinfo_cap_flag = CAP_HVM | CAP_DirectIO
+type physinfo_cap_flag =
+  | CAP_HVM
+  | CAP_PV
+  | CAP_DirectIO
 type physinfo = {
   threads_per_core : int;
   cores_per_socket : int;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c 
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 2e1b29ce33..ce8dbba437 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -32,6 +32,7 @@
 
 #define XC_WANT_COMPAT_MAP_FOREIGN_API
 #include <xenctrl.h>
+#include <xen-tools/libs.h>
 
 #include "mmap_stubs.h"
 
@@ -119,6 +120,31 @@ static void 
domain_handle_of_uuid_string(xen_domain_handle_t h,
 #undef X
 }
 
+/*
+ * Various fields which are a bitmap in the C ABI are converted to lists of
+ * integers in the Ocaml ABI for more idiomatic handling.
+ */
+static value c_bitmap_to_ocaml_list(unsigned int bitmap)
+{
+       CAMLparam0();
+       CAMLlocal2(list, tmp);
+
+       list = tmp = Val_emptylist;
+
+       for ( unsigned int i = 0; bitmap; i++, bitmap >>= 1 )
+       {
+               if ( !(bitmap & 1) )
+                       continue;
+
+               tmp = caml_alloc_small(2, Tag_cons);
+               Field(tmp, 0) = Val_int(i);
+               Field(tmp, 1) = list;
+               list = tmp;
+       }
+
+       CAMLreturn(list);
+}
+
 CAMLprim value stub_xc_domain_create(value xch, value config)
 {
        CAMLparam2(xch, config);
@@ -315,16 +341,25 @@ static value alloc_domaininfo(xc_domaininfo_t * info)
        Store_field(result, 15, tmp);
 
 #if defined(__i386__) || defined(__x86_64__)
-       /* emulation_flags: x86_arch_emulation_flags list; */
-       tmp = emul_list = Val_emptylist;
-       for (i = 0; i < 10; i++) {
-               if ((info->arch_config.emulation_flags >> i) & 1) {
-                       tmp = caml_alloc_small(2, Tag_cons);
-                       Field(tmp, 0) = Val_int(i);
-                       Field(tmp, 1) = emul_list;
-                       emul_list = tmp;
-               }
-       }
+       /*
+        * emulation_flags: x86_arch_emulation_flags list;
+        *
+        * These BUILD_BUG_ON()'s map the C ABI to the Ocaml ABI.  If they
+        * trip, xenctrl.ml{,i} need updating to match.
+        */
+       BUILD_BUG_ON(XEN_X86_EMU_LAPIC    != (1u <<  0));
+       BUILD_BUG_ON(XEN_X86_EMU_HPET     != (1u <<  1));
+       BUILD_BUG_ON(XEN_X86_EMU_PM       != (1u <<  2));
+       BUILD_BUG_ON(XEN_X86_EMU_RTC      != (1u <<  3));
+       BUILD_BUG_ON(XEN_X86_EMU_IOAPIC   != (1u <<  4));
+       BUILD_BUG_ON(XEN_X86_EMU_PIC      != (1u <<  5));
+       BUILD_BUG_ON(XEN_X86_EMU_VGA      != (1u <<  6));
+       BUILD_BUG_ON(XEN_X86_EMU_IOMMU    != (1u <<  7));
+       BUILD_BUG_ON(XEN_X86_EMU_PIT      != (1u <<  8));
+       BUILD_BUG_ON(XEN_X86_EMU_USE_PIRQ != (1u <<  9));
+       BUILD_BUG_ON(XEN_X86_EMU_VPCI     != (1u << 10));
+       BUILD_BUG_ON(XEN_X86_EMU_ALL      != 0x7ff);
+       emul_list = c_bitmap_to_ocaml_list(info->arch_config.emulation_flags);
 
        /* xen_x86_arch_domainconfig */
        x86_arch_config = caml_alloc_tuple(1);
@@ -635,7 +670,7 @@ CAMLprim value stub_xc_send_debug_keys(value xch, value 
keys)
 CAMLprim value stub_xc_physinfo(value xch)
 {
        CAMLparam1(xch);
-       CAMLlocal3(physinfo, cap_list, tmp);
+       CAMLlocal2(physinfo, cap_list);
        xc_physinfo_t c_physinfo;
        int r;
 
@@ -646,15 +681,17 @@ CAMLprim value stub_xc_physinfo(value xch)
        if (r)
                failwith_xc(_H(xch));
 
-       tmp = cap_list = Val_emptylist;
-       for (r = 0; r < 2; r++) {
-               if ((c_physinfo.capabilities >> r) & 1) {
-                       tmp = caml_alloc_small(2, Tag_cons);
-                       Field(tmp, 0) = Val_int(r);
-                       Field(tmp, 1) = cap_list;
-                       cap_list = tmp;
-               }
-       }
+       /*
+        * capabilities: physinfo_cap_flag list;
+        *
+        * These BUILD_BUG_ON()'s map the C ABI to the Ocaml ABI.  If they
+        * trip, xenctrl.ml{,i} need updating to match.
+        */
+       BUILD_BUG_ON(XEN_SYSCTL_PHYSCAP_hvm      != (1u <<  0));
+       BUILD_BUG_ON(XEN_SYSCTL_PHYSCAP_pv       != (1u <<  1));
+       BUILD_BUG_ON(XEN_SYSCTL_PHYSCAP_directio != (1u <<  2));
+       BUILD_BUG_ON(XEN_SYSCTL_PHYSCAP_MAX      != 
XEN_SYSCTL_PHYSCAP_directio);
+       cap_list = c_bitmap_to_ocaml_list(c_physinfo.capabilities);
 
        physinfo = caml_alloc_tuple(10);
        Store_field(physinfo, 0, Val_int(c_physinfo.threads_per_core));
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 36b3f8c429..5401f9c2fe 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -90,6 +90,10 @@ struct xen_sysctl_tbuf_op {
  /* The platform supports direct access to I/O devices with IOMMU. */
 #define _XEN_SYSCTL_PHYSCAP_directio     2
 #define XEN_SYSCTL_PHYSCAP_directio  (1u<<_XEN_SYSCTL_PHYSCAP_directio)
+
+/* Max XEN_SYSCTL_PHYSCAP_* constant.  Used for ABI checking. */
+#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_directio
+
 struct xen_sysctl_physinfo {
     uint32_t threads_per_core;
     uint32_t cores_per_socket;
-- 
2.11.0


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.