All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files
@ 2017-08-27  8:36 Zhongze Liu
  2017-08-27  8:36 ` [PATCH v2 1/6] libxc: add xc_domain_remove_from_physmap to wrap XENMEM_remove_from_physmap Zhongze Liu
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Zhongze Liu @ 2017-08-27  8:36 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, Stefano Stabellini, Zhongze Liu

This series implements the new xl config entry proposed in [1]. Users can use
the new config entry to statically setup shared memory areas among VMs that
don't have grant table support so that they could communicate with each other
through the static shared memory areas.

[1] Proposla to allow setting up shared memory areas between VMs from xl config file:
  https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html

v2:
  * fixed several code style issues.
  * introduce MMU__SHARE_MEM in flask av permissions, and add a check to this.
    permission in the flask hook for xsm_map_gmfn_foreign.
  * support rolling back during creation on partial failure.
  * refcounting the sshm path instead of using "alive" and "zombie" to label the
    master and counting the slaves.

Cheers,

Zhongze Liu (6):
  libxc: add xc_domain_remove_from_physmap to wrap
    XENMEM_remove_from_physmap
  libxl: introduce a new structure to represent static shared memory
    regions
  libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config
    files
  xsm: flask: change the dummy xsm policy and flask hook for
    map_gmfn_foregin
  libxl: support mapping static shared memory areas during domain
    creation
  libxl: support unmapping static shared memory areas during domain
    destruction

 tools/flask/policy/modules/xen.if   |   2 +
 tools/libxc/include/xenctrl.h       |   4 +
 tools/libxc/xc_domain.c             |  11 +
 tools/libxl/Makefile                |   4 +-
 tools/libxl/libxl.h                 |   4 +
 tools/libxl/libxl_arch.h            |   6 +
 tools/libxl/libxl_arm.c             |  15 ++
 tools/libxl/libxl_create.c          |  27 ++
 tools/libxl/libxl_domain.c          |   5 +
 tools/libxl/libxl_internal.h        |  15 ++
 tools/libxl/libxl_sshm.c            | 480 ++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_types.idl         |  34 ++-
 tools/libxl/libxl_x86.c             |  18 ++
 tools/libxl/libxlu_sshm.c           | 210 ++++++++++++++++
 tools/libxl/libxlutil.h             |   6 +
 tools/xl/xl_parse.c                 |  24 +-
 xen/arch/arm/mm.c                   |   2 +-
 xen/arch/x86/mm/p2m.c               |   2 +-
 xen/include/xsm/dummy.h             |   8 +-
 xen/include/xsm/xsm.h               |   7 +-
 xen/xsm/flask/hooks.c               |  10 +-
 xen/xsm/flask/policy/access_vectors |   4 +
 22 files changed, 883 insertions(+), 15 deletions(-)
 create mode 100644 tools/libxl/libxl_sshm.c
 create mode 100644 tools/libxl/libxlu_sshm.c

-- 
2.14.0


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/6] libxc: add xc_domain_remove_from_physmap to wrap XENMEM_remove_from_physmap
  2017-08-27  8:36 [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Zhongze Liu
@ 2017-08-27  8:36 ` Zhongze Liu
  2017-08-27  8:36 ` [PATCH v2 2/6] libxl: introduce a new structure to represent static shared memory regions Zhongze Liu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Zhongze Liu @ 2017-08-27  8:36 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Julien Grall, Stefano Stabellini, Ian Jackson, Zhongze Liu

This is for the proposal "Allow setting up shared memory areas between VMs
from xl config file". See:

  https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html

Then plan is to use XENMEM_add_to_physmap_batch to map the shared pages from
one domU to another and use XENMEM_remove_from_physmap to cancel the sharing.
A wrapper to XENMEM_add_to_physmap_batch was added in the following commit:

  commit 20e725e9364cff4a29945f66986ecd88cca8743d

Now add the wrapper to XENMEM_remove_from_physmap.

Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Wei Liu <wei.liu2@citrix.com>

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: xen-devel@lists.xen.org
---
 tools/libxc/include/xenctrl.h |  4 ++++
 tools/libxc/xc_domain.c       | 11 +++++++++++
 2 files changed, 15 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index c7710b8f36..0ff15a9255 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1381,6 +1381,10 @@ int xc_domain_add_to_physmap_batch(xc_interface *xch,
                                    xen_pfn_t *gfpns,
                                    int *errs);
 
+int xc_domain_remove_from_physmap(xc_interface *xch,
+                                  domid_t domid,
+                                  xen_pfn_t gpfn);
+
 int xc_domain_populate_physmap(xc_interface *xch,
                                uint32_t domid,
                                unsigned long nr_extents,
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 3bab4e8bab..e6b32792c0 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1077,6 +1077,17 @@ out:
     return rc;
 }
 
+int xc_domain_remove_from_physmap(xc_interface *xch,
+                                  domid_t domid,
+                                  xen_pfn_t gpfn)
+{
+    struct xen_remove_from_physmap xrfp = {
+        .domid = domid,
+        .gpfn = gpfn,
+    };
+    return do_memory_op(xch, XENMEM_remove_from_physmap, &xrfp, sizeof(xrfp));
+}
+
 int xc_domain_claim_pages(xc_interface *xch,
                                uint32_t domid,
                                unsigned long nr_pages)
-- 
2.14.0


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/6] libxl: introduce a new structure to represent static shared memory regions
  2017-08-27  8:36 [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Zhongze Liu
  2017-08-27  8:36 ` [PATCH v2 1/6] libxc: add xc_domain_remove_from_physmap to wrap XENMEM_remove_from_physmap Zhongze Liu
@ 2017-08-27  8:36 ` Zhongze Liu
  2017-08-27  8:36 ` [PATCH v2 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files Zhongze Liu
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Zhongze Liu @ 2017-08-27  8:36 UTC (permalink / raw)
  To: xen-devel
  Cc: Ian Jackson, Julien Grall, Stefano Stabellini, Wei Liu, Zhongze Liu

Add a new structure to the IDL familiy to represent static shared memory regions
as proposed in the proposal "Allow setting up shared memory areas between VMs
from xl config file" (see [1]).

Also fix some code style issues (indentation, trailling whitespaces).

[1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html

Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: xen-devel@lists.xen.org
---
v2: mention in the commit msg that several style corrections have been done
---
 tools/libxl/libxl.h         |  4 ++++
 tools/libxl/libxl_types.idl | 34 +++++++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 229e289750..3ee788642f 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -2237,6 +2237,10 @@ int libxl_fd_set_nonblock(libxl_ctx *ctx, int fd, int nonblock);
 int libxl_qemu_monitor_command(libxl_ctx *ctx, uint32_t domid,
                                const char *command_line, char **output);
 
+/* Constants for libxl_static_shm */
+#define LIBXL_SSHM_RANGE_UNKNOWN UINT64_MAX
+#define LIBXL_SSHM_ID_MAXLEN    128
+
 #include <libxl_event.h>
 
 #endif /* LIBXL_H */
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 6e80d36256..5ba8a878dc 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -472,7 +472,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("blkdev_start",    string),
 
     ("vnuma_nodes", Array(libxl_vnode_info, "num_vnuma_nodes")),
-    
+
     ("device_model_version", libxl_device_model_version),
     ("device_model_stubdomain", libxl_defbool),
     # if you set device_model you must set device_model_version too
@@ -543,10 +543,10 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("keymap",           string),
                                        ("sdl",              libxl_sdl_info),
                                        ("spice",            libxl_spice_info),
-                                       
+
                                        ("gfx_passthru",     libxl_defbool),
                                        ("gfx_passthru_kind", libxl_gfx_passthru_kind),
-                                       
+
                                        ("serial",           string),
                                        ("boot",             string),
                                        ("usb",              libxl_defbool),
@@ -779,6 +779,33 @@ libxl_device_channel = Struct("device_channel", [
            ])),
 ])
 
+libxl_sshm_cachepolicy = Enumeration("sshm_cachepolicy", [
+    (-1, "UNKNOWN"),
+    (0,  "ARM_NORMAL"),  # ARM policies should be < 32
+    (32,  "X86_NORMAL"), # X86 policies should be >= 32
+    ], init_val = "LIBXL_SSHM_CHCHE_POLICY_UNKNOWN")
+
+libxl_sshm_prot = Enumeration("sshm_prot", [
+    (-1, "UNKNOWN"),
+    (3,  "RW"),
+    ], init_val = "LIBXL_SSHM_PROT_UNKNOWN")
+
+libxl_sshm_role = Enumeration("sshm_role", [
+    (-1, "UNKNOWN"),
+    (0,  "MASTER"),
+    (1,  "SLAVE"),
+    ], init_val = "LIBXL_SSHM_ROLE_UNKNOWN")
+
+libxl_static_shm = Struct("static_shm", [
+    ("id", string),
+    ("offset", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}),
+    ("begin", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}),
+    ("end", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}),
+    ("prot", libxl_sshm_prot, {'init_val': 'LIBXL_SSHM_PROT_UNKNOWN'}),
+    ("cache_policy", libxl_sshm_cachepolicy, {'init_val': 'LIBXL_SSHM_CACHEPOLICY_UNKNOWN'}),
+    ("role", libxl_sshm_role, {'init_val': 'LIBXL_SSHM_ROLE_UNKNOWN'}),
+])
+
 libxl_domain_config = Struct("domain_config", [
     ("c_info", libxl_domain_create_info),
     ("b_info", libxl_domain_build_info),
@@ -797,6 +824,7 @@ libxl_domain_config = Struct("domain_config", [
     ("channels", Array(libxl_device_channel, "num_channels")),
     ("usbctrls", Array(libxl_device_usbctrl, "num_usbctrls")),
     ("usbdevs", Array(libxl_device_usbdev, "num_usbdevs")),
+    ("sshms", Array(libxl_static_shm, "num_sshms")),
 
     ("on_poweroff", libxl_action_on_shutdown),
     ("on_reboot", libxl_action_on_shutdown),
-- 
2.14.0


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files
  2017-08-27  8:36 [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Zhongze Liu
  2017-08-27  8:36 ` [PATCH v2 1/6] libxc: add xc_domain_remove_from_physmap to wrap XENMEM_remove_from_physmap Zhongze Liu
  2017-08-27  8:36 ` [PATCH v2 2/6] libxl: introduce a new structure to represent static shared memory regions Zhongze Liu
@ 2017-08-27  8:36 ` Zhongze Liu
  2017-09-01 16:03   ` Wei Liu
  2017-09-01 16:25   ` Wei Liu
  2017-08-27  8:36 ` [PATCH v2 4/6] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin Zhongze Liu
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Zhongze Liu @ 2017-08-27  8:36 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Zhongze Liu, George Dunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall,
	Jan Beulich

Add the parsing utils for the newly introduced libxl_static_sshm struct
to the libxl/libxlu_* family. And add realated parsing code in xl to
parse the struct from xl config files. This is for the proposal "Allow
setting up shared memory areas between VMs from xl config file" (see [1]).

[1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html

Signed-off-by: Zhongze Liu <blackskygg@gmail.com>

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: xen-devel@lists.xen.org
---
v2:
  * fix all the 'Yoda conditions'
  * use the predefined XC_PAGE_MASK instead of defining a new one
---
 tools/libxl/Makefile      |   2 +-
 tools/libxl/libxlu_sshm.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxlutil.h   |   6 ++
 tools/xl/xl_parse.c       |  24 +++++-
 4 files changed, 240 insertions(+), 2 deletions(-)
 create mode 100644 tools/libxl/libxlu_sshm.c

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 082af8f716..3b63fb2cad 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -175,7 +175,7 @@ AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h _paths.h \
 AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
 AUTOSRCS += _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c
 LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \
-	libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o
+	libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o libxlu_sshm.o
 $(LIBXLU_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
 
 $(TEST_PROG_OBJS) _libxl.api-for-check: CFLAGS += $(CFLAGS_libxentoollog)
diff --git a/tools/libxl/libxlu_sshm.c b/tools/libxl/libxlu_sshm.c
new file mode 100644
index 0000000000..5276ff9395
--- /dev/null
+++ b/tools/libxl/libxlu_sshm.c
@@ -0,0 +1,210 @@
+#include "libxl_osdeps.h" /* must come before any other headers */
+#include "libxlu_internal.h"
+#include "xenctrl.h"
+
+#include <ctype.h>
+
+#define PARAM_RE(EXPR) "^\\s*" EXPR "\\s*(,|$)"
+#define WORD_RE         "([_a-zA-Z0-9]+)"
+#define EQU_RE         PARAM_RE(WORD_RE "\\s*=\\s*" WORD_RE)
+
+#define RET_INVAL(msg, curr_str)  do {              \
+        xlu__sshm_err(cfg, msg, curr_str);          \
+        rc = EINVAL;                                \
+        goto out;                                   \
+    } while(0)
+
+/* set a member in libxl_static_shm and report an error if it's respecified,
+ * @curr_str indicates the head of the remaining string. */
+#define SET_VAL(var, name, type, value, curr_str)  do {                 \
+        if ((var) != LIBXL_SSHM_##type##_UNKNOWN && (var) != value) {   \
+            RET_INVAL("\"" name "\" respecified", curr_str);            \
+        }                                                               \
+        (var) = value;                                                  \
+    } while(0)
+
+
+static void xlu__sshm_err(XLU_Config *cfg, const char *msg,
+                          const char *curr_str) {
+    fprintf(cfg->report,
+            "%s: config parsing error in shared_memory: %s at '%s'\n",
+            cfg->config_source, msg, curr_str);
+}
+
+static int parse_prot(XLU_Config *cfg, char *str, libxl_sshm_prot *prot)
+{
+    int rc;
+    libxl_sshm_prot new_prot;
+
+    if (!strcmp(str, "rw")) {
+        new_prot = LIBXL_SSHM_PROT_RW;
+    } else {
+        RET_INVAL("invalid permission flags", str);
+    }
+
+    SET_VAL(*prot, "permission flags", PROT, new_prot, str);
+
+    rc = 0;
+
+ out:
+    return rc;
+}
+
+static int parse_cachepolicy(XLU_Config *cfg, char *str,
+                             libxl_sshm_cachepolicy *policy)
+{
+    int rc;
+    libxl_sshm_cachepolicy new_policy;
+
+    if (!strcmp(str, "ARM_normal")) {
+        new_policy = LIBXL_SSHM_CACHEPOLICY_ARM_NORMAL;
+    } else if (!strcmp(str, "x86_normal")) {
+        new_policy = LIBXL_SSHM_CACHEPOLICY_X86_NORMAL;
+    } else {
+        RET_INVAL("invalid cache policy", str);
+    }
+
+    SET_VAL(*policy, "cache policy", CACHEPOLICY, new_policy, str);
+    rc = 0;
+
+ out:
+    return rc;
+}
+
+/* handle key = value pairs */
+static int handle_equ(XLU_Config *cfg, char *key, char *val,
+                      libxl_static_shm *sshm)
+{
+    int rc;
+
+    if (!strcmp(key, "id")) {
+        if (strlen(val) > LIBXL_SSHM_ID_MAXLEN) { RET_INVAL("id too long", val); }
+        if (sshm->id && !strcmp(sshm->id, val)) {
+            RET_INVAL("id respecified", val);
+        }
+
+        sshm->id = strdup(val);
+        if (!sshm->id) {
+            fprintf(stderr, "sshm parser out of memory\n");
+            rc = ENOMEM;
+            goto out;
+        }
+    } else if (!strcmp(key, "role")) {
+        libxl_sshm_role new_role;
+
+        if (!strcmp("master", val)) {
+            new_role = LIBXL_SSHM_ROLE_MASTER;
+        } else if (!strcmp("slave", val)) {
+            new_role = LIBXL_SSHM_ROLE_SLAVE;
+        } else {
+            RET_INVAL("invalid role", val);
+        }
+
+        SET_VAL(sshm->role, "role", ROLE, new_role, val);
+    } else if (!strcmp(key, "begin") ||
+               !strcmp(key, "end") ||
+               !strcmp(key, "offset")) {
+        char *endptr;
+        int base = 10;
+        uint64_t new_addr;
+
+        /* Could be in hex form. Note that we don't need to check the length here,
+         * for val[] is NULL-terminated */
+        if (val[0] == '0' && val[1] == 'x') { base = 16; }
+        new_addr = strtoull(val, &endptr, base);
+        if (errno == ERANGE || *endptr)
+            RET_INVAL("invalid begin/end/offset", val);
+        if (new_addr & ~XC_PAGE_MASK)
+            RET_INVAL("begin/end/offset is not a multiple of 4K", val);
+
+        /* begin or end */
+        if (key[0] == 'b') {
+            SET_VAL(sshm->begin, "beginning address", RANGE, new_addr, val);
+        } else if(key[0] == 'e'){
+            SET_VAL(sshm->end, "ending address", RANGE, new_addr, val);
+        } else {
+            SET_VAL(sshm->offset, "offset", RANGE, new_addr, val);
+        }
+    } else if (!strcmp(key, "prot")) {
+        rc = parse_prot(cfg, val, &sshm->prot);
+        if (rc) { goto out; }
+    } else if (!strcmp(key, "cache_policy")) {
+        rc = parse_cachepolicy(cfg, val, &sshm->cache_policy);
+        if (rc) { goto out; }
+    } else {
+        RET_INVAL("invalid option", key);
+    }
+
+    rc = 0;
+
+ out:
+    return rc;
+}
+
+int xlu_sshm_parse(XLU_Config *cfg, const char *spec,
+                   libxl_static_shm *sshm)
+{
+    int rc;
+    regex_t equ_rec;
+    char *buf2 = NULL, *ptr = NULL;
+    regmatch_t pmatch[3];
+
+    rc = regcomp(&equ_rec, EQU_RE, REG_EXTENDED);
+    if (rc) {
+        fprintf(stderr, "sshm parser failed to initialize\n");
+        goto out;
+    }
+
+    buf2 = ptr = strdup(spec);
+    if (!buf2) {
+        fprintf(stderr, "sshm parser out of memory\n");
+        rc = ENOMEM;
+        goto out;
+    }
+
+    /* main parsing loop */
+    while (true) {
+        if (!*ptr) { break; }
+        if (regexec(&equ_rec, ptr, 3, pmatch, 0))
+            RET_INVAL("unrecognized token", ptr);
+
+        ptr[pmatch[1].rm_eo] = '\0';
+        ptr[pmatch[2].rm_eo] = '\0';
+        rc = handle_equ(cfg, ptr + pmatch[1].rm_so,
+                        ptr + pmatch[2].rm_so, sshm);
+        if (rc) { goto out; }
+
+        ptr += pmatch[0].rm_eo;
+    }
+
+    if (*ptr) { RET_INVAL("invalid syntax", ptr); }
+
+    /* do some early checks */
+    if (!sshm->id) {
+        RET_INVAL("id not specified", spec);
+    }
+    if (sshm->begin == LIBXL_SSHM_RANGE_UNKNOWN) {
+        RET_INVAL("begin address not specified", spec);
+    }
+    if (sshm->end == LIBXL_SSHM_RANGE_UNKNOWN) {
+        RET_INVAL("end address not specified", spec);
+    }
+    if (sshm->begin > sshm->end) {
+        RET_INVAL("begin address larger that end address", spec);
+    }
+
+    rc = 0;
+
+ out:
+    if (buf2) { free(buf2); }
+    regfree(&equ_rec);
+    return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
index e81b644c01..ee39cb5bdc 100644
--- a/tools/libxl/libxlutil.h
+++ b/tools/libxl/libxlutil.h
@@ -118,6 +118,12 @@ int xlu_rdm_parse(XLU_Config *cfg, libxl_rdm_reserve *rdm, const char *str);
 int xlu_vif_parse_rate(XLU_Config *cfg, const char *rate,
                        libxl_device_nic *nic);
 
+/*
+ * static shared memory specification parsing
+ */
+int xlu_sshm_parse(XLU_Config *cfg, const char *spec,
+                   libxl_static_shm *sshm);
+
 #endif /* LIBXLUTIL_H */
 
 /*
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 5c2bf17222..82d955b8b9 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -813,7 +813,7 @@ void parse_config_data(const char *config_source,
     long l, vcpus = 0;
     XLU_Config *config;
     XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms,
-                   *usbctrls, *usbdevs, *p9devs;
+                   *usbctrls, *usbdevs, *p9devs, *sshms;
     XLU_ConfigList *channels, *ioports, *irqs, *iomem, *viridian, *dtdevs,
                    *mca_caps;
     int num_ioports, num_irqs, num_iomem, num_cpus, num_viridian, num_mca_caps;
@@ -1392,6 +1392,28 @@ void parse_config_data(const char *config_source,
         }
     }
 
+    if (!xlu_cfg_get_list (config, "static_shm", &sshms, 0, 0)) {
+        d_config->num_sshms = 0;
+        d_config->sshms = NULL;
+        while ((buf = xlu_cfg_get_listitem (sshms, d_config->num_sshms)) != NULL) {
+            libxl_static_shm *sshm;
+            char *buf2 = strdup(buf);
+            int ret;
+
+            sshm = ARRAY_EXTEND_INIT_NODEVID(d_config->sshms,
+                                             d_config->num_sshms,
+                                             libxl_static_shm_init);
+            ret = xlu_sshm_parse(config, buf2, sshm);
+            if (ret) {
+                fprintf(stderr,
+                        "xl: Invalid argument for static_shm: %s", buf2);
+                exit(EXIT_FAILURE);
+            }
+
+            free(buf2);
+        }
+    }
+
     if (!xlu_cfg_get_list(config, "p9", &p9devs, 0, 0)) {
         libxl_device_p9 *p9;
         char *security_model = NULL;
-- 
2.14.0


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 4/6] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin
  2017-08-27  8:36 [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Zhongze Liu
                   ` (2 preceding siblings ...)
  2017-08-27  8:36 ` [PATCH v2 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files Zhongze Liu
@ 2017-08-27  8:36 ` Zhongze Liu
  2017-08-28  8:29   ` Jan Beulich
  2017-08-27  8:36 ` [PATCH v2 5/6] libxl: support mapping static shared memory areas during domain creation Zhongze Liu
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Zhongze Liu @ 2017-08-27  8:36 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Zhongze Liu, George Dunlap,
	Andrew Cooper, Ian Jackson, Julien Grall, Jan Beulich,
	Daniel De Graaf

The original dummy xsm_map_gmfn_foregin checks if source domain has the proper
privileges over the target domain. Under this policy, it's not allowed if a Dom0
wants to map pages from one DomU to another, which restricts some useful yet not
dangerous use cases of the API, such as sharing pages among DomU's by calling
XENMEM_add_to_physmap from Dom0.

For the dummy xsm_map_gmfn_foregin, change to policy to: IFF the current domain
has the proper privileges on (d) and (t), grant the access.

For the flask side: 1) Introduce a new av permission MMU__SHARE_MEM to denote if
two domains can share memory through map_gmfn_foregin. 2) Change to hook to
grant the access IFF the current domain has proper MMU privileges on (d) and (t),
and MMU__SHARE_MEM is allowed between (d) and (t). 3) Modify the default xen.te
to allow MMU__SHARE_MEM for normal domains that allow grant mapping/event
channels.

References to the xsm check have also been updated to pass the current
domain as a new parameter.

This is for the proposal "Allow setting up shared memory areas between VMs
from xl config file" (see [1]).

[1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html

Signed-off-by: Zhongze Liu <blackskygg@gmail.com>

Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: xen-devel@lists.xen.org
---
v2:
  * Preserve the error code in both the dummy xsm callback and the flask hook.
  * Also check if (d) and (t) can share memory in the flask hook.
---
 tools/flask/policy/modules/xen.if   |  2 ++
 xen/arch/arm/mm.c                   |  2 +-
 xen/arch/x86/mm/p2m.c               |  2 +-
 xen/include/xsm/dummy.h             |  8 ++++++--
 xen/include/xsm/xsm.h               |  7 ++++---
 xen/xsm/flask/hooks.c               | 10 ++++++++--
 xen/xsm/flask/policy/access_vectors |  4 ++++
 7 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if
index ed0df4f010..edb7dc8b50 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -127,6 +127,8 @@ define(`domain_comms', `
 	domain_event_comms($1, $2)
 	allow $1 $2:grant { map_read map_write copy unmap };
 	allow $2 $1:grant { map_read map_write copy unmap };
+	allow $1 $2:mmu share_mem;
+	allow $2 $1:mmu share_mem;
 ')
 
 # domain_self_comms(domain)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index a810a056d7..9ec78d8c03 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1284,7 +1284,7 @@ int xenmem_add_to_physmap_one(
             return -EINVAL;
         }
 
-        rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od);
+        rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, d, od);
         if ( rc )
         {
             rcu_unlock_domain(od);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index e8a57d118c..a547fd00c0 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2545,7 +2545,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
     if ( tdom == fdom )
         goto out;
 
-    rc = xsm_map_gmfn_foreign(XSM_TARGET, tdom, fdom);
+    rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, tdom, fdom);
     if ( rc )
         goto out;
 
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 62fcea6f04..1a42d85d27 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -525,10 +525,14 @@ static XSM_INLINE int xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,
     return xsm_default_action(action, d1, d2);
 }
 
-static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, struct domain *t)
+static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *cd,
+                                           struct domain *d, struct domain *t)
 {
+    int rc;
     XSM_ASSERT_ACTION(XSM_TARGET);
-    return xsm_default_action(action, d, t);
+    rc = xsm_default_action(action, cd, d);
+    if (rc) return rc;
+    return xsm_default_action(action, cd, t);
 }
 
 static XSM_INLINE int xsm_hvm_param(XSM_DEFAULT_ARG struct domain *d, unsigned long op)
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 60c0fd6a62..a20654a803 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -85,7 +85,7 @@ struct xsm_operations {
     int (*memory_pin_page) (struct domain *d1, struct domain *d2, struct page_info *page);
     int (*add_to_physmap) (struct domain *d1, struct domain *d2);
     int (*remove_from_physmap) (struct domain *d1, struct domain *d2);
-    int (*map_gmfn_foreign) (struct domain *d, struct domain *t);
+    int (*map_gmfn_foreign) (struct domain *cd, struct domain *d, struct domain *t);
     int (*claim_pages) (struct domain *d);
 
     int (*console_io) (struct domain *d, int cmd);
@@ -372,9 +372,10 @@ static inline int xsm_remove_from_physmap(xsm_default_t def, struct domain *d1,
     return xsm_ops->remove_from_physmap(d1, d2);
 }
 
-static inline int xsm_map_gmfn_foreign (xsm_default_t def, struct domain *d, struct domain *t)
+static inline int xsm_map_gmfn_foreign (xsm_default_t def, struct domain *cd,
+                                        struct domain *d, struct domain *t)
 {
-    return xsm_ops->map_gmfn_foreign(d, t);
+    return xsm_ops->map_gmfn_foreign(cd, d, t);
 }
 
 static inline int xsm_claim_pages(xsm_default_t def, struct domain *d)
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 91146275bb..c80e21e7ee 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1165,9 +1165,15 @@ static int flask_remove_from_physmap(struct domain *d1, struct domain *d2)
     return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP);
 }
 
-static int flask_map_gmfn_foreign(struct domain *d, struct domain *t)
+static int flask_map_gmfn_foreign(struct domain *cd,
+                                  struct domain *d, struct domain *t)
 {
-    return domain_has_perm(d, t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
+    int rc;
+    rc = domain_has_perm(cd, d, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
+    if (rc) return rc;
+    rc = domain_has_perm(cd, t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
+    if (rc) return rc;
+    return domain_has_perm(d, t, SECCLASS_MMU, MMU__SHARE_MEM);
 }
 
 static int flask_hvm_param(struct domain *d, unsigned long op)
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 1f7eb35fc8..0ba7505387 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -383,6 +383,10 @@ class mmu
 # Allow a privileged domain to install a map of a page it does not own.  Used
 # for stub domain device models with the PV framebuffer.
     target_hack
+# Checked when using map_gmfn_foreign to share memory:
+#  source = domain whose memory is being shared
+#  target = client domain
+    share_mem
 }
 
 # control of the paging_domctl split by subop
-- 
2.14.0


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 5/6] libxl: support mapping static shared memory areas during domain creation
  2017-08-27  8:36 [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Zhongze Liu
                   ` (3 preceding siblings ...)
  2017-08-27  8:36 ` [PATCH v2 4/6] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin Zhongze Liu
@ 2017-08-27  8:36 ` Zhongze Liu
  2017-09-01 16:23   ` Wei Liu
  2017-08-27  8:36 ` [PATCH v2 6/6] libxl: support unmapping static shared memory areas during domain destruction Zhongze Liu
  2017-10-10 23:55 ` [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Stefano Stabellini
  6 siblings, 1 reply; 20+ messages in thread
From: Zhongze Liu @ 2017-08-27  8:36 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Zhongze Liu, George Dunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall,
	Jan Beulich

Add libxl__sshm_add to map shared pages from one DomU to another, The mapping
process involves the follwing steps:

  * Set defaults and check for further errors in the static_shm configs:
    overlapping areas, invalid ranges, duplicated master domain,
    no master domain etc.
  * Write infomation of static shared memory areas into the appropriate
    xenstore paths.
  * Use xc_domain_add_to_physmap_batch to do the page sharing.
  * Set the refcount of the shared region accordingly

Temporarily mark this as unsupported on x86 because calling p2m_add_foregin on
two domU's is currently not allowd on x86 (see the comments in
x86/mm/p2m.c:p2m_add_foregin for more details).

This is for the proposal "Allow setting up shared memory areas between VMs
from xl config file" (see [1]).

[1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html

Signed-off-by: Zhongze Liu <blackskygg@gmail.com>

Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: xen-devel@lists.xen.org
---
v2:
  * coding style corrections
  * replace libxl__xs_get_sshmpath() with a macro
  * use the predefined XC_PAGE_SIZE instead of hardcoding 12
  * avoid calling xc_domain_add_to_physmap many time during xs retry
  * unmapping mapped pages on partial failure
  * refcounting the node
---
 tools/libxl/Makefile         |   2 +-
 tools/libxl/libxl_arch.h     |   6 +
 tools/libxl/libxl_arm.c      |  15 ++
 tools/libxl/libxl_create.c   |  27 ++++
 tools/libxl/libxl_internal.h |  13 ++
 tools/libxl/libxl_sshm.c     | 375 +++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_x86.c      |  18 +++
 7 files changed, 455 insertions(+), 1 deletion(-)
 create mode 100644 tools/libxl/libxl_sshm.c

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 3b63fb2cad..fd624b28f3 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -138,7 +138,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
 			libxl_dom_suspend.o libxl_dom_save.o libxl_usb.o \
 			libxl_vtpm.o libxl_nic.o libxl_disk.o libxl_console.o \
 			libxl_cpupool.o libxl_mem.o libxl_sched.o libxl_tmem.o \
-			libxl_9pfs.o libxl_domain.o \
+			libxl_9pfs.o libxl_domain.o libxl_sshm.o \
                         $(LIBXL_OBJS-y)
 LIBXL_OBJS += libxl_genid.o
 LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
index 5e1fc6060e..1d681d8863 100644
--- a/tools/libxl/libxl_arch.h
+++ b/tools/libxl/libxl_arch.h
@@ -71,6 +71,12 @@ int libxl__arch_extra_memory(libxl__gc *gc,
                              const libxl_domain_build_info *info,
                              uint64_t *out);
 
+_hidden
+bool libxl__arch_domain_support_sshm(const libxl_domain_build_info *b_info);
+
+_hidden
+int libxl__arch_domain_sshm_cachepolicy_setdefault(libxl_static_shm *sshm);
+
 #if defined(__i386__) || defined(__x86_64__)
 
 #define LAPIC_BASE_ADDRESS  0xfee00000
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index d842d888eb..0975109c0c 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -1065,6 +1065,21 @@ void libxl__arch_domain_build_info_acpi_setdefault(
     libxl_defbool_setdefault(&b_info->acpi, false);
 }
 
+bool libxl__arch_domain_support_sshm(const libxl_domain_build_info *b_info)
+{
+    return true;
+}
+
+int libxl__arch_domain_sshm_cachepolicy_setdefault(libxl_static_shm *sshm)
+{
+    if (sshm->cache_policy == LIBXL_SSHM_CACHEPOLICY_UNKNOWN)
+        sshm->cache_policy = LIBXL_SSHM_CACHEPOLICY_ARM_NORMAL;
+    if (sshm->cache_policy >= LIBXL_SSHM_CACHEPOLICY_X86_NORMAL)
+        return ERROR_INVAL;
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1158303e1a..f0193c84ba 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -501,6 +501,14 @@ int libxl__domain_build(libxl__gc *gc,
         ret = ERROR_INVAL;
         goto out;
     }
+
+    /* the p2m has been setup, we could map the static shared memory now. */
+    ret = libxl__sshm_add(gc, domid, d_config->sshms, d_config->num_sshms);
+    if (ret != 0) {
+        LOG(ERROR, "failed to map static shared memory");
+        goto out;
+    }
+
     ret = libxl__build_post(gc, domid, info, state, vments, localents);
 out:
     return ret;
@@ -918,6 +926,25 @@ static void initiate_domain_create(libxl__egc *egc,
         goto error_out;
     }
 
+    if (d_config->num_sshms != 0 &&
+        !libxl__arch_domain_support_sshm(&d_config->b_info)) {
+        LOGD(ERROR, domid, "static_shm is not supported by this domain type.");
+        ret = ERROR_INVAL;
+        goto error_out;
+    }
+
+    for (i = 0; i < d_config->num_sshms; ++i) {
+        ret = libxl__sshm_setdefault(gc, domid, &d_config->sshms[i]);
+        if (ret) {
+            LOGD(ERROR, domid, "Unable to set defaults for static sshm");
+            goto error_out;
+        }
+    }
+
+    ret = libxl__sshm_check_overlap(gc, domid,
+                                    d_config->sshms, d_config->num_sshms);
+    if (ret) goto error_out;
+
     ret = libxl__domain_make(gc, d_config, &domid, &state->config);
     if (ret) {
         LOGD(ERROR, domid, "cannot make domain: %d", ret);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 724750967c..7ab2d40c49 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4352,6 +4352,19 @@ static inline bool libxl__acpi_defbool_val(const libxl_domain_build_info *b_info
 }
 #endif
 
+/*
+ * Set up static shared ram pages for HVM domains to communicate
+ *
+ * This function should only be called after the memory map is constructed
+ * and before any further memory access. */
+_hidden int libxl__sshm_add(libxl__gc *gc, uint32_t domid,
+                            libxl_static_shm *sshm, int len);
+
+_hidden int libxl__sshm_check_overlap(libxl__gc *gc, uint32_t domid,
+                                      libxl_static_shm *sshms, int len);
+_hidden int libxl__sshm_setdefault(libxl__gc *gc, uint32_t domid,
+                                   libxl_static_shm *sshm);
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_sshm.c b/tools/libxl/libxl_sshm.c
new file mode 100644
index 0000000000..a4091a3056
--- /dev/null
+++ b/tools/libxl/libxl_sshm.c
@@ -0,0 +1,375 @@
+#include "libxl_osdeps.h"
+#include "libxl_internal.h"
+#include "libxl_arch.h"
+
+#define SSHM_PATH(id) GCSPRINTF("/local/static_shm/%s", id)
+
+#define SSHM_ERROR(domid, sshmid, f, ...)                               \
+    LOGD(ERROR, domid, "static_shm id = %s: " f, sshmid, ##__VA_ARGS__)
+
+
+/* Set default values for libxl_static_shm */
+int libxl__sshm_setdefault(libxl__gc *gc, uint32_t domid,
+                           libxl_static_shm *sshm)
+{
+    int rc;
+
+    if (sshm->role == LIBXL_SSHM_ROLE_UNKNOWN)
+        sshm->role = LIBXL_SSHM_ROLE_SLAVE;
+    if (sshm->prot == LIBXL_SSHM_PROT_UNKNOWN)
+        sshm->prot = LIBXL_SSHM_PROT_RW;
+
+    /* role-specific checks */
+    if (sshm->role == LIBXL_SSHM_ROLE_SLAVE) {
+        if (sshm->offset == LIBXL_SSHM_RANGE_UNKNOWN)
+            sshm->offset = 0;
+        if (sshm->cache_policy != LIBXL_SSHM_CACHEPOLICY_UNKNOWN) {
+            SSHM_ERROR(domid, sshm->id,
+                       "cache_policy is only applicable to master domains");
+            rc = ERROR_INVAL;
+            goto out;
+        }
+    } else {
+        if (sshm->offset != LIBXL_SSHM_RANGE_UNKNOWN) {
+            SSHM_ERROR(domid, sshm->id,
+                       "offset is only applicable to slave domains");
+            rc = ERROR_INVAL;
+            goto out;
+        }
+
+        rc = libxl__arch_domain_sshm_cachepolicy_setdefault(sshm);
+        if (rc) {
+            SSHM_ERROR(domid, sshm->id,
+                       "cache policy not supported on this platform");
+            goto out;
+        }
+    }
+
+    rc = 0;
+out:
+    return rc;
+}
+
+/* Compare function for sorting sshm ranges by sshm->begin */
+static int sshm_range_cmp(const void *a, const void *b)
+{
+    libxl_static_shm *const *sshma = a, *const *sshmb = b;
+    return (*sshma)->begin > (*sshmb)->begin ? 1 : -1;
+}
+
+/* check if the sshm slave configs in @sshm overlap */
+int libxl__sshm_check_overlap(libxl__gc *gc, uint32_t domid,
+                                     libxl_static_shm *sshms, int len)
+{
+
+    const libxl_static_shm **slave_sshms = NULL;
+    int num_slaves;
+    int i;
+
+    if (!len) return 0;
+
+    slave_sshms = libxl__calloc(gc, len, sizeof(slave_sshms[0]));
+    num_slaves = 0;
+    for (i = 0; i < len; ++i) {
+        if (sshms[i].role == LIBXL_SSHM_ROLE_SLAVE)
+            slave_sshms[num_slaves++] = sshms + i;
+    }
+    qsort(slave_sshms, num_slaves, sizeof(slave_sshms[0]), sshm_range_cmp);
+
+    for (i = 0; i < num_slaves - 1; ++i) {
+        if (slave_sshms[i+1]->begin < slave_sshms[i]->end) {
+            SSHM_ERROR(domid, slave_sshms[i+1]->id, "slave ranges overlap.");
+            return ERROR_INVAL;
+        }
+    }
+
+    return 0;
+}
+
+/* The caller have to guarentee that sshm->begin < sshm->end */
+static int libxl__sshm_do_map(libxl__gc *gc, uint32_t mid, uint32_t sid,
+                              libxl_static_shm *sshm,
+                              uint64_t mbegin, uint64_t mend)
+{
+    int rc;
+    int i;
+    unsigned int num_mpages, num_spages, offset;
+    int *errs;
+    xen_ulong_t *idxs;
+    xen_pfn_t *gpfns;
+
+    num_mpages = (mend - mbegin) >> XC_PAGE_SHIFT;
+    num_spages = (sshm->end - sshm->begin) >> XC_PAGE_SHIFT;
+    offset = sshm->offset >> XC_PAGE_SHIFT;
+
+    /* Check range. Test offset < mpages first to avoid overflow */
+    if ((offset >= num_mpages) || (num_mpages - offset < num_spages)) {
+        SSHM_ERROR(sid, sshm->id, "exceeds master's address space.");
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    /* fill out the pfn's and do the mapping */
+    errs = libxl__calloc(gc, num_spages, sizeof(int));
+    idxs = libxl__calloc(gc, num_spages, sizeof(xen_ulong_t));
+    gpfns = libxl__calloc(gc, num_spages, sizeof(xen_pfn_t));
+    for (i = 0; i < num_spages; i++) {
+        idxs[i] = (mbegin >> XC_PAGE_SHIFT) + offset + i;
+        gpfns[i]= (sshm->begin >> XC_PAGE_SHIFT) + i;
+    }
+    rc = xc_domain_add_to_physmap_batch(CTX->xch,
+                                        sid, mid,
+                                        XENMAPSPACE_gmfn_foreign,
+                                        num_spages,
+                                        idxs, gpfns, errs);
+
+    for (i = 0; i < num_spages; i++) {
+        if (errs[i]) {
+            SSHM_ERROR(sid, sshm->id,
+                       "can't map at address 0x%"PRIx64".",
+                       gpfns[i] << XC_PAGE_SHIFT);
+            rc = ERROR_FAIL;
+        }
+    }
+    if (rc) {
+        /* role back successfully mapped pages */
+        SSHM_ERROR(sid, sshm->id, "errors occurred, canceling operation.");
+        for (i = 0; i < num_spages; i++) {
+            if (xc_domain_remove_from_physmap(CTX->xch, sid, gpfns[i])) {
+                SSHM_ERROR(sid, sshm->id, "can't unmap page at 0x%"PRIx64".",
+                           gpfns[i] << XC_PAGE_SHIFT);
+            }
+        }
+        goto out;
+    }
+
+    rc = 0;
+out:
+    return rc;
+}
+
+static int libxl__sshm_incref(libxl__gc *gc, xs_transaction_t xt,
+                              const char *sshm_path)
+{
+    int rc, count;
+    const char *count_path, *count_string;
+
+    count_path = GCSPRINTF("%s/users", sshm_path);
+    rc = libxl__xs_read_checked(gc, xt, count_path, &count_string);
+    if (rc) goto out;
+    count = atoi(count_string);
+
+    count_string = GCSPRINTF("%d", count+1);
+    rc = libxl__xs_write_checked(gc, xt, count_path, count_string);
+    if (rc) goto out;
+
+    rc = 0;
+out:
+    return rc;
+}
+
+static int libxl__sshm_add_slave(libxl__gc *gc, uint32_t domid,
+                                 libxl_static_shm *sshm)
+{
+    int rc;
+    const char *sshm_path, *slave_path, *dom_path, *dom_sshm_path, *dom_role_path;
+    char *ents[9];
+    const char *xs_value;
+    libxl_static_shm master_sshm;
+    uint32_t master_domid;
+    xs_transaction_t xt = XBT_NULL;
+    bool isretry;
+
+    sshm_path = SSHM_PATH(sshm->id);
+    slave_path = GCSPRINTF("%s/slaves/%"PRIu32, sshm_path, domid);
+    dom_path = libxl__xs_get_dompath(gc, domid);
+    /* the domain should be in xenstore by now */
+    assert(dom_path);
+    dom_sshm_path = GCSPRINTF("%s/static_shm/%s", dom_path, sshm->id);
+    dom_role_path = GCSPRINTF("%s/role", dom_sshm_path);
+
+    /* prepare the slave xenstore entries */
+    ents[0] = "begin";
+    ents[1] = GCSPRINTF("0x%"PRIx64, sshm->begin);
+    ents[2] = "end";
+    ents[3] = GCSPRINTF("0x%"PRIx64, sshm->end);
+    ents[4] = "offset";
+    ents[5] = GCSPRINTF("0x%"PRIx64, sshm->offset);
+    ents[6] = "prot";
+    ents[7] = libxl__strdup(gc, libxl_sshm_prot_to_string(sshm->prot));
+    ents[8] = NULL;
+
+    isretry = false;
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &xt);
+        if (rc) goto out;
+
+        if (!libxl__xs_read(gc, xt, sshm_path)) {
+            SSHM_ERROR(domid, sshm->id, "no master found.");
+            rc = ERROR_FAIL;
+            goto out;
+        }
+
+        /* every ID can appear in each domain at most once */
+        if (libxl__xs_read(gc, xt, dom_sshm_path)) {
+            SSHM_ERROR(domid, sshm->id,
+                       "domain tried to map the same ID twice.");
+            rc = ERROR_FAIL;
+            goto out;
+        }
+
+        /* look at the master info and see if we could do the mapping */
+        rc = libxl__xs_read_checked(gc, xt,
+                                    GCSPRINTF("%s/prot", sshm_path),
+                                    &xs_value);
+        if (rc) goto out;
+        libxl_sshm_prot_from_string(xs_value, &master_sshm.prot);
+
+        rc = libxl__xs_read_checked(gc, xt,
+                                    GCSPRINTF("%s/begin", sshm_path),
+                                    &xs_value);
+        if (rc) goto out;
+        master_sshm.begin = strtoull(xs_value, NULL, 16);
+
+        rc = libxl__xs_read_checked(gc, xt,
+                                    GCSPRINTF("%s/end", sshm_path),
+                                    &xs_value);
+        if (rc) goto out;
+        master_sshm.end = strtoull(xs_value, NULL, 16);
+
+        rc = libxl__xs_read_checked(gc, xt,
+                                    GCSPRINTF("%s/master", sshm_path),
+                                    &xs_value);
+        if (rc) goto out;
+        master_domid = strtoull(xs_value, NULL, 16);
+
+        if (sshm->prot == LIBXL_SSHM_PROT_UNKNOWN) {
+            sshm->prot = master_sshm.prot;
+        }
+        /* check if the slave is asking too much permission */
+        if (master_sshm.prot < sshm->prot) {
+            SSHM_ERROR(domid, sshm->id, "slave is asking too much permission.");
+            rc = ERROR_INVAL;
+            goto out;
+        }
+
+        /* all checks passed, do the job */
+        if (!isretry) {
+            rc = libxl__sshm_do_map(gc, master_domid, domid, sshm,
+                                    master_sshm.begin, master_sshm.end);
+            if (rc) goto out;
+        }
+
+        /* write the result to xenstore and commit */
+        rc = libxl__xs_write_checked(gc, xt, dom_role_path, "slave");
+        if (rc) goto out;
+        rc = libxl__xs_writev(gc, xt, slave_path, ents);
+        if (rc) goto out;
+        rc = libxl__sshm_incref(gc, xt, sshm_path);
+        if (rc) goto out;
+
+        rc = libxl__xs_transaction_commit(gc, &xt);
+        if (!rc) break;
+        if (rc < 0) goto out;
+        isretry = true;
+    }
+
+    rc = 0;
+out:
+    libxl__xs_transaction_abort(gc, &xt);
+    return rc;
+}
+
+static int libxl__sshm_add_master(libxl__gc *gc, uint32_t domid,
+                                  libxl_static_shm *sshm)
+{
+    int rc;
+    const char *sshm_path, *dom_path, *dom_role_path;
+    char *ents[13];
+    struct xs_permissions noperm;
+    xs_transaction_t xt = XBT_NULL;
+
+    sshm_path = SSHM_PATH(sshm->id);
+    dom_path = libxl__xs_get_dompath(gc, domid);
+    /* the domain should be in xenstore by now */
+    assert(dom_path);
+    dom_role_path = GCSPRINTF("%s/static_shm/%s/role", dom_path, sshm->id);
+
+    /* prepare the xenstore entries */
+    ents[0] = "master";
+    ents[1] = GCSPRINTF("%"PRIu32, domid);
+    ents[2] = "begin";
+    ents[3] = GCSPRINTF("0x%"PRIx64, sshm->begin);
+    ents[4] = "end";
+    ents[5] = GCSPRINTF("0x%"PRIx64, sshm->end);
+    ents[6] = "prot";
+    ents[7] = libxl__strdup(gc, libxl_sshm_prot_to_string(sshm->prot));
+    ents[8] = "cache_policy";
+    ents[9] = libxl__strdup(gc,
+                            libxl_sshm_cachepolicy_to_string(sshm->cache_policy));
+    ents[10] = "users";
+    ents[11] = "1";
+    ents[12] = NULL;
+
+    /* could only be accessed by Dom0 */
+    noperm.id = 0;
+    noperm.perms = XS_PERM_NONE;
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &xt);
+        if (rc) goto out;
+
+        if (!libxl__xs_read(gc, xt, sshm_path)) {
+            /* every ID can appear in each domain at most once */
+            if (libxl__xs_read(gc, xt, dom_role_path)) {
+                SSHM_ERROR(domid, sshm->id,
+                           "domain tried to map the same ID twice.");
+                rc = ERROR_FAIL;
+                goto out;
+            }
+            rc = libxl__xs_write_checked(gc, xt, dom_role_path, "master");
+            if (rc) goto out;;
+
+            libxl__xs_mknod(gc, xt, sshm_path, &noperm, 1);
+            libxl__xs_writev(gc, xt, sshm_path, ents);
+        } else {
+            SSHM_ERROR(domid, sshm->id, "can only have one master.");
+            rc = ERROR_FAIL;
+            goto out;
+        }
+
+        rc = libxl__xs_transaction_commit(gc, &xt);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
+
+    rc = 0;
+out:
+    libxl__xs_transaction_abort(gc, &xt);
+    return rc;
+}
+
+int libxl__sshm_add(libxl__gc *gc,  uint32_t domid,
+                    libxl_static_shm *sshms, int len)
+{
+    int rc, i;
+
+    for (i = 0; i < len; ++i) {
+        if (sshms[i].role == LIBXL_SSHM_ROLE_SLAVE) {
+            rc = libxl__sshm_add_slave(gc, domid, sshms+i);
+        } else {
+            rc = libxl__sshm_add_master(gc, domid, sshms+i);
+        }
+        if (rc)  return rc;
+    }
+
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 455f6f0bed..bd792b6da9 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -587,6 +587,24 @@ void libxl__arch_domain_build_info_acpi_setdefault(
     libxl_defbool_setdefault(&b_info->acpi, true);
 }
 
+bool libxl__arch_domain_support_sshm(const libxl_domain_build_info *b_info)
+{
+    /* FIXME: Mark this as unsupported for calling p2m_add_foreign on two
+     * DomU's is currently not allowd on x86, see the comments in
+     * x86/mm/p2m.c: p2m_add_foreign */
+     return false;
+}
+
+int libxl__arch_domain_sshm_cachepolicy_setdefault(libxl_static_shm *sshm)
+{
+    if (sshm->cache_policy == LIBXL_SSHM_CACHEPOLICY_UNKNOWN)
+        sshm->cache_policy = LIBXL_SSHM_CACHEPOLICY_X86_NORMAL;
+    if (sshm->cache_policy < LIBXL_SSHM_CACHEPOLICY_X86_NORMAL)
+        return ERROR_INVAL;
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
2.14.0


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 6/6] libxl: support unmapping static shared memory areas during domain destruction
  2017-08-27  8:36 [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Zhongze Liu
                   ` (4 preceding siblings ...)
  2017-08-27  8:36 ` [PATCH v2 5/6] libxl: support mapping static shared memory areas during domain creation Zhongze Liu
@ 2017-08-27  8:36 ` Zhongze Liu
  2017-10-10 23:55 ` [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Stefano Stabellini
  6 siblings, 0 replies; 20+ messages in thread
From: Zhongze Liu @ 2017-08-27  8:36 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Julien Grall, Stefano Stabellini, Ian Jackson, Zhongze Liu

Add libxl__sshm_del to unmap static shared memory areas mapped by
libxl__sshm_add during domain creation. The unmapping process is:

* For a master: decrease the refcount of the sshm region, if the refcount
  reaches 0, cleanup the whole sshm path.
* For a slave: unmap the shared pages, and cleanup related xs entries.
  decrease the refcount of the sshm region, if the refcount reaches 0,
  cleanup the whole sshm path.

This is for the proposal "Allow setting up shared memory areas between VMs
from xl config file" (see [1]).

[1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html

Signed-off-by: Zhongze Liu <blackskygg@gmail.com>

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: xen-devel@lists.xen.org
---
v2:
  * use the predefined XC_PAGE_SIZE instead of hardcoding 12
  * refcounting the node
---
 tools/libxl/libxl_domain.c   |   5 +++
 tools/libxl/libxl_internal.h |   2 +
 tools/libxl/libxl_sshm.c     | 105 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+)

diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index 08eccd082b..73ac856fb4 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -1028,6 +1028,11 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
         goto out;
     }
 
+    rc = libxl__sshm_del(gc, domid);
+    if (rc) {
+        LOGD(ERROR, domid, "Deleting static shm failed.");
+    }
+
     if (libxl__device_pci_destroy_all(gc, domid) < 0)
         LOGD(ERROR, domid, "Pci shutdown failed");
     rc = xc_domain_pause(ctx->xch, domid);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 7ab2d40c49..731823dc1e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4360,6 +4360,8 @@ static inline bool libxl__acpi_defbool_val(const libxl_domain_build_info *b_info
 _hidden int libxl__sshm_add(libxl__gc *gc, uint32_t domid,
                             libxl_static_shm *sshm, int len);
 
+_hidden int libxl__sshm_del(libxl__gc *gc, uint32_t domid);
+
 _hidden int libxl__sshm_check_overlap(libxl__gc *gc, uint32_t domid,
                                       libxl_static_shm *sshms, int len);
 _hidden int libxl__sshm_setdefault(libxl__gc *gc, uint32_t domid,
diff --git a/tools/libxl/libxl_sshm.c b/tools/libxl/libxl_sshm.c
index a4091a3056..edfde0a905 100644
--- a/tools/libxl/libxl_sshm.c
+++ b/tools/libxl/libxl_sshm.c
@@ -86,6 +86,111 @@ int libxl__sshm_check_overlap(libxl__gc *gc, uint32_t domid,
     return 0;
 }
 
+/* Decrease the refcount of an sshm. When refcount reaches 0,
+ * clean up the whole sshm path */
+static void libxl__sshm_decref(libxl__gc *gc, xs_transaction_t xt,
+                               const char *sshm_path)
+{
+    int count;
+    const char *count_path, *count_string;
+
+    count_path = GCSPRINTF("%s/users", sshm_path);
+    if (libxl__xs_read_checked(gc, xt, count_path, &count_string))
+        return;
+    count = atoi(count_string);
+
+    if (--count == 0) {
+        libxl__xs_path_cleanup(gc, xt, sshm_path);
+        return;
+    }
+
+    count_string = GCSPRINTF("%d", count);
+    libxl__xs_write_checked(gc, xt, count_path, count_string);
+
+    return;
+}
+
+static void libxl__sshm_do_unmap(libxl__gc *gc, uint32_t domid, const char *id,
+                                 uint64_t begin, uint64_t end)
+{
+    begin >>= XC_PAGE_SHIFT;
+    end >>= XC_PAGE_SHIFT;
+    for (; begin < end; ++begin) {
+        if (xc_domain_remove_from_physmap(CTX->xch, domid, begin)) {
+            SSHM_ERROR(domid, id,
+                       "unable to unmap shared page at 0x%"PRIx64".",
+                       begin);
+        }
+    }
+}
+
+static void libxl__sshm_del_slave(libxl__gc *gc, xs_transaction_t xt,
+                                  uint32_t domid, const char *id, bool isretry)
+{
+    const char *slave_path, *begin_str, *end_str;
+    uint64_t begin, end;
+
+    slave_path = GCSPRINTF("%s/slaves/%"PRIu32, SSHM_PATH(id), domid);
+
+    begin_str = libxl__xs_read(gc, xt, GCSPRINTF("%s/begin", slave_path));
+    end_str = libxl__xs_read(gc, xt, GCSPRINTF("%s/end", slave_path));
+    begin = strtoull(begin_str, NULL, 16);
+    end = strtoull(end_str, NULL, 16);
+
+    /* Avoid calling do_unmap many times in case of xs transaction retry */
+    if (!isretry)
+        libxl__sshm_do_unmap(gc, domid, id, begin, end);
+
+    libxl__xs_path_cleanup(gc, xt, slave_path);
+}
+
+/* Delete static_shm entries in the xensotre. */
+int libxl__sshm_del(libxl__gc *gc,  uint32_t domid)
+{
+    int rc, i;
+    bool isretry;
+    xs_transaction_t xt = XBT_NULL;
+    const char *dom_path, *dom_sshm_path, *role;
+    char **sshm_ents;
+    unsigned int sshm_num;
+
+    dom_path = libxl__xs_get_dompath(gc, domid);
+    dom_sshm_path = GCSPRINTF("%s/static_shm", dom_path);
+
+    isretry = false;
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &xt);
+        if (rc) goto out;
+
+        if (libxl__xs_read(gc, xt, dom_sshm_path)) {
+            sshm_ents = libxl__xs_directory(gc, xt, dom_sshm_path, &sshm_num);
+            if (!sshm_ents) continue;
+
+            for (i = 0; i < sshm_num; ++i) {
+                role = libxl__xs_read(gc, xt,
+                                      GCSPRINTF("%s/%s/role",
+                                                dom_sshm_path,
+                                                sshm_ents[i]));
+                assert(role);
+                if (!strncmp(role, "slave", 5))
+                    libxl__sshm_del_slave(gc, xt, domid, sshm_ents[i], isretry);
+
+                libxl__sshm_decref(gc, xt, SSHM_PATH(sshm_ents[i]));
+            }
+        }
+
+        rc = libxl__xs_transaction_commit(gc, &xt);
+        if (!rc) break;
+        if (rc < 0) goto out;
+         isretry = true;
+    }
+
+    rc = 0;
+out:
+    libxl__xs_transaction_abort(gc, &xt);
+    return rc;
+}
+
 /* The caller have to guarentee that sshm->begin < sshm->end */
 static int libxl__sshm_do_map(libxl__gc *gc, uint32_t mid, uint32_t sid,
                               libxl_static_shm *sshm,
-- 
2.14.0


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/6] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin
  2017-08-27  8:36 ` [PATCH v2 4/6] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin Zhongze Liu
@ 2017-08-28  8:29   ` Jan Beulich
  2017-08-28 11:01     ` Zhongze Liu
  2017-08-28 14:24     ` George Dunlap
  0 siblings, 2 replies; 20+ messages in thread
From: Jan Beulich @ 2017-08-28  8:29 UTC (permalink / raw)
  To: Zhongze Liu
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, xen-devel, Julien Grall, Daniel De Graaf

>>> On 27.08.17 at 10:36, <blackskygg@gmail.com> wrote:
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1284,7 +1284,7 @@ int xenmem_add_to_physmap_one(
>              return -EINVAL;
>          }
>  
> -        rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od);
> +        rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, d, od);
>          if ( rc )
>          {
>              rcu_unlock_domain(od);
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -2545,7 +2545,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
>      if ( tdom == fdom )
>          goto out;
>  
> -    rc = xsm_map_gmfn_foreign(XSM_TARGET, tdom, fdom);
> +    rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, tdom, fdom);

I continue to dislike the added arguments here, as being pointless
to pass. I'm not the maintainer of either of the modified files, so I
won't (and can't) veto the change though.

> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -525,10 +525,14 @@ static XSM_INLINE int xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,
>      return xsm_default_action(action, d1, d2);
>  }
>  
> -static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, struct domain *t)
> +static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *cd,
> +                                           struct domain *d, struct domain *t)
>  {
> +    int rc;
>      XSM_ASSERT_ACTION(XSM_TARGET);

Missing blank line between declaration and statements.

> -    return xsm_default_action(action, d, t);
> +    rc = xsm_default_action(action, cd, d);
> +    if (rc) return rc;

Coding style. In any event, as suggested before the whole thing is
easier to write as

> +    return xsm_default_action(action, cd, t);

    return xsm_default_action(action, cd, d) ?: xsm_default_action(action, cd, t);

anyway imo (suitably split across lines if needed, of course).

> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1165,9 +1165,15 @@ static int flask_remove_from_physmap(struct domain *d1, struct domain *d2)
>      return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP);
>  }
>  
> -static int flask_map_gmfn_foreign(struct domain *d, struct domain *t)
> +static int flask_map_gmfn_foreign(struct domain *cd,
> +                                  struct domain *d, struct domain *t)
>  {
> -    return domain_has_perm(d, t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
> +    int rc;
> +    rc = domain_has_perm(cd, d, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
> +    if (rc) return rc;
> +    rc = domain_has_perm(cd, t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
> +    if (rc) return rc;
> +    return domain_has_perm(d, t, SECCLASS_MMU, MMU__SHARE_MEM);
>  }

At least the style problems mentioned above apply here too.

Jan


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/6] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin
  2017-08-28  8:29   ` Jan Beulich
@ 2017-08-28 11:01     ` Zhongze Liu
  2017-08-28 11:18       ` Jan Beulich
  2017-08-28 14:24     ` George Dunlap
  1 sibling, 1 reply; 20+ messages in thread
From: Zhongze Liu @ 2017-08-28 11:01 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, xen-devel, Julien Grall, Daniel De Graaf

Hi Jan,

Thanks for having a look at the patch.

2017-08-28 16:29 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>> On 27.08.17 at 10:36, <blackskygg@gmail.com> wrote:
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -1284,7 +1284,7 @@ int xenmem_add_to_physmap_one(
>>              return -EINVAL;
>>          }
>>
>> -        rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od);
>> +        rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, d, od);
>>          if ( rc )
>>          {
>>              rcu_unlock_domain(od);
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -2545,7 +2545,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
>>      if ( tdom == fdom )
>>          goto out;
>>
>> -    rc = xsm_map_gmfn_foreign(XSM_TARGET, tdom, fdom);
>> +    rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, tdom, fdom);
>
> I continue to dislike the added arguments here, as being pointless
> to pass. I'm not the maintainer of either of the modified files, so I
> won't (and can't) veto the change though.
>
>> --- a/xen/include/xsm/dummy.h
>> +++ b/xen/include/xsm/dummy.h
>> @@ -525,10 +525,14 @@ static XSM_INLINE int xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,
>>      return xsm_default_action(action, d1, d2);
>>  }
>>
>> -static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, struct domain *t)
>> +static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *cd,
>> +                                           struct domain *d, struct domain *t)
>>  {
>> +    int rc;
>>      XSM_ASSERT_ACTION(XSM_TARGET);
>
> Missing blank line between declaration and statements.

Sorry. Will fix this.

>
>> -    return xsm_default_action(action, d, t);
>> +    rc = xsm_default_action(action, cd, d);
>> +    if (rc) return rc;
>
> Coding style. In any event, as suggested before the whole thing is
> easier to write as
>
>> +    return xsm_default_action(action, cd, t);
>
>     return xsm_default_action(action, cd, d) ?: xsm_default_action(action, cd, t);

But aren't we going to preserve the error code here?

Cheers,

Zhongze Liu

>
> anyway imo (suitably split across lines if needed, of course).
>
>> --- a/xen/xsm/flask/hooks.c
>> +++ b/xen/xsm/flask/hooks.c
>> @@ -1165,9 +1165,15 @@ static int flask_remove_from_physmap(struct domain *d1, struct domain *d2)
>>      return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP);
>>  }
>>
>> -static int flask_map_gmfn_foreign(struct domain *d, struct domain *t)
>> +static int flask_map_gmfn_foreign(struct domain *cd,
>> +                                  struct domain *d, struct domain *t)
>>  {
>> -    return domain_has_perm(d, t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
>> +    int rc;
>> +    rc = domain_has_perm(cd, d, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
>> +    if (rc) return rc;
>> +    rc = domain_has_perm(cd, t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
>> +    if (rc) return rc;
>> +    return domain_has_perm(d, t, SECCLASS_MMU, MMU__SHARE_MEM);
>>  }
>
> At least the style problems mentioned above apply here too.
>
> Jan
>

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/6] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin
  2017-08-28 11:01     ` Zhongze Liu
@ 2017-08-28 11:18       ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-08-28 11:18 UTC (permalink / raw)
  To: Zhongze Liu
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, xen-devel, Julien Grall, Daniel De Graaf

>>> On 28.08.17 at 13:01, <blackskygg@gmail.com> wrote:
> 2017-08-28 16:29 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>>>>> On 27.08.17 at 10:36, <blackskygg@gmail.com> wrote:
>>> -static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, struct domain *t)
>>> +static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *cd,
>>> +                                           struct domain *d, struct domain *t)
>>>  {
>>> +    int rc;
>>>      XSM_ASSERT_ACTION(XSM_TARGET);
>>
>> Missing blank line between declaration and statements.
> 
> Sorry. Will fix this.
> 
>>
>>> -    return xsm_default_action(action, d, t);
>>> +    rc = xsm_default_action(action, cd, d);
>>> +    if (rc) return rc;
>>
>> Coding style. In any event, as suggested before the whole thing is
>> easier to write as
>>
>>> +    return xsm_default_action(action, cd, t);
>>
>>     return xsm_default_action(action, cd, d) ?: xsm_default_action(action, cd, t);
> 
> But aren't we going to preserve the error code here?

I don't understand the question - if the first function invocation
returns an error, that is what the function here will return. Else
it returns what the second xsm_default_action() invocation
hands back.

Jan


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/6] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin
  2017-08-28  8:29   ` Jan Beulich
  2017-08-28 11:01     ` Zhongze Liu
@ 2017-08-28 14:24     ` George Dunlap
  2017-08-28 14:53       ` Jan Beulich
  1 sibling, 1 reply; 20+ messages in thread
From: George Dunlap @ 2017-08-28 14:24 UTC (permalink / raw)
  To: Jan Beulich, Zhongze Liu
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, xen-devel, Julien Grall, Daniel De Graaf

On 08/28/2017 09:29 AM, Jan Beulich wrote:
>>>> On 27.08.17 at 10:36, <blackskygg@gmail.com> wrote:
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -1284,7 +1284,7 @@ int xenmem_add_to_physmap_one(
>>              return -EINVAL;
>>          }
>>  
>> -        rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od);
>> +        rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, d, od);
>>          if ( rc )
>>          {
>>              rcu_unlock_domain(od);
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -2545,7 +2545,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
>>      if ( tdom == fdom )
>>          goto out;
>>  
>> -    rc = xsm_map_gmfn_foreign(XSM_TARGET, tdom, fdom);
>> +    rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, tdom, fdom);
> 
> I continue to dislike the added arguments here, as being pointless
> to pass. I'm not the maintainer of either of the modified files, so I
> won't (and can't) veto the change though.

You mean, you think xsm_map_gmfn_foreign() can look up 'current' itself?

If not can you be more explicit what you'd prefer?

  -George

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/6] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin
  2017-08-28 14:24     ` George Dunlap
@ 2017-08-28 14:53       ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-08-28 14:53 UTC (permalink / raw)
  To: George Dunlap, Zhongze Liu
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, xen-devel, Julien Grall, Daniel De Graaf

>>> On 28.08.17 at 16:24, <george.dunlap@citrix.com> wrote:
> On 08/28/2017 09:29 AM, Jan Beulich wrote:
>>>>> On 27.08.17 at 10:36, <blackskygg@gmail.com> wrote:
>>> --- a/xen/arch/arm/mm.c
>>> +++ b/xen/arch/arm/mm.c
>>> @@ -1284,7 +1284,7 @@ int xenmem_add_to_physmap_one(
>>>              return -EINVAL;
>>>          }
>>>  
>>> -        rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od);
>>> +        rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, d, od);
>>>          if ( rc )
>>>          {
>>>              rcu_unlock_domain(od);
>>> --- a/xen/arch/x86/mm/p2m.c
>>> +++ b/xen/arch/x86/mm/p2m.c
>>> @@ -2545,7 +2545,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
>>>      if ( tdom == fdom )
>>>          goto out;
>>>  
>>> -    rc = xsm_map_gmfn_foreign(XSM_TARGET, tdom, fdom);
>>> +    rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, tdom, fdom);
>> 
>> I continue to dislike the added arguments here, as being pointless
>> to pass. I'm not the maintainer of either of the modified files, so I
>> won't (and can't) veto the change though.
> 
> You mean, you think xsm_map_gmfn_foreign() can look up 'current' itself?

Yes, that's what I had suggested before (in more detail than I
have given here); I also don't think it would be
xsm_map_gmfn_foreign(), but rather whatever functions back
it.

Jan


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files
  2017-08-27  8:36 ` [PATCH v2 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files Zhongze Liu
@ 2017-09-01 16:03   ` Wei Liu
  2017-09-01 17:56     ` Zhongze Liu
  2017-09-01 16:25   ` Wei Liu
  1 sibling, 1 reply; 20+ messages in thread
From: Wei Liu @ 2017-09-01 16:03 UTC (permalink / raw)
  To: Zhongze Liu
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, George Dunlap,
	Andrew Cooper, Ian Jackson, xen-devel, Julien Grall, Jan Beulich

On Sun, Aug 27, 2017 at 04:36:12PM +0800, Zhongze Liu wrote:
> Add the parsing utils for the newly introduced libxl_static_sshm struct
> to the libxl/libxlu_* family. And add realated parsing code in xl to
> parse the struct from xl config files. This is for the proposal "Allow
> setting up shared memory areas between VMs from xl config file" (see [1]).
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html
> 
> Signed-off-by: Zhongze Liu <blackskygg@gmail.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

I am wondering if there is a chance to extract the key=value parsing
into standalone function and use it everywhere though.

> 
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: xen-devel@lists.xen.org

