Re: [Xen-devel] [PATCH v3 0/6] ARM hypercall ABI: 64 bit ready

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 16 Oct 2012, Ian Campbell wrote:
> On Thu, 2012-08-16 at 15:48 +0100, Stefano Stabellini wrote:
> > In this version of the patch series I have introduced conversion macros
> > to convert a XEN_GUEST_HANDLE_PARAM into a XEN_GUEST_HANDLE a vice
> > versa. Most of the problematic cases come from xen/arch/x86 code, in
> > order to spot them I wrote a simple debug patch that change the
> > definition of XEN_GUEST_HANDLE_PARAM to be different from
> > XEN_GUEST_HANDLE on x86 too. I am attaching the debug patch to this
> > email. 
> 
> This (quoted below) seems like a useful patch from the PoV of catching
> these sorts of things early on x86 before they break ARM.
> 
> It doesn't seem like it should have any impact on the generated code,
> should we perhaps apply it?

Nope, it shouldn't have any impact.
Having it in the tree would be a clear improvement!


> I needed the addition of the following to actually make it work though.
> 
> diff --git a/xen/include/asm-x86/guest_access.h 
> b/xen/include/asm-x86/guest_acce
> index ca700c9..33b4afd 100644
> --- a/xen/include/asm-x86/guest_access.h
> +++ b/xen/include/asm-x86/guest_access.h
> @@ -54,22 +54,24 @@
>  
>  /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
>  #define guest_handle_to_param(hnd, type) ({                  \
> +    typeof((hnd).p) _x = (hnd).p;                            \
> +    XEN_GUEST_HANDLE_PARAM(type) _y = { _x };                \
>      /* type checking: make sure that the pointers inside     \
>       * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
>       * the same type, then return hnd */                     \
> -    (void)((typeof(&(hnd).p)) 0 ==                           \
> -        (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \
> -    (hnd);                                                   \
> +    (void)(&_x == &_y.p);                                    \
> +    _y;                                                      \
>  })
>  
>  /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
> -#define guest_handle_from_param(hnd, type) ({                \
> -    /* type checking: make sure that the pointers inside     \
> -     * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
> -     * the same type, then return hnd */                     \
> -    (void)((typeof(&(hnd).p)) 0 ==                           \
> -        (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \
> -    (hnd);                                                   \
> +#define guest_handle_from_param(hnd, type) ({               \
> +    typeof((hnd).p) _x = (hnd).p;                           \
> +    XEN_GUEST_HANDLE(type) _y = { _x };                     \
> +    /* type checking: make sure that the pointers inside    \
> +     * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of   \
> +     * the same type, then return hnd */                    \
> +    (void)(&_x == &_y.p);                                   \
> +    _y;                                                     \
>  })
>  
>  #define guest_handle_for_field(hnd, type, fld)          \

I would argue that these changes are the right thing to do anyway.
The original code is unreadable.

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