* [Xen-devel] [PATCH 3/3] tools/ocaml: Introduce xenctrl ABI build-time checks
2019-09-09 17:12 [Xen-devel] [PATCH 1/3] tools/ocaml: Add missing X86_EMU_VPCI Ian Jackson
2019-09-09 17:12 ` [Xen-devel] [PATCH 2/3] tools/ocaml: Add missing CAP_PV Ian Jackson
@ 2019-09-09 17:12 ` Ian Jackson
2019-09-10 8:29 ` Christian Lindig
1 sibling, 1 reply; 5+ messages in thread
From: Ian Jackson @ 2019-09-09 17:12 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Ian Jackson, Julien Grall,
Andrew Cooper, Christian Lindig, Jan Beulich, David Scott
c/s f089fddd941 broke the Ocaml ABI by renumering
XEN_SYSCTL_PHYSCAP_directio without adjusting the Ocaml
physinfo_cap_flag enumeration.
Add build machinery which will check the ABI correspondence.
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@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: Script with ad-hoc parsers rather than handwritten BUILD_BUG_ON
list (which was out of date even in v1 of this patch!)
---
.gitignore | 1 +
tools/ocaml/libs/xc/Makefile | 5 +++
tools/ocaml/libs/xc/abi-check | 84 +++++++++++++++++++++++++++++++++++++
tools/ocaml/libs/xc/xenctrl_stubs.c | 69 +++++++++++++++++++++---------
xen/include/public/sysctl.h | 4 ++
5 files changed, 143 insertions(+), 20 deletions(-)
create mode 100755 tools/ocaml/libs/xc/abi-check
diff --git a/.gitignore b/.gitignore
index 3c947ac948..3ada0c4f0b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -392,6 +392,7 @@ tools/ocaml/libs/xentoollog/_xtl_levels.*
tools/ocaml/libs/xentoollog/xentoollog.ml
tools/ocaml/libs/xentoollog/xentoollog.mli
tools/ocaml/libs/xs/paths.ml
+tools/ocaml/libs/xc/xenctrl_abi_check.h
tools/ocaml/xenstored/_paths.h
tools/ocaml/xenstored/oxenstored
tools/ocaml/xenstored/oxenstored.conf
diff --git a/tools/ocaml/libs/xc/Makefile b/tools/ocaml/libs/xc/Makefile
index d24b0144d0..ac780627d2 100644
--- a/tools/ocaml/libs/xc/Makefile
+++ b/tools/ocaml/libs/xc/Makefile
@@ -31,4 +31,9 @@ install: $(LIBS) META
uninstall:
$(OCAMLFIND) remove -destdir $(OCAMLDESTDIR) xenctrl
+xenctrl_stubs.o: xenctrl_abi_check.h
+
+xenctrl_abi_check.h: abi-check xenctrl_stubs.c xenctrl.ml
+ $(PERL) -w $^ >$@.tmp && mv -f $@.tmp $@
+
include $(TOPLEVEL)/Makefile.rules
diff --git a/tools/ocaml/libs/xc/abi-check b/tools/ocaml/libs/xc/abi-check
new file mode 100755
index 0000000000..c987cd8454
--- /dev/null
+++ b/tools/ocaml/libs/xc/abi-check
@@ -0,0 +1,84 @@
+#!/usr/bin/perl -w
+
+use strict;
+use Data::Dumper;
+
+our %enums;
+
+@ARGV == 2 or die;
+our ($c, $o) = @ARGV;
+
+open STDIN, "<", $c or die $!;
+
+our $cline = -1;
+our $ei;
+
+while (<>) {
+ if ($cline == -1) {
+ if (m/c_bitmap_to_ocaml_list/) {
+ $cline = 0;
+ $ei = { };
+ }
+ } else {
+ $cline++;
+ m{^\s+/\* \s+ ! \s+ (.*?) \s* \*/\s*$}x or die "$cline $_ ?";
+ my @vals = split /\s+/, $1;
+ if ($cline == 1 && !@vals) {
+ $cline = -1;
+ } elsif ($cline == 1 && @vals == 3) {
+ $ei->{$_} = shift @vals foreach qw(OType OPrefix Mangle);
+ } elsif ($cline == 2 && @vals == 3) {
+ $ei->{$_} = shift @vals foreach qw(CPrefix CFinal CFinalHow);
+ die if $enums{ $ei->{OType} };
+ $enums{ $ei->{OType} } = $ei;
+ $cline = -1;
+ } else {
+ die "$_ ?";
+ }
+ }
+}
+
+sub expect ($$) {
+ printf "BUILD_BUG_ON( %-30s != %-10s );\n", @_ or die $!;
+}
+
+open STDIN, "<", $o or die $!;
+my $cval;
+$ei = undef;
+my $bitnum = 0;
+while (<>) {
+ if (!$ei) {
+ if (m{^type \s+ (\w+) \s* \= \s* $/}x && $enums{$1}) {
+ $ei = $enums{$1};
+ $cval = '';
+ $bitnum = 0;
+ }
+ } else {
+ if (m{^\s+ \| \s* $ei->{OPrefix} (\w+) \s*$}x) {
+ $cval = $1;
+ if ($ei->{Mangle} eq 'lc') {
+ $cval = lc $cval;
+ } elsif ($ei->{Mangle} eq 'none') {
+ } else {
+ die;
+ }
+ $cval = $ei->{CPrefix}.$cval;
+ expect($cval, "(1u << $bitnum)");
+ $bitnum++;
+ } elsif (m/^\w|\{/) {
+ if ($ei->{CFinalHow} eq 'max') {
+ expect($ei->{CFinal}, "(1u << ".($bitnum-1).")");
+ } elsif ($ei->{CFinalHow} eq 'all') {
+ expect($ei->{CFinal}, "(1u << $bitnum)-1u");
+ } else {
+ die Dumper($ei)." ?";
+ }
+ $ei = undef;
+ } elsif (!m{\S}) {
+ } else {
+ die "$_ ?";
+ }
+ }
+}
+
+close STDOUT or die $!;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 2e1b29ce33..352a6bd2d6 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,39 @@ 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
+ /* ! */
+ /*
+ * All calls to this function must be in a form suitable
+ * for xenctrl_abi_check. The parsing there is ad-hoc.
+ */
+ (unsigned int bitmap)
+{
+ CAMLparam0();
+ CAMLlocal2(list, tmp);
+
+#include "xenctrl_abi_check.h"
+
+ 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 +349,13 @@ 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;
+ */
+ emul_list = c_bitmap_to_ocaml_list
+ /* ! x86_arch_emulation_flags X86_EMU_ none */
+ /* ! XEN_X86_EMU_ XEN_X86_EMU_ALL all */
+ (info->arch_config.emulation_flags);
/* xen_x86_arch_domainconfig */
x86_arch_config = caml_alloc_tuple(1);
@@ -635,7 +666,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 +677,13 @@ 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;
+ */
+ cap_list = c_bitmap_to_ocaml_list
+ /* ! physinfo_cap_flag CAP_ lc */
+ /* ! XEN_SYSCTL_PHYSCAP_ XEN_SYSCTL_PHYSCAP_MAX max */
+ (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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread