Re: [Xen-devel] [PATCH 07/25] xen (ARM, x86): add errno-returning functions for copy

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Christopher Clark [mailto:christopher.w.clark@xxxxxxxxx]
> Sent: 01 December 2018 01:33
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall
> <julien.grall@xxxxxxx>; Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; George
> Dunlap <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>;
> Jan Beulich <jbeulich@xxxxxxxx>; Konrad Rzeszutek Wilk
> <konrad.wilk@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Tim
> (Xen.org) <tim@xxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Roger Pau Monne
> <roger.pau@xxxxxxxxxx>; Rich Persaud <persaur@xxxxxxxxx>; Ross Philipson
> <ross.philipson@xxxxxxxxx>; Eric Chanudet <eric.chanudet@xxxxxxxxx>; James
> McKenzie <voreekf@xxxxxxxxxxxxx>; Jason Andryuk <jandryuk@xxxxxxxxx>;
> Daniel Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> Subject: [PATCH 07/25] xen (ARM, x86): add errno-returning functions for
> copy
> 
> Applied to both x86 and ARM headers.
> 
> Signed-off-by: Christopher Clark <christopher.clark6@xxxxxxxxxxxxxx>
> ---
>  xen/include/asm-arm/guest_access.h | 25 +++++++++++++++++++++++++
>  xen/include/asm-x86/guest_access.h | 29 +++++++++++++++++++++++++++++
>  xen/include/xen/guest_access.h     |  3 +++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-
> arm/guest_access.h
> index 224d2a0..7b6f89c 100644
> --- a/xen/include/asm-arm/guest_access.h
> +++ b/xen/include/asm-arm/guest_access.h
> @@ -24,6 +24,11 @@ int access_guest_memory_by_ipa(struct domain *d,
> paddr_t ipa, void *buf,
>  #define __raw_copy_from_guest raw_copy_from_guest
>  #define __raw_clear_guest raw_clear_guest
> 
> +#define raw_copy_from_guest_errno(dst, src, len)             \
> +    (raw_copy_from_guest((dst), (src), (len)) ? -EFAULT : 0)
> +#define raw_copy_to_guest_errno(dst, src, len)               \
> +    (raw_copy_to_guest((dst), (src), (len)) ? -EFAULT : 0)
> +
>  /* Remainder copied from x86 -- could be common? */
> 
>  /* Is the guest handle a NULL reference? */
> @@ -113,6 +118,26 @@ int access_guest_memory_by_ipa(struct domain *d,
> paddr_t ipa, void *buf,
>      raw_copy_from_guest(_d, _s, sizeof(*_d));           \
>  })
> 
> +/* errno returning copy functions */
> +#define copy_from_guest_offset_errno(ptr, hnd, off, nr) ({              \
> +            const typeof(*(ptr)) *_s = (hnd).p;                         \
> +            typeof(*(ptr)) *_d = (ptr);                                 \
> +            raw_copy_from_guest_errno(_d, _s + (off), sizeof(*_d) *
> (nr)); \
> +        })
> +
> +#define copy_field_to_guest_errno(hnd, ptr, field) ({           \
> +            const typeof(&(ptr)->field) _s = &(ptr)->field;     \
> +            void *_d = &(hnd).p->field;                         \
> +            ((void)(&(hnd).p->field == &(ptr)->field));         \
> +            raw_copy_to_guest_errno(_d, _s, sizeof(*_s));       \
> +        })
> +
> +#define copy_field_from_guest_errno(ptr, hnd, field) ({         \
> +            const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
> +            typeof(&(ptr)->field) _d = &(ptr)->field;           \
> +            raw_copy_from_guest_errno(_d, _s, sizeof(*_d));     \
> +        })
> +
>  /*
>   * Pre-validate a guest handle.
>   * Allows use of faster __copy_* functions.
> diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-
> x86/guest_access.h
> index ca700c9..9391cd3 100644
> --- a/xen/include/asm-x86/guest_access.h
> +++ b/xen/include/asm-x86/guest_access.h
> @@ -38,6 +38,15 @@
>       clear_user_hvm((dst), (len)) :             \
>       clear_user((dst), (len)))
> 
> +#define raw_copy_from_guest_errno(dst, src, len)                        \
> +    (is_hvm_vcpu(current) ?                                             \
> +     copy_from_user_hvm((dst), (src), (len)) :                         \
> +     (copy_from_user((dst), (src), (len)) ? -EFAULT : 0))

AFAICT copy_from_user_hvm() doesn't return -ve errno (it has comment "/* fake a 
copy_to_user() return code */" on the return line) so I think your bracketing 
is wrong here...

> +#define raw_copy_to_guest_errno(dst, src, len)          \
> +    (is_hvm_vcpu(current) ?                             \
> +     copy_to_user_hvm((dst), (src), (len)) :           \
> +     (copy_to_user((dst), (src), (len)) ? -EFAULT : 0))
> +

...and similarly here.

>  /* Is the guest handle a NULL reference? */
>  #define guest_handle_is_null(hnd)        ((hnd).p == NULL)
> 
> @@ -121,6 +130,26 @@
>      raw_copy_from_guest(_d, _s, sizeof(*_d));           \
>  })
> 
> +/* errno returning copy functions */
> +#define copy_from_guest_offset_errno(ptr, hnd, off, nr) ({              \
> +            const typeof(*(ptr)) *_s = (hnd).p;                         \
> +            typeof(*(ptr)) *_d = (ptr);                                 \
> +            raw_copy_from_guest_errno(_d, _s + (off), sizeof(*_d) *
> (nr)); \
> +        })
> +
> +#define copy_field_to_guest_errno(hnd, ptr, field) ({           \
> +            const typeof(&(ptr)->field) _s = &(ptr)->field;     \
> +            void *_d = &(hnd).p->field;                         \
> +            ((void)(&(hnd).p->field == &(ptr)->field));         \
> +            raw_copy_to_guest_errno(_d, _s, sizeof(*_s));       \
> +        })
> +
> +#define copy_field_from_guest_errno(ptr, hnd, field) ({         \
> +            const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
> +            typeof(&(ptr)->field) _d = &(ptr)->field;           \
> +            raw_copy_from_guest_errno(_d, _s, sizeof(*_d));     \
> +        })
> +
>  /*
>   * Pre-validate a guest handle.
>   * Allows use of faster __copy_* functions.
> diff --git a/xen/include/xen/guest_access.h
> b/xen/include/xen/guest_access.h
> index 09989df..3494c5f 100644
> --- a/xen/include/xen/guest_access.h
> +++ b/xen/include/xen/guest_access.h
> @@ -26,6 +26,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_errno(ptr, hnd, 0, nr)
> +
>  #define __clear_guest(hnd, nr)                          \
>      __clear_guest_offset(hnd, 0, nr)
> 

Given that the only errno possible seems to be EFAULT, I do have to question 
why you need these changes?

  Paul

> --
> 2.1.4


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