Mailing List Archive

Handle GNTST_eagain status from GNTTABOP_map_grant_ref and
GNTTABOP_copy operations properly to allow usage of xenpaging without
causing crashes or data corruption.

Replace all relevant HYPERVISOR_grant_table_op() calls with a retry
loop. This loop is implemented as a macro to allow different
GNTTABOP_* args. It will sleep up to 33 seconds and wait for the
page to be paged in again.

All ->status checks were updated to use the GNTST_* namespace. All
return values are converted from GNTST_* namespace to 0/-EINVAL, since
all callers did not use the actual return value.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>

This is a port to the mainline Linux tree. This required dropping many backend
drivers which have not yet made it in. Additionally, several of the referenced
functions have moved and/or been refactored. I attempted to minimize changes
while keeping the same semantics.

Signed-off-by: Adin Scannell <adin@scannell.ca>

Index: linux/drivers/block/xen-blkback/blkback.c
===================================================================
---
drivers/block/xen-blkback/blkback.c | 4 ++-
drivers/xen/grant-table.c | 7 ++++-
drivers/xen/xenbus/xenbus_client.c | 4 +++
include/xen/grant_table.h | 39 +++++++++++++++++++++++++++++++++++
include/xen/interface/grant_table.h | 6 ++++-
5 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 15ec4db..d3fb290 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -390,7 +390,9 @@ static int xen_blkbk_map(struct blkif_request *req,
* the page from the other domain.
*/
for (i = 0; i < nseg; i++) {
- if (unlikely(map[i].status != 0)) {
+ if (unlikely(map[i].status == GNTST_eagain))
+ gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map[i]);
+ if (unlikely(map[i].status != GNTST_okay)) {
pr_debug(DRV_PFX "invalid buffer -- could not remap it\n");
map[i].handle = BLKBACK_INVALID_HANDLE;
ret |= 1;
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index bf1c094..48826c3 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -464,9 +464,12 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,

for (i = 0; i < count; i++) {
/* Do not add to override if the map failed. */
- if (map_ops[i].status)
+ if (map_ops[i].status != GNTST_okay && map_ops[i].status != GNTST_eagain)
continue;

+ if (map_ops[i].status == GNTST_eagain)
+ return -EAGAIN;
+
if (map_ops[i].flags & GNTMAP_contains_pte) {
pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
(map_ops[i].host_addr & ~PAGE_MASK));
@@ -565,7 +568,7 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
return -ENOSYS;
}

- BUG_ON(rc || setup.status);
+ BUG_ON(rc || (setup.status != GNTST_okay));

rc = arch_gnttab_map_shared(frames, nr_gframes, gnttab_max_grant_frames(),
&shared);
diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
index 1906125..d123c78 100644
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -455,6 +455,8 @@ int xenbus_map_ring_valloc(struct xenbus_device *dev, int gnt_ref, void **vaddr)
if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
BUG();

+ if (op.status == GNTST_eagain)
+ gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, (&op));
if (op.status != GNTST_okay) {
free_vm_area(area);
xenbus_dev_fatal(dev, op.status,
@@ -499,6 +501,8 @@ int xenbus_map_ring(struct xenbus_device *dev, int gnt_ref,
if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
BUG();

+ if (op.status == GNTST_eagain)
+ gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, (&op));
if (op.status != GNTST_okay) {
xenbus_dev_fatal(dev, op.status,
"mapping in shared page %d from domain %d",
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index 11e2dfc..46fac85 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -37,6 +37,8 @@
#ifndef __ASM_GNTTAB_H__
#define __ASM_GNTTAB_H__

+#include <linux/delay.h>
+
#include <asm/page.h>

#include <xen/interface/xen.h>
@@ -160,4 +162,41 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
struct page **pages, unsigned int count);

+#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p) \
+do { \
+ u8 __hc_delay = 1; \
+ int __ret; \
+ while (unlikely((__HCarg_p)->status == GNTST_eagain && __hc_delay)) { \
+ msleep(__hc_delay++); \
+ __ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1); \
+ BUG_ON(__ret); \
+ } \
+ if (__hc_delay == 0) { \
+ printk(KERN_ERR "%s: gnt busy\n", __func__,); \
+ (__HCarg_p)->status = GNTST_bad_page; \
+ } \
+ if ((__HCarg_p)->status != GNTST_okay) \
+ printk(KERN_ERR "%s: gnt status %x\n", \
+ __func__, (__HCarg_p)->status); \
+} while(0)
+
+#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p) \
+do { \
+ u8 __hc_delay = 1; \
+ int __ret; \
+ do { \
+ __ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1); \
+ BUG_ON(__ret); \
+ if ((__HCarg_p)->status == GNTST_eagain) \
+ msleep(__hc_delay++); \
+ } while ((__HCarg_p)->status == GNTST_eagain && __hc_delay); \
+ if (__hc_delay == 0) { \
+ printk(KERN_ERR "%s: gnt busy\n", __func__); \
+ (__HCarg_p)->status = GNTST_bad_page; \
+ } \
+ if ((__HCarg_p)->status != GNTST_okay) \
+ printk(KERN_ERR "%s: gnt status %x\n", \
+ __func__, (__HCarg_p)->status); \
+} while(0)
+
#endif /* __ASM_GNTTAB_H__ */
diff --git a/include/xen/interface/grant_table.h b/include/xen/interface/grant_table.h
index 39e5717..ba04080 100644
--- a/include/xen/interface/grant_table.h
+++ b/include/xen/interface/grant_table.h
@@ -363,6 +363,8 @@ DEFINE_GUEST_HANDLE_STRUCT(gnttab_query_size);
#define GNTST_permission_denied (-8) /* Not enough privilege for operation. */
#define GNTST_bad_page (-9) /* Specified page was invalid for op. */
#define GNTST_bad_copy_arg (-10) /* copy arguments cross page boundary */
+#define GNTST_address_too_big (-11) /* transfer page address too large. */
+#define GNTST_eagain (-12) /* Could not map at the moment. Retry. */

#define GNTTABOP_error_msgs { \
"okay", \
@@ -375,7 +377,9 @@ DEFINE_GUEST_HANDLE_STRUCT(gnttab_query_size);
"no spare translation slot in the I/O MMU", \
"permission denied", \
"bad page", \
- "copy arguments cross page boundary" \
+ "copy arguments cross page boundary", \
+ "page address size too large", \
+ "could not map at the moment, retry" \
}

#endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */
--
1.6.2.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel