Re: [Xen-devel] [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> 
>>> wrote:
> Note: this change does not make any difference on x86 and ia64.
> 
> 
> XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers
> stored in memory from guest pointers as hypercall parameters.

I have to admit that really dislike this, to a large part because of
the follow up patch that clutters the corresponding function
declarations even further. Plus I see no mechanism to convert
between the two, yet I can't see how - long term at least - you
could get away without such conversion.

Is it really a well thought through and settled upon decision to
make guest handles 64 bits wide even on 32-bit ARM? After all,
both x86 and PPC got away without doing so (and I think your
xen_ulong_t swipe would have broken PPC quite badly).

Jan

> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
>  xen/include/asm-arm/guest_access.h |    2 +-
>  xen/include/public/arch-arm.h      |   17 +++++++++++++----
>  xen/include/public/arch-ia64.h     |    1 +
>  xen/include/public/arch-x86/xen.h  |    1 +
>  4 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/include/asm-arm/guest_access.h 
> b/xen/include/asm-arm/guest_access.h
> index 0fceae6..7a955cb 100644
> --- a/xen/include/asm-arm/guest_access.h
> +++ b/xen/include/asm-arm/guest_access.h
> @@ -30,7 +30,7 @@ unsigned long raw_clear_guest(void *to, unsigned len);
>  /* Cast a guest handle to the specified type of handle. */
>  #define guest_handle_cast(hnd, type) ({         \
>      type *_x = (hnd).p;                         \
> -    (XEN_GUEST_HANDLE(type)) { _x };            \
> +    (XEN_GUEST_HANDLE(type)) { {_x } };            \
>  })
>  
>  #define guest_handle_from_ptr(ptr, type)        \
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 2ae6548..d17d645 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -51,18 +51,27 @@
>  
>  #define XEN_HYPERCALL_TAG   0XEA1
>  
> +#define uint64_aligned_t uint64_t __attribute__((aligned(8)))
>  
>  #ifndef __ASSEMBLY__
> -#define ___DEFINE_XEN_GUEST_HANDLE(name, type) \
> -    typedef struct { type *p; } __guest_handle_ ## name
> +#define ___DEFINE_XEN_GUEST_HANDLE(name, type)                  \
> +    typedef struct { type *p; }                                 \
> +        __guest_handle_ ## name;                                \
> +    typedef struct { union { type *p; uint64_aligned_t q; }; }  \
> +        __guest_handle_64_ ## name;
>  
>  #define __DEFINE_XEN_GUEST_HANDLE(name, type) \
>      ___DEFINE_XEN_GUEST_HANDLE(name, type);   \
>      ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
>  #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name, 
> name)
> -#define __XEN_GUEST_HANDLE(name)        __guest_handle_ ## name
> +#define __XEN_GUEST_HANDLE(name)        __guest_handle_64_ ## name
>  #define XEN_GUEST_HANDLE(name)          __XEN_GUEST_HANDLE(name)
> -#define set_xen_guest_handle_raw(hnd, val)  do { (hnd).p = val; } while (0)
> +/* this is going to be changes on 64 bit */
> +#define XEN_GUEST_HANDLE_PARAM(name)    __guest_handle_ ## name
> +#define set_xen_guest_handle_raw(hnd, val)                  \
> +    do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0;   \
> +         (hnd).p = val;                                     \
> +    } while ( 0 )
>  #ifdef __XEN_TOOLS__
>  #define get_xen_guest_handle(val, hnd)  do { val = (hnd).p; } while (0)
>  #endif
> diff --git a/xen/include/public/arch-ia64.h b/xen/include/public/arch-ia64.h
> index c9da5d4..97583ea 100644
> --- a/xen/include/public/arch-ia64.h
> +++ b/xen/include/public/arch-ia64.h
> @@ -47,6 +47,7 @@
>  
>  #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name, 
> name)
>  #define XEN_GUEST_HANDLE(name)          __guest_handle_ ## name
> +#define XEN_GUEST_HANDLE_PARAM(name)    XEN_GUEST_HANDLE(name)
>  #define XEN_GUEST_HANDLE_64(name)       XEN_GUEST_HANDLE(name)
>  #define uint64_aligned_t                uint64_t
>  #define set_xen_guest_handle_raw(hnd, val)  do { (hnd).p = val; } while (0)
> diff --git a/xen/include/public/arch-x86/xen.h 
> b/xen/include/public/arch-x86/xen.h
> index 1c186d7..8ee5437 100644
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -44,6 +44,7 @@
>  #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name, 
> name)
>  #define __XEN_GUEST_HANDLE(name)        __guest_handle_ ## name
>  #define XEN_GUEST_HANDLE(name)          __XEN_GUEST_HANDLE(name)
> +#define XEN_GUEST_HANDLE_PARAM(name)    XEN_GUEST_HANDLE(name)
>  #define set_xen_guest_handle_raw(hnd, val)  do { (hnd).p = val; } while (0)
>  #ifdef __XEN_TOOLS__
>  #define get_xen_guest_handle(val, hnd)  do { val = (hnd).p; } while (0)
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx 
> http://lists.xen.org/xen-devel 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel