Re: [Xen-devel] [PATCH v8 3/8] libxl: support mapping static shared memory areas during domain creation

2018-10-31 Thread Ian Jackson
Julien Grall writes ("Re: [PATCH v8 3/8] libxl: support mapping static shared 
memory areas during domain creation"):
> On 10/30/18 3:36 PM, Ian Jackson wrote:
> > This cache policy stuff is odd.  I couldn't see it being used by the
> > hypervisor.  Why is it even there ?
> 
> We decided to not implement them in the hypervisor yet but still provide 
> the xl interface for it. Would there be any issue to extern the 
> interface later on?

If you haven't implemented the hypervisor part, how do you know what
it will be like ?  Doing things the way you are means committing to
API design decisions very early.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 3/8] libxl: support mapping static shared memory areas during domain creation

2018-10-30 Thread Julien Grall



On 10/30/18 3:36 PM, Ian Jackson wrote:

Stefano Stabellini writes ("[PATCH v8 3/8] libxl: support mapping static shared 
memory areas during domain creation"):

+_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  0xfee0

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 25dc3de..054ad58 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -1138,6 +1138,21 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc 
*gc,
  libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH);
  }
  
+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;


This cache policy stuff is odd.  I couldn't see it being used by the
hypervisor.  Why is it even there ?
We decided to not implement them in the hypervisor yet but still provide 
the xl interface for it. Would there be any issue to extern the 
interface later on?


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 3/8] libxl: support mapping static shared memory areas during domain creation

2018-10-30 Thread Ian Jackson
Stefano Stabellini writes ("[PATCH v8 3/8] libxl: support mapping static shared 
memory areas during domain creation"):
> +_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  0xfee0
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 25dc3de..054ad58 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -1138,6 +1138,21 @@ void 
> libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
>  libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH);
>  }
>  
> +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;

This cache policy stuff is odd.  I couldn't see it being used by the
hypervisor.  Why is it even there ?

The same question about prot.

>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 320dbed..45ae9e4 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -513,6 +513,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;
> @@ -949,6 +957,25 @@ static void initiate_domain_create(libxl__egc *egc,
>  goto error_out;
>  }
>  
> +if (d_config->num_sshms != 0 &&
> +!libxl__arch_domain_support_sshm(_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, _config->sshms[i]);
> +if (ret) {
> +LOGD(ERROR, domid, "Unable to set defaults for static shm");
> +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, >build_state, );
>  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 43947b1..6f31a3d 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -4434,6 +4434,20 @@ static inline const char 
> *libxl__qemu_qmp_path(libxl__gc *gc, int domid)
>  #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
>   * c-basic-offset: 4
> diff --git a/tools/libxl/libxl_sshm.c b/tools/libxl/libxl_sshm.c
> new file mode 100644
> index 000..f61b80c
> --- /dev/null
> +++ b/tools/libxl/libxl_sshm.c
> @@ -0,0 +1,405 @@
> +#include "libxl_osdeps.h"
> +#include "libxl_internal.h"
> +#include "libxl_arch.h"
> +
> +#define SSHM_PATH(id) GCSPRINTF("/libxl/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_SLAVE &&
> +sshm->role != LIBXL_SSHM_ROLE_MASTER)
> +return ERROR_INVAL;
> +if (sshm->begin & ~XC_PAGE_MASK ||
> +sshm->size & ~XC_PAGE_MASK ||
> +(sshm->offset != LIBXL_SSHM_RANGE_UNKNOWN &&
> +sshm->offset & ~XC_PAGE_MASK)) {
> +SSHM_ERROR(domid, 

Re: [Xen-devel] [PATCH v8 3/8] libxl: support mapping static shared memory areas during domain creation

2018-10-24 Thread Oleksandr Tyshchenko
Hi, Stefano

On Tue, Oct 9, 2018 at 2:39 AM Stefano Stabellini
 wrote:
>
> From: Zhongze Liu 
>
> Author: Zhongze Liu 
>
> Add libxl__sshm_add to map shared pages from one DomU to another, The mapping
> process involves the following steps:
>
>   * Set defaults and check for further errors in the static_shm configs:
> overlapping areas, invalid ranges, duplicated master domain,
> not page aligned, no master domain etc.
>   * Use xc_domain_add_to_physmap_batch to map the shared pages to slaves
>   * When some of the pages can't be successfully mapped, roll back any
> successfully mapped pages so that the system stays in a consistent state.
>   * Write information about static shared memory areas into the appropriate
> xenstore paths and set the refcount of the shared region accordingly.
>
> Temporarily mark this as unsupported on x86 because 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 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 
> Signed-off-by: Stefano Stabellini 
>
> Cc: Wei Liu 
> Cc: Ian Jackson 
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> Cc: xen-de...@lists.xen.org
> ---
> Changes in v5:
> - fix typos
> - add comments
> - add value checks (including alignment checks) in sshm_setdefaults
> ---
>  tools/libxl/Makefile |   3 +-
>  tools/libxl/libxl_arch.h |   6 +
>  tools/libxl/libxl_arm.c  |  15 ++
>  tools/libxl/libxl_create.c   |  27 +++
>  tools/libxl/libxl_internal.h |  14 ++
>  tools/libxl/libxl_sshm.c | 405 
> +++
>  tools/libxl/libxl_x86.c  |  19 ++
>  7 files changed, 488 insertions(+), 1 deletion(-)
>  create mode 100644 tools/libxl/libxl_sshm.c
>
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 6da342e..53af186 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -140,7 +140,8 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o 
> libxl_dm.o libxl_pci.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_vdispl.o \
> -   libxl_pvcalls.o libxl_vsnd.o libxl_vkb.o 
> $(LIBXL_OBJS-y)
> +   libxl_pvcalls.o libxl_vsnd.o libxl_vkb.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 930570e..63c26cc 100644
> --- a/tools/libxl/libxl_arch.h
> +++ b/tools/libxl/libxl_arch.h
> @@ -73,6 +73,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  0xfee0
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 25dc3de..054ad58 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -1138,6 +1138,21 @@ void 
> libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
>  libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH);
>  }
>
> +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 320dbed..45ae9e4 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -513,6 +513,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;
> @@ -949,6 +957,25 @@ static void initiate_domain_create(libxl__egc *egc,
>  goto error_out;
>  }