Correctly trim your CC list next time please. A lot of hypervisor people
don't need to get CCed on this patch.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 5/6] libxl: support mapping static shared memory areas during domain creation
  2017-08-27  8:36 ` [PATCH v2 5/6] libxl: support mapping static shared memory areas during domain creation Zhongze Liu
@ 2017-09-01 16:23   ` Wei Liu
  2017-09-01 17:49     ` Zhongze Liu
  0 siblings, 1 reply; 20+ messages in thread
From: Wei Liu @ 2017-09-01 16:23 UTC (permalink / raw)
  To: Zhongze Liu
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, George Dunlap,
	Andrew Cooper, Ian Jackson, xen-devel, Julien Grall, Jan Beulich

On Sun, Aug 27, 2017 at 04:36:14PM +0800, Zhongze Liu wrote:
> Add libxl__sshm_add to map shared pages from one DomU to another, The mapping
> process involves the follwing steps:
> 
>   * Set defaults and check for further errors in the static_shm configs:
>     overlapping areas, invalid ranges, duplicated master domain,
>     no master domain etc.
>   * Write infomation of static shared memory areas into the appropriate
>     xenstore paths.
>   * Use xc_domain_add_to_physmap_batch to do the page sharing.
>   * Set the refcount of the shared region accordingly
> 
> Temporarily mark this as unsupported on x86 because calling p2m_add_foregin on
> two domU's is currently not allowd on x86 (see the comments in
> x86/mm/p2m.c:p2m_add_foregin for more details).
> 
> This is for the proposal "Allow setting up shared memory areas between VMs
> from xl config file" (see [1]).
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html
> 
> Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
[...]
> +
> +        rc = libxl__xs_read_checked(gc, xt,
> +                                    GCSPRINTF("%s/master", sshm_path),
> +                                    &xs_value);
> +        if (rc) goto out;
> +        master_domid = strtoull(xs_value, NULL, 16);
> +
> +        if (sshm->prot == LIBXL_SSHM_PROT_UNKNOWN) {
> +            sshm->prot = master_sshm.prot;
> +        }
> +        /* check if the slave is asking too much permission */
> +        if (master_sshm.prot < sshm->prot) {
> +            SSHM_ERROR(domid, sshm->id, "slave is asking too much permission.");
> +            rc = ERROR_INVAL;
> +            goto out;
> +        }
> +
> +        /* all checks passed, do the job */
> +        if (!isretry) {
> +            rc = libxl__sshm_do_map(gc, master_domid, domid, sshm,
> +                                    master_sshm.begin, master_sshm.end);
> +            if (rc) goto out;

You probably need to roll back the mapping should the transaction gets
aborted in this loop.

> +        }
> +
> +        /* write the result to xenstore and commit */
> +        rc = libxl__xs_write_checked(gc, xt, dom_role_path, "slave");
> +        if (rc) goto out;
> +        rc = libxl__xs_writev(gc, xt, slave_path, ents);
> +        if (rc) goto out;
> +        rc = libxl__sshm_incref(gc, xt, sshm_path);
> +        if (rc) goto out;
> +
> +        rc = libxl__xs_transaction_commit(gc, &xt);
> +        if (!rc) break;
> +        if (rc < 0) goto out;
> +        isretry = true;
> +    }
> +
> +    rc = 0;
> +out:
> +    libxl__xs_transaction_abort(gc, &xt);
> +    return rc;
> +}
> +
> +static int libxl__sshm_add_master(libxl__gc *gc, uint32_t domid,
> +                                  libxl_static_shm *sshm)
> +{
> +

No refcount increment here?

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files
  2017-08-27  8:36 ` [PATCH v2 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files Zhongze Liu
  2017-09-01 16:03   ` Wei Liu
@ 2017-09-01 16:25   ` Wei Liu
  1 sibling, 0 replies; 20+ messages in thread
From: Wei Liu @ 2017-09-01 16:25 UTC (permalink / raw)
  To: Zhongze Liu
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, George Dunlap,
	Andrew Cooper, Ian Jackson, xen-devel, Julien Grall, Jan Beulich

On Sun, Aug 27, 2017 at 04:36:12PM +0800, Zhongze Liu wrote:
> Add the parsing utils for the newly introduced libxl_static_sshm struct
> to the libxl/libxlu_* family. And add realated parsing code in xl to
> parse the struct from xl config files. This is for the proposal "Allow
> setting up shared memory areas between VMs from xl config file" (see [1]).
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html
> 
> Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
> 

BTW when you resend this patch should go last because it depends on
others.

And you didn't patch documents (manpage, xenstore path doc).

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 5/6] libxl: support mapping static shared memory areas during domain creation
  2017-09-01 16:23   ` Wei Liu
@ 2017-09-01 17:49     ` Zhongze Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Zhongze Liu @ 2017-09-01 17:49 UTC (permalink / raw)
  To: Wei Liu
  Cc: Tim Deegan, Stefano Stabellini, George Dunlap, Andrew Cooper,
	Ian Jackson, xen-devel, Julien Grall, Jan Beulich

