All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range
@ 2020-08-27 17:35 Edwin Török
  2020-08-27 17:35 ` [PATCH v1 1/9] tools/ocaml: use common macros for manipulating mmap_interface Edwin Török
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Edwin Török @ 2020-08-27 17:35 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Christian Lindig, David Scott,
	Ian Jackson, Wei Liu, Andrew Cooper,
	Signed-off-by : Juergen Gross

oxenstored currently depends on 2 unstable interfaces from libxenctrl:
* Xenctrl.map_foreign_range
* Xenctrl.domain_getinfo

It is desirable to reduce the use of unstable APIs in xenstored, so that
an update to the hypervisor doesn't break xenstored.

The C version of xenstored has dropped the usage of map_foreign_range in:
38eeb3864de40aa568c48f9f26271c141c62b50b tools/xenstored: Drop mapping of the ring via foreign map
This also made the MFN in oxenstored redundant, which was dropped in:
122b52230aa5b79d65e18b8b77094027faa2f8e2 tools/xenstore: don't store domU's mfn of ring page in xenstored

This series ports those commits and dependencies to oxenstored.

First of all oxenstored currently doesn't have bindings to xengnttab.
There are upstream bindings available at https://github.com/mirage/ocaml-gnt.
A reduced form of that is imported into oxenstored that removes external dependencies
such as Lwt and Io_page.

This also requires changes to xenmmap interface to make it safer: there are now 2 ways to unmap a
Xenmmap.mmap_interface, so we need to use the type system to ensure that we can't call the wrong
one.

Also cleaned up various minor issues in xenmmap bindings (e.g. allocating more bytes than necessary,
due to a confusion between bytes and words in function parameters).

I've tested that I can boot a Linux and Windows VM after these changes.

Note: I thought about replacing Xenmmap.mmap_interface with Bigarray.Array1.t. However Bigarrays
can't be unmapped at arbitrary point in time by design: they can only be GCed.
We require more precise control in oxenstored, so I retained xenmmap as it is, I don't think it can
be simplified further.

A git tree with this and the other series is available at:
https://gitlab.com/edwintorok/xen/-/compare/master...for-upstream

Edwin Török (9):
  tools/ocaml: use common macros for manipulating mmap_interface
  tools/ocaml/libs/mmap: allocate correct number of bytes
  tools/ocaml/libs/mmap: Expose stub_mmap_alloc
  tools/ocaml/libs/xb: import gnttab stubs from mirage
  tools/ocaml: safer Xenmmap interface
  tools/ocaml/xenstored: use gnttab instead of xenctrl's
    foreign_map_range
  tools/ocaml/xenstored: don't store domU's mfn of ring page
  tools/ocaml/libs/mmap: mark mmap/munmap as blocking
  tools/ocaml/libs/mmap: Clean up unused read/write

 tools/ocaml/libs/mmap/mmap_stubs.h    |  11 ++-
 tools/ocaml/libs/mmap/xenmmap.ml      |  17 +++--
 tools/ocaml/libs/mmap/xenmmap.mli     |  13 ++--
 tools/ocaml/libs/mmap/xenmmap_stubs.c |  86 ++++++++-------------
 tools/ocaml/libs/xb/xb.ml             |  10 +--
 tools/ocaml/libs/xb/xb.mli            |   4 +-
 tools/ocaml/libs/xb/xs_ring_stubs.c   |  14 ++--
 tools/ocaml/libs/xc/xenctrl.ml        |   6 +-
 tools/ocaml/libs/xc/xenctrl.mli       |   5 +-
 tools/ocaml/xenstored/Makefile        |  11 ++-
 tools/ocaml/xenstored/domain.ml       |   9 +--
 tools/ocaml/xenstored/domains.ml      |  13 ++--
 tools/ocaml/xenstored/gnt.ml          |  62 +++++++++++++++
 tools/ocaml/xenstored/gnt.mli         |  87 +++++++++++++++++++++
 tools/ocaml/xenstored/gnttab_stubs.c  | 106 ++++++++++++++++++++++++++
 tools/ocaml/xenstored/process.ml      |  16 ++--
 tools/ocaml/xenstored/xenstored.ml    |  11 +--
 17 files changed, 362 insertions(+), 119 deletions(-)
 create mode 100644 tools/ocaml/xenstored/gnt.ml
 create mode 100644 tools/ocaml/xenstored/gnt.mli
 create mode 100644 tools/ocaml/xenstored/gnttab_stubs.c

-- 
2.25.1



^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH v1 1/9] tools/ocaml: use common macros for manipulating mmap_interface
  2020-08-27 17:35 [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range Edwin Török
@ 2020-08-27 17:35 ` Edwin Török
  2020-08-27 17:35 ` [PATCH v1 2/9] tools/ocaml/libs/mmap: allocate correct number of bytes Edwin Török
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Edwin Török @ 2020-08-27 17:35 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Christian Lindig, David Scott,
	Ian Jackson, Wei Liu

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/libs/mmap/mmap_stubs.h    |  7 +++++++
 tools/ocaml/libs/mmap/xenmmap_stubs.c |  2 --
 tools/ocaml/libs/xb/xs_ring_stubs.c   | 14 +++++---------
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/tools/ocaml/libs/mmap/mmap_stubs.h b/tools/ocaml/libs/mmap/mmap_stubs.h
index 65e4239890..816ba6a724 100644
--- a/tools/ocaml/libs/mmap/mmap_stubs.h
+++ b/tools/ocaml/libs/mmap/mmap_stubs.h
@@ -30,4 +30,11 @@ struct mmap_interface
 	int len;
 };
 
+#ifndef Data_abstract_val
+#define Data_abstract_val(v) ((void*) Op_val(v))
+#endif
+
+#define Intf_val(a) ((struct mmap_interface *) Data_abstract_val(a))
+#define Intf_data_val(a) (Intf_val(a)->addr)
+
 #endif
diff --git a/tools/ocaml/libs/mmap/xenmmap_stubs.c b/tools/ocaml/libs/mmap/xenmmap_stubs.c
index e2ce088e25..b811990a89 100644
--- a/tools/ocaml/libs/mmap/xenmmap_stubs.c
+++ b/tools/ocaml/libs/mmap/xenmmap_stubs.c
@@ -28,8 +28,6 @@
 #include <caml/fail.h>
 #include <caml/callback.h>
 
-#define Intf_val(a) ((struct mmap_interface *) a)
-
 static int mmap_interface_init(struct mmap_interface *intf,
                                int fd, int pflag, int mflag,
                                int len, int offset)
diff --git a/tools/ocaml/libs/xb/xs_ring_stubs.c b/tools/ocaml/libs/xb/xs_ring_stubs.c
index 7537a23949..9b6e3209fe 100644
--- a/tools/ocaml/libs/xb/xs_ring_stubs.c
+++ b/tools/ocaml/libs/xb/xs_ring_stubs.c
@@ -34,8 +34,6 @@
 
 #include "mmap_stubs.h"
 
-#define GET_C_STRUCT(a) ((struct mmap_interface *) a)
-
 /*
  * Bytes_val has been introduced by Ocaml 4.06.1. So define our own version
  * if needed.
@@ -51,12 +49,11 @@ CAMLprim value ml_interface_read(value ml_interface,
 	CAMLparam3(ml_interface, ml_buffer, ml_len);
 	CAMLlocal1(ml_result);
 
-	struct mmap_interface *interface = GET_C_STRUCT(ml_interface);
 	unsigned char *buffer = Bytes_val(ml_buffer);
 	int len = Int_val(ml_len);
 	int result;
 
-	struct xenstore_domain_interface *intf = interface->addr;
+	struct xenstore_domain_interface *intf = Intf_data_val(ml_interface);
 	XENSTORE_RING_IDX cons, prod; /* offsets only */
 	int total_data, data;
 	uint32_t connection;
@@ -110,12 +107,11 @@ CAMLprim value ml_interface_write(value ml_interface,
 	CAMLparam3(ml_interface, ml_buffer, ml_len);
 	CAMLlocal1(ml_result);
 
-	struct mmap_interface *interface = GET_C_STRUCT(ml_interface);
 	const unsigned char *buffer = Bytes_val(ml_buffer);
 	int len = Int_val(ml_len);
 	int result;
 
-	struct xenstore_domain_interface *intf = interface->addr;
+	struct xenstore_domain_interface *intf = Intf_data_val(ml_interface);
 	XENSTORE_RING_IDX cons, prod;
 	int total_space, space;
 	uint32_t connection;
@@ -165,7 +161,7 @@ exit:
 CAMLprim value ml_interface_set_server_features(value interface, value v)
 {
 	CAMLparam2(interface, v);
-	struct xenstore_domain_interface *intf = GET_C_STRUCT(interface)->addr;
+	struct xenstore_domain_interface *intf = Intf_data_val(interface);
 
 	intf->server_features = Int_val(v);
 
@@ -175,7 +171,7 @@ CAMLprim value ml_interface_set_server_features(value interface, value v)
 CAMLprim value ml_interface_get_server_features(value interface)
 {
 	CAMLparam1(interface);
-	struct xenstore_domain_interface *intf = GET_C_STRUCT(interface)->addr;
+	struct xenstore_domain_interface *intf = Intf_data_val(interface);
 
 	CAMLreturn(Val_int (intf->server_features));
 }
@@ -183,7 +179,7 @@ CAMLprim value ml_interface_get_server_features(value interface)
 CAMLprim value ml_interface_close(value interface)
 {
 	CAMLparam1(interface);
-	struct xenstore_domain_interface *intf = GET_C_STRUCT(interface)->addr;
+	struct xenstore_domain_interface *intf = Intf_data_val(interface);
 	int i;
 
 	intf->req_cons = intf->req_prod = intf->rsp_cons = intf->rsp_prod = 0;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v1 2/9] tools/ocaml/libs/mmap: allocate correct number of bytes
  2020-08-27 17:35 [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range Edwin Török
  2020-08-27 17:35 ` [PATCH v1 1/9] tools/ocaml: use common macros for manipulating mmap_interface Edwin Török
@ 2020-08-27 17:35 ` Edwin Török
  2020-08-27 17:35 ` [PATCH v1 3/9] tools/ocaml/libs/mmap: Expose stub_mmap_alloc Edwin Török
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Edwin Török @ 2020-08-27 17:35 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Christian Lindig, David Scott,
	Ian Jackson, Wei Liu

OCaml memory allocation functions use words as units,
unless explicitly documented otherwise.
Thus we were allocating more memory than necessary,
caml_alloc should've been called with the parameter '2',
but was called with a lot more.
To account for future changes in the struct keep using sizeof,
but round up and convert to number of words.

For OCaml 1 word = sizeof(value)

The Wsize_bsize macro converts bytes to words.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/libs/mmap/xenmmap_stubs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/ocaml/libs/mmap/xenmmap_stubs.c b/tools/ocaml/libs/mmap/xenmmap_stubs.c
index b811990a89..4d09c5a6e6 100644
--- a/tools/ocaml/libs/mmap/xenmmap_stubs.c
+++ b/tools/ocaml/libs/mmap/xenmmap_stubs.c
@@ -28,6 +28,8 @@
 #include <caml/fail.h>
 #include <caml/callback.h>
 
+#define Wsize_bsize_round(n) (Wsize_bsize( (n) + sizeof(value) - 1 ))
+
 static int mmap_interface_init(struct mmap_interface *intf,
                                int fd, int pflag, int mflag,
                                int len, int offset)
@@ -57,7 +59,7 @@ CAMLprim value stub_mmap_init(value fd, value pflag, value mflag,
 	default: caml_invalid_argument("maptype");
 	}
 
