From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Ian Campbell <ian.campbell@citrix.com>
Cc: tim@xen.org, julien.grall@linaro.org,
Ian Jackson <ian.jackson@eu.citrix.com>,
stefano.stabellini@eu.citrix.com, xen-devel@lists.xen.org
Subject: Re: [PATCH XEN RFC] libxc: use an uncached mapping of guest ram in domain builder
Date: Wed, 18 Dec 2013 16:14:49 -0500 [thread overview]
Message-ID: <20131218211449.GA11717@phenom.dumpdata.com> (raw)
In-Reply-To: <1387387847-19821-1-git-send-email-ian.campbell@citrix.com>
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@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
> 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@lists.xen.org
> http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2013-12-18 21:14 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-18 17:28 [PATCH RFC] xen: arm: use uncached foreign mappings when building guests Ian Campbell
2013-12-18 17:30 ` [PATCH LINUX RFC] xen: privcmd: implement IOCTL_PRIVCMD_MMAPBATCH_V2_UNCACHED Ian Campbell
2013-12-18 19:24 ` Stefano Stabellini
2013-12-18 21:16 ` Konrad Rzeszutek Wilk
2013-12-20 9:19 ` Ian Campbell
2013-12-20 14:13 ` Konrad Rzeszutek Wilk
2013-12-20 14:18 ` Ian Campbell
2013-12-20 14:38 ` Konrad Rzeszutek Wilk
2013-12-20 14:44 ` Ian Campbell
2013-12-18 17:30 ` [PATCH XEN RFC] libxc: use an uncached mapping of guest ram in domain builder Ian Campbell
2013-12-18 21:14 ` Konrad Rzeszutek Wilk [this message]
2013-12-20 9:19 ` Ian Campbell
2013-12-18 18:41 ` [PATCH RFC] xen: arm: use uncached foreign mappings when building guests David Vrabel
2013-12-19 10:10 ` Ian Campbell
2013-12-19 11:23 ` Stefano Stabellini
2013-12-19 11:29 ` Ian Campbell
2014-01-02 15:10 ` David Vrabel
2014-01-06 10:05 ` Ian Campbell
2013-12-19 4:16 ` Julien Grall
2013-12-19 4:26 ` Julien Grall
2013-12-19 14:30 ` Ian Campbell
2014-01-06 12:10 ` Ian Campbell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131218211449.GA11717@phenom.dumpdata.com \
--to=konrad.wilk@oracle.com \
--cc=ian.campbell@citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=julien.grall@linaro.org \
--cc=stefano.stabellini@eu.citrix.com \
--cc=tim@xen.org \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.