Hi Wei,

Thanks for reviewing.

2017-09-02 0:23 GMT+08:00 Wei Liu <wei.liu2@citrix.com>:
> On Sun, Aug 27, 2017 at 04:36:14PM +0800, Zhongze Liu wrote:
>> Add libxl__sshm_add to map shared pages from one DomU to another, The mapping
>> process involves the follwing steps:
>>
>>   * Set defaults and check for further errors in the static_shm configs:
>>     overlapping areas, invalid ranges, duplicated master domain,
>>     no master domain etc.
>>   * Write infomation of static shared memory areas into the appropriate
>>     xenstore paths.
>>   * Use xc_domain_add_to_physmap_batch to do the page sharing.
>>   * Set the refcount of the shared region accordingly
>>
>> Temporarily mark this as unsupported on x86 because calling p2m_add_foregin on
>> two domU's is currently not allowd on x86 (see the comments in
>> x86/mm/p2m.c:p2m_add_foregin for more details).
>>
>> This is for the proposal "Allow setting up shared memory areas between VMs
>> from xl config file" (see [1]).
>>
>> [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html
>>
>> Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
> [...]
>> +
>> +        rc = libxl__xs_read_checked(gc, xt,
>> +                                    GCSPRINTF("%s/master", sshm_path),
>> +                                    &xs_value);
>> +        if (rc) goto out;
>> +        master_domid = strtoull(xs_value, NULL, 16);
>> +
>> +        if (sshm->prot == LIBXL_SSHM_PROT_UNKNOWN) {
>> +            sshm->prot = master_sshm.prot;
>> +        }
>> +        /* check if the slave is asking too much permission */
>> +        if (master_sshm.prot < sshm->prot) {
>> +            SSHM_ERROR(domid, sshm->id, "slave is asking too much permission.");
>> +            rc = ERROR_INVAL;
>> +            goto out;
>> +        }
>> +
>> +        /* all checks passed, do the job */
>> +        if (!isretry) {
>> +            rc = libxl__sshm_do_map(gc, master_domid, domid, sshm,
>> +                                    master_sshm.begin, master_sshm.end);
>> +            if (rc) goto out;
>
> You probably need to roll back the mapping should the transaction gets
> aborted in this loop.

OK. I'll move the rollback code to the abort path.

>
>> +        }
>> +
>> +        /* write the result to xenstore and commit */
>> +        rc = libxl__xs_write_checked(gc, xt, dom_role_path, "slave");
>> +        if (rc) goto out;
>> +        rc = libxl__xs_writev(gc, xt, slave_path, ents);
>> +        if (rc) goto out;
>> +        rc = libxl__sshm_incref(gc, xt, sshm_path);
>> +        if (rc) goto out;
>> +
>> +        rc = libxl__xs_transaction_commit(gc, &xt);
>> +        if (!rc) break;
>> +        if (rc < 0) goto out;
>> +        isretry = true;
>> +    }
>> +
>> +    rc = 0;
>> +out:
>> +    libxl__xs_transaction_abort(gc, &xt);
>> +    return rc;
>> +}
>> +
>> +static int libxl__sshm_add_master(libxl__gc *gc, uint32_t domid,
>> +                                  libxl_static_shm *sshm)
>> +{
>> +
>
> No refcount increment here?

For every sshm ID, this will be called only once. And in the xenstore
ents array, I've  already
set "users = 1", so there's no need to call libxl__sshm_incref().


Cheers,

Zhongze Liu

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files
  2017-09-01 16:03   ` Wei Liu
@ 2017-09-01 17:56     ` Zhongze Liu
  2017-09-05 13:10       ` Wei Liu
  0 siblings, 1 reply; 20+ messages in thread
From: Zhongze Liu @ 2017-09-01 17:56 UTC (permalink / raw)
  To: Wei Liu
  Cc: Tim Deegan, Stefano Stabellini, George Dunlap, Andrew Cooper,
	Ian Jackson, xen-devel, Julien Grall, Jan Beulich

2017-09-02 0:03 GMT+08:00 Wei Liu <wei.liu2@citrix.com>:
> On Sun, Aug 27, 2017 at 04:36:12PM +0800, Zhongze Liu wrote:
>> Add the parsing utils for the newly introduced libxl_static_sshm struct
>> to the libxl/libxlu_* family. And add realated parsing code in xl to
>> parse the struct from xl config files. This is for the proposal "Allow
>> setting up shared memory areas between VMs from xl config file" (see [1]).
>>
>> [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html
>>
>> Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
>
> I am wondering if there is a chance to extract the key=value parsing
> into standalone function and use it everywhere though.

I think this is a good idea.

>
>>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Tim Deegan <tim@xen.org>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>> Cc: Julien Grall <julien.grall@arm.com>
>> Cc: xen-devel@lists.xen.org
>
> Correctly trim your CC list next time please. A lot of hypervisor people
> don't need to get CCed on this patch.

Actually, this list is generated by get_maintainer.pl. Maybe next time I should
go carefully through the list and remove some of the names manually.

Cheers,

Zhongze Liu

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files
  2017-09-01 17:56     ` Zhongze Liu
@ 2017-09-05 13:10       ` Wei Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Wei Liu @ 2017-09-05 13:10 UTC (permalink / raw)
  To: Zhongze Liu
  Cc: Tim Deegan, Stefano Stabellini, Wei Liu, George Dunlap,
	Andrew Cooper, Ian Jackson, xen-devel, Julien Grall, Jan Beulich

On Sat, Sep 02, 2017 at 01:56:35AM +0800, Zhongze Liu wrote:
> 2017-09-02 0:03 GMT+08:00 Wei Liu <wei.liu2@citrix.com>:
> > On Sun, Aug 27, 2017 at 04:36:12PM +0800, Zhongze Liu wrote:
> >> Add the parsing utils for the newly introduced libxl_static_sshm struct
> >> to the libxl/libxlu_* family. And add realated parsing code in xl to
> >> parse the struct from xl config files. This is for the proposal "Allow
> >> setting up shared memory areas between VMs from xl config file" (see [1]).
> >>
> >> [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html
> >>
> >> Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
> >
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> >
> > I am wondering if there is a chance to extract the key=value parsing
> > into standalone function and use it everywhere though.
> 
> I think this is a good idea.

To be clear, I'm not saying that's a prerequisite for this series.

> 
> >
> >>
> >> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> >> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> >> Cc: Jan Beulich <jbeulich@suse.com>
> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> Cc: Stefano Stabellini <sstabellini@kernel.org>
> >> Cc: Tim Deegan <tim@xen.org>
> >> Cc: Wei Liu <wei.liu2@citrix.com>
> >> Cc: Julien Grall <julien.grall@arm.com>
> >> Cc: xen-devel@lists.xen.org
> >
> > Correctly trim your CC list next time please. A lot of hypervisor people
> > don't need to get CCed on this patch.
> 
> Actually, this list is generated by get_maintainer.pl. Maybe next time I should
> go carefully through the list and remove some of the names manually.
> 

You could have used it the wrong way.

Either

  $ ./scripts/get_maintainers.pl -f FILE # not patch

or

  $ ./scripts/get_maintainers.pl    PATCH # not file

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files
  2017-08-27  8:36 [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Zhongze Liu
                   ` (5 preceding siblings ...)
  2017-08-27  8:36 ` [PATCH v2 6/6] libxl: support unmapping static shared memory areas during domain destruction Zhongze Liu
@ 2017-10-10 23:55 ` Stefano Stabellini
  2017-10-11  8:26   ` Zhongze Liu
  6 siblings, 1 reply; 20+ messages in thread
From: Stefano Stabellini @ 2017-10-10 23:55 UTC (permalink / raw)
  To: Zhongze Liu; +Cc: Julien Grall, Stefano Stabellini, xen-devel

On Sun, 27 Aug 2017, Zhongze Liu wrote:
> This series implements the new xl config entry proposed in [1]. Users can use
> the new config entry to statically setup shared memory areas among VMs that
> don't have grant table support so that they could communicate with each other
> through the static shared memory areas.
> 
> [1] Proposla to allow setting up shared memory areas between VMs from xl config file:
>   https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html
> 
> v2:
>   * fixed several code style issues.
>   * introduce MMU__SHARE_MEM in flask av permissions, and add a check to this.
>     permission in the flask hook for xsm_map_gmfn_foreign.
>   * support rolling back during creation on partial failure.
>   * refcounting the sshm path instead of using "alive" and "zombie" to label the
>     master and counting the slaves.

Hey Zhongze,

any plans on sending an update of this series?


> Cheers,
> 
> Zhongze Liu (6):
>   libxc: add xc_domain_remove_from_physmap to wrap
>     XENMEM_remove_from_physmap
>   libxl: introduce a new structure to represent static shared memory
>     regions
>   libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config
>     files
>   xsm: flask: change the dummy xsm policy and flask hook for
>     map_gmfn_foregin
>   libxl: support mapping static shared memory areas during domain
>     creation
>   libxl: support unmapping static shared memory areas during domain
>     destruction
> 
>  tools/flask/policy/modules/xen.if   |   2 +
>  tools/libxc/include/xenctrl.h       |   4 +
>  tools/libxc/xc_domain.c             |  11 +
>  tools/libxl/Makefile                |   4 +-
>  tools/libxl/libxl.h                 |   4 +
>  tools/libxl/libxl_arch.h            |   6 +
>  tools/libxl/libxl_arm.c             |  15 ++
>  tools/libxl/libxl_create.c          |  27 ++
>  tools/libxl/libxl_domain.c          |   5 +
>  tools/libxl/libxl_internal.h        |  15 ++
>  tools/libxl/libxl_sshm.c            | 480 ++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_types.idl         |  34 ++-
>  tools/libxl/libxl_x86.c             |  18 ++
>  tools/libxl/libxlu_sshm.c           | 210 ++++++++++++++++
>  tools/libxl/libxlutil.h             |   6 +
>  tools/xl/xl_parse.c                 |  24 +-
>  xen/arch/arm/mm.c                   |   2 +-
>  xen/arch/x86/mm/p2m.c               |   2 +-
>  xen/include/xsm/dummy.h             |   8 +-
>  xen/include/xsm/xsm.h               |   7 +-
>  xen/xsm/flask/hooks.c               |  10 +-
>  xen/xsm/flask/policy/access_vectors |   4 +
>  22 files changed, 883 insertions(+), 15 deletions(-)
>  create mode 100644 tools/libxl/libxl_sshm.c
>  create mode 100644 tools/libxl/libxlu_sshm.c
> 
> -- 
> 2.14.0
> 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files
  2017-10-10 23:55 ` [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Stefano Stabellini
@ 2017-10-11  8:26   ` Zhongze Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Zhongze Liu @ 2017-10-11  8:26 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Julien Grall, xen-devel

Hi Stefano,

Sorry for the long delay. I've been busy with other school stuff recently so
I can only use my spare time on this. The current situation is: the
code has already been completed, but I'm still drafting the man pages
and there are approximately 20% left. I'll try my best to update it within
the following one week or so.

Sorry again for failing to schedule my time effectively.

Cheers,

Zhongze Liu.

2017-10-11 7:55 GMT+08:00 Stefano Stabellini <sstabellini@kernel.org>:
> On Sun, 27 Aug 2017, Zhongze Liu wrote:
>> This series implements the new xl config entry proposed in [1]. Users can use
>> the new config entry to statically setup shared memory areas among VMs that
>> don't have grant table support so that they could communicate with each other
>> through the static shared memory areas.
>>
>> [1] Proposla to allow setting up shared memory areas between VMs from xl config file:
>>   https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html
>>
>> v2:
>>   * fixed several code style issues.
>>   * introduce MMU__SHARE_MEM in flask av permissions, and add a check to this.
>>     permission in the flask hook for xsm_map_gmfn_foreign.
>>   * support rolling back during creation on partial failure.
>>   * refcounting the sshm path instead of using "alive" and "zombie" to label the
>>     master and counting the slaves.
>
> Hey Zhongze,
>
> any plans on sending an update of this series?
>
>
>> Cheers,
>>
>> Zhongze Liu (6):
>>   libxc: add xc_domain_remove_from_physmap to wrap
>>     XENMEM_remove_from_physmap
>>   libxl: introduce a new structure to represent static shared memory
>>     regions
>>   libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config
>>     files
>>   xsm: flask: change the dummy xsm policy and flask hook for
>>     map_gmfn_foregin
>>   libxl: support mapping static shared memory areas during domain
>>     creation
>>   libxl: support unmapping static shared memory areas during domain
>>     destruction
>>
>>  tools/flask/policy/modules/xen.if   |   2 +
>>  tools/libxc/include/xenctrl.h       |   4 +
>>  tools/libxc/xc_domain.c             |  11 +
>>  tools/libxl/Makefile                |   4 +-
>>  tools/libxl/libxl.h                 |   4 +
>>  tools/libxl/libxl_arch.h            |   6 +
>>  tools/libxl/libxl_arm.c             |  15 ++
>>  tools/libxl/libxl_create.c          |  27 ++
>>  tools/libxl/libxl_domain.c          |   5 +
>>  tools/libxl/libxl_internal.h        |  15 ++
>>  tools/libxl/libxl_sshm.c            | 480 ++++++++++++++++++++++++++++++++++++
>>  tools/libxl/libxl_types.idl         |  34 ++-
>>  tools/libxl/libxl_x86.c             |  18 ++
>>  tools/libxl/libxlu_sshm.c           | 210 ++++++++++++++++
>>  tools/libxl/libxlutil.h             |   6 +
>>  tools/xl/xl_parse.c                 |  24 +-
>>  xen/arch/arm/mm.c                   |   2 +-
>>  xen/arch/x86/mm/p2m.c               |   2 +-
>>  xen/include/xsm/dummy.h             |   8 +-
>>  xen/include/xsm/xsm.h               |   7 +-
>>  xen/xsm/flask/hooks.c               |  10 +-
>>  xen/xsm/flask/policy/access_vectors |   4 +
>>  22 files changed, 883 insertions(+), 15 deletions(-)
>>  create mode 100644 tools/libxl/libxl_sshm.c
>>  create mode 100644 tools/libxl/libxlu_sshm.c
>>
>> --
>> 2.14.0
>>

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-10-11  8:26 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-27  8:36 [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Zhongze Liu
2017-08-27  8:36 ` [PATCH v2 1/6] libxc: add xc_domain_remove_from_physmap to wrap XENMEM_remove_from_physmap Zhongze Liu
2017-08-27  8:36 ` [PATCH v2 2/6] libxl: introduce a new structure to represent static shared memory regions Zhongze Liu
2017-08-27  8:36 ` [PATCH v2 3/6] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files Zhongze Liu
2017-09-01 16:03   ` Wei Liu
2017-09-01 17:56     ` Zhongze Liu
2017-09-05 13:10       ` Wei Liu
2017-09-01 16:25   ` Wei Liu
2017-08-27  8:36 ` [PATCH v2 4/6] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin Zhongze Liu
2017-08-28  8:29   ` Jan Beulich
2017-08-28 11:01     ` Zhongze Liu
2017-08-28 11:18       ` Jan Beulich
2017-08-28 14:24     ` George Dunlap
2017-08-28 14:53       ` Jan Beulich
2017-08-27  8:36 ` [PATCH v2 5/6] libxl: support mapping static shared memory areas during domain creation Zhongze Liu
2017-09-01 16:23   ` Wei Liu
2017-09-01 17:49     ` Zhongze Liu
2017-08-27  8:36 ` [PATCH v2 6/6] libxl: support unmapping static shared memory areas during domain destruction Zhongze Liu
2017-10-10 23:55 ` [PATCH v2 0/6] Allow setting up shared memory areas between VMs from xl config files Stefano Stabellini
2017-10-11  8:26   ` Zhongze Liu

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.