-	result = caml_alloc(sizeof(struct mmap_interface), Abstract_tag);
+	result = caml_alloc(Wsize_bsize_round(sizeof(struct mmap_interface)), Abstract_tag);
 
 	if (mmap_interface_init(Intf_val(result), Int_val(fd),
 	                        c_pflag, c_mflag,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v1 3/9] tools/ocaml/libs/mmap: Expose stub_mmap_alloc
  2020-08-27 17:35 [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range Edwin Török
  2020-08-27 17:35 ` [PATCH v1 1/9] tools/ocaml: use common macros for manipulating mmap_interface Edwin Török
  2020-08-27 17:35 ` [PATCH v1 2/9] tools/ocaml/libs/mmap: allocate correct number of bytes Edwin Török
@ 2020-08-27 17:35 ` Edwin Török
  2020-08-27 17:35 ` [PATCH v1 4/9] tools/ocaml/libs/xb: import gnttab stubs from mirage Edwin Török
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Edwin Török @ 2020-08-27 17:35 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Christian Lindig, David Scott,
	Ian Jackson, Wei Liu

This also handles mmap errors better by using the `uerror` helper
to raise a proper exception using `errno`.

Changed type of `len` from `int` to `size_t`: at construction time we
ensure the length is >= 0, so we can reflect this by using an unsigned
type. The type is unsigned at the C API level, and a negative integer
would just get translated to a very large unsigned number otherwise.

mmap also takes off_t and size_t, so using int64 would be more generic
here, however we only ever use this interface to map rings, so keeping
the `int` sizes is fine.
OCaml itself only uses `ints` for mapping bigarrays, and int64 for just
the offset.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/libs/mmap/mmap_stubs.h    |  4 +++-
 tools/ocaml/libs/mmap/xenmmap_stubs.c | 31 +++++++++++++++++----------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/tools/ocaml/libs/mmap/mmap_stubs.h b/tools/ocaml/libs/mmap/mmap_stubs.h
index 816ba6a724..3352594e38 100644
--- a/tools/ocaml/libs/mmap/mmap_stubs.h
+++ b/tools/ocaml/libs/mmap/mmap_stubs.h
@@ -27,7 +27,7 @@
 struct mmap_interface
 {
 	void *addr;
-	int len;
+	size_t len;
 };
 
 #ifndef Data_abstract_val
@@ -37,4 +37,6 @@ struct mmap_interface
 #define Intf_val(a) ((struct mmap_interface *) Data_abstract_val(a))
 #define Intf_data_val(a) (Intf_val(a)->addr)
 
+value stub_mmap_alloc(void *addr, size_t len);
+
 #endif
diff --git a/tools/ocaml/libs/mmap/xenmmap_stubs.c b/tools/ocaml/libs/mmap/xenmmap_stubs.c
index 4d09c5a6e6..9c1126c6a2 100644
--- a/tools/ocaml/libs/mmap/xenmmap_stubs.c
+++ b/tools/ocaml/libs/mmap/xenmmap_stubs.c
@@ -27,16 +27,18 @@
 #include <caml/custom.h>
 #include <caml/fail.h>
 #include <caml/callback.h>
+#include <caml/unixsupport.h>
 
 #define Wsize_bsize_round(n) (Wsize_bsize( (n) + sizeof(value) - 1 ))
 
-static int mmap_interface_init(struct mmap_interface *intf,
-                               int fd, int pflag, int mflag,
-                               int len, int offset)
+value stub_mmap_alloc(void *addr, size_t len)
 {
-	intf->len = len;
-	intf->addr = mmap(NULL, len, pflag, mflag, fd, offset);
-	return (intf->addr == MAP_FAILED) ? errno : 0;
+	CAMLparam0();
+	CAMLlocal1(result);
+	result = caml_alloc(Wsize_bsize_round(sizeof(struct mmap_interface)), Abstract_tag);
+	Intf_val(result)->addr = addr;
+	Intf_val(result)->len = len;
+	CAMLreturn(result);
 }
 
 CAMLprim value stub_mmap_init(value fd, value pflag, value mflag,
@@ -45,6 +47,8 @@ CAMLprim value stub_mmap_init(value fd, value pflag, value mflag,
 	CAMLparam5(fd, pflag, mflag, len, offset);
 	CAMLlocal1(result);
 	int c_pflag, c_mflag;
+	void* addr;
+	size_t length;
 
 	switch (Int_val(pflag)) {
 	case 0: c_pflag = PROT_READ; break;
@@ -59,12 +63,17 @@ CAMLprim value stub_mmap_init(value fd, value pflag, value mflag,
 	default: caml_invalid_argument("maptype");
 	}
 
-	result = caml_alloc(Wsize_bsize_round(sizeof(struct mmap_interface)), Abstract_tag);
+	if (Int_val(len) < 0)
+		caml_invalid_argument("negative size");
+	if (Int_val(offset) < 0)
+		caml_invalid_argument("negative offset");
+	length = Int_val(len);
 
-	if (mmap_interface_init(Intf_val(result), Int_val(fd),
-	                        c_pflag, c_mflag,
-	                        Int_val(len), Int_val(offset)))
-		caml_failwith("mmap");
+	addr = mmap(NULL, length, c_pflag, c_mflag, fd, Int_val(offset));
+	if (MAP_FAILED == addr)
+		uerror("mmap", Nothing);
+
+	result = stub_mmap_alloc(addr, length);
 	CAMLreturn(result);
 }
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v1 4/9] tools/ocaml/libs/xb: import gnttab stubs from mirage
  2020-08-27 17:35 [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range Edwin Török
                   ` (2 preceding siblings ...)
  2020-08-27 17:35 ` [PATCH v1 3/9] tools/ocaml/libs/mmap: Expose stub_mmap_alloc Edwin Török
@ 2020-08-27 17:35 ` Edwin Török
  2020-08-27 17:35 ` [PATCH v1 5/9] tools/ocaml: safer Xenmmap interface Edwin Török
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Edwin Török @ 2020-08-27 17:35 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Christian Lindig, David Scott,
	Ian Jackson, Wei Liu

Upstream URL: https://github.com/mirage/ocaml-gnt
Mirage is part of the Xen project and the license is compatible,
copyright headers are retained.

Changes from upstream:
* cut down dependencies: dropped Lwt, replaced Io_page with Xenmmap
* only import Gnttab and not Gntshr

This is for xenstored's use only which needs a way to grant map
the xenstore ring without using xenctrl.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/xenstored/Makefile       |  11 ++-
 tools/ocaml/xenstored/gnt.ml         |  60 +++++++++++++++
 tools/ocaml/xenstored/gnt.mli        |  86 ++++++++++++++++++++++
 tools/ocaml/xenstored/gnttab_stubs.c | 106 +++++++++++++++++++++++++++
 4 files changed, 260 insertions(+), 3 deletions(-)
 create mode 100644 tools/ocaml/xenstored/gnt.ml
 create mode 100644 tools/ocaml/xenstored/gnt.mli
 create mode 100644 tools/ocaml/xenstored/gnttab_stubs.c

diff --git a/tools/ocaml/xenstored/Makefile b/tools/ocaml/xenstored/Makefile
index 692a62584e..3490c4ff4e 100644
--- a/tools/ocaml/xenstored/Makefile
+++ b/tools/ocaml/xenstored/Makefile
@@ -7,6 +7,7 @@ CFLAGS += -include $(XEN_ROOT)/tools/config.h
 CFLAGS-$(CONFIG_SYSTEMD)  += $(SYSTEMD_CFLAGS)
 LDFLAGS-$(CONFIG_SYSTEMD) += $(SYSTEMD_LIBS)
 
+CFLAGS  += $(CFLAGS_libxengnttab) -I../libs/mmap
 CFLAGS  += $(CFLAGS-y)
 CFLAGS  += $(APPEND_CFLAGS)
 LDFLAGS += $(LDFLAGS-y)
@@ -18,12 +19,15 @@ OCAMLINCLUDE += \
 	-I $(OCAML_TOPLEVEL)/libs/xc \
 	-I $(OCAML_TOPLEVEL)/libs/eventchn
 
-LIBS = syslog.cma syslog.cmxa poll.cma poll.cmxa
+LIBS = syslog.cma syslog.cmxa poll.cma poll.cmxa gnt.cma gnt.cmxa
 syslog_OBJS = syslog
 syslog_C_OBJS = syslog_stubs
 poll_OBJS = poll
 poll_C_OBJS = select_stubs
-OCAML_LIBRARY = syslog poll
+gnt_OBJS = gnt
+gnt_C_OBJS = gnttab_stubs
+LIBS_gnt += $(LDLIBS_libxengnttab)
+OCAML_LIBRARY = syslog poll gnt
 
 LIBS += systemd.cma systemd.cmxa
 systemd_OBJS = systemd
@@ -58,7 +62,7 @@ OBJS = paths \
 	process \
 	xenstored
 
-INTF = symbol.cmi trie.cmi syslog.cmi systemd.cmi poll.cmi
+INTF = symbol.cmi trie.cmi syslog.cmi systemd.cmi poll.cmi gnt.cmi
 
 XENSTOREDLIBS = \
 	unix.cmxa \
@@ -66,6 +70,7 @@ XENSTOREDLIBS = \
 	-ccopt -L -ccopt . systemd.cmxa \
 	-ccopt -L -ccopt . poll.cmxa \
 	-ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/mmap $(OCAML_TOPLEVEL)/libs/mmap/xenmmap.cmxa \
+	-ccopt -L -ccopt . gnt.cmxa \
 	-ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/eventchn $(OCAML_TOPLEVEL)/libs/eventchn/xeneventchn.cmxa \
 	-ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/xc $(OCAML_TOPLEVEL)/libs/xc/xenctrl.cmxa \
 	-ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/xb $(OCAML_TOPLEVEL)/libs/xb/xenbus.cmxa \
diff --git a/tools/ocaml/xenstored/gnt.ml b/tools/ocaml/xenstored/gnt.ml
new file mode 100644
index 0000000000..65f0334b7c
--- /dev/null
+++ b/tools/ocaml/xenstored/gnt.ml
@@ -0,0 +1,60 @@
+(*
+ * Copyright (c) 2010 Anil Madhavapeddy <anil@recoil.org>
+ * Copyright (C) 2012-2014 Citrix Inc
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *)
+
+type gntref = int
+type domid = int
+
+let console = 0 (* public/grant_table.h:GNTTAB_RESERVED_CONSOLE *)
+let xenstore = 1 (* public/grant_table.h:GNTTAB_RESERVED_XENSTORE *)
+
+type grant_handle (* handle to a mapped grant *)
+
+module Gnttab = struct
+  type interface
+
+  external interface_open': unit -> interface = "stub_gnttab_interface_open"
+
+  let interface_open () =
+    try
+      interface_open' ()
+    with e ->
+      Printf.fprintf stderr "Failed to open grant table device: ENOENT\n";
+      Printf.fprintf stderr "Does this system have Xen userspace grant table support?\n";
+      Printf.fprintf stderr "On linux try:\n";
+      Printf.fprintf stderr "  sudo modprobe xen-gntdev\n%!";
+      raise e
+
+  external interface_close: interface -> unit = "stub_gnttab_interface_close"
+
+  type grant = {
+    domid: domid;
+    ref: gntref;
+  }
+
+  module Local_mapping = struct
+    type t = Xenmmap.mmap_interface
+
+    let to_pages t = t
+  end
+
+  external unmap_exn : interface -> Local_mapping.t -> unit = "stub_gnttab_unmap"
+
+  external map_fresh_exn: interface -> gntref -> domid -> bool -> Local_mapping.t = "stub_gnttab_map_fresh"
+
+  let map_exn interface grant writable =
+      map_fresh_exn interface grant.ref grant.domid writable
+end
diff --git a/tools/ocaml/xenstored/gnt.mli b/tools/ocaml/xenstored/gnt.mli
new file mode 100644
index 0000000000..302e13b05d
--- /dev/null
+++ b/tools/ocaml/xenstored/gnt.mli
@@ -0,0 +1,86 @@
+(*
+ * Copyright (c) 2010 Anil Madhavapeddy <anil@recoil.org>
+ * Copyright (C) 2012-2014 Citrix Inc
+ * 
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *)
+
+(** Allow a local xen domain to read/write memory exported ("granted")
+    from foreign domains. Safe memory sharing is a building block of all
+    xen inter-domain communication protocols such as those for virtual
+    network and disk devices.
+
+    Foreign domains will explicitly "grant" us access to certain memory
+    regions such as disk buffers. These regions are uniquely identified
+    by the pair of (foreign domain id, integer reference) which is
+    passed to us over some existing channel (typically via xenstore keys
+    or via structures in previously-shared memory region).
+*)
+
+(** {2 Common interface} *)
+
+type gntref = int
+(** Type of a grant table index, called a grant reference in
+    Xen's terminology. *)
+
+(** {2 Receiving foreign pages} *)
+
+module Gnttab : sig
+  type interface
+  (** A connection to the grant device, needed for mapping/unmapping *)
+
+  val interface_open: unit -> interface
+  (** Open a connection to the grant device. This must be done before any
+      calls to map or unmap. *)
+
+  val interface_close: interface -> unit
+  (** Close a connection to the grant device. Any future calls to map or
+      unmap will fail. *)
+
+  type grant = {
+    domid: int;
+    (** foreign domain who is exporting memory *)
+    ref: gntref;
+    (** id which identifies the specific export in the foreign domain *)
+  }
+  (** A foreign domain must explicitly "grant" us memory and send us the
+      "reference". The pair of (foreign domain id, reference) uniquely
+      identifies the block of memory. This pair ("grant") is transmitted
+      to us out-of-band, usually either via xenstore during device setup or
+      via a shared memory ring structure. *)
+
+  module Local_mapping : sig
+    type t
+    (** Abstract type representing a locally-mapped shared memory page *)
+
+    val to_pages: t -> Xenmmap.mmap_interface
+  end
+
+  val map_exn : interface -> grant -> bool -> Local_mapping.t
+  (** [map_exn if grant writable] creates a single mapping from
+      [grant] that will be writable if [writable] is [true]. *)
+
+  val unmap_exn: interface -> Local_mapping.t -> unit
+  (** Unmap a single mapping (which may involve multiple grants). Throws a
+      Failure if unsuccessful. *)
+end
+
+val console: gntref
+(** In xen-4.2 and later, the domain builder will allocate one of the
+    reserved grant table entries and use it to pre-authorise the console
+    backend domain. *)
+
+val xenstore: gntref
+(** In xen-4.2 and later, the domain builder will allocate one of the
+    reserved grant table entries and use it to pre-authorise the xenstore
+    backend domain. *)
diff --git a/tools/ocaml/xenstored/gnttab_stubs.c b/tools/ocaml/xenstored/gnttab_stubs.c
new file mode 100644
index 0000000000..f0b4ab237f
--- /dev/null
+++ b/tools/ocaml/xenstored/gnttab_stubs.c
@@ -0,0 +1,106 @@
+/*
+ * Copyright (C) 2012-2013 Citrix Inc
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <errno.h>
+
+/* For PROT_READ | PROT_WRITE */
+#include <sys/mman.h>
+
+#define CAML_NAME_SPACE
+#include <caml/alloc.h>
+#include <caml/memory.h>
+#include <caml/signals.h>
+#include <caml/fail.h>
+#include <caml/callback.h>
+#include <caml/bigarray.h>
+
+#include "xengnttab.h"
+#include "mmap_stubs.h"
+
+#ifndef Data_abstract_val
+#define Data_abstract_val(v) ((void*) Op_val(v))
+#endif
+
+#define _G(__g) (*((xengnttab_handle**)Data_abstract_val(__g)))
+
+CAMLprim value stub_gnttab_interface_open(void)
+{
+	CAMLparam0();
+	CAMLlocal1(result);
+	xengnttab_handle *xgh;
+
+	xgh = xengnttab_open(NULL, 0);
+	if (xgh == NULL)
+		caml_failwith("Failed to open interface");
+	result = caml_alloc(1, Abstract_tag);
+	_G(result) = xgh;
+
+	CAMLreturn(result);
+}
+
+CAMLprim value stub_gnttab_interface_close(value xgh)
+{
+	CAMLparam1(xgh);
+
+	xengnttab_close(_G(xgh));
+
+	CAMLreturn(Val_unit);
+}
+
+#define _M(__m) ((struct mmap_interface*)Data_abstract_val(__m))
+#define XEN_PAGE_SHIFT 12
+
+CAMLprim value stub_gnttab_unmap(value xgh, value array)
+{
+	CAMLparam2(xgh, array);
+	int result;
+
+	caml_enter_blocking_section();
+	result = xengnttab_unmap(_G(xgh), _M(array)->addr, _M(array)->len >> XEN_PAGE_SHIFT);
+	caml_leave_blocking_section();
+
+	if(result!=0) {
+		caml_failwith("Failed to unmap grant");
+	}
+
+	CAMLreturn(Val_unit);
+}
+
+CAMLprim value stub_gnttab_map_fresh(
+	value xgh,
+	value reference,
+	value domid,
+	value writable
+	)
+{
+	CAMLparam4(xgh, reference, domid, writable);
+	CAMLlocal1(contents);
+	void *map;
+
+	caml_enter_blocking_section();
+	map = xengnttab_map_grant_ref(_G(xgh), Int_val(domid), Int_val(reference),
+		Bool_val(writable)?PROT_READ | PROT_WRITE:PROT_READ);
+	caml_leave_blocking_section();
+
+	if(map==NULL) {
+		caml_failwith("Failed to map grant ref");
+	}
+	contents = stub_mmap_alloc(map, 1 << XEN_PAGE_SHIFT);
+	CAMLreturn(contents);
+}
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v1 5/9] tools/ocaml: safer Xenmmap interface
  2020-08-27 17:35 [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range Edwin Török
                   ` (3 preceding siblings ...)
  2020-08-27 17:35 ` [PATCH v1 4/9] tools/ocaml/libs/xb: import gnttab stubs from mirage Edwin Török
@ 2020-08-27 17:35 ` Edwin Török
  2020-08-27 17:35 ` [PATCH v1 6/9] tools/ocaml/xenstored: use gnttab instead of xenctrl's foreign_map_range Edwin Török
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Edwin Török @ 2020-08-27 17:35 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Christian Lindig, David Scott,
	Ian Jackson, Wei Liu

Xenmmap.mmap_interface is created from multiple places:
* via mmap(), which needs to be unmap()-ed
* xc_map_foreign_range
* xengnttab_map_grant_ref

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/libs/mmap/xenmmap.ml  | 14 ++++++++++++--
 tools/ocaml/libs/mmap/xenmmap.mli | 11 ++++++++---
 tools/ocaml/libs/xb/xb.ml         | 10 +++++-----
 tools/ocaml/libs/xb/xb.mli        |  4 ++--
 tools/ocaml/libs/xc/xenctrl.ml    |  6 ++++--
 tools/ocaml/libs/xc/xenctrl.mli   |  5 ++---
 tools/ocaml/xenstored/domain.ml   |  2 +-
 tools/ocaml/xenstored/gnt.ml      | 14 ++++++++------
 tools/ocaml/xenstored/gnt.mli     |  3 ++-
 9 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/tools/ocaml/libs/mmap/xenmmap.ml b/tools/ocaml/libs/mmap/xenmmap.ml
index 44b67c89d2..af258942a0 100644
--- a/tools/ocaml/libs/mmap/xenmmap.ml
+++ b/tools/ocaml/libs/mmap/xenmmap.ml
@@ -15,17 +15,27 @@
  *)
 
 type mmap_interface
+type t = mmap_interface * (mmap_interface -> unit)
+
 
 type mmap_prot_flag = RDONLY | WRONLY | RDWR
 type mmap_map_flag = SHARED | PRIVATE
 
 (* mmap: fd -> prot_flag -> map_flag -> length -> offset -> interface *)
-external mmap: Unix.file_descr -> mmap_prot_flag -> mmap_map_flag
+external mmap': Unix.file_descr -> mmap_prot_flag -> mmap_map_flag
 		-> int -> int -> mmap_interface = "stub_mmap_init"
-external unmap: mmap_interface -> unit = "stub_mmap_final"
 (* read: interface -> start -> length -> data *)
 external read: mmap_interface -> int -> int -> string = "stub_mmap_read"
 (* write: interface -> data -> start -> length -> unit *)
 external write: mmap_interface -> string -> int -> int -> unit = "stub_mmap_write"
 (* getpagesize: unit -> size of page *)
+external unmap': mmap_interface -> unit = "stub_mmap_final"
+(* getpagesize: unit -> size of page *)
+let make ?(unmap=unmap') interface = interface, unmap
 external getpagesize: unit -> int = "stub_mmap_getpagesize"
+
+let to_interface (intf, _) = intf
+let mmap fd prot_flag map_flag length offset =
+	let map = mmap' fd prot_flag map_flag length offset in
+	make map ~unmap:unmap'
+let unmap (map, do_unmap) = do_unmap map
diff --git a/tools/ocaml/libs/mmap/xenmmap.mli b/tools/ocaml/libs/mmap/xenmmap.mli
index 8f92ed6310..075b24eab4 100644
--- a/tools/ocaml/libs/mmap/xenmmap.mli
+++ b/tools/ocaml/libs/mmap/xenmmap.mli
@@ -14,15 +14,20 @@
  * GNU Lesser General Public License for more details.
  *)
 
+type t
 type mmap_interface
 type mmap_prot_flag = RDONLY | WRONLY | RDWR
 type mmap_map_flag = SHARED | PRIVATE
 
-external mmap : Unix.file_descr -> mmap_prot_flag -> mmap_map_flag -> int -> int
-             -> mmap_interface = "stub_mmap_init"
-external unmap : mmap_interface -> unit = "stub_mmap_final"
 external read : mmap_interface -> int -> int -> string = "stub_mmap_read"
 external write : mmap_interface -> string -> int -> int -> unit
                = "stub_mmap_write"
 
+val mmap : Unix.file_descr -> mmap_prot_flag -> mmap_map_flag -> int -> int -> t
+val unmap : t -> unit
+
+val make: ?unmap:(mmap_interface -> unit) -> mmap_interface -> t 
+
+val to_interface: t -> mmap_interface
+
 external getpagesize : unit -> int = "stub_mmap_getpagesize"
diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml
index 104d319d77..4ddf741420 100644
--- a/tools/ocaml/libs/xb/xb.ml
+++ b/tools/ocaml/libs/xb/xb.ml
@@ -28,7 +28,7 @@ let _ =
 
 type backend_mmap =
 {
-	mmap: Xenmmap.mmap_interface;     (* mmaped interface = xs_ring *)
+	mmap: Xenmmap.t;     (* mmaped interface = xs_ring *)
 	eventchn_notify: unit -> unit; (* function to notify through eventchn *)
 	mutable work_again: bool;
 }
@@ -59,7 +59,7 @@ let reconnect t = match t.backend with
 		(* should never happen, so close the connection *)
 		raise End_of_file
 	| Xenmmap backend ->
-		Xs_ring.close backend.mmap;
+		Xs_ring.close Xenmmap.(to_interface backend.mmap);
 		backend.eventchn_notify ();
 		(* Clear our old connection state *)
 		Queue.clear t.pkt_in;
@@ -77,7 +77,7 @@ let read_fd back _con b len =
 
 let read_mmap back _con b len =
 	let s = Bytes.make len '\000' in
-	let rd = Xs_ring.read back.mmap s len in
+	let rd = Xs_ring.read Xenmmap.(to_interface back.mmap) s len in
 	Bytes.blit s 0 b 0 rd;
 	back.work_again <- (rd > 0);
 	if rd > 0 then
@@ -93,7 +93,7 @@ let write_fd back _con b len =
 	Unix.write_substring back.fd b 0 len
 
 let write_mmap back _con s len =
-	let ws = Xs_ring.write_substring back.mmap s len in
+	let ws = Xs_ring.write_substring Xenmmap.(to_interface back.mmap) s len in
 	if ws > 0 then
 		back.eventchn_notify ();
 	ws
@@ -167,7 +167,7 @@ let open_fd fd = newcon (Fd { fd = fd; })
 
 let open_mmap mmap notifyfct =
 	(* Advertise XENSTORE_SERVER_FEATURE_RECONNECTION *)
-	Xs_ring.set_server_features mmap (Xs_ring.Server_features.singleton Xs_ring.Server_feature.Reconnection);
+	Xs_ring.set_server_features (Xenmmap.to_interface mmap) (Xs_ring.Server_features.singleton Xs_ring.Server_feature.Reconnection);
 	newcon (Xenmmap {
 		mmap = mmap;
 		eventchn_notify = notifyfct;
diff --git a/tools/ocaml/libs/xb/xb.mli b/tools/ocaml/libs/xb/xb.mli
index 3a00da6cdd..0184d77ffc 100644
--- a/tools/ocaml/libs/xb/xb.mli
+++ b/tools/ocaml/libs/xb/xb.mli
@@ -59,7 +59,7 @@ exception Noent
 exception Invalid
 exception Reconnect
 type backend_mmap = {
-  mmap : Xenmmap.mmap_interface;
+  mmap : Xenmmap.t;
   eventchn_notify : unit -> unit;
   mutable work_again : bool;
 }
@@ -86,7 +86,7 @@ val output : t -> bool
 val input : t -> bool
 val newcon : backend -> t
 val open_fd : Unix.file_descr -> t
-val open_mmap : Xenmmap.mmap_interface -> (unit -> unit) -> t
+val open_mmap : Xenmmap.t -> (unit -> unit) -> t
 val close : t -> unit
 val is_fd : t -> bool
 val is_mmap : t -> bool
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 497ded7ce2..e80cacf22c 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -256,9 +256,11 @@ external domain_set_memmap_limit: handle -> domid -> int64 -> unit
 external domain_memory_increase_reservation: handle -> domid -> int64 -> unit
        = "stub_xc_domain_memory_increase_reservation"
 
-external map_foreign_range: handle -> domid -> int
+external map_foreign_range': handle -> domid -> int
                          -> nativeint -> Xenmmap.mmap_interface
-       = "stub_map_foreign_range"
+			 = "stub_map_foreign_range"
+let map_foreign_range handle domid port mfn =
+	Xenmmap.make (map_foreign_range' handle domid port mfn)
 
 external domain_assign_device: handle -> domid -> (int * int * int * int) -> unit
        = "stub_xc_domain_assign_device"
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index f7f6ec570d..9f268a87fa 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -196,9 +196,8 @@ external domain_set_memmap_limit : handle -> domid -> int64 -> unit
 external domain_memory_increase_reservation :
   handle -> domid -> int64 -> unit
   = "stub_xc_domain_memory_increase_reservation"
-external map_foreign_range :
-  handle -> domid -> int -> nativeint -> Xenmmap.mmap_interface
-  = "stub_map_foreign_range"
+val map_foreign_range :
+  handle -> domid -> int -> nativeint -> Xenmmap.t
 
 external domain_assign_device: handle -> domid -> (int * int * int * int) -> unit
        = "stub_xc_domain_assign_device"
diff --git a/tools/ocaml/xenstored/domain.ml b/tools/ocaml/xenstored/domain.ml
index aeb185ff7e..2d9c1f5d09 100644
--- a/tools/ocaml/xenstored/domain.ml
+++ b/tools/ocaml/xenstored/domain.ml
@@ -23,7 +23,7 @@ type t =
 {
 	id: Xenctrl.domid;
 	mfn: nativeint;
-	interface: Xenmmap.mmap_interface;
+	interface: Xenmmap.t;
 	eventchn: Event.t;
 	mutable remote_port: int;
 	mutable port: Xeneventchn.t option;
diff --git a/tools/ocaml/xenstored/gnt.ml b/tools/ocaml/xenstored/gnt.ml
index 65f0334b7c..bef2d3e850 100644
--- a/tools/ocaml/xenstored/gnt.ml
+++ b/tools/ocaml/xenstored/gnt.ml
@@ -45,16 +45,18 @@ module Gnttab = struct
     ref: gntref;
   }
 
+  external unmap_exn : interface -> Xenmmap.mmap_interface -> unit = "stub_gnttab_unmap"
+
+  external map_fresh_exn: interface -> gntref -> domid -> bool -> Xenmmap.mmap_interface = "stub_gnttab_map_fresh"
+
   module Local_mapping = struct
     type t = Xenmmap.mmap_interface
 
-    let to_pages t = t
+    let to_pages interface t =
+      Xenmmap.make t ~unmap:(unmap_exn interface)
   end
 
-  external unmap_exn : interface -> Local_mapping.t -> unit = "stub_gnttab_unmap"
-
-  external map_fresh_exn: interface -> gntref -> domid -> bool -> Local_mapping.t = "stub_gnttab_map_fresh"
-
   let map_exn interface grant writable =
-      map_fresh_exn interface grant.ref grant.domid writable
+    map_fresh_exn interface grant.ref grant.domid writable
+
 end
diff --git a/tools/ocaml/xenstored/gnt.mli b/tools/ocaml/xenstored/gnt.mli
index 302e13b05d..13ab4c7ead 100644
--- a/tools/ocaml/xenstored/gnt.mli
+++ b/tools/ocaml/xenstored/gnt.mli
@@ -53,6 +53,7 @@ module Gnttab : sig
     ref: gntref;
     (** id which identifies the specific export in the foreign domain *)
   }
+
   (** A foreign domain must explicitly "grant" us memory and send us the
       "reference". The pair of (foreign domain id, reference) uniquely
       identifies the block of memory. This pair ("grant") is transmitted
@@ -63,7 +64,7 @@ module Gnttab : sig
     type t
     (** Abstract type representing a locally-mapped shared memory page *)
 
-    val to_pages: t -> Xenmmap.mmap_interface
+    val to_pages: interface -> t -> Xenmmap.t
   end
 
   val map_exn : interface -> grant -> bool -> Local_mapping.t
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v1 6/9] tools/ocaml/xenstored: use gnttab instead of xenctrl's foreign_map_range
  2020-08-27 17:35 [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range Edwin Török
                   ` (4 preceding siblings ...)
  2020-08-27 17:35 ` [PATCH v1 5/9] tools/ocaml: safer Xenmmap interface Edwin Török
@ 2020-08-27 17:35 ` Edwin Török
  2020-08-27 17:35 ` [PATCH v1 7/9] tools/ocaml/xenstored: don't store domU's mfn of ring page Edwin Török
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Edwin Török @ 2020-08-27 17:35 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Christian Lindig, David Scott,
	Ian Jackson, Wei Liu, Andrew Cooper

This is an oxenstored port of the following C xenstored commit:
38eeb3864de40aa568c48f9f26271c141c62b50b tools/xenstored: Drop mapping of the ring via foreign map

Now only Xenctrl.domain_getinfo remains as the last use of unstable xenctrl interface
in oxenstored.

Depends on: tools/ocaml: safer Xenmmap interface
(without it the code would build but the wrong unmap function would get
 called on domain destruction)

CC: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/xenstored/domains.ml   | 7 +++++--
 tools/ocaml/xenstored/xenstored.ml | 3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/ocaml/xenstored/domains.ml b/tools/ocaml/xenstored/domains.ml
index 17fe2fa257..d9cb693751 100644
--- a/tools/ocaml/xenstored/domains.ml
+++ b/tools/ocaml/xenstored/domains.ml
@@ -22,6 +22,7 @@ let xc = Xenctrl.interface_open ()
 
 type domains = {
 	eventchn: Event.t;
+	gnttab: Gnt.Gnttab.interface;
 	table: (Xenctrl.domid, Domain.t) Hashtbl.t;
 
 	(* N.B. the Queue module is not thread-safe but oxenstored is single-threaded. *)
@@ -42,8 +43,9 @@ type domains = {
 	mutable n_penalised: int; (* Number of domains with less than maximum credit *)
 }
 
-let init eventchn on_first_conflict_pause = {
+let init eventchn gnttab on_first_conflict_pause = {
 	eventchn = eventchn;
+	gnttab;
 	table = Hashtbl.create 10;
 	doms_conflict_paused = Queue.create ();
 	doms_with_conflict_penalty = Queue.create ();
@@ -123,7 +125,8 @@ let resume _doms _domid =
 	()
 
 let create doms domid mfn port =
-	let interface = Xenctrl.map_foreign_range xc domid (Xenmmap.getpagesize()) mfn in
+	let mapping = Gnt.(Gnttab.map_exn doms.gnttab { domid; ref = xenstore} true) in
+	let interface = Gnt.Gnttab.Local_mapping.to_pages doms.gnttab mapping in
 	let dom = Domain.make domid mfn port interface doms.eventchn in
 	Hashtbl.add doms.table domid dom;
 	Domain.bind_interdomain dom;
diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
index f3e4697dea..a232e4c616 100644
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -265,6 +265,7 @@ let _ =
 
 	let store = Store.create () in
 	let eventchn = Event.init () in
+	let gnttab = Gnt.Gnttab.interface_open () in
 	let next_frequent_ops = ref 0. in
 	let advance_next_frequent_ops () =
 		next_frequent_ops := (Unix.gettimeofday () +. !Define.conflict_max_history_seconds)
@@ -272,7 +273,7 @@ let _ =
 	let delay_next_frequent_ops_by duration =
 		next_frequent_ops := !next_frequent_ops +. duration
 	in
-	let domains = Domains.init eventchn advance_next_frequent_ops in
+	let domains = Domains.init eventchn gnttab advance_next_frequent_ops in
 
 	(* For things that need to be done periodically but more often
 	 * than the periodic_ops function *)
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v1 7/9] tools/ocaml/xenstored: don't store domU's mfn of ring page
  2020-08-27 17:35 [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range Edwin Török
                   ` (5 preceding siblings ...)
  2020-08-27 17:35 ` [PATCH v1 6/9] tools/ocaml/xenstored: use gnttab instead of xenctrl's foreign_map_range Edwin Török
@ 2020-08-27 17:35 ` Edwin Török
  2020-08-27 17:35 ` [PATCH v1 8/9] tools/ocaml/libs/mmap: mark mmap/munmap as blocking Edwin Török
  2020-08-27 17:36 ` [PATCH v1 9/9] tools/ocaml/libs/mmap: Clean up unused read/write Edwin Török
  8 siblings, 0 replies; 10+ messages in thread
From: Edwin Török @ 2020-08-27 17:35 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Christian Lindig, David Scott,
	Ian Jackson, Wei Liu, Signed-off-by : Juergen Gross

This is a port of the following C xenstored commit
122b52230aa5b79d65e18b8b77094027faa2f8e2 tools/xenstore: don't store domU's mfn of ring page in xenstored

Backwards compat: accept a domain dump both with and without MFN.

CC: Signed-off-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/xenstored/domain.ml    |  7 ++-----
 tools/ocaml/xenstored/domains.ml   |  6 +++---
 tools/ocaml/xenstored/process.ml   | 16 +++++-----------
 tools/ocaml/xenstored/xenstored.ml |  8 ++++----
 4 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/tools/ocaml/xenstored/domain.ml b/tools/ocaml/xenstored/domain.ml
index 2d9c1f5d09..b11a2f39f5 100644
--- a/tools/ocaml/xenstored/domain.ml
+++ b/tools/ocaml/xenstored/domain.ml
@@ -22,7 +22,6 @@ let warn  fmt = Logging.warn  "domain" fmt
 type t =
 {
 	id: Xenctrl.domid;
-	mfn: nativeint;
 	interface: Xenmmap.t;
 	eventchn: Event.t;
 	mutable remote_port: int;
@@ -41,7 +40,6 @@ let is_dom0 d = d.id = 0
 let get_path dom = "/local/domain/" ^ (sprintf "%u" dom.id)
 let get_id domain = domain.id
 let get_interface d = d.interface
-let get_mfn d = d.mfn
 let get_remote_port d = d.remote_port
 let get_port d = d.port
 
@@ -62,7 +60,7 @@ let string_of_port = function
 | Some x -> string_of_int (Xeneventchn.to_int x)
 
 let dump d chan =
-	fprintf chan "dom,%d,%nd,%d\n" d.id d.mfn d.remote_port
+	fprintf chan "dom,%d,%d\n" d.id d.remote_port
 
 let notify dom = match dom.port with
 | None ->
@@ -88,9 +86,8 @@ let close dom =
 	Xenmmap.unmap dom.interface;
 	()
 
-let make id mfn remote_port interface eventchn = {
+let make id remote_port interface eventchn = {
 	id = id;
-	mfn = mfn;
 	remote_port = remote_port;
 	interface = interface;
 	eventchn = eventchn;
diff --git a/tools/ocaml/xenstored/domains.ml b/tools/ocaml/xenstored/domains.ml
index d9cb693751..0dfeed193a 100644
--- a/tools/ocaml/xenstored/domains.ml
+++ b/tools/ocaml/xenstored/domains.ml
@@ -124,10 +124,10 @@ let cleanup doms =
 let resume _doms _domid =
 	()
 
-let create doms domid mfn port =
+let create doms domid port =
 	let mapping = Gnt.(Gnttab.map_exn doms.gnttab { domid; ref = xenstore} true) in
 	let interface = Gnt.Gnttab.Local_mapping.to_pages doms.gnttab mapping in
-	let dom = Domain.make domid mfn port interface doms.eventchn in
+	let dom = Domain.make domid port interface doms.eventchn in
 	Hashtbl.add doms.table domid dom;
 	Domain.bind_interdomain dom;
 	dom
@@ -147,7 +147,7 @@ let create0 doms =
 			port, interface
 		)
 		in
-	let dom = Domain.make 0 Nativeint.zero port interface doms.eventchn in
+	let dom = Domain.make 0 port interface doms.eventchn in
 	Hashtbl.add doms.table 0 dom;
 	Domain.bind_interdomain dom;
 	Domain.notify dom;
diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
index ff5c9484fc..73d7411e59 100644
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -97,10 +97,6 @@ let do_debug con t _domains cons data =
 	| "watches" :: _ ->
 		let watches = Connections.debug cons in
 		Some (watches ^ "\000")
-	| "mfn" :: domid :: _ ->
-		let domid = int_of_string domid in
-		let con = Connections.find_domain cons domid in
-		may (fun dom -> Printf.sprintf "%nd\000" (Domain.get_mfn dom)) (Connection.get_domain con)
 	| _ -> None
 	with _ -> None
 
@@ -409,20 +405,18 @@ let do_introduce con _t domains cons data =
 	let dom =
 		if Domains.exist domains domid then
 			let edom = Domains.find domains domid in
-			if (Domain.get_mfn edom) = mfn && (Connections.find_domain cons domid) != con then begin
-				(* Use XS_INTRODUCE for recreating the xenbus event-channel. *)
-				edom.remote_port <- port;
-				Domain.bind_interdomain edom;
-			end;
+			(* Use XS_INTRODUCE for recreating the xenbus event-channel. *)
+			edom.remote_port <- port;
+			Domain.bind_interdomain edom;
 			edom
 		else try
-			let ndom = Domains.create domains domid mfn port in
+			let ndom = Domains.create domains domid port in
 			Connections.add_domain cons ndom;
 			Connections.fire_spec_watches cons "@introduceDomain";
 			ndom
 		with _ -> raise Invalid_Cmd_Args
 	in
-	if (Domain.get_remote_port dom) <> port || (Domain.get_mfn dom) <> mfn then
+	if (Domain.get_remote_port dom) <> port then
 		raise Domain_not_match
 
 let do_release con _t domains cons data =
diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
index a232e4c616..0e23bec69d 100644
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -152,9 +152,9 @@ let from_channel_f chan domain_f watch_f store_f =
 			let l = String.split ',' line in
 			try
 				match l with
-				| "dom" :: domid :: mfn :: port :: []->
+				| "dom" :: domid :: _ :: port :: []
+				| "dom" :: domid :: port :: [] ->
 					domain_f (int_of_string domid)
-					         (Nativeint.of_string mfn)
 					         (int_of_string port)
 				| "watch" :: domid :: path :: token :: [] ->
 					watch_f (int_of_string domid)
@@ -178,10 +178,10 @@ let from_channel store cons doms chan =
 	(* don't let the permission get on our way, full perm ! *)
 	let op = Store.get_ops store Perms.Connection.full_rights in
 
-	let domain_f domid mfn port =
+	let domain_f domid port =
 		let ndom =
 			if domid > 0 then
-				Domains.create doms domid mfn port
+				Domains.create doms domid port
 			else
 				Domains.create0 doms
 			in
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v1 8/9] tools/ocaml/libs/mmap: mark mmap/munmap as blocking
  2020-08-27 17:35 [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range Edwin Török
                   ` (6 preceding siblings ...)
  2020-08-27 17:35 ` [PATCH v1 7/9] tools/ocaml/xenstored: don't store domU's mfn of ring page Edwin Török
@ 2020-08-27 17:35 ` Edwin Török
  2020-08-27 17:36 ` [PATCH v1 9/9] tools/ocaml/libs/mmap: Clean up unused read/write Edwin Török
  8 siblings, 0 replies; 10+ messages in thread
From: Edwin Török @ 2020-08-27 17:35 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Christian Lindig, David Scott,
	Ian Jackson, Wei Liu

These functions can potentially take some time,
so allow other OCaml code to proceed meanwhile (if any).

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/libs/mmap/xenmmap_stubs.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/ocaml/libs/mmap/xenmmap_stubs.c b/tools/ocaml/libs/mmap/xenmmap_stubs.c
index 9c1126c6a2..21feceea0e 100644
--- a/tools/ocaml/libs/mmap/xenmmap_stubs.c
+++ b/tools/ocaml/libs/mmap/xenmmap_stubs.c
@@ -28,6 +28,7 @@
 #include <caml/fail.h>
 #include <caml/callback.h>
 #include <caml/unixsupport.h>
+#include <caml/signals.h>
 
 #define Wsize_bsize_round(n) (Wsize_bsize( (n) + sizeof(value) - 1 ))
 
@@ -69,7 +70,9 @@ CAMLprim value stub_mmap_init(value fd, value pflag, value mflag,
 		caml_invalid_argument("negative offset");
 	length = Int_val(len);
 
+	caml_enter_blocking_section();
 	addr = mmap(NULL, length, c_pflag, c_mflag, fd, Int_val(offset));
+	caml_leave_blocking_section();
 	if (MAP_FAILED == addr)
 		uerror("mmap", Nothing);
 
@@ -80,10 +83,15 @@ CAMLprim value stub_mmap_init(value fd, value pflag, value mflag,
 CAMLprim value stub_mmap_final(value intf)
 {
 	CAMLparam1(intf);
+	struct mmap_interface interface = *Intf_val(intf);
 
-	if (Intf_val(intf)->addr != MAP_FAILED)
-		munmap(Intf_val(intf)->addr, Intf_val(intf)->len);
+	/* mark it as freed, in case munmap below fails, so we don't retry it */
 	Intf_val(intf)->addr = MAP_FAILED;
+	if (interface.addr != MAP_FAILED) {
+		caml_enter_blocking_section();
+		munmap(interface.addr, interface.len);
+		caml_leave_blocking_section();
+	}
 
 	CAMLreturn(Val_unit);
 }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH v1 9/9] tools/ocaml/libs/mmap: Clean up unused read/write
  2020-08-27 17:35 [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range Edwin Török
                   ` (7 preceding siblings ...)
  2020-08-27 17:35 ` [PATCH v1 8/9] tools/ocaml/libs/mmap: mark mmap/munmap as blocking Edwin Török
@ 2020-08-27 17:36 ` Edwin Török
  8 siblings, 0 replies; 10+ messages in thread
From: Edwin Török @ 2020-08-27 17:36 UTC (permalink / raw)
  To: xen-devel
  Cc: Edwin Török, Christian Lindig, David Scott,
	Ian Jackson, Wei Liu

Xenmmap is only modified by the ring functions,
these functions are unused.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/libs/mmap/xenmmap.ml      |  5 ----
 tools/ocaml/libs/mmap/xenmmap.mli     |  4 ---
 tools/ocaml/libs/mmap/xenmmap_stubs.c | 41 ---------------------------
 3 files changed, 50 deletions(-)

diff --git a/tools/ocaml/libs/mmap/xenmmap.ml b/tools/ocaml/libs/mmap/xenmmap.ml
index af258942a0..e17a62e607 100644
--- a/tools/ocaml/libs/mmap/xenmmap.ml
+++ b/tools/ocaml/libs/mmap/xenmmap.ml
@@ -24,11 +24,6 @@ type mmap_map_flag = SHARED | PRIVATE
 (* mmap: fd -> prot_flag -> map_flag -> length -> offset -> interface *)
 external mmap': Unix.file_descr -> mmap_prot_flag -> mmap_map_flag
 		-> int -> int -> mmap_interface = "stub_mmap_init"
-(* read: interface -> start -> length -> data *)
-external read: mmap_interface -> int -> int -> string = "stub_mmap_read"
-(* write: interface -> data -> start -> length -> unit *)
-external write: mmap_interface -> string -> int -> int -> unit = "stub_mmap_write"
-(* getpagesize: unit -> size of page *)
 external unmap': mmap_interface -> unit = "stub_mmap_final"
 (* getpagesize: unit -> size of page *)
 let make ?(unmap=unmap') interface = interface, unmap
diff --git a/tools/ocaml/libs/mmap/xenmmap.mli b/tools/ocaml/libs/mmap/xenmmap.mli
index 075b24eab4..abf2a50131 100644
--- a/tools/ocaml/libs/mmap/xenmmap.mli
+++ b/tools/ocaml/libs/mmap/xenmmap.mli
@@ -19,10 +19,6 @@ type mmap_interface
 type mmap_prot_flag = RDONLY | WRONLY | RDWR
 type mmap_map_flag = SHARED | PRIVATE
 
-external read : mmap_interface -> int -> int -> string = "stub_mmap_read"
-external write : mmap_interface -> string -> int -> int -> unit
-               = "stub_mmap_write"
-
 val mmap : Unix.file_descr -> mmap_prot_flag -> mmap_map_flag -> int -> int -> t
 val unmap : t -> unit
 
diff --git a/tools/ocaml/libs/mmap/xenmmap_stubs.c b/tools/ocaml/libs/mmap/xenmmap_stubs.c
index 21feceea0e..ec0431efb5 100644
--- a/tools/ocaml/libs/mmap/xenmmap_stubs.c
+++ b/tools/ocaml/libs/mmap/xenmmap_stubs.c
@@ -96,47 +96,6 @@ CAMLprim value stub_mmap_final(value intf)
 	CAMLreturn(Val_unit);
 }
 
-CAMLprim value stub_mmap_read(value intf, value start, value len)
-{
-	CAMLparam3(intf, start, len);
-	CAMLlocal1(data);
-	int c_start;
-	int c_len;
-
-	c_start = Int_val(start);
-	c_len = Int_val(len);
-
-	if (c_start > Intf_val(intf)->len)
-		caml_invalid_argument("start invalid");
-	if (c_start + c_len > Intf_val(intf)->len)
-		caml_invalid_argument("len invalid");
-
-	data = caml_alloc_string(c_len);
-	memcpy((char *) data, Intf_val(intf)->addr + c_start, c_len);
-
-	CAMLreturn(data);
-}
-
-CAMLprim value stub_mmap_write(value intf, value data,
-                               value start, value len)
-{
-	CAMLparam4(intf, data, start, len);
-	int c_start;
-	int c_len;
-
-	c_start = Int_val(start);
-	c_len = Int_val(len);
-
-	if (c_start > Intf_val(intf)->len)
-		caml_invalid_argument("start invalid");
-	if (c_start + c_len > Intf_val(intf)->len)
-		caml_invalid_argument("len invalid");
-
-	memcpy(Intf_val(intf)->addr + c_start, (char *) data, c_len);
-
-	CAMLreturn(Val_unit);
-}
-
 CAMLprim value stub_mmap_getpagesize(value unit)
 {
 	CAMLparam1(unit);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-08-27 17:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-27 17:35 [PATCH v1 0/9] tools/ocaml: use gnttab instead of map_foreign_range Edwin Török
2020-08-27 17:35 ` [PATCH v1 1/9] tools/ocaml: use common macros for manipulating mmap_interface Edwin Török
2020-08-27 17:35 ` [PATCH v1 2/9] tools/ocaml/libs/mmap: allocate correct number of bytes Edwin Török
2020-08-27 17:35 ` [PATCH v1 3/9] tools/ocaml/libs/mmap: Expose stub_mmap_alloc Edwin Török
2020-08-27 17:35 ` [PATCH v1 4/9] tools/ocaml/libs/xb: import gnttab stubs from mirage Edwin Török
2020-08-27 17:35 ` [PATCH v1 5/9] tools/ocaml: safer Xenmmap interface Edwin Török
2020-08-27 17:35 ` [PATCH v1 6/9] tools/ocaml/xenstored: use gnttab instead of xenctrl's foreign_map_range Edwin Török
2020-08-27 17:35 ` [PATCH v1 7/9] tools/ocaml/xenstored: don't store domU's mfn of ring page Edwin Török
2020-08-27 17:35 ` [PATCH v1 8/9] tools/ocaml/libs/mmap: mark mmap/munmap as blocking Edwin Török
2020-08-27 17:36 ` [PATCH v1 9/9] tools/ocaml/libs/mmap: Clean up unused read/write Edwin Török

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.