[Xen-devel] [PATCH v2 05/18] xen: add simple errno-returning macros for copy from guest

[Christopher Clark <christopher.w.clark@xxxxxxxxx>]

Adds: copy_from_guest_errno(ptr, hnd, nr)
 and: copy_from_guest_offset_errno(ptr, hnd, off, nr)

Inputs are identical to copy_from_guest and copy_from_guest_offset:

    - ptr: pointer to local memory to write into
    - hnd: guest handle to read from
    -  nr: number of records to copy
    - off: offset from the source handle to start the copy from.

The new versions perform the same work as the existing copy_from_guest and
copy_from_guest_offset, but differ in return value when the function does
not succeed in performing a complete copy: the new functions return an errno
value on error, rather than the number of bytes that could not be copied.

This is to allow for improvements in tidy error handling flow at call sites
in the common case.
eg. to enable this sequence:

    ret = copy_from_guest_errno(&iov, iovs, 1);
    if ( ret )
        goto out;
    ...

instead of the prior:

    if ( copy_from_guest(&iov, iovs, 1) )
    {
        ret = -EFAULT;
        goto out;
    }
        ...

or assigning ret a default value of -EFAULT at point of declaration.

In (almost?) all cases the result of copy_from_guest must be checked
and the error code if one is to be generated is (always?) EFAULT.
This change moves that check and error code translation into common code.

This errno-returning function interface originates from Bromium's uxen and
an additional motivation for introducing this is to simplify comparison and
maintenance of common code between argo and v4v.

Applied to both x86 and ARM headers.

Signed-off-by: Christopher Clark <christopher.clark6@xxxxxxxxxxxxxx>

---
v1 #7 feedback, Paul: corrected and significantly simplified implementation

The need for this interface was questioned during the first series review,
(which wasn't much helped by the implementation or commit message that was
provided in that patch, sorry) -- it wasn't the primary focus of the series,
so wasn't forefront in attention beforehand. However, I maintain that this
interface is better for nearly all the call sites using the guest access
macros, and will simplify code using them -- that plus easing work across
the uxen and Xen codebases together provides reasonable justification for
inclusion in the common code.

 xen/include/asm-arm/guest_access.h | 3 +++
 xen/include/asm-x86/guest_access.h | 3 +++
 xen/include/xen/guest_access.h     | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/xen/include/asm-arm/guest_access.h 
b/xen/include/asm-arm/guest_access.h
index 224d2a0..8722858 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -97,6 +97,9 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa, 
void *buf,
     typeof(*(ptr)) *_d = (ptr);                         \
     raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
 })
+#define copy_from_guest_offset_errno(ptr, hnd, off, nr)  \
+    (copy_from_guest_offset((ptr), (hnd), (off), (nr)) ? \
+        -EFAULT : 0)
 
 /* Copy sub-field of a structure to guest context via a guest handle. */
 #define copy_field_to_guest(hnd, ptr, field) ({         \
diff --git a/xen/include/asm-x86/guest_access.h 
b/xen/include/asm-x86/guest_access.h
index ca700c9..9399480 100644
--- a/xen/include/asm-x86/guest_access.h
+++ b/xen/include/asm-x86/guest_access.h
@@ -100,6 +100,9 @@
     typeof(*(ptr)) *_d = (ptr);                         \
     raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
 })
+#define copy_from_guest_offset_errno(ptr, hnd, off, nr)  \
+    (copy_from_guest_offset((ptr), (hnd), (off), (nr)) ? \
+        -EFAULT : 0)                                     \
 
 #define clear_guest_offset(hnd, off, nr) ({    \
     void *_d = (hnd).p;                        \
diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h
index 09989df..4a8ea1f 100644
--- a/xen/include/xen/guest_access.h
+++ b/xen/include/xen/guest_access.h
@@ -17,6 +17,9 @@
 #define copy_from_guest(ptr, hnd, nr)                   \
     copy_from_guest_offset(ptr, hnd, 0, nr)
 
+#define copy_from_guest_errno(ptr, hnd, nr)             \
+    (copy_from_guest_offset(ptr, hnd, 0, nr) ? -EFAULT : 0)
+
 #define clear_guest(hnd, nr)                            \
     clear_guest_offset(hnd, 0, nr)
 
-- 
2.7.4


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