thr3ads.net - Xen devel - [PATCH 0/7] Optionally add libvchan to stubdom [Apr 2013]

If this information is useful, please help other people find it:
Share via:

Marek Marczykowski

2013-Apr-21 01:46 UTC

[PATCH 2/7] libvchan: create xenstore entries in one transaction

This will prevent race when client waits for server with xs_watch - all
entries should appear at once.

Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
---
 tools/libvchan/init.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c
index f0d2505..dbdcc6c 100644
--- a/tools/libvchan/init.c
+++ b/tools/libvchan/init.c
@@ -234,6 +234,7 @@ static int init_xs_srv(struct libxenvchan *ctrl, int domain,
const char* xs_base
 	char buf[64];
 	char ref[16];
 	char* domid_str = NULL;
+	xs_transaction_t xs_trans = NULL;
 	xs = xs_domain_open();
 	if (!xs)
 		goto fail;
@@ -249,20 +250,29 @@ static int init_xs_srv(struct libxenvchan *ctrl, int
domain, const char* xs_base
 	perms[1].id = domain;
 	perms[1].perms = XS_PERM_READ;
 
+retry_transaction:
+	xs_trans = xs_transaction_start(xs);
+	if (!xs_trans)
+		goto fail_xs_open;
+
 	snprintf(ref, sizeof ref, "%d", ring_ref);
 	snprintf(buf, sizeof buf, "%s/ring-ref", xs_base);
-	if (!xs_write(xs, 0, buf, ref, strlen(ref)))
+	if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
 		goto fail_xs_open;
-	if (!xs_set_permissions(xs, 0, buf, perms, 2))
+	if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
 		goto fail_xs_open;
 
 	snprintf(ref, sizeof ref, "%d", ctrl->event_port);
 	snprintf(buf, sizeof buf, "%s/event-channel", xs_base);
-	if (!xs_write(xs, 0, buf, ref, strlen(ref)))
+	if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
 		goto fail_xs_open;
-	if (!xs_set_permissions(xs, 0, buf, perms, 2))
+	if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
 		goto fail_xs_open;
 
+	if (!xs_transaction_end(xs, xs_trans, 0))
+		if (errno == EAGAIN)
+			goto retry_transaction;
+
 	ret = 0;
  fail_xs_open:
 	free(domid_str);
-- 
1.8.1.4

Marek Marczykowski

2013-Apr-22 01:44 UTC

head link

[PATCH 1/7] minios: enhance xenstore available for stubdoms

Update xs*_{open,close} to match new API (mainly introduce xs_open and
xs_close).
Add xs_transaction_{start,end} and xs_{get,set}_permissions - mostly
based on tools/xenstore/xs.c.

Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
---
 extras/mini-os/lib/xs.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 192 insertions(+), 3 deletions(-)

diff --git a/extras/mini-os/lib/xs.c b/extras/mini-os/lib/xs.c
index a2a1220..ff524c3 100644
--- a/extras/mini-os/lib/xs.c
+++ b/extras/mini-os/lib/xs.c
@@ -18,14 +18,25 @@ static inline int _xs_fileno(struct xs_handle *h) {
     return (intptr_t) h;
 }
 
-struct xs_handle *xs_daemon_open()
-{
-    int fd = alloc_fd(FTYPE_XENBUS);
+struct xs_handle *xs_open(unsigned long flags) {
+    int fd;
+    if (flags != 0)
+        return NULL;
+    fd = alloc_fd(FTYPE_XENBUS);
     files[fd].xenbus.events = NULL;
     printk("xs_daemon_open -> %d, %p\n", fd,
&files[fd].xenbus.events);
     return (void*)(intptr_t) fd;
 }
 
+struct xs_handle *xs_daemon_open()
+{
+    return xs_open(0);
+}
+
+struct xs_handle *xs_domain_open() {
+    return xs_open(0);
+}
+
 void xs_daemon_close(struct xs_handle *h)
 {
     int fd = _xs_fileno(h);
@@ -35,6 +46,12 @@ void xs_daemon_close(struct xs_handle *h)
     files[fd].type = FTYPE_NONE;
 }
 
+void xs_close(struct xs_handle *h)
+{
+    if (h)
+        xs_daemon_close(h);
+}
+
 int xs_fileno(struct xs_handle *h)
 {
     return _xs_fileno(h);
@@ -186,4 +203,176 @@ bool xs_unwatch(struct xs_handle *h, const char *path,
const char *token)
     printk("xs_unwatch(%s, %s)\n", path, token);
     return xs_bool(xenbus_unwatch_path_token(XBT_NULL, path, token));
 }
+
+xs_transaction_t xs_transaction_start(struct xs_handle *h)
+{
+    xenbus_transaction_t xbt;
+
+    if (!xs_bool(xenbus_transaction_start(&xbt)))
+        return XBT_NULL;
+    return xbt;
+}
+
+bool xs_transaction_end(struct xs_handle *h, xs_transaction_t t, bool abort)
+{
+    int retry;
+
+    if (!xs_bool(xenbus_transaction_end(t, abort, &retry)))
+        return false;
+    if (retry) {
+        errno = EAGAIN;
+        return false;
+    }
+    return true;
+}
+
+/* Convert strings to permissions.  False if a problem. */
+bool xs_strings_to_perms(struct xs_permissions *perms, unsigned int num,
+			 const char *strings)
+{
+	const char *p;
+	char *end;
+	unsigned int i;
+
+	for (p = strings, i = 0; i < num; i++) {
+		/* "r", "w", or "b" for both. */
+		switch (*p) {
+		case ''r'':
+			perms[i].perms = XS_PERM_READ;
+			break;
+		case ''w'':
+			perms[i].perms = XS_PERM_WRITE;
+			break;
+		case ''b'':
+			perms[i].perms = XS_PERM_READ|XS_PERM_WRITE;
+			break;
+		case ''n'':
+			perms[i].perms = XS_PERM_NONE;
+			break;
+		default:
+			errno = EINVAL;
+			return false;
+		} 
+		p++;
+		perms[i].id = strtol(p, &end, 0);
+		if (*end || !*p) {
+			errno = EINVAL;
+			return false;
+		}
+		p = end + 1;
+	}
+	return true;
+}
+
+/* Convert permissions to a string (up to len MAX_STRLEN(unsigned int)+1). */
+bool xs_perm_to_string(const struct xs_permissions *perm,
+                       char *buffer, size_t buf_len)
+{
+	switch ((int)perm->perms) {
+	case XS_PERM_WRITE:
+		*buffer = ''w'';
+		break;
+	case XS_PERM_READ:
+		*buffer = ''r'';
+		break;
+	case XS_PERM_READ|XS_PERM_WRITE:
+		*buffer = ''b'';
+		break;
+	case XS_PERM_NONE:
+		*buffer = ''n'';
+		break;
+	default:
+		errno = EINVAL;
+		return false;
+	}
+	snprintf(buffer+1, buf_len-1, "%i", (int)perm->id);
+	return true;
+}
+
+/* Given a string and a length, count how many strings (nul terms). */
+unsigned int xs_count_strings(const char *strings, unsigned int len)
+{
+	unsigned int num;
+	const char *p;
+
+	for (p = strings, num = 0; p < strings + len; p++)
+		if (*p == ''\0'')
+			num++;
+
+	return num;
+}
+
+/* Get permissions of node (first element is owner).
+ * Returns malloced array, or NULL: call free() after use.
+ */
+struct xs_permissions *xs_get_permissions(struct xs_handle *h,
+					  xs_transaction_t t,
+					  const char *path, unsigned int *num)
+{
+	char *strings;
+	unsigned int len;
+	struct xs_permissions *ret;
+
+	strings = xs_single(h, t, XS_GET_PERMS, path, &len);
+	if (!strings)
+		return NULL;
+
+	/* Count the strings: each one perms then domid. */
+	*num = xs_count_strings(strings, len);
+
+	/* Transfer to one big alloc for easy freeing. */
+	ret = malloc(*num * sizeof(struct xs_permissions));
+	if (!ret) {
+		free(strings);
+		return NULL;
+	}
+
+	if (!xs_strings_to_perms(ret, *num, strings)) {
+		free(ret);
+		ret = NULL;
+	}
+
+	free(strings);
+	return ret;
+}
+
+/* Set permissions of node (must be owner).
+ * Returns false on failure.
+ */
+bool xs_set_permissions(struct xs_handle *h,
+			xs_transaction_t t,
+			const char *path,
+			struct xs_permissions *perms,
+			unsigned int num_perms)
+{
+	unsigned int i;
+	struct write_req iov[1+num_perms];
+
+	iov[0].data = (void *)path;
+	iov[0].len = strlen(path) + 1;
+
+	for (i = 0; i < num_perms; i++) {
+		char buffer[MAX_STRLEN(unsigned int)+1];
+
+		if (!xs_perm_to_string(&perms[i], buffer, sizeof(buffer)))
+			goto unwind;
+
+		iov[i+1].data = strdup(buffer);
+		iov[i+1].len = strlen(buffer) + 1;
+		if (!iov[i+1].data)
+			goto unwind;
+	}
+
+	if (!xs_bool(xs_talkv(h, t, XS_SET_PERMS, iov, 1+num_perms, NULL)))
+		goto unwind;
+	for (i = 0; i < num_perms; i++)
+		free((void*)iov[i+1].data);
+	return true;
+
+unwind:
+	num_perms = i;
+	for (i = 0; i < num_perms; i++)
+		free((void*)iov[i+1].data);
+	return false;
+}
 #endif
-- 
1.8.1.4

Marek Marczykowski

2013-Apr-22 01:50 UTC

head link

[PATCH 3/7] libvchan: remove unnecessary includes

Those headers aren''t used in libvchan, so remove the includes. This
makes possible to compile libvchan in stubdom (mini-os doesn''t provide
them).

Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
---
 tools/libvchan/init.c        | 1 -
 tools/libvchan/io.c          | 1 -
 tools/libvchan/libxenvchan.h | 1 -
 3 files changed, 3 deletions(-)

diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c
index dbdcc6c..8a66cef 100644
--- a/tools/libvchan/init.c
+++ b/tools/libvchan/init.c
@@ -32,7 +32,6 @@
 #include <sys/types.h>
 #include <sys/mman.h>
 #include <sys/ioctl.h>
-#include <sys/user.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdint.h>
diff --git a/tools/libvchan/io.c b/tools/libvchan/io.c
index 3040099..fea9687 100644
--- a/tools/libvchan/io.c
+++ b/tools/libvchan/io.c
@@ -32,7 +32,6 @@
 #include <sys/types.h>
 #include <sys/mman.h>
 #include <sys/ioctl.h>
-#include <sys/uio.h>
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
diff --git a/tools/libvchan/libxenvchan.h b/tools/libvchan/libxenvchan.h
index 6365d36..07d1b59 100644
--- a/tools/libvchan/libxenvchan.h
+++ b/tools/libvchan/libxenvchan.h
@@ -44,7 +44,6 @@
  */
 
 #include <xen/io/libxenvchan.h>
-#include <xen/sys/evtchn.h>
 #include <xenctrl.h>
 
 struct libxenvchan_ring {
-- 
1.8.1.4

Marek Marczykowski

2013-Apr-22 02:10 UTC

head link

[PATCH 6/7] stubdom: make libvchan available in stubdom

Add CONFIG_VCHAN to enable libvchan build for stubdom. By default
disabled in all cases, but one can enable it when needed.

To enable libvchan in your stubdom, you need CONFIG_VCHAN=y in
minios.conf and append "libvchan" to target dependencies in
stubdom/Makefile

Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
---
 .gitignore              |  1 +
 extras/mini-os/Makefile |  5 +++++
 stubdom/Makefile        | 14 ++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/.gitignore b/.gitignore
index 6753368..d7787ff 100644
--- a/.gitignore
+++ b/.gitignore
@@ -127,6 +127,7 @@ stubdom/ocaml-*
 stubdom/polarssl-*
 stubdom/gmp-*
 stubdom/tpm_emulator-*
+stubdom/libvchan-*
 stubdom/lwip/
 stubdom/ioemu/
 stubdom/stubdompath.sh
diff --git a/extras/mini-os/Makefile b/extras/mini-os/Makefile
index 50d038b..fc85f34 100644
--- a/extras/mini-os/Makefile
+++ b/extras/mini-os/Makefile
@@ -32,6 +32,7 @@ CONFIG_CONSFRONT ?= y
 CONFIG_XENBUS ?= y
 CONFIG_XC ?=y
 CONFIG_LWIP ?= $(lwip)
+CONFIG_VCHAN ?= n
 
 # Export config items as compiler directives
 flags-$(CONFIG_START_NETWORK) += -DCONFIG_START_NETWORK
@@ -163,6 +164,10 @@ APP_LDLIBS += -lm
 LDLIBS += -lc
 endif
 
+ifeq ($(CONFIG_VCHAN),y)
+APP_LDLIBS += -L$(XEN_ROOT)/stubdom/libvchan-$(XEN_TARGET_ARCH) -lxenvchan
+endif
+
 ifneq ($(APP_OBJS)-$(lwip),-y)
 OBJS := $(filter-out $(OBJ_DIR)/daytime.o, $(OBJS))
 endif
diff --git a/stubdom/Makefile b/stubdom/Makefile
index 427e4d6..893148a 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -309,6 +309,11 @@ mk-headers-$(XEN_TARGET_ARCH): ioemu/linkfarm.stamp
 	  ln -sf $(XEN_ROOT)/tools/libxc/$(XEN_TARGET_ARCH)/*.c . && \
 	  ln -sf $(XEN_ROOT)/tools/libxc/$(XEN_TARGET_ARCH)/*.h . && \
 	  ln -sf $(XEN_ROOT)/tools/libxc/$(XEN_TARGET_ARCH)/Makefile . )
+	mkdir -p libvchan-$(XEN_TARGET_ARCH)
+	[ -h libvchan-$(XEN_TARGET_ARCH) ] || ( cd libvchan-$(XEN_TARGET_ARCH)
&& \
+	  ln -sf $(XEN_ROOT)/tools/libvchan/*.c . && \
+	  ln -sf $(XEN_ROOT)/tools/libvchan/*.h . && \
+	  ln -sf $(XEN_ROOT)/tools/libvchan/Makefile . )
 	mkdir -p xenstore
 	[ -h xenstore/Makefile ] || ( cd xenstore && \
 	  ln -sf $(XEN_ROOT)/tools/xenstore/*.c . && \
@@ -336,6 +341,15 @@ libxc-$(XEN_TARGET_ARCH)/libxenctrl.a: cross-zlib
  libxc-$(XEN_TARGET_ARCH)/libxenguest.a: libxc-$(XEN_TARGET_ARCH)/libxenctrl.a
 
 #######
+# libvchan
+#######
+
+.PHONY: libvchan
+libvchan: libvchan-$(XEN_TARGET_ARCH)/libxenvchan.a
+libvchan-$(XEN_TARGET_ARCH)/libxenvchan.a:
+	CPPFLAGS="$(TARGET_CPPFLAGS)" CFLAGS="$(TARGET_CFLAGS)"
$(MAKE) DESTDIR= -C libvchan-$(XEN_TARGET_ARCH) libxenvchan.a
+
+#######
 # ioemu
 #######
 
-- 
1.8.1.4

Marek Marczykowski

2013-Apr-23 03:16 UTC

head link

[PATCH 4/7] minios: add gnttab_find_grant_of_page

Needed to implement minios_gntshr_munmap without introducing additional
mapping (frame->gref).

Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
---
 extras/mini-os/gnttab.c         | 23 +++++++++++++++++++++++
 extras/mini-os/include/gnttab.h |  1 +
 2 files changed, 24 insertions(+)

diff --git a/extras/mini-os/gnttab.c b/extras/mini-os/gnttab.c
index 2f1b3d7..ec32e69 100644
--- a/extras/mini-os/gnttab.c
+++ b/extras/mini-os/gnttab.c
@@ -157,6 +157,29 @@ gnttab_alloc_and_grant(void **map)
     return gref;
 }
 
+/* This function is used by gnttab_munmap call. It is rather rare to call it -
+ * mostly at sthudown, so save memory at price of efficiency */
+grant_ref_t
+gnttab_find_grant_of_page(void *map)
+{
+    unsigned long flags;
+    int i, mfn;
+    grant_ref_t ref = 0;
+
+    mfn = virt_to_mfn(map);
+    local_irq_save(flags);
+    for (i = NR_RESERVED_ENTRIES; i < NR_GRANT_ENTRIES; i++) {
+        if (!gnttab_table[i].flags)
+            continue;
+        if (gnttab_table[i].frame == mfn) {
+            ref = i;
+            break;
+        }
+    }
+    local_irq_restore(flags);
+    return ref;
+}
+
 static const char * const gnttabop_error_msgs[] = GNTTABOP_error_msgs;
 
 const char *
diff --git a/extras/mini-os/include/gnttab.h b/extras/mini-os/include/gnttab.h
index acd6c39..fb69daf 100644
--- a/extras/mini-os/include/gnttab.h
+++ b/extras/mini-os/include/gnttab.h
@@ -10,6 +10,7 @@ grant_ref_t gnttab_grant_access(domid_t domid, unsigned long
frame,
 grant_ref_t gnttab_grant_transfer(domid_t domid, unsigned long pfn);
 unsigned long gnttab_end_transfer(grant_ref_t gref);
 int gnttab_end_access(grant_ref_t ref);
+grant_ref_t gnttab_find_grant_of_page(void *map);
 const char *gnttabop_error(int16_t status);
 void fini_gnttab(void);
 
-- 
1.8.1.4

Marek Marczykowski

2013-Apr-23 03:17 UTC

head link

[PATCH 5/7] libxc: implement gntshr for minios

Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
---
 tools/libxc/xc_minios.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/tools/libxc/xc_minios.c b/tools/libxc/xc_minios.c
index dec4d73..eeaf966 100644
--- a/tools/libxc/xc_minios.c
+++ b/tools/libxc/xc_minios.c
@@ -26,6 +26,7 @@
 #include <mini-os/mm.h>
 #include <mini-os/lib.h>
 #include <mini-os/gntmap.h>
+#include <mini-os/gnttab.h>
 #include <mini-os/events.h>
 #include <mini-os/wait.h>
 #include <sys/mman.h>
@@ -519,6 +520,74 @@ static struct xc_osdep_ops minios_gnttab_ops = {
     },
 };
 
+static xc_osdep_handle minios_gntshr_open(xc_gntshr *xcg)
+{
+    return (xc_osdep_handle)0;
+}
+
+static int minios_gntshr_close(xc_gntshr *xcg, xc_osdep_handle h)
+{
+    return 0;
+}
+
+static void *minios_gntshr_share_pages(xc_gntshr *xch, xc_osdep_handle h,
+                                      uint32_t domid, int count,
+                                      uint32_t *refs, int writable,
+                                      uint32_t notify_offset,
+                                      evtchn_port_t notify_port)
+{
+    int i, mfn;
+    void *area = NULL;
+
+    if (notify_offset != -1 || notify_port != -1) {
+        printf("minios_gntshr_share_pages: notify not
implemented\n");
+        errno = ENOSYS;
+        return NULL;
+    }
+
+    area = memalign(XC_PAGE_SIZE, count * XC_PAGE_SIZE);
+
+    if (!area) {
+        PERROR("minios_gntshr_share_pages: alloc failed");
+        return NULL;
+    }
+
+    for (i = 0; i < count; i++) {
+        mfn = virtual_to_mfn(area + i*XC_PAGE_SIZE);
+        refs[i] = gnttab_grant_access(domid, mfn, !writable);
+    }
+
+    return area;
+}
+
+static int minios_gntshr_munmap(xc_gntshr *xcg, xc_osdep_handle h,
+                               void *start_address, uint32_t count)
+{
+    int i;
+    uint32_t gref;
+
+    for (i = 0; i < count; i++) {
+        gref = gnttab_find_grant_of_page(start_address + i * PAGE_SIZE);
+        if (!gref) {
+            errno = EINVAL;
+            return -1;
+        }
+        gnttab_end_access(gref);
+    }
+
+    return 0;
+}
+
+static struct xc_osdep_ops minios_gntshr_ops = {
+    .open = &minios_gntshr_open,
+    .close = &minios_gntshr_close,
+
+    .u.gntshr = {
+        .share_pages = &minios_gntshr_share_pages,
+        .munmap = &minios_gntshr_munmap,
+    },
+};
+
 static struct xc_osdep_ops *minios_osdep_init(xc_interface *xch, enum
xc_osdep_type type)
 {
     switch ( type )
@@ -529,6 +598,8 @@ static struct xc_osdep_ops *minios_osdep_init(xc_interface
*xch, enum xc_osdep_t
         return &minios_evtchn_ops;
     case XC_OSDEP_GNTTAB:
         return &minios_gnttab_ops;
+    case XC_OSDEP_GNTSHR:
+        return &minios_gntshr_ops;
     default:
         return NULL;
     }
-- 
1.8.1.4

Marek Marczykowski

2013-Apr-23 03:25 UTC

head link

[PATCH 7/7] libvchan: do not use xc_gntshr_share_page_notify in Mini-OS

It isn''t implemented there yet.

Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
---
 tools/libvchan/init.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c
index 8a66cef..54aa08e 100644
--- a/tools/libvchan/init.c
+++ b/tools/libvchan/init.c
@@ -78,9 +78,14 @@ static int init_gnt_srv(struct libxenvchan *ctrl, int domain)
 	uint32_t ring_ref = -1;
 	void *ring;
 
+#ifdef __MINIOS__
+	/* _notify part not implemented in Mini-OS */
+	ring = xc_gntshr_share_page(ctrl->gntshr, domain, &ring_ref, 1);
+#else
 	ring = xc_gntshr_share_page_notify(ctrl->gntshr, domain,
 			&ring_ref, 1, offsetof(struct vchan_interface, srv_live),
 			ctrl->event_port);
+#endif
 
 	if (!ring)
 		goto out;
-- 
1.8.1.4

Marek Marczykowski

2013-Apr-26 14:45 UTC

head link

[PATCH 0/7] Optionally add libvchan to stubdom

This patch series enhances stubdomain with libvchan. It is disabled by default,
but can be easily enabled in mini-os.cfg when needed.
In order to do so, some parts of minios needs to be extended:
 - xenstore (to support transactions)
 - libxc - to support xc_gntshr

This series assumes "libxc: fix xc_gntshr_munmap" already applied.

One unsolved problem is implementation of xc_gntshr_share_page_notify. On Linux,
there is kernel, which can notify remote in case of server process death. In
stubdom there is no such place - if exit() is called anywhere, the whole
stubdom is terminated. Also the common case is stubdom destruction (the case
for ioemu stubdom - it is destroyed by toolstack at domain shutdown), I
don''t
know if stubdom even support clean shutdown.
Anyway I can implement some notification (which needs some additional info
stored by minios version of gnttab), just for clean libxenvchan_close(). But it
is rather rare case.
Some workaround can be checking for domain state at client side, but IMHO it
isn''t the best way (although still used in original Qubes OS vchan
version).
Any ideas?

Patch 2 is independent - should be also useful for normal Linux case.

Marek Marczykowski (7):
  minios: enhance xenstore available for stubdoms
  libvchan: create xenstore entries in one transaction
  libvchan: remove unnecessary includes
  minios: add gnttab_find_grant_of_page
  libxc: implement gntshr for minios
  stubdom: make libvchan available in stubdom
  libvchan: do not use xc_gntshr_share_page_notify in Mini-OS

 .gitignore                      |   1 +
 extras/mini-os/Makefile         |   5 ++
 extras/mini-os/gnttab.c         |  23 +++++
 extras/mini-os/include/gnttab.h |   1 +
 extras/mini-os/lib/xs.c         | 195 +++++++++++++++++++++++++++++++++++++++-
 stubdom/Makefile                |  14 +++
 tools/libvchan/init.c           |  24 +++--
 tools/libvchan/io.c             |   1 -
 tools/libvchan/libxenvchan.h    |   1 -
 tools/libxc/xc_minios.c         |  71 +++++++++++++++
 10 files changed, 326 insertions(+), 10 deletions(-)

-- 
1.8.1.4

George Dunlap

2013-Apr-26 14:55 UTC

head link

Re: [PATCH 0/7] Optionally add libvchan to stubdom

On Fri, Apr 26, 2013 at 3:45 PM, Marek Marczykowski
<marmarek@invisiblethingslab.com> wrote:> This patch series enhances stubdomain with libvchan. It is disabled by
default,
> but can be easily enabled in mini-os.cfg when needed.
> In order to do so, some parts of minios needs to be extended:
>  - xenstore (to support transactions)
>  - libxc - to support xc_gntshr
>
> This series assumes "libxc: fix xc_gntshr_munmap" already
applied.
>
> One unsolved problem is implementation of xc_gntshr_share_page_notify. On
Linux,
> there is kernel, which can notify remote in case of server process death.
In
> stubdom there is no such place - if exit() is called anywhere, the whole
> stubdom is terminated. Also the common case is stubdom destruction (the
case
> for ioemu stubdom - it is destroyed by toolstack at domain shutdown), I
don''t
> know if stubdom even support clean shutdown.
> Anyway I can implement some notification (which needs some additional info
> stored by minios version of gnttab), just for clean libxenvchan_close().
But it
> is rather rare case.
> Some workaround can be checking for domain state at client side, but IMHO
it
> isn''t the best way (although still used in original Qubes OS vchan
version).
> Any ideas?
>
> Patch 2 is independent - should be also useful for normal Linux case.
I assume your targeting this for 4.4?

 -George

Marek Marczykowski

2013-Apr-26 14:56 UTC

head link

Re: [PATCH 0/7] Optionally add libvchan to stubdom

On 26.04.2013 16:55, George Dunlap wrote:> On Fri, Apr 26, 2013 at 3:45 PM, Marek Marczykowski
> <marmarek@invisiblethingslab.com> wrote:
>> This patch series enhances stubdomain with libvchan. It is disabled by
default,
>> but can be easily enabled in mini-os.cfg when needed.
>> In order to do so, some parts of minios needs to be extended:
>>  - xenstore (to support transactions)
>>  - libxc - to support xc_gntshr
>>
>> This series assumes "libxc: fix xc_gntshr_munmap" already
applied.
>>
>> One unsolved problem is implementation of xc_gntshr_share_page_notify.
On Linux,
>> there is kernel, which can notify remote in case of server process
death. In
>> stubdom there is no such place - if exit() is called anywhere, the
whole
>> stubdom is terminated. Also the common case is stubdom destruction (the
case
>> for ioemu stubdom - it is destroyed by toolstack at domain shutdown), I
don''t
>> know if stubdom even support clean shutdown.
>> Anyway I can implement some notification (which needs some additional
info
>> stored by minios version of gnttab), just for clean
libxenvchan_close(). But it
>> is rather rare case.
>> Some workaround can be checking for domain state at client side, but
IMHO it
>> isn''t the best way (although still used in original Qubes OS
vchan version).
>> Any ideas?
>>
>> Patch 2 is independent - should be also useful for normal Linux case.
> 
> I assume your targeting this for 4.4?
Probably yes, I assume it is too late for 4.3, right?

-- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

George Dunlap

2013-Apr-26 14:57 UTC

head link

Re: [PATCH 0/7] Optionally add libvchan to stubdom

On 26/04/13 15:56, Marek Marczykowski wrote:> On 26.04.2013 16:55, George Dunlap wrote:
>> On Fri, Apr 26, 2013 at 3:45 PM, Marek Marczykowski
>> <marmarek@invisiblethingslab.com> wrote:
>>> This patch series enhances stubdomain with libvchan. It is disabled
by default,
>>> but can be easily enabled in mini-os.cfg when needed.
>>> In order to do so, some parts of minios needs to be extended:
>>>   - xenstore (to support transactions)
>>>   - libxc - to support xc_gntshr
>>>
>>> This series assumes "libxc: fix xc_gntshr_munmap" already
applied.
>>>
>>> One unsolved problem is implementation of
xc_gntshr_share_page_notify. On Linux,
>>> there is kernel, which can notify remote in case of server process
death. In
>>> stubdom there is no such place - if exit() is called anywhere, the
whole
>>> stubdom is terminated. Also the common case is stubdom destruction
(the case
>>> for ioemu stubdom - it is destroyed by toolstack at domain
shutdown), I don''t
>>> know if stubdom even support clean shutdown.
>>> Anyway I can implement some notification (which needs some
additional info
>>> stored by minios version of gnttab), just for clean
libxenvchan_close(). But it
>>> is rather rare case.
>>> Some workaround can be checking for domain state at client side,
but IMHO it
>>> isn''t the best way (although still used in original Qubes
OS vchan version).
>>> Any ideas?
>>>
>>> Patch 2 is independent - should be also useful for normal Linux
case.
>> I assume your targeting this for 4.4?
> Probably yes, I assume it is too late for 4.3, right?
Unless you''ve got a really amazing justification, yes. :-)

  -George

Marek Marczykowski

2013-Apr-26 15:02 UTC

head link

Re: [PATCH 0/7] Optionally add libvchan to stubdom

On 26.04.2013 16:57, George Dunlap wrote:> On 26/04/13 15:56, Marek Marczykowski wrote:
>> On 26.04.2013 16:55, George Dunlap wrote:
>>> On Fri, Apr 26, 2013 at 3:45 PM, Marek Marczykowski
>>> <marmarek@invisiblethingslab.com> wrote:
>>>> This patch series enhances stubdomain with libvchan. It is
disabled by
>>>> default,
>>>> but can be easily enabled in mini-os.cfg when needed.
>>>> In order to do so, some parts of minios needs to be extended:
>>>>   - xenstore (to support transactions)
>>>>   - libxc - to support xc_gntshr
>>>>
>>>> This series assumes "libxc: fix xc_gntshr_munmap"
already applied.
>>>>
>>>> One unsolved problem is implementation of
xc_gntshr_share_page_notify. On
>>>> Linux,
>>>> there is kernel, which can notify remote in case of server
process death. In
>>>> stubdom there is no such place - if exit() is called anywhere,
the whole
>>>> stubdom is terminated. Also the common case is stubdom
destruction (the case
>>>> for ioemu stubdom - it is destroyed by toolstack at domain
shutdown), I don''t
>>>> know if stubdom even support clean shutdown.
>>>> Anyway I can implement some notification (which needs some
additional info
>>>> stored by minios version of gnttab), just for clean
libxenvchan_close().
>>>> But it
>>>> is rather rare case.
>>>> Some workaround can be checking for domain state at client
side, but IMHO it
>>>> isn''t the best way (although still used in original
Qubes OS vchan version).
>>>> Any ideas?
>>>>
>>>> Patch 2 is independent - should be also useful for normal Linux
case.
>>> I assume your targeting this for 4.4?
>> Probably yes, I assume it is too late for 4.3, right?
> 
> Unless you''ve got a really amazing justification, yes. :-)
Hmm... let me try: "it is disable in default build". But it
isn''t entirely
true, especially not for patch no 2.

Anyway I''ll be happy to hear some comments on this.

-- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Ian Campbell

2013-Apr-26 15:03 UTC

head link

Re: [PATCH 0/7] Optionally add libvchan to stubdom

The maintainers for most of this code are Samuel (stubdoms) and Daniel
(libvchan). I''ve CCd them both.

I assume this isn''t being proposed for 4.3 at this stage?

On Fri, 2013-04-26 at 15:45 +0100, Marek Marczykowski
wrote:> This patch series enhances stubdomain with libvchan. It is disabled by
default,
> but can be easily enabled in mini-os.cfg when needed.
> In order to do so, some parts of minios needs to be extended:
>  - xenstore (to support transactions)
>  - libxc - to support xc_gntshr
> 
> This series assumes "libxc: fix xc_gntshr_munmap" already
applied.
> 
> One unsolved problem is implementation of xc_gntshr_share_page_notify. On
Linux,
> there is kernel, which can notify remote in case of server process death.
In
> stubdom there is no such place - if exit() is called anywhere, the whole
> stubdom is terminated. Also the common case is stubdom destruction (the
case
> for ioemu stubdom - it is destroyed by toolstack at domain shutdown), I
don''t
> know if stubdom even support clean shutdown.
> Anyway I can implement some notification (which needs some additional info
> stored by minios version of gnttab), just for clean libxenvchan_close().
But it
> is rather rare case.
> Some workaround can be checking for domain state at client side, but IMHO
it
> isn''t the best way (although still used in original Qubes OS vchan
version).
> Any ideas?
> 
> Patch 2 is independent - should be also useful for normal Linux case.
> 
> Marek Marczykowski (7):
>   minios: enhance xenstore available for stubdoms
>   libvchan: create xenstore entries in one transaction
>   libvchan: remove unnecessary includes
>   minios: add gnttab_find_grant_of_page
>   libxc: implement gntshr for minios
>   stubdom: make libvchan available in stubdom
>   libvchan: do not use xc_gntshr_share_page_notify in Mini-OS
> 
>  .gitignore                      |   1 +
>  extras/mini-os/Makefile         |   5 ++
>  extras/mini-os/gnttab.c         |  23 +++++
>  extras/mini-os/include/gnttab.h |   1 +
>  extras/mini-os/lib/xs.c         | 195
+++++++++++++++++++++++++++++++++++++++-
>  stubdom/Makefile                |  14 +++
>  tools/libvchan/init.c           |  24 +++--
>  tools/libvchan/io.c             |   1 -
>  tools/libvchan/libxenvchan.h    |   1 -
>  tools/libxc/xc_minios.c         |  71 +++++++++++++++
>  10 files changed, 326 insertions(+), 10 deletions(-)
>

Daniel De Graaf

2013-Apr-26 16:41 UTC

head link

Re: [PATCH 0/7] Optionally add libvchan to stubdom

On 04/26/2013 11:03 AM, Ian Campbell wrote:> The maintainers for most of this code are Samuel (stubdoms) and Daniel
> (libvchan). I''ve CCd them both.
>
> I assume this isn''t being proposed for 4.3 at this stage?
>
> On Fri, 2013-04-26 at 15:45 +0100, Marek Marczykowski wrote:
>> This patch series enhances stubdomain with libvchan. It is disabled by
default,
>> but can be easily enabled in mini-os.cfg when needed.
>> In order to do so, some parts of minios needs to be extended:
>>   - xenstore (to support transactions)
>>   - libxc - to support xc_gntshr
>>
>> This series assumes "libxc: fix xc_gntshr_munmap" already
applied.
>>
>> One unsolved problem is implementation of xc_gntshr_share_page_notify.
On Linux,
>> there is kernel, which can notify remote in case of server process
death. In
>> stubdom there is no such place - if exit() is called anywhere, the
whole
>> stubdom is terminated. Also the common case is stubdom destruction (the
case
>> for ioemu stubdom - it is destroyed by toolstack at domain shutdown), I
don''t
>> know if stubdom even support clean shutdown.
This also mirrors the case where the Linux kernel running a libvchan process has
a crash: there is no way for notification to happen in that case.
>> Anyway I can implement some notification (which needs some additional
info
>> stored by minios version of gnttab), just for clean
libxenvchan_close(). But it
>> is rather rare case.
If it''s not already done, the close function should do the byte clear
on its own so
that a mini-os application doing a proper teardown (perhaps using atexit to
handle
calls to exit()) would notify the other end the same way as a process under
Linux.
>> Some workaround can be checking for domain state at client side, but
IMHO it
>> isn''t the best way (although still used in original Qubes OS
vchan version).
>> Any ideas?
The only way to properly handle a domain crash on the other side is to watch for
that - probably via xenstore watches. This all depends on what exactly the vchan
is being used for - a domain crashing may trigger other actions that cause the
vchan client to be notified independent of the vchan''s own
notifications.
>> Patch 2 is independent - should be also useful for normal Linux case.
I think just this patch may have reason to go in 4.3, especially if you have
something that would see the race condition it fixes.  You''ll need to
discuss
that more with George, however.
>> Marek Marczykowski (7):
>>    minios: enhance xenstore available for stubdoms
>>    libvchan: create xenstore entries in one transaction
>>    libvchan: remove unnecessary includes
>>    minios: add gnttab_find_grant_of_page
>>    libxc: implement gntshr for minios
>>    stubdom: make libvchan available in stubdom
>>    libvchan: do not use xc_gntshr_share_page_notify in Mini-OS
>>
-- 
Daniel De Graaf
National Security Agency

David Vrabel

2013-Apr-26 18:04 UTC

head link

Re: [PATCH 2/7] libvchan: create xenstore entries in one transaction

On 21/04/13 02:46, Marek Marczykowski wrote:> This will prevent race when client waits for server with xs_watch - all
> entries should appear at once.
Aren''t xenstore writes strictly ordered?  The client just needs to read
the keys in the inverse order to avoid races, yes?

David
> 
> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
> ---
>  tools/libvchan/init.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c
> index f0d2505..dbdcc6c 100644
> --- a/tools/libvchan/init.c
> +++ b/tools/libvchan/init.c
> @@ -234,6 +234,7 @@ static int init_xs_srv(struct libxenvchan *ctrl, int
domain, const char* xs_base
>  	char buf[64];
>  	char ref[16];
>  	char* domid_str = NULL;
> +	xs_transaction_t xs_trans = NULL;
>  	xs = xs_domain_open();
>  	if (!xs)
>  		goto fail;
> @@ -249,20 +250,29 @@ static int init_xs_srv(struct libxenvchan *ctrl, int
domain, const char* xs_base
>  	perms[1].id = domain;
>  	perms[1].perms = XS_PERM_READ;
>  
> +retry_transaction:
> +	xs_trans = xs_transaction_start(xs);
> +	if (!xs_trans)
> +		goto fail_xs_open;
> +
>  	snprintf(ref, sizeof ref, "%d", ring_ref);
>  	snprintf(buf, sizeof buf, "%s/ring-ref", xs_base);
> -	if (!xs_write(xs, 0, buf, ref, strlen(ref)))
> +	if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
>  		goto fail_xs_open;
> -	if (!xs_set_permissions(xs, 0, buf, perms, 2))
> +	if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
>  		goto fail_xs_open;
>  
>  	snprintf(ref, sizeof ref, "%d", ctrl->event_port);
>  	snprintf(buf, sizeof buf, "%s/event-channel", xs_base);
> -	if (!xs_write(xs, 0, buf, ref, strlen(ref)))
> +	if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
>  		goto fail_xs_open;
> -	if (!xs_set_permissions(xs, 0, buf, perms, 2))
> +	if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
>  		goto fail_xs_open;
>  
> +	if (!xs_transaction_end(xs, xs_trans, 0))
> +		if (errno == EAGAIN)
> +			goto retry_transaction;
> +
>  	ret = 0;
>   fail_xs_open:
>  	free(domid_str);

Samuel Thibault

2013-Apr-28 22:34 UTC

head link

Re: [PATCH 1/7] minios: enhance xenstore available for stubdoms

Marek Marczykowski, le Mon 22 Apr 2013 03:44:30 +0200, a écrit
:> Add xs_transaction_{start,end} and xs_{get,set}_permissions - mostly
> based on tools/xenstore/xs.c.
It''s probably not a good idea to duplicate this code, we''ll
most
probably forget to apply changes on one to the other. I''m not sure how
to proceed best. Ideally libxenstore could be ported to mini-os. It
however seems like quite a change.

Samuel

Samuel Thibault

2013-Apr-28 22:41 UTC

head link

Re: [PATCH 4/7] minios: add gnttab_find_grant_of_page

Marek Marczykowski, le Tue 23 Apr 2013 05:16:21 +0200, a écrit
:> Needed to implement minios_gntshr_munmap without introducing additional
> mapping (frame->gref).
I agree on the principle, given the comment you have put.
> +/* This function is used by gnttab_munmap call. It is rather rare to call
it -
> + * mostly at sthudown, so save memory at price of efficiency */
Better not introduce the typo, however ;)
> +grant_ref_t
> +gnttab_find_grant_of_page(void *map)
> +{
> +    unsigned long flags;
> +    int i, mfn;
> +    grant_ref_t ref = 0;
> +
> +    mfn = virt_to_mfn(map);
> +    local_irq_save(flags);
Why disabling IRQs?

Samuel

Marek Marczykowski

2013-Apr-28 22:43 UTC

head link

Re: [PATCH 2/7] libvchan: create xenstore entries in one transaction

On 26.04.2013 20:04, David Vrabel wrote:> On 21/04/13 02:46, Marek Marczykowski wrote:
>> This will prevent race when client waits for server with xs_watch - all
>> entries should appear at once.
> 
> Aren''t xenstore writes strictly ordered?  The client just needs to
read
> the keys in the inverse order to avoid races, yes?
IMO this isn''t the best idea - application gives only xenstore
directory to
libxenvchan_*_init, I see no reason to assume anything in application:
a) what entries will be created/expected there,
b) in which order,
c) and it will not be changed in the future.

Adding it in one transaction seems to be better idea. Also this will fix
hypothetical race between two simultaneously started servers - when entries
are added without transaction it can happen to end up with event-channel from
one server and ring-ref from another.

> David
> 
>>
>> Signed-off-by: Marek Marczykowski
<marmarek@invisiblethingslab.com>
>> ---
>>  tools/libvchan/init.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c
>> index f0d2505..dbdcc6c 100644
>> --- a/tools/libvchan/init.c
>> +++ b/tools/libvchan/init.c
>> @@ -234,6 +234,7 @@ static int init_xs_srv(struct libxenvchan *ctrl,
int domain, const char* xs_base
>>  	char buf[64];
>>  	char ref[16];
>>  	char* domid_str = NULL;
>> +	xs_transaction_t xs_trans = NULL;
>>  	xs = xs_domain_open();
>>  	if (!xs)
>>  		goto fail;
>> @@ -249,20 +250,29 @@ static int init_xs_srv(struct libxenvchan *ctrl,
int domain, const char* xs_base
>>  	perms[1].id = domain;
>>  	perms[1].perms = XS_PERM_READ;
>>  
>> +retry_transaction:
>> +	xs_trans = xs_transaction_start(xs);
>> +	if (!xs_trans)
>> +		goto fail_xs_open;
>> +
>>  	snprintf(ref, sizeof ref, "%d", ring_ref);
>>  	snprintf(buf, sizeof buf, "%s/ring-ref", xs_base);
>> -	if (!xs_write(xs, 0, buf, ref, strlen(ref)))
>> +	if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
>>  		goto fail_xs_open;
>> -	if (!xs_set_permissions(xs, 0, buf, perms, 2))
>> +	if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
>>  		goto fail_xs_open;
>>  
>>  	snprintf(ref, sizeof ref, "%d", ctrl->event_port);
>>  	snprintf(buf, sizeof buf, "%s/event-channel", xs_base);
>> -	if (!xs_write(xs, 0, buf, ref, strlen(ref)))
>> +	if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
>>  		goto fail_xs_open;
>> -	if (!xs_set_permissions(xs, 0, buf, perms, 2))
>> +	if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
>>  		goto fail_xs_open;
>>  
>> +	if (!xs_transaction_end(xs, xs_trans, 0))
>> +		if (errno == EAGAIN)
>> +			goto retry_transaction;
>> +
>>  	ret = 0;
>>   fail_xs_open:
>>  	free(domid_str);
> 

-- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Samuel Thibault

2013-Apr-28 22:44 UTC

head link

Re: [PATCH 5/7] libxc: implement gntshr for minios

Marek Marczykowski, le Tue 23 Apr 2013 05:17:59 +0200, a écrit
:> +static int minios_gntshr_munmap(xc_gntshr *xcg, xc_osdep_handle h,
> +                               void *start_address, uint32_t count)
> +{
> +    int i;
> +    uint32_t gref;
> +
> +    for (i = 0; i < count; i++) {
> +        gref = gnttab_find_grant_of_page(start_address + i * PAGE_SIZE);
This can become quite expensive. Can we really not afford allocating the
list of grants, keeping its adresse in the xc_osdep_handle?

Samuel

Samuel Thibault

2013-Apr-28 22:46 UTC

head link

Re: [PATCH 6/7] stubdom: make libvchan available in stubdom

Marek Marczykowski, le Mon 22 Apr 2013 04:10:10 +0200, a écrit
:> Add CONFIG_VCHAN to enable libvchan build for stubdom. By default
> disabled in all cases, but one can enable it when needed.
> 
> To enable libvchan in your stubdom, you need CONFIG_VCHAN=y in
> minios.conf and append "libvchan" to target dependencies in
> stubdom/Makefile
> 
> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

But it''ll depend on the other stubdom changes of course.

Samuel

Samuel Thibault

2013-Apr-28 22:48 UTC

head link

Re: [PATCH 7/7] libvchan: do not use xc_gntshr_share_page_notify in Mini-OS

Marek Marczykowski, le Tue 23 Apr 2013 05:25:00 +0200, a écrit
:> It isn''t implemented there yet.
Doesn''t libvchan need it? It should probably be at least documented
somewhere what does not work in the mini-os case, e.g. stubdom/README.

Samuel

Marek Marczykowski

2013-Apr-28 22:51 UTC

head link

Re: [PATCH 4/7] minios: add gnttab_find_grant_of_page

On 29.04.2013 00:41, Samuel Thibault wrote:> Marek Marczykowski, le Tue 23 Apr 2013 05:16:21 +0200, a écrit :
>> Needed to implement minios_gntshr_munmap without introducing additional
>> mapping (frame->gref).
> 
> I agree on the principle, given the comment you have put.
> 
>> +/* This function is used by gnttab_munmap call. It is rather rare to
call it -
>> + * mostly at sthudown, so save memory at price of efficiency */
> 
> Better not introduce the typo, however ;)
Oops ;)
>> +grant_ref_t
>> +gnttab_find_grant_of_page(void *map)
>> +{
>> +    unsigned long flags;
>> +    int i, mfn;
>> +    grant_ref_t ref = 0;
>> +
>> +    mfn = virt_to_mfn(map);
>> +    local_irq_save(flags);
> 
> Why disabling IRQs?
I''ve just copied it from get_free_entry. But indeed, as I
don''t modify the
table here, probably it can be dropped.

-- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Samuel Thibault

2013-Apr-28 22:52 UTC

head link

Re: [PATCH 0/7] Optionally add libvchan to stubdom

Marek Marczykowski, le Fri 26 Apr 2013 16:45:05 +0200, a écrit
:> One unsolved problem is implementation of xc_gntshr_share_page_notify. On
Linux,
> there is kernel, which can notify remote in case of server process death.
In
> stubdom there is no such place - if exit() is called anywhere, the whole
> stubdom is terminated.
We could implement an atexit.
> Also the common case is stubdom destruction (the case
> for ioemu stubdom - it is destroyed by toolstack at domain shutdown), I
don''t
> know if stubdom even support clean shutdown.
It does since recently, see the app_shutdown() hook.

Samuel

Marek Marczykowski

2013-Apr-28 23:00 UTC

head link

Re: [PATCH 7/7] libvchan: do not use xc_gntshr_share_page_notify in Mini-OS

On 29.04.2013 00:48, Samuel Thibault wrote:> Marek Marczykowski, le Tue 23 Apr 2013 05:25:00 +0200, a écrit :
>> It isn''t implemented there yet.
> 
> Doesn''t libvchan need it? It should probably be at least
documented
> somewhere what does not work in the mini-os case, e.g. stubdom/README.
It can be simulated by manual event channel fire in libxenvchan_close, which I
should do. Will be in the next patch version.
IIUC there is no case in which (properly written) vchan server application
will terminate without ability to libxenvchan_close(), but mini-os will still
be alive and can notify the client.

-- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Marek Marczykowski

2013-Apr-28 23:08 UTC

head link

Re: [PATCH 0/7] Optionally add libvchan to stubdom

On 29.04.2013 00:52, Samuel Thibault wrote:> Marek Marczykowski, le Fri 26 Apr 2013 16:45:05 +0200, a écrit :
>> One unsolved problem is implementation of xc_gntshr_share_page_notify.
On Linux,
>> there is kernel, which can notify remote in case of server process
death. In
>> stubdom there is no such place - if exit() is called anywhere, the
whole
>> stubdom is terminated.
> 
> We could implement an atexit.
> 
>> Also the common case is stubdom destruction (the case
>> for ioemu stubdom - it is destroyed by toolstack at domain shutdown), I
don''t
>> know if stubdom even support clean shutdown.
> 
> It does since recently, see the app_shutdown() hook.
Nice.
Any chances to call it for ioemu stubdomains by libxl (even if main domain
crashes)?
Currently I see no users of app_shutdown(), right?

So one solution is to require all libvchan users register libxenvchan_close
with atexit.

Or perhaps the better idea is implement properly xc_gntshr_share_page_notify
with some callback from shutdown_thread?

-- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Marek Marczykowski

2013-Apr-28 23:17 UTC

head link

Re: [PATCH 5/7] libxc: implement gntshr for minios

On 29.04.2013 00:44, Samuel Thibault wrote:> Marek Marczykowski, le Tue 23 Apr 2013 05:17:59 +0200, a écrit :
>> +static int minios_gntshr_munmap(xc_gntshr *xcg, xc_osdep_handle h,
>> +                               void *start_address, uint32_t count)
>> +{
>> +    int i;
>> +    uint32_t gref;
>> +
>> +    for (i = 0; i < count; i++) {
>> +        gref = gnttab_find_grant_of_page(start_address + i *
PAGE_SIZE);
> 
> This can become quite expensive. Can we really not afford allocating the
> list of grants, keeping its adresse in the xc_osdep_handle?
count is number of pages, so most likely gnttab_find_grant_of_page() will be
called no more than few times. Also most likely munmap will be called very
rarely (connection close).
Anyway I haven''t calculated how much memory such mapping will require
and how
much memory is still available in stubdom. Do you have some numbers?

What can be xc_osdep_handle? In (almost?) every case it is file descriptor.
Can it be some memory pointer?

-- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Samuel Thibault

2013-Apr-28 23:22 UTC

head link

Re: [PATCH 5/7] libxc: implement gntshr for minios

Marek Marczykowski, le Mon 29 Apr 2013 01:17:16 +0200, a écrit
:> On 29.04.2013 00:44, Samuel Thibault wrote:
> > Marek Marczykowski, le Tue 23 Apr 2013 05:17:59 +0200, a écrit :
> >> +static int minios_gntshr_munmap(xc_gntshr *xcg, xc_osdep_handle
h,
> >> +                               void *start_address, uint32_t
count)
> >> +{
> >> +    int i;
> >> +    uint32_t gref;
> >> +
> >> +    for (i = 0; i < count; i++) {
> >> +        gref = gnttab_find_grant_of_page(start_address + i *
PAGE_SIZE);
> > 
> > This can become quite expensive. Can we really not afford allocating
the
> > list of grants, keeping its adresse in the xc_osdep_handle?
> 
> count is number of pages,
But that could be somehow big if using e.g. a 1MiB buffer.
> Also most likely munmap will be called very rarely (connection close).
Quadratic complexity is still not a good thing :)
> Anyway I haven''t calculated how much memory such mapping will
require
Well, just the gref per page, so about 0.1% of the memory you allocate
with memalign.
> and how much memory is still available in stubdom. Do you have some
> numbers?
Well, it''s essentially up to the user.
> What can be xc_osdep_handle? In (almost?) every case it is file descriptor.
> Can it be some memory pointer?
IIRC Xen already assumes that a pointer can fit into an unsigned long,
yes. In the current implementation of minios it is true at least.

Samuel

Samuel Thibault

2013-Apr-30 21:22 UTC

head link

Re: [PATCH 0/7] Optionally add libvchan to stubdom

Marek Marczykowski, le Mon 29 Apr 2013 01:08:52 +0200, a écrit
:> > It does since recently, see the app_shutdown() hook.
> 
> Nice.
> Any chances to call it for ioemu stubdomains by libxl (even if main domain
> crashes)?
I''m not sure what you mean. app_shutdown is called whenever the domain
is shut down (not destroyed).
> Currently I see no users of app_shutdown(), right?
Indeed.
> So one solution is to require all libvchan users register libxenvchan_close
> with atexit.
I''d say libvchan should do it by itself. In a Linux system it would be
the kernel which would clean that for the user. It''s better to make
libvchan behave the same in mini-os.

Gow, atexit can''t be undone, so libvchan could rather call atexit just
once, and record a list of chans to be libxenchan_close()-d in the
registered atexit hook.
> Or perhaps the better idea is implement properly
xc_gntshr_share_page_notify
> with some callback from shutdown_thread?
I''d say both are needed, since termination of the domain can happen
either from the inside (exit()), or from the outside (shutdown).

For the shutdown part, I''m not sure how to translate that into a
posixish interface. I''d tend to say add an atshutdown() call similar
to atexit(), so libvchan can just register what it needs to be done at
shutdown.
> > We could implement an atexit.
BTW, by "We", I mean including you :) (I don''t really have
much time for
this ATM)

Samuel

Xen devel - Apr 2013 - [PATCH 0/7] Optionally add libvchan to stubdom

[PATCH 2/7] libvchan: create xenstore entries in one transaction

[PATCH 1/7] minios: enhance xenstore available for stubdoms

[PATCH 3/7] libvchan: remove unnecessary includes

[PATCH 6/7] stubdom: make libvchan available in stubdom

[PATCH 4/7] minios: add gnttab_find_grant_of_page

[PATCH 5/7] libxc: implement gntshr for minios

[PATCH 7/7] libvchan: do not use xc_gntshr_share_page_notify in Mini-OS

[PATCH 0/7] Optionally add libvchan to stubdom

Re: [PATCH 0/7] Optionally add libvchan to stubdom

Re: [PATCH 0/7] Optionally add libvchan to stubdom

Re: [PATCH 0/7] Optionally add libvchan to stubdom

Re: [PATCH 0/7] Optionally add libvchan to stubdom

Re: [PATCH 0/7] Optionally add libvchan to stubdom

Re: [PATCH 0/7] Optionally add libvchan to stubdom

Re: [PATCH 2/7] libvchan: create xenstore entries in one transaction

Re: [PATCH 1/7] minios: enhance xenstore available for stubdoms

Re: [PATCH 4/7] minios: add gnttab_find_grant_of_page

Re: [PATCH 2/7] libvchan: create xenstore entries in one transaction

Re: [PATCH 5/7] libxc: implement gntshr for minios

Re: [PATCH 6/7] stubdom: make libvchan available in stubdom

Re: [PATCH 7/7] libvchan: do not use xc_gntshr_share_page_notify in Mini-OS

Re: [PATCH 4/7] minios: add gnttab_find_grant_of_page

Re: [PATCH 0/7] Optionally add libvchan to stubdom

Re: [PATCH 7/7] libvchan: do not use xc_gntshr_share_page_notify in Mini-OS

Re: [PATCH 0/7] Optionally add libvchan to stubdom

Re: [PATCH 5/7] libxc: implement gntshr for minios

Re: [PATCH 5/7] libxc: implement gntshr for minios

Re: [PATCH 0/7] Optionally add libvchan to stubdom