[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH XEN RFC] libxc: use an uncached mapping of guest ram in domain builder



On Wed, Dec 18, 2013 at 05:30:47PM +0000, Ian Campbell wrote:
> VERY MUCH A WIP.
> 
> On ARM guest OSes are started with MMU and Caches disables (as they are on
> native) however caching is enabled in the domain running the builder and
> therefore we must use an explcitly uncached mapping, otherwise when the guest
> starts running it may not see them.
> 
> Cache flushes are not sufficient because there is a race between the flush and
> the unmap, where the processor may speculatively fill a cache line. Thanks to
> Catalin Marinas and Marc Zyngier for pointing this out.
> 
> Therefore use the newly introduced IOCTL_PRIVCMD_MMAPBATCH_V2_UNCACHED on ARM
> when mapping guest memory from xc_dom_* (which means xc_dom_boot_domU_map in
> practice).  This avoids issues with the processor dirtying cache lines while
> the guest will then fail to see because it starts with MMU and caches
> disabled.

Why not make the default IOCTL_PRIVCMD_MMAPBATCH_V2 when running under ARM
do this?

> 
> The xc_map_foreign_* functions here are a bit of a twisty maze, so far I have
> just updated exactly the call path which is used by the function which I
> needed to update. I have not yet considered non-xc_linux*.c nor tried to
> disentangle this into a saner patch, nor any level of consistency with the
> other foreign mapping functions. I've not even tried to compile on x86.
> 
> This applies on top of a revert of "tools: libxc: flush data cache after
> loading images into guest memory" (a0035ecc0d82) which I will include in the
> eventual proper posting.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> ---
>  tools/include/xen-sys/Linux/privcmd.h |    2 ++
>  tools/libxc/xc_core_x86.c             |    4 ++--
>  tools/libxc/xc_dom_boot.c             |    2 +-
>  tools/libxc/xc_domain_restore.c       |    4 ++--
>  tools/libxc/xc_domain_save.c          |    4 ++--
>  tools/libxc/xc_foreign_memory.c       |   19 ++++++++++++++-----
>  tools/libxc/xc_gnttab.c               |    2 +-
>  tools/libxc/xc_linux_osdep.c          |   18 +++++++++++++-----
>  tools/libxc/xc_private.h              |    3 +++
>  tools/libxc/xenctrl.h                 |    5 +++--
>  tools/libxc/xenctrl_osdep_ENOSYS.c    |    4 ++--
>  tools/libxc/xenctrlosdep.h            |    4 ++--
>  12 files changed, 47 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/include/xen-sys/Linux/privcmd.h 
> b/tools/include/xen-sys/Linux/privcmd.h
> index 5be860a..e3a92a0 100644
> --- a/tools/include/xen-sys/Linux/privcmd.h
> +++ b/tools/include/xen-sys/Linux/privcmd.h
> @@ -88,5 +88,7 @@ typedef struct privcmd_mmapbatch_v2 {
>       _IOC(_IOC_NONE, 'P', 3, sizeof(privcmd_mmapbatch_t))
>  #define IOCTL_PRIVCMD_MMAPBATCH_V2                           \
>       _IOC(_IOC_NONE, 'P', 4, sizeof(privcmd_mmapbatch_v2_t))
> +#define IOCTL_PRIVCMD_MMAPBATCH_V2_UNCACHED                  \
> +     _IOC(_IOC_NONE, 'P', 5, sizeof(privcmd_mmapbatch_v2_t))
>  
>  #endif /* __LINUX_PUBLIC_PRIVCMD_H__ */
> diff --git a/tools/libxc/xc_core_x86.c b/tools/libxc/xc_core_x86.c
> index e328dcf..8e73bc4 100644
> --- a/tools/libxc/xc_core_x86.c
> +++ b/tools/libxc/xc_core_x86.c
> @@ -131,7 +131,7 @@ xc_core_arch_map_p2m_rw(xc_interface *xch, struct 
> domain_info_context *dinfo, xc
>      live_p2m_frame_list =
>          xc_map_foreign_pages(xch, dom, PROT_READ,
>                               p2m_frame_list_list,
> -                             P2M_FLL_ENTRIES);
> +                             P2M_FLL_ENTRIES, 1);
>  
>      if ( !live_p2m_frame_list )
>      {
> @@ -159,7 +159,7 @@ xc_core_arch_map_p2m_rw(xc_interface *xch, struct 
> domain_info_context *dinfo, xc
>      *live_p2m = xc_map_foreign_pages(xch, dom,
>                                      rw ? (PROT_READ | PROT_WRITE) : 
> PROT_READ,
>                                      p2m_frame_list,
> -                                    P2M_FL_ENTRIES);
> +                                    P2M_FL_ENTRIES, 1);
>  
>      if ( !*live_p2m )
>      {
> diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> index fdfeaf8..691fc2b 100644
> --- a/tools/libxc/xc_dom_boot.c
> +++ b/tools/libxc/xc_dom_boot.c
> @@ -179,7 +179,7 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, 
> xen_pfn_t pfn,
>      for ( i = 0; i < count; i++ )
>          entries[i].mfn = xc_dom_p2m_host(dom, pfn + i);
>  
> -    ptr = xc_map_foreign_ranges(dom->xch, dom->guest_domid,
> +    ptr = xc_map_foreign_ranges_uncached(dom->xch, dom->guest_domid,
>                  count << page_shift, PROT_READ | PROT_WRITE, 1 << page_shift,
>                  entries, count);
>      if ( ptr == NULL )
> diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
> index 80769a7..a4c621b 100644
> --- a/tools/libxc/xc_domain_restore.c
> +++ b/tools/libxc/xc_domain_restore.c
> @@ -1885,7 +1885,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, 
> uint32_t dom,
>              if ( (i == (dinfo->p2m_size-1)) || (j == MAX_BATCH_SIZE) )
>              {
>                  region_base = xc_map_foreign_pages(
> -                    xch, dom, PROT_READ | PROT_WRITE, region_mfn, j);
> +                    xch, dom, PROT_READ | PROT_WRITE, region_mfn, j, 1);
>                  if ( region_base == NULL )
>                  {
>                      PERROR("map batch failed");
> @@ -2222,7 +2222,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, 
> uint32_t dom,
>  
>      /* Copy the P2M we've constructed to the 'live' P2M */
>      if ( !(ctx->live_p2m = xc_map_foreign_pages(xch, dom, PROT_WRITE,
> -                                           p2m_frame_list, P2M_FL_ENTRIES)) )
> +                                                p2m_frame_list, 
> P2M_FL_ENTRIES, 1)) )
>      {
>          PERROR("Couldn't map p2m table");
>          goto out;
> diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
> index 42c4752..00c97f7 100644
> --- a/tools/libxc/xc_domain_save.c
> +++ b/tools/libxc/xc_domain_save.c
> @@ -631,7 +631,7 @@ static xen_pfn_t *map_and_save_p2m_table(xc_interface 
> *xch,
>      live_p2m_frame_list =
>          xc_map_foreign_pages(xch, dom, PROT_READ,
>                               p2m_frame_list_list,
> -                             P2M_FLL_ENTRIES);
> +                             P2M_FLL_ENTRIES, 1);
>      if ( !live_p2m_frame_list )
>      {
>          PERROR("Couldn't map p2m_frame_list");
> @@ -666,7 +666,7 @@ static xen_pfn_t *map_and_save_p2m_table(xc_interface 
> *xch,
>  
>      p2m = xc_map_foreign_pages(xch, dom, PROT_READ,
>                                 p2m_frame_list,
> -                               P2M_FL_ENTRIES);
> +                               P2M_FL_ENTRIES, 1);
>      if ( !p2m )
>      {
>          PERROR("Couldn't map p2m table");
> diff --git a/tools/libxc/xc_foreign_memory.c b/tools/libxc/xc_foreign_memory.c
> index 7dfc817..ac4793d 100644
> --- a/tools/libxc/xc_foreign_memory.c
> +++ b/tools/libxc/xc_foreign_memory.c
> @@ -21,7 +21,7 @@
>  #include "xc_private.h"
>  
>  void *xc_map_foreign_pages(xc_interface *xch, uint32_t dom, int prot,
> -                           const xen_pfn_t *arr, int num)
> +                           const xen_pfn_t *arr, int num, int cached)
>  {
>      void *res;
>      int i, *err;
> @@ -35,7 +35,7 @@ void *xc_map_foreign_pages(xc_interface *xch, uint32_t dom, 
> int prot,
>      if (!err)
>          return NULL;
>  
> -    res = xc_map_foreign_bulk(xch, dom, prot, arr, err, num);
> +    res = xc_map_foreign_bulk(xch, dom, prot, arr, err, num, cached);
>      if (res) {
>          for (i = 0; i < num; i++) {
>              if (err[i]) {
> @@ -63,7 +63,15 @@ void *xc_map_foreign_ranges(xc_interface *xch, uint32_t 
> dom,
>                              privcmd_mmap_entry_t entries[], int nentries)
>  {
>      return xch->ops->u.privcmd.map_foreign_ranges(xch, xch->ops_handle,
> -                                                  dom, size, prot, 
> chunksize, entries, nentries);
> +                                                  dom, size, prot, 
> chunksize, entries, nentries, 1);
> +}
> +
> +void *xc_map_foreign_ranges_uncached(xc_interface *xch, uint32_t dom,
> +                            size_t size, int prot, size_t chunksize,
> +                            privcmd_mmap_entry_t entries[], int nentries)
> +{
> +    return xch->ops->u.privcmd.map_foreign_ranges(xch, xch->ops_handle,
> +                                                  dom, size, prot, 
> chunksize, entries, nentries, 0);
>  }
>  
>  void *xc_map_foreign_batch(xc_interface *xch, uint32_t dom, int prot,
> @@ -74,10 +82,11 @@ void *xc_map_foreign_batch(xc_interface *xch, uint32_t 
> dom, int prot,
>  }
>  
>  void *xc_map_foreign_bulk(xc_interface *xch, uint32_t dom, int prot,
> -                          const xen_pfn_t *arr, int *err, unsigned int num)
> +                          const xen_pfn_t *arr, int *err, unsigned int num,
> +                          int cached)
>  {
>      return xch->ops->u.privcmd.map_foreign_bulk(xch, xch->ops_handle,
> -                                                dom, prot, arr, err, num);
> +                                                dom, prot, arr, err, num, 
> cached);
>  }
>  
>  /* stub for all not yet converted OSes */
> diff --git a/tools/libxc/xc_gnttab.c b/tools/libxc/xc_gnttab.c
> index 79dab40..4774c69 100644
> --- a/tools/libxc/xc_gnttab.c
> +++ b/tools/libxc/xc_gnttab.c
> @@ -114,7 +114,7 @@ static void *_gnttab_map_table(xc_interface *xch, int 
> domid, int *gnt_num)
>          pfn_list[i] = frame_list[i];
>  
>      gnt = xc_map_foreign_pages(xch, domid, PROT_READ, pfn_list,
> -                               setup.nr_frames);
> +                               setup.nr_frames, 1);
>      if ( !gnt )
>      {
>          ERROR("Could not map grant table\n");
> diff --git a/tools/libxc/xc_linux_osdep.c b/tools/libxc/xc_linux_osdep.c
> index 73860a2..09aae37 100644
> --- a/tools/libxc/xc_linux_osdep.c
> +++ b/tools/libxc/xc_linux_osdep.c
> @@ -246,7 +246,8 @@ out:
>  
>  static void *linux_privcmd_map_foreign_bulk(xc_interface *xch, 
> xc_osdep_handle h,
>                                              uint32_t dom, int prot,
> -                                            const xen_pfn_t *arr, int *err, 
> unsigned int num)
> +                                            const xen_pfn_t *arr, int *err,
> +                                            unsigned int num, int cached)
>  {
>      int fd = (int)h;
>      privcmd_mmapbatch_v2_t ioctlx;
> @@ -254,6 +255,11 @@ static void *linux_privcmd_map_foreign_bulk(xc_interface 
> *xch, xc_osdep_handle h
>      unsigned int i;
>      int rc;
>  
> +#if defined(__i386__) || defined(__x86_64__)
> +    /* No need for cache maintenance on x86 */
> +    cached = 1;
> +#endif
> +
>      addr = mmap(NULL, (unsigned long)num << XC_PAGE_SHIFT, prot, MAP_SHARED,
>                  fd, 0);
>      if ( addr == MAP_FAILED )
> @@ -268,7 +274,9 @@ static void *linux_privcmd_map_foreign_bulk(xc_interface 
> *xch, xc_osdep_handle h
>      ioctlx.arr = arr;
>      ioctlx.err = err;
>  
> -    rc = ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH_V2, &ioctlx);
> +    rc = ioctl(fd, cached ? IOCTL_PRIVCMD_MMAPBATCH_V2
> +                          : IOCTL_PRIVCMD_MMAPBATCH_V2_UNCACHED,
> +               &ioctlx);
>  
>      /* Command was recognized, some gfn in arr are in paging state */
>      if ( rc < 0 && errno == ENOENT )
> @@ -384,7 +392,7 @@ static void *linux_privcmd_map_foreign_range(xc_interface 
> *xch, xc_osdep_handle
>      for ( i = 0; i < num; i++ )
>          arr[i] = mfn + i;
>  
> -    ret = xc_map_foreign_pages(xch, dom, prot, arr, num);
> +    ret = xc_map_foreign_pages(xch, dom, prot, arr, num, 1);
>      free(arr);
>      return ret;
>  }
> @@ -392,7 +400,7 @@ static void *linux_privcmd_map_foreign_range(xc_interface 
> *xch, xc_osdep_handle
>  static void *linux_privcmd_map_foreign_ranges(xc_interface *xch, 
> xc_osdep_handle h,
>                                                uint32_t dom, size_t size, int 
> prot,
>                                                size_t chunksize, 
> privcmd_mmap_entry_t entries[],
> -                                              int nentries)
> +                                              int nentries, int cached)
>  {
>      xen_pfn_t *arr;
>      int num_per_entry;
> @@ -411,7 +419,7 @@ static void 
> *linux_privcmd_map_foreign_ranges(xc_interface *xch, xc_osdep_handle
>          for ( j = 0; j < num_per_entry; j++ )
>              arr[i * num_per_entry + j] = entries[i].mfn + j;
>  
> -    ret = xc_map_foreign_pages(xch, dom, prot, arr, num);
> +    ret = xc_map_foreign_pages(xch, dom, prot, arr, num, 1);
>      free(arr);
>      return ret;
>  }
> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
> index 92271c9..e040179 100644
> --- a/tools/libxc/xc_private.h
> +++ b/tools/libxc/xc_private.h
> @@ -294,6 +294,9 @@ int do_memory_op(xc_interface *xch, int cmd, void *arg, 
> size_t len);
>  void *xc_map_foreign_ranges(xc_interface *xch, uint32_t dom,
>                              size_t size, int prot, size_t chunksize,
>                              privcmd_mmap_entry_t entries[], int nentries);
> +void *xc_map_foreign_ranges_uncached(xc_interface *xch, uint32_t dom,
> +                            size_t size, int prot, size_t chunksize,
> +                            privcmd_mmap_entry_t entries[], int nentries);
>  
>  int xc_get_pfn_type_batch(xc_interface *xch, uint32_t dom,
>                            unsigned int num, xen_pfn_t *);
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 6e58ebe..b20731c 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -1324,7 +1324,7 @@ void *xc_map_foreign_range(xc_interface *xch, uint32_t 
> dom,
>                              unsigned long mfn );
>  
>  void *xc_map_foreign_pages(xc_interface *xch, uint32_t dom, int prot,
> -                           const xen_pfn_t *arr, int num );
> +                           const xen_pfn_t *arr, int num, int cached);
>  
>  /**
>   * DEPRECATED - use xc_map_foreign_bulk() instead.
> @@ -1342,7 +1342,8 @@ void *xc_map_foreign_batch(xc_interface *xch, uint32_t 
> dom, int prot,
>   * set to the corresponding errno value.
>   */
>  void *xc_map_foreign_bulk(xc_interface *xch, uint32_t dom, int prot,
> -                          const xen_pfn_t *arr, int *err, unsigned int num);
> +                          const xen_pfn_t *arr, int *err, unsigned int num,
> +                       int cached);
>  
>  /**
>   * Translates a virtual address in the context of a given domain and
> diff --git a/tools/libxc/xenctrl_osdep_ENOSYS.c 
> b/tools/libxc/xenctrl_osdep_ENOSYS.c
> index 4821342..d26c696 100644
> --- a/tools/libxc/xenctrl_osdep_ENOSYS.c
> +++ b/tools/libxc/xenctrl_osdep_ENOSYS.c
> @@ -42,7 +42,7 @@ static void *ENOSYS_privcmd_map_foreign_batch(xc_interface 
> *xch, xc_osdep_handle
>  }
>  
>  static void *ENOSYS_privcmd_map_foreign_bulk(xc_interface *xch, 
> xc_osdep_handle h, uint32_t dom, int prot,
> -                                     const xen_pfn_t *arr, int *err, 
> unsigned int num)
> +                                     const xen_pfn_t *arr, int *err, 
> unsigned int num, int cached)
>  {
>      IPRINTF(xch, "ENOSYS_privcmd %p: map_foreign_buld: dom%d prot %#x arr %p 
> err %p num %d\n", h, dom, prot, arr, err, num);
>      return MAP_FAILED;
> @@ -57,7 +57,7 @@ static void *ENOSYS_privcmd_map_foreign_range(xc_interface 
> *xch, xc_osdep_handle
>  
>  static void *ENOSYS_privcmd_map_foreign_ranges(xc_interface *xch, 
> xc_osdep_handle h, uint32_t dom, size_t size, int prot,
>                                         size_t chunksize, 
> privcmd_mmap_entry_t entries[],
> -                                       int nentries)
> +                                       int nentries, int cached)
>  {
>      IPRINTF(xch, "ENOSYS_privcmd %p: map_foreign_ranges: dom%d size %zd prot 
> %#x chunksize %zd entries %p num %d\n", h, dom, size, prot, chunksize, 
> entries, nentries);
>      return MAP_FAILED;
> diff --git a/tools/libxc/xenctrlosdep.h b/tools/libxc/xenctrlosdep.h
> index e610a24..3daf757 100644
> --- a/tools/libxc/xenctrlosdep.h
> +++ b/tools/libxc/xenctrlosdep.h
> @@ -83,12 +83,12 @@ struct xc_osdep_ops
>              void *(*map_foreign_batch)(xc_interface *xch, xc_osdep_handle h, 
> uint32_t dom, int prot,
>                                         xen_pfn_t *arr, int num);
>              void *(*map_foreign_bulk)(xc_interface *xch, xc_osdep_handle h, 
> uint32_t dom, int prot,
> -                                      const xen_pfn_t *arr, int *err, 
> unsigned int num);
> +                                      const xen_pfn_t *arr, int *err, 
> unsigned int num, int cached);
>              void *(*map_foreign_range)(xc_interface *xch, xc_osdep_handle h, 
> uint32_t dom, int size, int prot,
>                                         unsigned long mfn);
>              void *(*map_foreign_ranges)(xc_interface *xch, xc_osdep_handle 
> h, uint32_t dom, size_t size, int prot,
>                                          size_t chunksize, 
> privcmd_mmap_entry_t entries[],
> -                                        int nentries);
> +                                        int nentries, int cached);
>          } privcmd;
>          struct {
>              int (*fd)(xc_evtchn *xce, xc_osdep_handle h);
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.