[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC PATCH] tools/libxl : add struct and parsing utils for the 'static_shm' xl config entry
On Wed, 19 Jul 2017, Zhongze Liu wrote: > Add a new struct libxl_static_shm in the libxl IDL for the proposed new xl > config entry 'static_shm' (see [1]), which allow the user to set up shared > memory areas among several VMs for communication. > > Add related parsing code to the libxl/libxlu_* family and xl/xl_parse.c > > [1]: [RFC v3]Proposal to allow setting up shared memory areas between VMs > from xl config file, > > https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg01741.html > > Signed-off-by: Zhongze Liu <blackskygg@xxxxxxxxx> > --- > Cc: Wei Liu <wei.liu2@xxxxxxxxxx> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx> > Cc: Julien Grall <julien.grall@xxxxxxx> > Cc: xen-devel@xxxxxxxxxxxxx > --- > tools/libxl/Makefile | 2 +- > tools/libxl/libxl.h | 10 ++ > tools/libxl/libxl_types.idl | 52 +++++++++ > tools/libxl/libxlu_sshm.c | 274 > ++++++++++++++++++++++++++++++++++++++++++++ > tools/libxl/libxlutil.h | 6 + > tools/xl/xl_parse.c | 24 +++- > 6 files changed, 366 insertions(+), 2 deletions(-) > create mode 100644 tools/libxl/libxlu_sshm.c > > diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile > index 2ffb78f5c4..b7effb188b 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/libxl.h b/tools/libxl/libxl.h > index 7cf0f31f68..cf3cbe1ba1 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -2228,6 +2228,16 @@ 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); > > + > +/* Functions to stattically set up shared memory regions between two domains ^ statically ^double space > + * for shm-based communication. */ > + > +#define LIBXL_SSHM_RANGE_UNKNOWN UINT64_MAX > + > +/* TODO: int libxl_sshm_add(libxl_ctx *ctx, uint32_t domid, > + * libxl_static_shm *sshm); > + */ > + > #include <libxl_event.h> > > #endif /* LIBXL_H */ > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 8a9849c643..8c68b45add 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -779,6 +779,57 @@ libxl_device_channel = Struct("device_channel", [ > ])), > ]) > > +# static shared memory cacheability attributes > +libxl_sshm_cacheattr = Enumeration("sshm_cacheattr", [ > + (-1, "UNKNOWN"), > + (0, "UC"), > + (1, "WC"), #x86 only > + (4, "WT"), > + (5, "WP"), #x86 only > + (6, "WB"), > + (7, "SUC"), #x86 only > + (8, "BUFFERABLE"), #ARM only > + (9, "WA"), #ARM only > + ], init_val = "LIBXL_SSHM_CACHEATTR_UNKNOWN") I would only specify UNKNOWN and WB for now. > +# static shared memory shareability attributes > +libxl_sshm_shareattr = Enumeration("sshm_shareattr", [ > + (-1, "UNKNOWN"), > + (0, "NON"), > + (2, "OUTER"), > + (3, "INNER"), > + ], init_val = "LIBXL_SSHM_SHAREATTR_UNKNOWN") > + > +libxl_sshm_prot = Enumeration("sshm_prot", [ > + (-1, "UNKNOWN"), > + (0, "N"), > + (1, "R"), > + (2, "W"), > + (4, "X"), > + (3, "RW"), > + (5, "RX"), > + (6, "WX"), > + (7, "RWX"), > + ], 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), > + ("begin", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}), > + ("end", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}), > + ("prot", libxl_sshm_prot), > + ("arm_shareattr", libxl_sshm_shareattr), > + ("arm_inner_cacheattr", libxl_sshm_cacheattr), > + ("arm_outer_cacheattr", libxl_sshm_cacheattr), I would have a single arm_cacheattr > + ("x86_cacheattr", libxl_sshm_cacheattr), > + ("role", libxl_sshm_role), > +]) > + > libxl_domain_config = Struct("domain_config", [ > ("c_info", libxl_domain_create_info), > ("b_info", libxl_domain_build_info), > @@ -797,6 +848,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), > diff --git a/tools/libxl/libxlu_sshm.c b/tools/libxl/libxlu_sshm.c > new file mode 100644 > index 0000000000..fcd65af4d9 > --- /dev/null > +++ b/tools/libxl/libxlu_sshm.c > @@ -0,0 +1,274 @@ > +#include "libxl_osdeps.h" /* must come before any other headers */ > +#include "libxlu_internal.h" > + > +#include <ctype.h> > + > +#define PARAM_RE(EXPR) "^\\s*" EXPR "\\s*(,|$)" > +#define KEY_RE "([_a-zA-Z0-9]+)" > +#define VAL_RE "([^ \t\n,]+)" > +#define EQU_RE PARAM_RE(KEY_RE "\\s*=\\s*" VAL_RE) > + > +#define MASK_4K ((uint64_t)0xfff) > +#define MAX_ID_LEN 128 > +#define CACHEATTR_ARM 0 > +#define CACHEATTR_X86 1 > + > +#define INVAL_ERR(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) { \ > + INVAL_ERR("\"" 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, "r") || !strcmp(str, "ro")) { > + new_prot = LIBXL_SSHM_PROT_R; > + } else if (!strcmp(str, "w") || !strcmp(str, "wo")) { > + new_prot = LIBXL_SSHM_PROT_W; > + } else if (!strcmp(str, "x") || !strcmp(str, "xo")) { > + new_prot = LIBXL_SSHM_PROT_X; > + } else if (!strcmp(str, "rw")) { > + new_prot = LIBXL_SSHM_PROT_RW; > + } else if (!strcmp(str, "rx")) { > + new_prot = LIBXL_SSHM_PROT_RX; > + } else if (!strcmp(str, "wx")) { > + new_prot = LIBXL_SSHM_PROT_WX; > + } else if (!strcmp(str, "rwx")) { > + new_prot = LIBXL_SSHM_PROT_RWX; > + } else if (!strcmp(str, "n")) { > + new_prot = LIBXL_SSHM_PROT_N; > + } else { > + INVAL_ERR("invalid permission flags", str); shouldn't this return an error? > + } > + > + SET_VAL(*prot, "permission flags", PROT, new_prot, str); > + > + rc = 0; > + > + out: > + return rc; > +} > + > +static int parse_cacheattr(XLU_Config *cfg, char *str, int arch, > + libxl_sshm_cacheattr *cattr) > +{ > + int rc; > + libxl_sshm_cacheattr new_cattr; > + > + if (!strcmp(str, "uc")) { > + new_cattr = LIBXL_SSHM_CACHEATTR_UC; > + } else if (!strcmp(str, "wc")) { > + if (CACHEATTR_X86 != arch) { > + INVAL_ERR("invalid cacheability attribute", str); > + } > + new_cattr = LIBXL_SSHM_CACHEATTR_WC; > + } else if (!strcmp(str, "wt")) { > + new_cattr = LIBXL_SSHM_CACHEATTR_WT; > + } else if (!strcmp(str, "wp")) { > + if (CACHEATTR_X86 != arch) { > + INVAL_ERR("invalid cacheability attribute", str); > + } > + new_cattr = LIBXL_SSHM_CACHEATTR_WP; > + } else if (!strcmp(str, "wb")) { > + new_cattr = LIBXL_SSHM_CACHEATTR_WB; > + } else if (!strcmp(str, "suc")) { > + if (CACHEATTR_X86 != arch) { > + INVAL_ERR("invalid cacheability attribute", str); > + } > + new_cattr = LIBXL_SSHM_CACHEATTR_SUC; > + } else if (!strcmp(str, "bufferable")) { > + if (CACHEATTR_ARM != arch) { > + INVAL_ERR("invalid cacheability attribute", str); > + } > + new_cattr = LIBXL_SSHM_CACHEATTR_BUFFERABLE; > + } else if (!strcmp(str, "wa")) { > + if (CACHEATTR_ARM != arch) { > + INVAL_ERR("invalid cacheability attribute", str); > + } > + new_cattr = LIBXL_SSHM_CACHEATTR_WA; > + } else { > + INVAL_ERR("invalid cacheability attribute", str); shouldn't this return an error? > + } I don't know if the other maintainers agree, but I think we should just check that str is "wb" and fail in all other cases. > + SET_VAL(*cattr, "cacheability attributes", CACHEATTR, new_cattr, str); > + rc = 0; > + > + out: > + return rc; > +} > + > +static int parse_shareattr(XLU_Config *cfg, char *str, > + libxl_sshm_shareattr *sattr) > +{ > + int rc; > + libxl_sshm_shareattr new_sattr; > + > + if (!strcmp(str, "non")) { > + new_sattr = LIBXL_SSHM_SHAREATTR_NON; > + } else if (!strcmp(str, "outer")) { > + new_sattr = LIBXL_SSHM_SHAREATTR_OUTER; > + } else if (!strcmp(str, "inner")) { > + new_sattr = LIBXL_SSHM_SHAREATTR_INNER; > + } else { > + INVAL_ERR("invalid arm shareability attribute", str); shouldn't this return an error? > + } > + > + SET_VAL(*sattr, "arm shareability attributes", SHAREATTR, new_sattr, > 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) > MAX_ID_LEN) { INVAL_ERR("id too long", val); } > + if (sshm->id && !strcmp(sshm->id, val)) { > + INVAL_ERR("id respecified", val); > + } > + > + if (NULL == (sshm->id = strdup(val))) { > + 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 { > + INVAL_ERR("invalid role", val); > + } > + > + SET_VAL(sshm->role, "role", ROLE, new_role, val); > + > + } else if (!strcmp(key, "begin") || !strcmp(key, "end")) { > + char *endptr; > + int base = 10; > + uint64_t new_bound; > + > + /* could be in hex form */ > + if ('0' == val[0] && 'x' == val[1]) { base = 16; } Shouldn't you check that val is at least 2 in length? > + new_bound = strtoull(val, &endptr, base); > + if (ERANGE == errno || *endptr) { > + INVAL_ERR("invalid begin/end", val); > + } > + if (new_bound & MASK_4K) { > + INVAL_ERR("begin/end is not a multiple of 4K", val); > + } > + > + /* begin or end */ > + if ('b' == key[0]) { > + SET_VAL(sshm->begin, "beginning address", RANGE, new_bound, val); > + } else { > + SET_VAL(sshm->end, "ending address", RANGE, new_bound, val); > + } > + } else if (!strcmp(key, "prot")) { > + rc = parse_prot(cfg, val, &sshm->prot); > + if (rc) { goto out; } > + } else if (!strcmp(key, "arm_inner_cacheattr")) { > + rc = parse_cacheattr(cfg, val, CACHEATTR_ARM, > + &sshm->arm_inner_cacheattr); > + if (rc) { goto out; } > + } else if (!strcmp(key, "arm_outer_cacheattr")) { > + rc = parse_cacheattr(cfg, val, CACHEATTR_ARM, > + &sshm->arm_outer_cacheattr); > + if (rc) { goto out; } > + } else if (!strcmp(key, "x86_cacheattr")) { > + rc = parse_cacheattr(cfg, val, CACHEATTR_X86, > + &sshm->x86_cacheattr); > + if (rc) { goto out; } > + } else if (!strcmp(key, "arm_shareattr")) { > + rc = parse_shareattr(cfg, val, &sshm->arm_shareattr); > + if (rc) { goto out; } > + } else { > + INVAL_ERR("invalid option", key); shouldn't this return an error? > + } > + > + 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; > + } > + > + if (NULL == (buf2 = ptr = strdup(spec))) { > + fprintf(stderr, "sshm parser out of memory\n"); > + rc = ENOMEM; > + goto out; > + } > + > + while (true) { > + if (!*ptr) { break; } > + if (regexec(&equ_rec, ptr, 3, pmatch, 0)) { > + INVAL_ERR("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) { INVAL_ERR("invalid syntax", ptr); } > + > + 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.13.3 > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |