* [PATCH v2 1/3] tools/libxencall: use hypercall buffer device if available
2018-06-18 7:18 [PATCH v2 0/3] fix hypercall buffer locking in memory Juergen Gross
@ 2018-06-18 7:18 ` Juergen Gross
2018-06-18 11:26 ` Ian Jackson
2018-06-18 7:18 ` [PATCH v2 2/3] tools/libxencalls: add new function to query hypercall buffer safety Juergen Gross
2018-06-18 7:18 ` [PATCH v2 3/3] tools/libxc: retry hypercall in case of EFAULT Juergen Gross
2 siblings, 1 reply; 6+ messages in thread
From: Juergen Gross @ 2018-06-18 7:18 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, wei.liu2, ian.jackson
Instead of using anonymous memory for hypercall buffers which is then
locked into memory, use the hypercall buffer device of the Linux
privcmd driver if available.
This has the advantage of needing just a single mmap() for allocating
the buffer and page migration or compaction can't make the buffer
unaccessible for the hypervisor.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- add some comments to tools/libs/call/private.h (Ian Jackson)
- some control flow modifications (Ian Jackson)
- rename /dev/xen/provcmd-buf to /dev/xen/hypercall (Andrew Cooper)
- omit fallback to xenfs (Ian Jackson)
- treat errors other than ENOENT as fatal (Ian Jackson)
---
tools/libs/call/core.c | 12 +++++++++-
tools/libs/call/linux.c | 58 +++++++++++++++++++++++++++++++++++++++++------
tools/libs/call/private.h | 7 +++++-
3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/tools/libs/call/core.c b/tools/libs/call/core.c
index f3a34009da..46ca61529e 100644
--- a/tools/libs/call/core.c
+++ b/tools/libs/call/core.c
@@ -19,7 +19,16 @@
static int all_restrict_cb(Xentoolcore__Active_Handle *ah, domid_t domid) {
xencall_handle *xcall = CONTAINER_OF(ah, *xcall, tc_ah);
- return xentoolcore__restrict_by_dup2_null(xcall->fd);
+ int rc;
+
+ rc = xentoolcore__restrict_by_dup2_null(xcall->buf_fd);
+ if ( rc )
+ goto out;
+
+ rc = xentoolcore__restrict_by_dup2_null(xcall->fd);
+
+out:
+ return rc;
}
xencall_handle *xencall_open(xentoollog_logger *logger, unsigned open_flags)
@@ -30,6 +39,7 @@ xencall_handle *xencall_open(xentoollog_logger *logger, unsigned open_flags)
if (!xcall) return NULL;
xcall->fd = -1;
+ xcall->buf_fd = -1;
xcall->tc_ah.restrict_callback = all_restrict_cb;
xentoolcore__register_active_handle(&xcall->tc_ah);
diff --git a/tools/libs/call/linux.c b/tools/libs/call/linux.c
index 3f1b691fe7..90c2691486 100644
--- a/tools/libs/call/linux.c
+++ b/tools/libs/call/linux.c
@@ -56,15 +56,28 @@ int osdep_xencall_open(xencall_handle *xcall)
}
xcall->fd = fd;
+
+ /*
+ * Try the same for the hypercall buffer device.
+ */
+ fd = open("/dev/xen/hypercall", O_RDWR|O_CLOEXEC);
+ if ( fd == -1 && errno != ENOENT )
+ {
+ PERROR("Error on trying to open hypercall buffer device");
+ return -1;
+ }
+ xcall->buf_fd = fd;
+
return 0;
}
int osdep_xencall_close(xencall_handle *xcall)
{
- int fd = xcall->fd;
- if (fd == -1)
- return 0;
- return close(fd);
+ if ( xcall->buf_fd >= 0 )
+ close(xcall->buf_fd);
+ if ( xcall->fd >= 0 )
+ close(xcall->fd);
+ return 0;
}
int osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall)
@@ -72,7 +85,22 @@ int osdep_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall)
return ioctl(xcall->fd, IOCTL_PRIVCMD_HYPERCALL, hypercall);
}
-void *osdep_alloc_pages(xencall_handle *xcall, size_t npages)
+static void *alloc_pages_bufdev(xencall_handle *xcall, size_t npages)
+{
+ void *p;
+
+ p = mmap(NULL, npages * PAGE_SIZE, PROT_READ|PROT_WRITE, MAP_SHARED,
+ xcall->buf_fd, 0);
+ if ( p == MAP_FAILED )
+ {
+ PERROR("alloc_pages: mmap failed");
+ p = NULL;
+ }
+
+ return p;
+}
+
+static void *alloc_pages_nobufdev(xencall_handle *xcall, size_t npages)
{
size_t size = npages * PAGE_SIZE;
void *p;
@@ -116,11 +144,27 @@ out:
return NULL;
}
+void *osdep_alloc_pages(xencall_handle *xcall, size_t npages)
+{
+ void *p;
+
+ if ( xcall->buf_fd >= 0 )
+ p = alloc_pages_bufdev(xcall, npages);
+ else
+ p = alloc_pages_nobufdev(xcall, npages);
+
+ return p;
+}
+
void osdep_free_pages(xencall_handle *xcall, void *ptr, size_t npages)
{
int saved_errno = errno;
- /* Recover the VMA flags. Maybe it's not necessary */
- madvise(ptr, npages * PAGE_SIZE, MADV_DOFORK);
+
+ if ( xcall->buf_fd < 0 )
+ {
+ /* Recover the VMA flags. Maybe it's not necessary */
+ madvise(ptr, npages * PAGE_SIZE, MADV_DOFORK);
+ }
munmap(ptr, npages * PAGE_SIZE);
/* We MUST propagate the hypercall errno, not unmap call's. */
diff --git a/tools/libs/call/private.h b/tools/libs/call/private.h
index 533f0c4a8b..21f992b37e 100644
--- a/tools/libs/call/private.h
+++ b/tools/libs/call/private.h
@@ -20,7 +20,12 @@
struct xencall_handle {
xentoollog_logger *logger, *logger_tofree;
unsigned flags;
- int fd;
+
+ /* partially with /dev/ no /dev/ */
+ /* initialised xen/hypercall xen/hypercall */
+ int fd; /* any >= 0 >= 0 */
+ int buf_fd; /* any >= 0 -1 */
+
Xentoolcore__Active_Handle tc_ah;
/*
--
2.13.7
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/3] tools/libxencalls: add new function to query hypercall buffer safety
2018-06-18 7:18 [PATCH v2 0/3] fix hypercall buffer locking in memory Juergen Gross
2018-06-18 7:18 ` [PATCH v2 1/3] tools/libxencall: use hypercall buffer device if available Juergen Gross
@ 2018-06-18 7:18 ` Juergen Gross
2018-06-18 7:18 ` [PATCH v2 3/3] tools/libxc: retry hypercall in case of EFAULT Juergen Gross
2 siblings, 0 replies; 6+ messages in thread
From: Juergen Gross @ 2018-06-18 7:18 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, wei.liu2, ian.jackson
Add a new function to query whether hypercall buffers are always safe
to access by the hypervisor or might result in EFAULT.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libs/call/Makefile | 2 +-
tools/libs/call/freebsd.c | 5 +++++
tools/libs/call/include/xencall.h | 7 +++++++
tools/libs/call/libxencall.map | 5 +++++
tools/libs/call/linux.c | 5 +++++
tools/libs/call/minios.c | 5 +++++
tools/libs/call/netbsd.c | 5 +++++
tools/libs/call/solaris.c | 5 +++++
8 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/tools/libs/call/Makefile b/tools/libs/call/Makefile
index 39dd207428..252d3973fc 100644
--- a/tools/libs/call/Makefile
+++ b/tools/libs/call/Makefile
@@ -2,7 +2,7 @@ XEN_ROOT = $(CURDIR)/../../..
include $(XEN_ROOT)/tools/Rules.mk
MAJOR = 1
-MINOR = 0
+MINOR = 1
SHLIB_LDFLAGS += -Wl,--version-script=libxencall.map
CFLAGS += -Werror -Wmissing-prototypes
diff --git a/tools/libs/call/freebsd.c b/tools/libs/call/freebsd.c
index cadd313c04..28bfd852c1 100644
--- a/tools/libs/call/freebsd.c
+++ b/tools/libs/call/freebsd.c
@@ -107,6 +107,11 @@ void osdep_free_pages(xencall_handle *xcall, void *ptr, size_t npages)
errno = saved_errno;
}
+int xencall_buffers_never_fault(xencall_handle *xcall)
+{
+ return 1;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libs/call/include/xencall.h b/tools/libs/call/include/xencall.h
index bafacdd2ba..0d09bc8eae 100644
--- a/tools/libs/call/include/xencall.h
+++ b/tools/libs/call/include/xencall.h
@@ -115,6 +115,13 @@ void xencall_free_buffer_pages(xencall_handle *xcall, void *p, size_t nr_pages);
void *xencall_alloc_buffer(xencall_handle *xcall, size_t size);
void xencall_free_buffer(xencall_handle *xcall, void *p);
+/*
+ * Are allocated hypercall buffers safe to be accessed by the hypervisor all
+ * the time?
+ * Returns 0 if EFAULT might be possible.
+ */
+int xencall_buffers_never_fault(xencall_handle *xcall);
+
#endif
/*
diff --git a/tools/libs/call/libxencall.map b/tools/libs/call/libxencall.map
index 2f96144f40..c482195b95 100644
--- a/tools/libs/call/libxencall.map
+++ b/tools/libs/call/libxencall.map
@@ -17,3 +17,8 @@ VERS_1.0 {
xencall_free_buffer_pages;
local: *; /* Do not expose anything by default */
};
+
+VERS_1.1 {
+ global:
+ xencall_buffers_never_fault;
+} VERS_1.0;
diff --git a/tools/libs/call/linux.c b/tools/libs/call/linux.c
index 90c2691486..d8a6306e04 100644
--- a/tools/libs/call/linux.c
+++ b/tools/libs/call/linux.c
@@ -171,6 +171,11 @@ void osdep_free_pages(xencall_handle *xcall, void *ptr, size_t npages)
errno = saved_errno;
}
+int xencall_buffers_never_fault(xencall_handle *xcall)
+{
+ return xcall->buf_fd >= 0;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libs/call/minios.c b/tools/libs/call/minios.c
index f04688f63c..9f7a96995f 100644
--- a/tools/libs/call/minios.c
+++ b/tools/libs/call/minios.c
@@ -70,6 +70,11 @@ void osdep_free_pages(xencall_handle *xcall, void *ptr, size_t npages)
free(ptr);
}
+int xencall_buffers_never_fault(xencall_handle *xcall)
+{
+ return 1;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libs/call/netbsd.c b/tools/libs/call/netbsd.c
index e96fbf16f7..a5502da377 100644
--- a/tools/libs/call/netbsd.c
+++ b/tools/libs/call/netbsd.c
@@ -110,6 +110,11 @@ int do_xen_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall)
return hypercall->retval;
}
+int xencall_buffers_never_fault(xencall_handle *xcall)
+{
+ return 1;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libs/call/solaris.c b/tools/libs/call/solaris.c
index 5aa330e4eb..c63b6a329a 100644
--- a/tools/libs/call/solaris.c
+++ b/tools/libs/call/solaris.c
@@ -86,6 +86,11 @@ int do_xen_hypercall(xencall_handle *xcall, privcmd_hypercall_t *hypercall)
return ioctl(fd, IOCTL_PRIVCMD_HYPERCALL, hypercall);
}
+int xencall_buffers_never_fault(xencall_handle *xcall)
+{
+ return 1;
+}
+
/*
* Local variables:
* mode: C
--
2.13.7
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] tools/libxc: retry hypercall in case of EFAULT
2018-06-18 7:18 [PATCH v2 0/3] fix hypercall buffer locking in memory Juergen Gross
2018-06-18 7:18 ` [PATCH v2 1/3] tools/libxencall: use hypercall buffer device if available Juergen Gross
2018-06-18 7:18 ` [PATCH v2 2/3] tools/libxencalls: add new function to query hypercall buffer safety Juergen Gross
@ 2018-06-18 7:18 ` Juergen Gross
2018-06-18 11:26 ` Ian Jackson
2 siblings, 1 reply; 6+ messages in thread
From: Juergen Gross @ 2018-06-18 7:18 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, wei.liu2, ian.jackson
A hypercall issued via the privcmd driver can very rarely return
-EFAULT even if the hypercall buffers are locked in memory. This
happens for hypercall buffers in user memory when the Linux kernel
is doing memory scans e.g. for page migration or compaction.
Retry the getpageframeinfo3 hypercall up to 2 times in case
-EFAULT is returned and the hypervisor might see invalid PTEs for
user hypercall buffers (which should be the case only if the kernel
doesn't offer a /dev/xen/hypercall node).
Signed-off-by: Juergen Gross <jgross@suse.com>
--
V2:
- rename do_domctl_retry() to do_domctl_retry_efault() (Andrew Cooper)
---
tools/libxc/xc_private.c | 2 +-
tools/libxc/xc_private.h | 25 ++++++++++++++++++++++---
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
index fcda981744..e6e3d9913c 100644
--- a/tools/libxc/xc_private.c
+++ b/tools/libxc/xc_private.c
@@ -224,7 +224,7 @@ int xc_get_pfn_type_batch(xc_interface *xch, uint32_t dom,
domctl.domain = dom;
domctl.u.getpageframeinfo3.num = num;
set_xen_guest_handle(domctl.u.getpageframeinfo3.array, arr);
- rc = do_domctl(xch, &domctl);
+ rc = do_domctl_retry_efault(xch, &domctl);
xc_hypercall_bounce_post(xch, arr);
return rc;
}
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index 03bc9a7776..03bdfca7d4 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -254,9 +254,13 @@ out1:
return ret;
}
-static inline int do_domctl(xc_interface *xch, struct xen_domctl *domctl)
+static inline int do_domctl_maybe_retry_efault(xc_interface *xch,
+ struct xen_domctl *domctl,
+ unsigned int retries)
{
int ret = -1;
+ unsigned int retry_cnt = 0;
+
DECLARE_HYPERCALL_BOUNCE(domctl, sizeof(*domctl), XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
domctl->interface_version = XEN_DOMCTL_INTERFACE_VERSION;
@@ -267,8 +271,11 @@ static inline int do_domctl(xc_interface *xch, struct xen_domctl *domctl)
goto out1;
}
- ret = xencall1(xch->xcall, __HYPERVISOR_domctl,
- HYPERCALL_BUFFER_AS_ARG(domctl));
+ do {
+ ret = xencall1(xch->xcall, __HYPERVISOR_domctl,
+ HYPERCALL_BUFFER_AS_ARG(domctl));
+ } while ( ret < 0 && errno == EFAULT && retry_cnt++ < retries );
+
if ( ret < 0 )
{
if ( errno == EACCES )
@@ -281,6 +288,18 @@ static inline int do_domctl(xc_interface *xch, struct xen_domctl *domctl)
return ret;
}
+static inline int do_domctl(xc_interface *xch, struct xen_domctl *domctl)
+{
+ return do_domctl_maybe_retry_efault(xch, domctl, 0);
+}
+
+static inline int do_domctl_retry_efault(xc_interface *xch, struct xen_domctl *domctl)
+{
+ unsigned int retries = xencall_buffers_never_fault(xch->xcall) ? 0 : 2;
+
+ return do_domctl_maybe_retry_efault(xch, domctl, retries);
+}
+
static inline int do_sysctl(xc_interface *xch, struct xen_sysctl *sysctl)
{
int ret = -1;
--
2.13.7
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 3/3] tools/libxc: retry hypercall in case of EFAULT
2018-06-18 7:18 ` [PATCH v2 3/3] tools/libxc: retry hypercall in case of EFAULT Juergen Gross
@ 2018-06-18 11:26 ` Ian Jackson
0 siblings, 0 replies; 6+ messages in thread
From: Ian Jackson @ 2018-06-18 11:26 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel, wei.liu2
Juergen Gross writes ("[PATCH v2 3/3] tools/libxc: retry hypercall in case of EFAULT"):
> A hypercall issued via the privcmd driver can very rarely return
> -EFAULT even if the hypercall buffers are locked in memory. This
> happens for hypercall buffers in user memory when the Linux kernel
> is doing memory scans e.g. for page migration or compaction.
>
> Retry the getpageframeinfo3 hypercall up to 2 times in case
> -EFAULT is returned and the hypervisor might see invalid PTEs for
> user hypercall buffers (which should be the case only if the kernel
> doesn't offer a /dev/xen/hypercall node).
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread