[PATCH 0/5] ARM hypercall ABI: 64 bit ready

[PATCH 0/5] ARM hypercall ABI: 64 bit ready

[Stefano Stabellini]

Hi all,
this patch series makes the necessary changes to make sure that the
current ARM hypercall ABI can be used as-is on 64 bit ARM platforms:

- it defines xen_ulong_t as uint64_t on ARM;
- it introduces a new macro to handle guest pointers, called
XEN_GUEST_HANDLE_PARAM (that has size 4 bytes on aarch and is going to
have size 8 bytes on aarch64);
- it replaces all the occurrences of XEN_GUEST_HANDLE in hypercall
parameters with XEN_GUEST_HANDLE_PARAM.


On x86 and ia64 things should stay exactly the same.

On ARM all the unsigned long and the guest pointers that are members of
a struct become size 8 byte (both aarch and aarch64).
However guest pointers that are passed as hypercall arguments in
registers are going to be 4 bytes on aarch and 8 bytes on aarch64.



It is based on Ian's arm-for-4.3 branch. 


Stefano Stabellini (5):
      xen: improve changes to xen_add_to_physmap
      xen/arm: introduce __lshrdi3 and __aeabi_llsr
      xen: few more xen_ulong_t substitutions
      xen: introduce XEN_GUEST_HANDLE_PARAM
      xen: replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when appropriate


 xen/arch/arm/domain.c              |    2 +-
 xen/arch/arm/domctl.c              |    2 +-
 xen/arch/arm/hvm.c                 |    2 +-
 xen/arch/arm/lib/Makefile          |    2 +-
 xen/arch/arm/lib/lshrdi3.S         |   54 ++++++++++++++++++++++++++++++++++++
 xen/arch/arm/mm.c                  |    2 +-
 xen/arch/arm/physdev.c             |    2 +-
 xen/arch/arm/sysctl.c              |    2 +-
 xen/arch/x86/cpu/mcheck/mce.c      |    2 +-
 xen/arch/x86/domain.c              |    2 +-
 xen/arch/x86/domctl.c              |    2 +-
 xen/arch/x86/efi/runtime.c         |    2 +-
 xen/arch/x86/hvm/hvm.c             |   26 ++++++++--------
 xen/arch/x86/microcode.c           |    2 +-
 xen/arch/x86/mm.c                  |   14 ++++----
 xen/arch/x86/mm/hap/hap.c          |    2 +-
 xen/arch/x86/mm/mem_event.c        |    2 +-
 xen/arch/x86/mm/paging.c           |    2 +-
 xen/arch/x86/mm/shadow/common.c    |    2 +-
 xen/arch/x86/physdev.c             |    2 +-
 xen/arch/x86/platform_hypercall.c  |    2 +-
 xen/arch/x86/sysctl.c              |    2 +-
 xen/arch/x86/traps.c               |    2 +-
 xen/arch/x86/x86_32/mm.c           |    2 +-
 xen/arch/x86/x86_32/traps.c        |    2 +-
 xen/arch/x86/x86_64/compat/mm.c    |    8 ++--
 xen/arch/x86/x86_64/domain.c       |    2 +-
 xen/arch/x86/x86_64/mm.c           |    2 +-
 xen/arch/x86/x86_64/traps.c        |    2 +-
 xen/common/compat/domain.c         |    2 +-
 xen/common/compat/grant_table.c    |    2 +-
 xen/common/compat/memory.c         |    2 +-
 xen/common/domain.c                |    2 +-
 xen/common/domctl.c                |    2 +-
 xen/common/event_channel.c         |    2 +-
 xen/common/grant_table.c           |   36 ++++++++++++------------
 xen/common/kernel.c                |    4 +-
 xen/common/kexec.c                 |   16 +++++-----
 xen/common/memory.c                |    4 +-
 xen/common/multicall.c             |    2 +-
 xen/common/schedule.c              |    2 +-
 xen/common/sysctl.c                |    2 +-
 xen/common/xenoprof.c              |    8 ++--
 xen/drivers/acpi/pmstat.c          |    2 +-
 xen/drivers/char/console.c         |    6 ++--
 xen/drivers/passthrough/iommu.c    |    2 +-
 xen/include/asm-arm/guest_access.h |    2 +-
 xen/include/asm-arm/hypercall.h    |    2 +-
 xen/include/asm-arm/mm.h           |    2 +-
 xen/include/asm-x86/hap.h          |    2 +-
 xen/include/asm-x86/hypercall.h    |   24 ++++++++--------
 xen/include/asm-x86/mem_event.h    |    2 +-
 xen/include/asm-x86/mm.h           |    8 ++--
 xen/include/asm-x86/paging.h       |    2 +-
 xen/include/asm-x86/processor.h    |    2 +-
 xen/include/asm-x86/shadow.h       |    2 +-
 xen/include/asm-x86/xenoprof.h     |    6 ++--
 xen/include/public/arch-arm.h      |   21 ++++++++++----
 xen/include/public/arch-ia64.h     |    1 +
 xen/include/public/arch-x86/xen.h  |    1 +
 xen/include/public/memory.h        |   11 ++++--
 xen/include/public/physdev.h       |    2 +-
 xen/include/public/version.h       |    2 +-
 xen/include/public/xen.h           |    4 +-
 xen/include/xen/acpi.h             |    4 +-
 xen/include/xen/hypercall.h        |   52 +++++++++++++++++-----------------
 xen/include/xen/iommu.h            |    2 +-
 xen/include/xen/tmem_xen.h         |    2 +-
 xen/include/xsm/xsm.h              |    4 +-
 xen/xsm/dummy.c                    |    2 +-
 xen/xsm/flask/flask_op.c           |    4 +-
 xen/xsm/flask/hooks.c              |    2 +-
 xen/xsm/xsm_core.c                 |    2 +-
 73 files changed, 243 insertions(+), 175 deletions(-)


Cheers,

Stefano

[PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Stefano Stabellini]

This is an incremental patch on top of
c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
compatibility, it is better to introduce foreign_domid as part of a
union containing both size and foreign_domid.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/include/public/memory.h |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index b2adfbe..b0af2fd 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -208,8 +208,12 @@ struct xen_add_to_physmap {
     /* Which domain to change the mapping for. */
     domid_t domid;
 
-    /* Number of pages to go through for gmfn_range */
-    uint16_t    size;
+    union {
+        /* Number of pages to go through for gmfn_range */
+        uint16_t    size;
+        /* IFF gmfn_foreign */
+        domid_t foreign_domid;
+    };
 
     /* Source mapping space. */
 #define XENMAPSPACE_shared_info  0 /* shared info page */
@@ -217,8 +221,7 @@ struct xen_add_to_physmap {
 #define XENMAPSPACE_gmfn         2 /* GMFN */
 #define XENMAPSPACE_gmfn_range   3 /* GMFN range */
 #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
-    uint16_t space;
-    domid_t foreign_domid; /* IFF gmfn_foreign */
+    unsigned int space;
 
 #define XENMAPIDX_grant_table_status 0x80000000
 
-- 
1.7.2.5

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Konrad Rzeszutek Wilk]

On Mon, Aug 06, 2012 at 03:12:01PM +0100, Stefano Stabellini wrote:
> This is an incremental patch on top of
> c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
> compatibility, it is better to introduce foreign_domid as part of a
> union containing both size and foreign_domid.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/include/public/memory.h |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index b2adfbe..b0af2fd 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
>      /* Which domain to change the mapping for. */
>      domid_t domid;
>  
> -    /* Number of pages to go through for gmfn_range */
> -    uint16_t    size;
> +    union {
> +        /* Number of pages to go through for gmfn_range */
> +        uint16_t    size;
> +        /* IFF gmfn_foreign */
> +        domid_t foreign_domid;
> +    };
>  
>      /* Source mapping space. */
>  #define XENMAPSPACE_shared_info  0 /* shared info page */
> @@ -217,8 +221,7 @@ struct xen_add_to_physmap {
>  #define XENMAPSPACE_gmfn         2 /* GMFN */
>  #define XENMAPSPACE_gmfn_range   3 /* GMFN range */
>  #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
> -    uint16_t space;
> -    domid_t foreign_domid; /* IFF gmfn_foreign */
> +    unsigned int space;

Should this be of uint32... ?

>  
>  #define XENMAPIDX_grant_table_status 0x80000000
>  
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Stefano Stabellini]

On Mon, 6 Aug 2012, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 06, 2012 at 03:12:01PM +0100, Stefano Stabellini wrote:
> > This is an incremental patch on top of
> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
> > compatibility, it is better to introduce foreign_domid as part of a
> > union containing both size and foreign_domid.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  xen/include/public/memory.h |   11 +++++++----
> >  1 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> > index b2adfbe..b0af2fd 100644
> > --- a/xen/include/public/memory.h
> > +++ b/xen/include/public/memory.h
> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
> >      /* Which domain to change the mapping for. */
> >      domid_t domid;
> >  
> > -    /* Number of pages to go through for gmfn_range */
> > -    uint16_t    size;
> > +    union {
> > +        /* Number of pages to go through for gmfn_range */
> > +        uint16_t    size;
> > +        /* IFF gmfn_foreign */
> > +        domid_t foreign_domid;
> > +    };
> >  
> >      /* Source mapping space. */
> >  #define XENMAPSPACE_shared_info  0 /* shared info page */
> > @@ -217,8 +221,7 @@ struct xen_add_to_physmap {
> >  #define XENMAPSPACE_gmfn         2 /* GMFN */
> >  #define XENMAPSPACE_gmfn_range   3 /* GMFN range */
> >  #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
> > -    uint16_t space;
> > -    domid_t foreign_domid; /* IFF gmfn_foreign */
> > +    unsigned int space;
> 
> Should this be of uint32... ?

Nope: this patch is a fix for:

diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 86d02c8..b2adfbe 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -212,11 +212,13 @@ struct xen_add_to_physmap {
     uint16_t    size;
 
     /* Source mapping space. */
-#define XENMAPSPACE_shared_info 0 /* shared info page */
-#define XENMAPSPACE_grant_table 1 /* grant table page */
-#define XENMAPSPACE_gmfn        2 /* GMFN */
-#define XENMAPSPACE_gmfn_range  3 /* GMFN range */
-    unsigned int space;
+#define XENMAPSPACE_shared_info  0 /* shared info page */
+#define XENMAPSPACE_grant_table  1 /* grant table page */
+#define XENMAPSPACE_gmfn         2 /* GMFN */
+#define XENMAPSPACE_gmfn_range   3 /* GMFN range */
+#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
+    uint16_t space;
+    domid_t foreign_domid; /* IFF gmfn_foreign */
 
 #define XENMAPIDX_grant_table_status 0x80000000

---

this is not upstream yet. So it brings space back to its original self.

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Jan Beulich]

>>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> This is an incremental patch on top of
> c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
> compatibility, it is better to introduce foreign_domid as part of a
> union containing both size and foreign_domid.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/include/public/memory.h |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index b2adfbe..b0af2fd 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
>      /* Which domain to change the mapping for. */
>      domid_t domid;
>  
> -    /* Number of pages to go through for gmfn_range */
> -    uint16_t    size;
> +    union {
> +        /* Number of pages to go through for gmfn_range */
> +        uint16_t    size;
> +        /* IFF gmfn_foreign */
> +        domid_t foreign_domid;
> +    };

But you're clear that this isn't standard C, and hence can't go
in this way?

Jan

>      /* Source mapping space. */
>  #define XENMAPSPACE_shared_info  0 /* shared info page */
> @@ -217,8 +221,7 @@ struct xen_add_to_physmap {
>  #define XENMAPSPACE_gmfn         2 /* GMFN */
>  #define XENMAPSPACE_gmfn_range   3 /* GMFN range */
>  #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
> -    uint16_t space;
> -    domid_t foreign_domid; /* IFF gmfn_foreign */
> +    unsigned int space;
>  
>  #define XENMAPIDX_grant_table_status 0x80000000
>  
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel 

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Stefano Stabellini]

On Mon, 6 Aug 2012, Jan Beulich wrote:
> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> wrote:
> > This is an incremental patch on top of
> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
> > compatibility, it is better to introduce foreign_domid as part of a
> > union containing both size and foreign_domid.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  xen/include/public/memory.h |   11 +++++++----
> >  1 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> > index b2adfbe..b0af2fd 100644
> > --- a/xen/include/public/memory.h
> > +++ b/xen/include/public/memory.h
> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
> >      /* Which domain to change the mapping for. */
> >      domid_t domid;
> >  
> > -    /* Number of pages to go through for gmfn_range */
> > -    uint16_t    size;
> > +    union {
> > +        /* Number of pages to go through for gmfn_range */
> > +        uint16_t    size;
> > +        /* IFF gmfn_foreign */
> > +        domid_t foreign_domid;
> > +    };
> 
> But you're clear that this isn't standard C, and hence can't go
> in this way?
> 

Why? It is c11 if I am not mistaken.

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Jan Beulich]

>>> On 06.08.12 at 17:43, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> wrote:
>> > This is an incremental patch on top of
>> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
>> > compatibility, it is better to introduce foreign_domid as part of a
>> > union containing both size and foreign_domid.
>> > 
>> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> > ---
>> >  xen/include/public/memory.h |   11 +++++++----
>> >  1 files changed, 7 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
>> > index b2adfbe..b0af2fd 100644
>> > --- a/xen/include/public/memory.h
>> > +++ b/xen/include/public/memory.h
>> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
>> >      /* Which domain to change the mapping for. */
>> >      domid_t domid;
>> >  
>> > -    /* Number of pages to go through for gmfn_range */
>> > -    uint16_t    size;
>> > +    union {
>> > +        /* Number of pages to go through for gmfn_range */
>> > +        uint16_t    size;
>> > +        /* IFF gmfn_foreign */
>> > +        domid_t foreign_domid;
>> > +    };
>> 
>> But you're clear that this isn't standard C, and hence can't go
>> in this way?
>> 
> 
> Why? It is c11 if I am not mistaken.

Yes. But the common baseline is C89.

Jan

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Stefano Stabellini]

On Mon, 6 Aug 2012, Jan Beulich wrote:
> >>> On 06.08.12 at 17:43, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> wrote:
> > On Mon, 6 Aug 2012, Jan Beulich wrote:
> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >> wrote:
> >> > This is an incremental patch on top of
> >> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
> >> > compatibility, it is better to introduce foreign_domid as part of a
> >> > union containing both size and foreign_domid.
> >> > 
> >> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >> > ---
> >> >  xen/include/public/memory.h |   11 +++++++----
> >> >  1 files changed, 7 insertions(+), 4 deletions(-)
> >> > 
> >> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> >> > index b2adfbe..b0af2fd 100644
> >> > --- a/xen/include/public/memory.h
> >> > +++ b/xen/include/public/memory.h
> >> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
> >> >      /* Which domain to change the mapping for. */
> >> >      domid_t domid;
> >> >  
> >> > -    /* Number of pages to go through for gmfn_range */
> >> > -    uint16_t    size;
> >> > +    union {
> >> > +        /* Number of pages to go through for gmfn_range */
> >> > +        uint16_t    size;
> >> > +        /* IFF gmfn_foreign */
> >> > +        domid_t foreign_domid;
> >> > +    };
> >> 
> >> But you're clear that this isn't standard C, and hence can't go
> >> in this way?
> >> 
> > 
> > Why? It is c11 if I am not mistaken.
> 
> Yes. But the common baseline is C89.

Do we need to keep it C89?
If I am not mistaken anonymous unions have been in GCC for more than 10
years now.

If we do want to keep it C89, considering that size was introduced only
recently, do you think that it would be OK for me to change the
interface and just add size to a union?
Like this:

union {
    /* Number of pages to go through for gmfn_range */
    uint16_t    size;
    /* IFF gmfn_foreign */
    domid_t foreign_domid;
} u;

Also CC'ing Jean.

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Jean Guyader]

On 7 August 2012 13:27, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >>> On 06.08.12 at 17:43, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> wrote:
>> > On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> >> wrote:
>> >> > This is an incremental patch on top of
>> >> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
>> >> > compatibility, it is better to introduce foreign_domid as part of a
>> >> > union containing both size and foreign_domid.
>> >> >
>> >> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> >> > ---
>> >> >  xen/include/public/memory.h |   11 +++++++----
>> >> >  1 files changed, 7 insertions(+), 4 deletions(-)
>> >> >
>> >> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
>> >> > index b2adfbe..b0af2fd 100644
>> >> > --- a/xen/include/public/memory.h
>> >> > +++ b/xen/include/public/memory.h
>> >> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
>> >> >      /* Which domain to change the mapping for. */
>> >> >      domid_t domid;
>> >> >
>> >> > -    /* Number of pages to go through for gmfn_range */
>> >> > -    uint16_t    size;
>> >> > +    union {
>> >> > +        /* Number of pages to go through for gmfn_range */
>> >> > +        uint16_t    size;
>> >> > +        /* IFF gmfn_foreign */
>> >> > +        domid_t foreign_domid;
>> >> > +    };
>> >>
>> >> But you're clear that this isn't standard C, and hence can't go
>> >> in this way?
>> >>
>> >
>> > Why? It is c11 if I am not mistaken.
>>
>> Yes. But the common baseline is C89.
>
> Do we need to keep it C89?
> If I am not mistaken anonymous unions have been in GCC for more than 10
> years now.
>

It's a public header so you can't assume that the only consumer will be GCC.

> If we do want to keep it C89, considering that size was introduced only
> recently, do you think that it would be OK for me to change the
> interface and just add size to a union?
> Like this:
>
> union {
>     /* Number of pages to go through for gmfn_range */
>     uint16_t    size;
>     /* IFF gmfn_foreign */
>     domid_t foreign_domid;
> } u;
>

You could probably do something like

#ifdef __GNUC__
# define UNION_NAME
#else
# define UNION_NAME u
#endif
union {
    /* Number of pages to go through for gmfn_range */
    uint16_t    size;
    /* IFF gmfn_foreign */
    domid_t foreign_domid;
} UNION_NAME;

It's not ideal but this way you keep the binary compatibility and you
also the code still cmopatible with GCC.

Jean

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Jan Beulich]

>>> On 07.08.12 at 14:40, Jean Guyader <jean.guyader@gmail.com> wrote:
> You could probably do something like
> 
> #ifdef __GNUC__
> # define UNION_NAME
> #else
> # define UNION_NAME u
> #endif
> union {
>     /* Number of pages to go through for gmfn_range */
>     uint16_t    size;
>     /* IFF gmfn_foreign */
>     domid_t foreign_domid;
> } UNION_NAME;
> 
> It's not ideal but this way you keep the binary compatibility and you
> also the code still cmopatible with GCC.

Ah, yes, something along those lines might be okay; we already
have similar things e.g. for __DECL_REG() (note the additional
use of __STRICT_ANSI__ there, though). The most problematic
thing here is the name space cluttering - UNION_NAME is certainly
not good, but __DECL_REG really is too unspecific too (yet we
got away with it so far).

Jan

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Stefano Stabellini]

On Tue, 7 Aug 2012, Jan Beulich wrote:
> >>> On 07.08.12 at 14:40, Jean Guyader <jean.guyader@gmail.com> wrote:
> > You could probably do something like
> > 
> > #ifdef __GNUC__
> > # define UNION_NAME
> > #else
> > # define UNION_NAME u
> > #endif
> > union {
> >     /* Number of pages to go through for gmfn_range */
> >     uint16_t    size;
> >     /* IFF gmfn_foreign */
> >     domid_t foreign_domid;
> > } UNION_NAME;
> > 
> > It's not ideal but this way you keep the binary compatibility and you
> > also the code still cmopatible with GCC.
> 
> Ah, yes, something along those lines might be okay; we already
> have similar things e.g. for __DECL_REG() (note the additional
> use of __STRICT_ANSI__ there, though).

But it is strict ANSI.


> The most problematic
> thing here is the name space cluttering - UNION_NAME is certainly
> not good, but __DECL_REG really is too unspecific too (yet we
> got away with it so far).

OK, I'll got for this strategy.

Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD?


#if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >= 201112L)
# define XEN_ADD_TO_PHYSMAP_ARG
#else
# define XEN_ADD_TO_PHYSMAP_ARG u
#endif

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Jan Beulich]

>>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD?

Sounds fine (and I like this better than the ..._ARG one you used
below.

> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >= 201112L)

#if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \
    (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L)

avoiding compilers to warn about the use of the possibly
undefined __STDC_VERSION__, which only got introduced
after C89 was already published (and which e.g. gcc indeed
doesn't define if the value would end up being below 199409L).

> # define XEN_ADD_TO_PHYSMAP_ARG
> #else
> # define XEN_ADD_TO_PHYSMAP_ARG u
> #endif

Also, please don't forget to #undef it after use.

Jan

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Ian Campbell]

On Wed, 2012-08-08 at 08:14 +0100, Jan Beulich wrote:
> >>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD?
> 
> Sounds fine (and I like this better than the ..._ARG one you used
> below.
> 
> > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >= 201112L)
> 
> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \
>     (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L)

The downside of this is that users of this header might need to change
their code depending on what compiler they actually build with today (or
even what options).

Is adding the ".u" throughout the Xen code base too intrusive?

> avoiding compilers to warn about the use of the possibly
> undefined __STDC_VERSION__, which only got introduced
> after C89 was already published (and which e.g. gcc indeed
> doesn't define if the value would end up being below 199409L).
> 
> > # define XEN_ADD_TO_PHYSMAP_ARG
> > #else
> > # define XEN_ADD_TO_PHYSMAP_ARG u
> > #endif
> 
> Also, please don't forget to #undef it after use.
> 
> Jan
> 

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Jan Beulich]

>>> On 08.08.12 at 09:45, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Wed, 2012-08-08 at 08:14 +0100, Jan Beulich wrote:
>> >>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
> wrote:
>> > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD?
>> 
>> Sounds fine (and I like this better than the ..._ARG one you used
>> below.
>> 
>> > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >= 
> 201112L)
>> 
>> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \
>>     (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L)
> 
> The downside of this is that users of this header might need to change
> their code depending on what compiler they actually build with today (or
> even what options).
> 
> Is adding the ".u" throughout the Xen code base too intrusive?

I don't know and didn't check; I think the goal was to avoid having
to change consumers that use gcc for compilation.

Jan

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Stefano Stabellini]

On Wed, 8 Aug 2012, Jan Beulich wrote:
> >>> On 08.08.12 at 09:45, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Wed, 2012-08-08 at 08:14 +0100, Jan Beulich wrote:
> >> >>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
> > wrote:
> >> > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD?
> >> 
> >> Sounds fine (and I like this better than the ..._ARG one you used
> >> below.
> >> 
> >> > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >= 
> > 201112L)
> >> 
> >> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \
> >>     (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L)
> > 
> > The downside of this is that users of this header might need to change
> > their code depending on what compiler they actually build with today (or
> > even what options).
> > 
> > Is adding the ".u" throughout the Xen code base too intrusive?
> 
> I don't know and didn't check; I think the goal was to avoid having
> to change consumers that use gcc for compilation.

For ARM is not an issue, but the size parameter can be used by out of
tree code (V4V?).
That's why I CC'ed Jean, I was hoping he was going to say that it is OK
to add ".u".

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Jean Guyader]

On 8 August 2012 10:51, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Wed, 8 Aug 2012, Jan Beulich wrote:
>> >>> On 08.08.12 at 09:45, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Wed, 2012-08-08 at 08:14 +0100, Jan Beulich wrote:
>> >> >>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> > wrote:
>> >> > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD?
>> >>
>> >> Sounds fine (and I like this better than the ..._ARG one you used
>> >> below.
>> >>
>> >> > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >=
>> > 201112L)
>> >>
>> >> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \
>> >>     (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L)
>> >
>> > The downside of this is that users of this header might need to change
>> > their code depending on what compiler they actually build with today (or
>> > even what options).
>> >
>> > Is adding the ".u" throughout the Xen code base too intrusive?
>>
>> I don't know and didn't check; I think the goal was to avoid having
>> to change consumers that use gcc for compilation.
>
> For ARM is not an issue, but the size parameter can be used by out of
> tree code (V4V?).
> That's why I CC'ed Jean, I was hoping he was going to say that it is OK
> to add ".u".

I have introduced the size field for XENMAPSPACE_gmfn_range and that is used
by hvmload when we want to relocated the memory because the pci hole needs to be
bigger.

Maybe something else use it by now, if not you could get away by just
updating the Xen
code to use .u.

Jean

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Stefano Stabellini]

On Wed, 8 Aug 2012, Jean Guyader wrote:
> On 8 August 2012 10:51, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
> > On Wed, 8 Aug 2012, Jan Beulich wrote:
> >> >>> On 08.08.12 at 09:45, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Wed, 2012-08-08 at 08:14 +0100, Jan Beulich wrote:
> >> >> >>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >> > wrote:
> >> >> > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD?
> >> >>
> >> >> Sounds fine (and I like this better than the ..._ARG one you used
> >> >> below.
> >> >>
> >> >> > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >=
> >> > 201112L)
> >> >>
> >> >> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \
> >> >>     (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L)
> >> >
> >> > The downside of this is that users of this header might need to change
> >> > their code depending on what compiler they actually build with today (or
> >> > even what options).
> >> >
> >> > Is adding the ".u" throughout the Xen code base too intrusive?
> >>
> >> I don't know and didn't check; I think the goal was to avoid having
> >> to change consumers that use gcc for compilation.
> >
> > For ARM is not an issue, but the size parameter can be used by out of
> > tree code (V4V?).
> > That's why I CC'ed Jean, I was hoping he was going to say that it is OK
> > to add ".u".
> 
> I have introduced the size field for XENMAPSPACE_gmfn_range and that is used
> by hvmload when we want to relocated the memory because the pci hole needs to be
> bigger.
> 
> Maybe something else use it by now, if not you could get away by just
> updating the Xen
> code to use .u.

Great, I'll do that then.

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[David Vrabel]

On 08/08/2012 11:03, Jean Guyader wrote:
>
> I have introduced the size field for XENMAPSPACE_gmfn_range and that is used
> by hvmload when we want to relocated the memory because the pci hole needs to be
> bigger.

Why do this so late? Rather than creating the domain with the correctly 
sized PCI hole?

David

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Jean Guyader]

On 8 August 2012 15:20, David Vrabel <dvrabel@cantab.net> wrote:
> On 08/08/2012 11:03, Jean Guyader wrote:
>>
>>
>> I have introduced the size field for XENMAPSPACE_gmfn_range and that is
>> used
>> by hvmload when we want to relocated the memory because the pci hole needs
>> to be
>> bigger.
>
>
> Why do this so late? Rather than creating the domain with the correctly
> sized PCI hole?
>

The PCI hole isn't fix in size, especially if you do passthrough. It's
not really easy to
compute outside of the domain you will have to ask Qemu. I think it's
fine how it's
done today.

Jean

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Jan Beulich]

>>> On 07.08.12 at 14:27, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >>> On 06.08.12 at 17:43, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> wrote:
>> > On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> >> wrote:
>> >> > This is an incremental patch on top of
>> >> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
>> >> > compatibility, it is better to introduce foreign_domid as part of a
>> >> > union containing both size and foreign_domid.
>> >> > 
>> >> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> >> > ---
>> >> >  xen/include/public/memory.h |   11 +++++++----
>> >> >  1 files changed, 7 insertions(+), 4 deletions(-)
>> >> > 
>> >> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
>> >> > index b2adfbe..b0af2fd 100644
>> >> > --- a/xen/include/public/memory.h
>> >> > +++ b/xen/include/public/memory.h
>> >> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
>> >> >      /* Which domain to change the mapping for. */
>> >> >      domid_t domid;
>> >> >  
>> >> > -    /* Number of pages to go through for gmfn_range */
>> >> > -    uint16_t    size;
>> >> > +    union {
>> >> > +        /* Number of pages to go through for gmfn_range */
>> >> > +        uint16_t    size;
>> >> > +        /* IFF gmfn_foreign */
>> >> > +        domid_t foreign_domid;
>> >> > +    };
>> >> 
>> >> But you're clear that this isn't standard C, and hence can't go
>> >> in this way?
>> >> 
>> > 
>> > Why? It is c11 if I am not mistaken.
>> 
>> Yes. But the common baseline is C89.
> 
> Do we need to keep it C89?

Yes, I think so. That's what we indeed can (and already do) expect
of compilers used on these headers. Even C99 would be too much
already.

> If I am not mistaken anonymous unions have been in GCC for more than 10
> years now.

With a few quirks here and there, yes. But gcc is not the measure
here anyway, we need to settle on the lowest common standard
that's largely accepted/implemented industry wide.

> If we do want to keep it C89, considering that size was introduced only
> recently, do you think that it would be OK for me to change the
> interface and just add size to a union?
> Like this:
> 
> union {
>     /* Number of pages to go through for gmfn_range */
>     uint16_t    size;
>     /* IFF gmfn_foreign */
>     domid_t foreign_domid;
> } u;
> 
> Also CC'ing Jean.

If that's okay with Jean (and implying eventual other users of the
interface - I hope he would know), then yes; the addition isn't in
anything that we have released so far.

The only other reasonable alternative I see would be to bump
__XEN_LATEST_INTERFACE_VERSION__ again, and have this
coded as (beware, ugly!)

#if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200
union {
#endif
    /* Number of pages to go through for gmfn_range */
    uint16_t    size;
#if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200
    /* IFF gmfn_foreign */
    domid_t foreign_domid;
} u;
#endif

Jan

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Ian Jackson]

Jan Beulich writes ("Re: [Xen-devel] [PATCH 1/5] xen: improve changes to xen_add_to_physmap"):
> #if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200
> union {
> #endif
>     /* Number of pages to go through for gmfn_range */
>     uint16_t    size;
> #if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200
>     /* IFF gmfn_foreign */
>     domid_t foreign_domid;
> } u;
> #endif

Has someone checked that on all supported platforms the size and
alignment of this union is identical to that of the original
uint16_t ?

Ian.

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Jan Beulich]

>>> On 07.08.12 at 17:24, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> Jan Beulich writes ("Re: [Xen-devel] [PATCH 1/5] xen: improve changes to 
> xen_add_to_physmap"):
>> #if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200
>> union {
>> #endif
>>     /* Number of pages to go through for gmfn_range */
>>     uint16_t    size;
>> #if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200
>>     /* IFF gmfn_foreign */
>>     domid_t foreign_domid;
>> } u;
>> #endif
> 
> Has someone checked that on all supported platforms the size and
> alignment of this union is identical to that of the original
> uint16_t ?

Since domid_t is uniformly a typedef of uint16_t, I don't think
there's any doubt in that.

Jan

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Mukesh Rathor]

On Mon, 6 Aug 2012 15:12:01 +0100
Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:

> This is an incremental patch on top of
> c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
> compatibility, it is better to introduce foreign_domid as part of a
> union containing both size and foreign_domid.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/include/public/memory.h |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index b2adfbe..b0af2fd 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
>      /* Which domain to change the mapping for. */
>      domid_t domid;
>  
> -    /* Number of pages to go through for gmfn_range */
> -    uint16_t    size;
> +    union {
> +        /* Number of pages to go through for gmfn_range */
> +        uint16_t    size;
> +        /* IFF gmfn_foreign */
> +        domid_t foreign_domid;
> +    };
>  
>      /* Source mapping space. */
>  #define XENMAPSPACE_shared_info  0 /* shared info page */
> @@ -217,8 +221,7 @@ struct xen_add_to_physmap {
>  #define XENMAPSPACE_gmfn         2 /* GMFN */
>  #define XENMAPSPACE_gmfn_range   3 /* GMFN range */
>  #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
> -    uint16_t space;
> -    domid_t foreign_domid; /* IFF gmfn_foreign */
> +    unsigned int space;
>  
>  #define XENMAPIDX_grant_table_status 0x80000000
>  

Is this the final version? I don't see it in today's xen unstable tree!
I have this in my tree:

struct xen_add_to_physmap {
    /* Which domain to change the mapping for. */
    domid_t domid;

    /* Number of pages to go through for gmfn_range */
    uint16_t    size;

    /* Source mapping space. */
#define XENMAPSPACE_shared_info 0 /* shared info page */
#define XENMAPSPACE_grant_table 1 /* grant table page */
#define XENMAPSPACE_gmfn        2 /* GMFN */
#define XENMAPSPACE_gmfn_range  3 /* GMFN range */
#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
    uint16_t space;
    domid_t foreign_domid;         /* IFF XENMAPSPACE_gmfn_foreign */


thanks,
Mukesh

Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap

[Stefano Stabellini]

On Sat, 11 Aug 2012, Mukesh Rathor wrote:
> On Mon, 6 Aug 2012 15:12:01 +0100
> Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> 
> > This is an incremental patch on top of
> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
> > compatibility, it is better to introduce foreign_domid as part of a
> > union containing both size and foreign_domid.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  xen/include/public/memory.h |   11 +++++++----
> >  1 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> > index b2adfbe..b0af2fd 100644
> > --- a/xen/include/public/memory.h
> > +++ b/xen/include/public/memory.h
> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap {
> >      /* Which domain to change the mapping for. */
> >      domid_t domid;
> >  
> > -    /* Number of pages to go through for gmfn_range */
> > -    uint16_t    size;
> > +    union {
> > +        /* Number of pages to go through for gmfn_range */
> > +        uint16_t    size;
> > +        /* IFF gmfn_foreign */
> > +        domid_t foreign_domid;
> > +    };
> >  
> >      /* Source mapping space. */
> >  #define XENMAPSPACE_shared_info  0 /* shared info page */
> > @@ -217,8 +221,7 @@ struct xen_add_to_physmap {
> >  #define XENMAPSPACE_gmfn         2 /* GMFN */
> >  #define XENMAPSPACE_gmfn_range   3 /* GMFN range */
> >  #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
> > -    uint16_t space;
> > -    domid_t foreign_domid; /* IFF gmfn_foreign */
> > +    unsigned int space;
> >  
> >  #define XENMAPIDX_grant_table_status 0x80000000
> >  
> 
> Is this the final version? I don't see it in today's xen unstable tree!
> I have this in my tree:
> 
> struct xen_add_to_physmap {
>     /* Which domain to change the mapping for. */
>     domid_t domid;
> 
>     /* Number of pages to go through for gmfn_range */
>     uint16_t    size;
> 
>     /* Source mapping space. */
> #define XENMAPSPACE_shared_info 0 /* shared info page */
> #define XENMAPSPACE_grant_table 1 /* grant table page */
> #define XENMAPSPACE_gmfn        2 /* GMFN */
> #define XENMAPSPACE_gmfn_range  3 /* GMFN range */
> #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
>     uint16_t space;
>     domid_t foreign_domid;         /* IFF XENMAPSPACE_gmfn_foreign */

We are still discussing how the final version should actually look like
but the last patch I sent is this one (for the for-4.3 tree):

http://marc.info/?l=xen-devel&m=134460098420461

[PATCH 2/5] xen/arm: introduce __lshrdi3 and __aeabi_llsr

[Stefano Stabellini]

Taken from Linux.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/lib/Makefile  |    2 +-
 xen/arch/arm/lib/lshrdi3.S |   54 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletions(-)
 create mode 100644 xen/arch/arm/lib/lshrdi3.S

diff --git a/xen/arch/arm/lib/Makefile b/xen/arch/arm/lib/Makefile
index cbbed68..4cf41f4 100644
--- a/xen/arch/arm/lib/Makefile
+++ b/xen/arch/arm/lib/Makefile
@@ -2,4 +2,4 @@ obj-y += memcpy.o memmove.o memset.o memzero.o
 obj-y += findbit.o setbit.o
 obj-y += setbit.o clearbit.o changebit.o
 obj-y += testsetbit.o testclearbit.o testchangebit.o
-obj-y += lib1funcs.o div64.o
+obj-y += lib1funcs.o lshrdi3.o div64.o
diff --git a/xen/arch/arm/lib/lshrdi3.S b/xen/arch/arm/lib/lshrdi3.S
new file mode 100644
index 0000000..3e8887e
--- /dev/null
+++ b/xen/arch/arm/lib/lshrdi3.S
@@ -0,0 +1,54 @@
+/* Copyright 1995, 1996, 1998, 1999, 2000, 2003, 2004, 2005
+   Free Software Foundation, Inc.
+
+This file is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by the
+Free Software Foundation; either version 2, or (at your option) any
+later version.
+
+In addition to the permissions in the GNU General Public License, the
+Free Software Foundation gives you unlimited permission to link the
+compiled version of this file into combinations with other programs,
+and to distribute those combinations without any restriction coming
+from the use of this file.  (The General Public License restrictions
+do apply in other respects; for example, they cover modification of
+the file, and distribution when not linked into a combine
+executable.)
+
+This file is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; see the file COPYING.  If not, write to
+the Free Software Foundation, 51 Franklin Street, Fifth Floor,
+Boston, MA 02110-1301, USA.  */
+
+
+#include <xen/config.h>
+#include "assembler.h"
+
+#ifdef __ARMEB__
+#define al r1
+#define ah r0
+#else
+#define al r0
+#define ah r1
+#endif
+
+ENTRY(__lshrdi3)
+ENTRY(__aeabi_llsr)
+
+	subs	r3, r2, #32
+	rsb	ip, r2, #32
+	movmi	al, al, lsr r2
+	movpl	al, ah, lsr r3
+ ARM(	orrmi	al, al, ah, lsl ip	)
+ THUMB(	lslmi	r3, ah, ip		)
+ THUMB(	orrmi	al, al, r3		)
+	mov	ah, ah, lsr r2
+	mov	pc, lr
+
+ENDPROC(__lshrdi3)
+ENDPROC(__aeabi_llsr)
-- 
1.7.2.5

Re: [PATCH 2/5] xen/arm: introduce __lshrdi3 and __aeabi_llsr

[Ian Campbell]

On Mon, 2012-08-06 at 15:12 +0100, Stefano Stabellini wrote:
> Taken from Linux.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Any idea what lshrdi means?

Anyway, given that this is presumably required by code which gcc
generates and that it comes direct from Linux:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

and pushed to my arm-for-4.3 branch.

> ---
>  xen/arch/arm/lib/Makefile  |    2 +-
>  xen/arch/arm/lib/lshrdi3.S |   54 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+), 1 deletions(-)
>  create mode 100644 xen/arch/arm/lib/lshrdi3.S
> 
> diff --git a/xen/arch/arm/lib/Makefile b/xen/arch/arm/lib/Makefile
> index cbbed68..4cf41f4 100644
> --- a/xen/arch/arm/lib/Makefile
> +++ b/xen/arch/arm/lib/Makefile
> @@ -2,4 +2,4 @@ obj-y += memcpy.o memmove.o memset.o memzero.o
>  obj-y += findbit.o setbit.o
>  obj-y += setbit.o clearbit.o changebit.o
>  obj-y += testsetbit.o testclearbit.o testchangebit.o
> -obj-y += lib1funcs.o div64.o
> +obj-y += lib1funcs.o lshrdi3.o div64.o
> diff --git a/xen/arch/arm/lib/lshrdi3.S b/xen/arch/arm/lib/lshrdi3.S
> new file mode 100644
> index 0000000..3e8887e
> --- /dev/null
> +++ b/xen/arch/arm/lib/lshrdi3.S
> @@ -0,0 +1,54 @@
> +/* Copyright 1995, 1996, 1998, 1999, 2000, 2003, 2004, 2005
> +   Free Software Foundation, Inc.
> +
> +This file is free software; you can redistribute it and/or modify it
> +under the terms of the GNU General Public License as published by the
> +Free Software Foundation; either version 2, or (at your option) any
> +later version.
> +
> +In addition to the permissions in the GNU General Public License, the
> +Free Software Foundation gives you unlimited permission to link the
> +compiled version of this file into combinations with other programs,
> +and to distribute those combinations without any restriction coming
> +from the use of this file.  (The General Public License restrictions
> +do apply in other respects; for example, they cover modification of
> +the file, and distribution when not linked into a combine
> +executable.)
> +
> +This file is distributed in the hope that it will be useful, but
> +WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with this program; see the file COPYING.  If not, write to
> +the Free Software Foundation, 51 Franklin Street, Fifth Floor,
> +Boston, MA 02110-1301, USA.  */
> +
> +
> +#include <xen/config.h>
> +#include "assembler.h"
> +
> +#ifdef __ARMEB__
> +#define al r1
> +#define ah r0
> +#else
> +#define al r0
> +#define ah r1
> +#endif
> +
> +ENTRY(__lshrdi3)
> +ENTRY(__aeabi_llsr)
> +
> +	subs	r3, r2, #32
> +	rsb	ip, r2, #32
> +	movmi	al, al, lsr r2
> +	movpl	al, ah, lsr r3
> + ARM(	orrmi	al, al, ah, lsl ip	)
> + THUMB(	lslmi	r3, ah, ip		)
> + THUMB(	orrmi	al, al, r3		)
> +	mov	ah, ah, lsr r2
> +	mov	pc, lr
> +
> +ENDPROC(__lshrdi3)
> +ENDPROC(__aeabi_llsr)

Re: [PATCH 2/5] xen/arm: introduce __lshrdi3 and __aeabi_llsr

[Stefano Stabellini]

On Thu, 9 Aug 2012, Ian Campbell wrote:
> On Mon, 2012-08-06 at 15:12 +0100, Stefano Stabellini wrote:
> > Taken from Linux.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> Any idea what lshrdi means?

I am not sure TBH, but it is one of the needed libgcc functions.


> Anyway, given that this is presumably required by code which gcc
> generates and that it comes direct from Linux:
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> and pushed to my arm-for-4.3 branch.
> 
> > ---
> >  xen/arch/arm/lib/Makefile  |    2 +-
> >  xen/arch/arm/lib/lshrdi3.S |   54 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 55 insertions(+), 1 deletions(-)
> >  create mode 100644 xen/arch/arm/lib/lshrdi3.S
> > 
> > diff --git a/xen/arch/arm/lib/Makefile b/xen/arch/arm/lib/Makefile
> > index cbbed68..4cf41f4 100644
> > --- a/xen/arch/arm/lib/Makefile
> > +++ b/xen/arch/arm/lib/Makefile
> > @@ -2,4 +2,4 @@ obj-y += memcpy.o memmove.o memset.o memzero.o
> >  obj-y += findbit.o setbit.o
> >  obj-y += setbit.o clearbit.o changebit.o
> >  obj-y += testsetbit.o testclearbit.o testchangebit.o
> > -obj-y += lib1funcs.o div64.o
> > +obj-y += lib1funcs.o lshrdi3.o div64.o
> > diff --git a/xen/arch/arm/lib/lshrdi3.S b/xen/arch/arm/lib/lshrdi3.S
> > new file mode 100644
> > index 0000000..3e8887e
> > --- /dev/null
> > +++ b/xen/arch/arm/lib/lshrdi3.S
> > @@ -0,0 +1,54 @@
> > +/* Copyright 1995, 1996, 1998, 1999, 2000, 2003, 2004, 2005
> > +   Free Software Foundation, Inc.
> > +
> > +This file is free software; you can redistribute it and/or modify it
> > +under the terms of the GNU General Public License as published by the
> > +Free Software Foundation; either version 2, or (at your option) any
> > +later version.
> > +
> > +In addition to the permissions in the GNU General Public License, the
> > +Free Software Foundation gives you unlimited permission to link the
> > +compiled version of this file into combinations with other programs,
> > +and to distribute those combinations without any restriction coming
> > +from the use of this file.  (The General Public License restrictions
> > +do apply in other respects; for example, they cover modification of
> > +the file, and distribution when not linked into a combine
> > +executable.)
> > +
> > +This file is distributed in the hope that it will be useful, but
> > +WITHOUT ANY WARRANTY; without even the implied warranty of
> > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +General Public License for more details.
> > +
> > +You should have received a copy of the GNU General Public License
> > +along with this program; see the file COPYING.  If not, write to
> > +the Free Software Foundation, 51 Franklin Street, Fifth Floor,
> > +Boston, MA 02110-1301, USA.  */
> > +
> > +
> > +#include <xen/config.h>
> > +#include "assembler.h"
> > +
> > +#ifdef __ARMEB__
> > +#define al r1
> > +#define ah r0
> > +#else
> > +#define al r0
> > +#define ah r1
> > +#endif
> > +
> > +ENTRY(__lshrdi3)
> > +ENTRY(__aeabi_llsr)
> > +
> > +	subs	r3, r2, #32
> > +	rsb	ip, r2, #32
> > +	movmi	al, al, lsr r2
> > +	movpl	al, ah, lsr r3
> > + ARM(	orrmi	al, al, ah, lsl ip	)
> > + THUMB(	lslmi	r3, ah, ip		)
> > + THUMB(	orrmi	al, al, r3		)
> > +	mov	ah, ah, lsr r2
> > +	mov	pc, lr
> > +
> > +ENDPROC(__lshrdi3)
> > +ENDPROC(__aeabi_llsr)
> 
> 
> 

[PATCH 3/5] xen: few more xen_ulong_t substitutions

[Stefano Stabellini]

There are still few unsigned long in the xen public interface: replace
them with xen_ulong_t.

Also typedef xen_ulong_t to uint64_t on ARM.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/include/public/arch-arm.h |    4 ++--
 xen/include/public/physdev.h  |    2 +-
 xen/include/public/version.h  |    2 +-
 xen/include/public/xen.h      |    4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 14ad0ab..2ae6548 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -122,8 +122,8 @@ typedef uint64_t xen_pfn_t;
 /* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */
 #define XEN_LEGACY_MAX_VCPUS 1
 
-typedef uint32_t xen_ulong_t;
-#define PRI_xen_ulong PRIx32
+typedef uint64_t xen_ulong_t;
+#define PRI_xen_ulong PRIx64
 
 struct vcpu_guest_context {
 #define _VGCF_online                   0
diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
index b78eeba..a4cf6eb 100644
--- a/xen/include/public/physdev.h
+++ b/xen/include/public/physdev.h
@@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
 #define PHYSDEVOP_apic_write             9
 struct physdev_apic {
     /* IN */
-    unsigned long apic_physbase;
+    xen_ulong_t apic_physbase;
     uint32_t reg;
     /* IN or OUT */
     uint32_t value;
diff --git a/xen/include/public/version.h b/xen/include/public/version.h
index 8742c2b..eb83eba 100644
--- a/xen/include/public/version.h
+++ b/xen/include/public/version.h
@@ -58,7 +58,7 @@ typedef char xen_changeset_info_t[64];
 
 #define XENVER_platform_parameters 5
 struct xen_platform_parameters {
-    unsigned long virt_start;
+    xen_ulong_t virt_start;
 };
 typedef struct xen_platform_parameters xen_platform_parameters_t;
 
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index b2f6c50..d635bbf 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
  * NB. The fields are natural register size for this architecture.
  */
 struct multicall_entry {
-    unsigned long op, result;
-    unsigned long args[6];
+    xen_ulong_t op, result;
+    xen_ulong_t args[6];
 };
 typedef struct multicall_entry multicall_entry_t;
 DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);
-- 
1.7.2.5

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Jan Beulich]

>>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> There are still few unsigned long in the xen public interface: replace
> them with xen_ulong_t.
> 
> Also typedef xen_ulong_t to uint64_t on ARM.

While this change by itself already looks suspicious to me, I don't
follow what the global replacement is going to be good for, in
particular when done in places that ARM should be completely
ignorant of, e.g.

> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
>  #define PHYSDEVOP_apic_write             9
>  struct physdev_apic {
>      /* IN */
> -    unsigned long apic_physbase;
> +    xen_ulong_t apic_physbase;
>      uint32_t reg;
>      /* IN or OUT */
>      uint32_t value;
>...
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
>   * NB. The fields are natural register size for this architecture.
>   */
>  struct multicall_entry {
> -    unsigned long op, result;
> -    unsigned long args[6];
> +    xen_ulong_t op, result;
> +    xen_ulong_t args[6];

And here I really start to wonder - what use is it to put all 64-bit
values here on a 32-bit arch? You certainly know a lot more about
ARM than me, but this looks pretty inefficient, the more that
you'll have to deal with checking the full values when converting
to e.g. pointers anyway, in order to avoid behavioral differences
between running on a 32- or 64-bit host. Zero-extending from
32-bits when in a 64-bit hypervisor wouldn't have this problem.

Jan

>  };
>  typedef struct multicall_entry multicall_entry_t;
>  DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Stefano Stabellini]

On Mon, 6 Aug 2012, Jan Beulich wrote:
> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> > There are still few unsigned long in the xen public interface: replace
> > them with xen_ulong_t.
> > 
> > Also typedef xen_ulong_t to uint64_t on ARM.
> 
> While this change by itself already looks suspicious to me, I don't
> follow what the global replacement is going to be good for, in
> particular when done in places that ARM should be completely
> ignorant of, e.g.

See below


> > --- a/xen/include/public/physdev.h
> > +++ b/xen/include/public/physdev.h
> > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
> >  #define PHYSDEVOP_apic_write             9
> >  struct physdev_apic {
> >      /* IN */
> > -    unsigned long apic_physbase;
> > +    xen_ulong_t apic_physbase;
> >      uint32_t reg;
> >      /* IN or OUT */
> >      uint32_t value;
> >...

This change is actually not required, considering that ARM doesn't have
an APIC. I changed apic_physbase to xen_ulong_t only for consistency,
but it wouldn't make any difference for ARM (or x86).
If you think that it is better to keep it unsigned long, I'll remove
this chunk for the patch.


> > --- a/xen/include/public/xen.h
> > +++ b/xen/include/public/xen.h
> > @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
> >   * NB. The fields are natural register size for this architecture.
> >   */
> >  struct multicall_entry {
> > -    unsigned long op, result;
> > -    unsigned long args[6];
> > +    xen_ulong_t op, result;
> > +    xen_ulong_t args[6];
> 
> And here I really start to wonder - what use is it to put all 64-bit
> values here on a 32-bit arch? You certainly know a lot more about
> ARM than me, but this looks pretty inefficient, the more that
> you'll have to deal with checking the full values when converting
> to e.g. pointers anyway, in order to avoid behavioral differences
> between running on a 32- or 64-bit host. Zero-extending from
> 32-bits when in a 64-bit hypervisor wouldn't have this problem.

Actually the multicall_entry change is wrong, thanks for point it out.

The idea is that pointers are always 8 bytes sized and 8 bytes aligned,
except when they are passed as hypercall arguments, in which case a 32
bit guest would use 32 bit pointers and a 64 bit guest would use 64 bit
pointers.

Considering that each field of a multicall_entry is usually passed as an
hypercall parameter, they should all remain unsigned long.

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Ian Campbell]

On Tue, 2012-08-07 at 13:08 +0100, Stefano Stabellini wrote:
> On Mon, 6 Aug 2012, Jan Beulich wrote:
> > >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> > > There are still few unsigned long in the xen public interface: replace
> > > them with xen_ulong_t.
> > > 
> > > Also typedef xen_ulong_t to uint64_t on ARM.
> > 
> > While this change by itself already looks suspicious to me, I don't
> > follow what the global replacement is going to be good for, in
> > particular when done in places that ARM should be completely
> > ignorant of, e.g.
> 
> See below
> 
> 
> > > --- a/xen/include/public/physdev.h
> > > +++ b/xen/include/public/physdev.h
> > > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
> > >  #define PHYSDEVOP_apic_write             9
> > >  struct physdev_apic {
> > >      /* IN */
> > > -    unsigned long apic_physbase;
> > > +    xen_ulong_t apic_physbase;
> > >      uint32_t reg;
> > >      /* IN or OUT */
> > >      uint32_t value;
> > >...
> 
> This change is actually not required, considering that ARM doesn't have
> an APIC. I changed apic_physbase to xen_ulong_t only for consistency,
> but it wouldn't make any difference for ARM (or x86).
> If you think that it is better to keep it unsigned long, I'll remove
> this chunk for the patch.
> 
> 
> > > --- a/xen/include/public/xen.h
> > > +++ b/xen/include/public/xen.h
> > > @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
> > >   * NB. The fields are natural register size for this architecture.
> > >   */
> > >  struct multicall_entry {
> > > -    unsigned long op, result;
> > > -    unsigned long args[6];
> > > +    xen_ulong_t op, result;
> > > +    xen_ulong_t args[6];
> > 
> > And here I really start to wonder - what use is it to put all 64-bit
> > values here on a 32-bit arch? You certainly know a lot more about
> > ARM than me, but this looks pretty inefficient, the more that
> > you'll have to deal with checking the full values when converting
> > to e.g. pointers anyway, in order to avoid behavioral differences
> > between running on a 32- or 64-bit host. Zero-extending from
> > 32-bits when in a 64-bit hypervisor wouldn't have this problem.
> 
> Actually the multicall_entry change is wrong, thanks for point it out.
> 
> The idea is that pointers are always 8 bytes sized and 8 bytes aligned,
> except when they are passed as hypercall arguments, in which case a 32
> bit guest would use 32 bit pointers and a 64 bit guest would use 64 bit
> pointers.
> 
> Considering that each field of a multicall_entry is usually passed as an
> hypercall parameter, they should all remain unsigned long.

If possible, please make them an explicitly sized type, even if it is
now 32 bits.

Ian.

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Jan Beulich]

>>> On 07.08.12 at 14:36, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2012-08-07 at 13:08 +0100, Stefano Stabellini wrote:
>> On Mon, 6 Aug 2012, Jan Beulich wrote:
>> > >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
> wrote:
>> > > There are still few unsigned long in the xen public interface: replace
>> > > them with xen_ulong_t.
>> > > 
>> > > Also typedef xen_ulong_t to uint64_t on ARM.
>> > 
>> > While this change by itself already looks suspicious to me, I don't
>> > follow what the global replacement is going to be good for, in
>> > particular when done in places that ARM should be completely
>> > ignorant of, e.g.
>> 
>> See below
>> 
>> 
>> > > --- a/xen/include/public/physdev.h
>> > > +++ b/xen/include/public/physdev.h
>> > > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
>> > >  #define PHYSDEVOP_apic_write             9
>> > >  struct physdev_apic {
>> > >      /* IN */
>> > > -    unsigned long apic_physbase;
>> > > +    xen_ulong_t apic_physbase;
>> > >      uint32_t reg;
>> > >      /* IN or OUT */
>> > >      uint32_t value;
>> > >...
>> 
>> This change is actually not required, considering that ARM doesn't have
>> an APIC. I changed apic_physbase to xen_ulong_t only for consistency,
>> but it wouldn't make any difference for ARM (or x86).
>> If you think that it is better to keep it unsigned long, I'll remove
>> this chunk for the patch.
>> 
>> 
>> > > --- a/xen/include/public/xen.h
>> > > +++ b/xen/include/public/xen.h
>> > > @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
>> > >   * NB. The fields are natural register size for this architecture.
>> > >   */
>> > >  struct multicall_entry {
>> > > -    unsigned long op, result;
>> > > -    unsigned long args[6];
>> > > +    xen_ulong_t op, result;
>> > > +    xen_ulong_t args[6];
>> > 
>> > And here I really start to wonder - what use is it to put all 64-bit
>> > values here on a 32-bit arch? You certainly know a lot more about
>> > ARM than me, but this looks pretty inefficient, the more that
>> > you'll have to deal with checking the full values when converting
>> > to e.g. pointers anyway, in order to avoid behavioral differences
>> > between running on a 32- or 64-bit host. Zero-extending from
>> > 32-bits when in a 64-bit hypervisor wouldn't have this problem.
>> 
>> Actually the multicall_entry change is wrong, thanks for point it out.
>> 
>> The idea is that pointers are always 8 bytes sized and 8 bytes aligned,
>> except when they are passed as hypercall arguments, in which case a 32
>> bit guest would use 32 bit pointers and a 64 bit guest would use 64 bit
>> pointers.
>> 
>> Considering that each field of a multicall_entry is usually passed as an
>> hypercall parameter, they should all remain unsigned long.
> 
> If possible, please make them an explicitly sized type, even if it is
> now 32 bits.

??? This structure is already shared between 32-bit and 64-bit
implementations, and the fields are intentionally fitting both by
using "unsigned long". Are you suggesting to add #ifdef-ery to
public/xen.h?

Jan

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Ian Campbell]

On Tue, 2012-08-07 at 14:13 +0100, Jan Beulich wrote:
> >>> On 07.08.12 at 14:36, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Tue, 2012-08-07 at 13:08 +0100, Stefano Stabellini wrote:
> >> On Mon, 6 Aug 2012, Jan Beulich wrote:
> >> > >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
> > wrote:
> >> > > There are still few unsigned long in the xen public interface: replace
> >> > > them with xen_ulong_t.
> >> > > 
> >> > > Also typedef xen_ulong_t to uint64_t on ARM.
> >> > 
> >> > While this change by itself already looks suspicious to me, I don't
> >> > follow what the global replacement is going to be good for, in
> >> > particular when done in places that ARM should be completely
> >> > ignorant of, e.g.
> >> 
> >> See below
> >> 
> >> 
> >> > > --- a/xen/include/public/physdev.h
> >> > > +++ b/xen/include/public/physdev.h
> >> > > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
> >> > >  #define PHYSDEVOP_apic_write             9
> >> > >  struct physdev_apic {
> >> > >      /* IN */
> >> > > -    unsigned long apic_physbase;
> >> > > +    xen_ulong_t apic_physbase;
> >> > >      uint32_t reg;
> >> > >      /* IN or OUT */
> >> > >      uint32_t value;
> >> > >...
> >> 
> >> This change is actually not required, considering that ARM doesn't have
> >> an APIC. I changed apic_physbase to xen_ulong_t only for consistency,
> >> but it wouldn't make any difference for ARM (or x86).
> >> If you think that it is better to keep it unsigned long, I'll remove
> >> this chunk for the patch.
> >> 
> >> 
> >> > > --- a/xen/include/public/xen.h
> >> > > +++ b/xen/include/public/xen.h
> >> > > @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
> >> > >   * NB. The fields are natural register size for this architecture.
> >> > >   */
> >> > >  struct multicall_entry {
> >> > > -    unsigned long op, result;
> >> > > -    unsigned long args[6];
> >> > > +    xen_ulong_t op, result;
> >> > > +    xen_ulong_t args[6];
> >> > 
> >> > And here I really start to wonder - what use is it to put all 64-bit
> >> > values here on a 32-bit arch? You certainly know a lot more about
> >> > ARM than me, but this looks pretty inefficient, the more that
> >> > you'll have to deal with checking the full values when converting
> >> > to e.g. pointers anyway, in order to avoid behavioral differences
> >> > between running on a 32- or 64-bit host. Zero-extending from
> >> > 32-bits when in a 64-bit hypervisor wouldn't have this problem.
> >> 
> >> Actually the multicall_entry change is wrong, thanks for point it out.
> >> 
> >> The idea is that pointers are always 8 bytes sized and 8 bytes aligned,
> >> except when they are passed as hypercall arguments, in which case a 32
> >> bit guest would use 32 bit pointers and a 64 bit guest would use 64 bit
> >> pointers.
> >> 
> >> Considering that each field of a multicall_entry is usually passed as an
> >> hypercall parameter, they should all remain unsigned long.
> > 
> > If possible, please make them an explicitly sized type, even if it is
> > now 32 bits.
> 
> ??? This structure is already shared between 32-bit and 64-bit
> implementations, and the fields are intentionally fitting both by
> using "unsigned long". Are you suggesting to add #ifdef-ery to
> public/xen.h?

Oh, I thought multicall_entry was in an arch header.

In any case I would have expected a typedef xen_multicall_arg_t or
something.

> 
> Jan
> 

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Jan Beulich]

>>> On 07.08.12 at 14:08, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>  wrote:
>> > --- a/xen/include/public/physdev.h
>> > +++ b/xen/include/public/physdev.h
>> > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
>> >  #define PHYSDEVOP_apic_write             9
>> >  struct physdev_apic {
>> >      /* IN */
>> > -    unsigned long apic_physbase;
>> > +    xen_ulong_t apic_physbase;
>> >      uint32_t reg;
>> >      /* IN or OUT */
>> >      uint32_t value;
>> >...
> 
> This change is actually not required, considering that ARM doesn't have
> an APIC. I changed apic_physbase to xen_ulong_t only for consistency,
> but it wouldn't make any difference for ARM (or x86).
> If you think that it is better to keep it unsigned long, I'll remove
> this chunk for the patch.

I'd prefer that. Also any others that you may not actually need
to be converted.

Also, seems I was misremembering how PPC dealt with this - they
indeed had xen_ulong_t defined to be a 64-bit value too.

> Considering that each field of a multicall_entry is usually passed as an
> hypercall parameter, they should all remain unsigned long.

That'll give you subtle bugs I'm afraid: do_memory_op()'s
encoding of a continuation start extent (into the 'cmd' value),
for example, depends on being able to store the full value into
the command field of the multicall structure. The limit checking
of the permitted number of extents therefore is different
between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and
compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would
neither find it very appealing to have do_memory_op() adjusted
for dealing with this new special case, nor am I sure that's the
only place your approach would cause problems if you excluded
the multicall structure from the model change.

Jan

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Ian Campbell]

On Tue, 2012-08-07 at 13:54 +0100, Jan Beulich wrote:
> >>> On 07.08.12 at 14:08, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> > On Mon, 6 Aug 2012, Jan Beulich wrote:
> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>  wrote:
> >> > --- a/xen/include/public/physdev.h
> >> > +++ b/xen/include/public/physdev.h
> >> > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
> >> >  #define PHYSDEVOP_apic_write             9
> >> >  struct physdev_apic {
> >> >      /* IN */
> >> > -    unsigned long apic_physbase;
> >> > +    xen_ulong_t apic_physbase;
> >> >      uint32_t reg;
> >> >      /* IN or OUT */
> >> >      uint32_t value;
> >> >...
> > 
> > This change is actually not required, considering that ARM doesn't have
> > an APIC. I changed apic_physbase to xen_ulong_t only for consistency,
> > but it wouldn't make any difference for ARM (or x86).
> > If you think that it is better to keep it unsigned long, I'll remove
> > this chunk for the patch.
> 
> I'd prefer that. Also any others that you may not actually need
> to be converted.
> 
> Also, seems I was misremembering how PPC dealt with this - they
> indeed had xen_ulong_t defined to be a 64-bit value too.
> 
> > Considering that each field of a multicall_entry is usually passed as an
> > hypercall parameter, they should all remain unsigned long.
> 
> That'll give you subtle bugs I'm afraid: do_memory_op()'s
> encoding of a continuation start extent (into the 'cmd' value),
> for example, depends on being able to store the full value into
> the command field of the multicall structure. The limit checking
> of the permitted number of extents therefore is different
> between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and
> compat (UINT_MAX >> MEMOP_EXTENT_SHIFT).

On compat this would be 2^(32-6) == 67 and a bit million extents. I
think we can live with this limit on 64bit ARM too.

We could have lived with it on 64bit X86 too but by the time we noticed
it was too late so we have to live with it.

>  I would
> neither find it very appealing to have do_memory_op() adjusted
> for dealing with this new special case, nor am I sure that's the
> only place your approach would cause problems if you excluded
> the multicall structure from the model change.

On the other hand it doesn't seem right for the multicall args to be
able to represent values which you couldn't pass to the regular
hypercall itself (since the 32 bit args are only 32 bit).

Perhaps if use of the upper portions for 32 bit guests were restricted
for the use of continuations only that might work -- it would rely on
the top half of the 64-bit x0 register surviving a trip back to 32 bit
mode and into hypervisor mode again. I think that can be expected (but
I'd have to double check a AArch64 docs to be sure). On the other-other
hand that would mean that a 32-on-32 guest would see different things to
a 32-on-64 guest which is bad.

I don't expect we will be able to get away with no compat layer what so
ever. At the minimum there will have to be compat hypercall entry points
which correctly extend the 32 bit arguments to 64 bit, and do the same
for the return value etc but if we can avoid the majority of the struct
munging (ideally completely avoiding the need for a hypercall arguments
translation area) then I think that is worth doing.

Ian.

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Stefano Stabellini]

On Tue, 7 Aug 2012, Jan Beulich wrote:
> > Considering that each field of a multicall_entry is usually passed as an
> > hypercall parameter, they should all remain unsigned long.
> 
> That'll give you subtle bugs I'm afraid: do_memory_op()'s
> encoding of a continuation start extent (into the 'cmd' value),
> for example, depends on being able to store the full value into
> the command field of the multicall structure. The limit checking
> of the permitted number of extents therefore is different
> between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and
> compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would
> neither find it very appealing to have do_memory_op() adjusted
> for dealing with this new special case, nor am I sure that's the
> only place your approach would cause problems if you excluded
> the multicall structure from the model change.

Given the way the continuation is implemented, the same problem can
also happen on x86.
In fact, considering that we don't use any compat code, and that
do_memory_op has the following check:

    /* Is size too large for us to encode a continuation? */
    if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) )
        return start_extent;

it would work as-is for ARM too.

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Ian Campbell]

On Wed, 2012-08-08 at 13:12 +0100, Stefano Stabellini wrote:
> On Tue, 7 Aug 2012, Jan Beulich wrote:
> > > Considering that each field of a multicall_entry is usually passed as an
> > > hypercall parameter, they should all remain unsigned long.
> > 
> > That'll give you subtle bugs I'm afraid: do_memory_op()'s
> > encoding of a continuation start extent (into the 'cmd' value),
> > for example, depends on being able to store the full value into
> > the command field of the multicall structure. The limit checking
> > of the permitted number of extents therefore is different
> > between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and
> > compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would
> > neither find it very appealing to have do_memory_op() adjusted
> > for dealing with this new special case, nor am I sure that's the
> > only place your approach would cause problems if you excluded
> > the multicall structure from the model change.
> 
> Given the way the continuation is implemented, the same problem can
> also happen on x86.
> In fact, considering that we don't use any compat code, and that
> do_memory_op has the following check:
> 
>     /* Is size too large for us to encode a continuation? */
>     if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) )
>         return start_extent;
> 
> it would work as-is for ARM too.

For 32-on-32 ARM it'll work. It'll need changing for 32-on-64 ARM
though. We might even consider making the limit the smaller number even
for 64-on-64 ARM.

We should make the limit MEMOP_MAX_EXTENTS and allow it to be per-arch,
on x86 it'll remain ULONG_MAX >> MEMOP_EXTENT_SHIFT.

Ian.

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Jan Beulich]

>>> On 08.08.12 at 14:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> On Tue, 7 Aug 2012, Jan Beulich wrote:
>> > Considering that each field of a multicall_entry is usually passed as an
>> > hypercall parameter, they should all remain unsigned long.
>> 
>> That'll give you subtle bugs I'm afraid: do_memory_op()'s
>> encoding of a continuation start extent (into the 'cmd' value),
>> for example, depends on being able to store the full value into
>> the command field of the multicall structure. The limit checking
>> of the permitted number of extents therefore is different
>> between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and
>> compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would
>> neither find it very appealing to have do_memory_op() adjusted
>> for dealing with this new special case, nor am I sure that's the
>> only place your approach would cause problems if you excluded
>> the multicall structure from the model change.
> 
> Given the way the continuation is implemented, the same problem can
> also happen on x86.

No. The compat wrapper, as pointed out there has a different
check on the maximum number of extents, and hence the
continuation index can't overflow.

> In fact, considering that we don't use any compat code, and that
> do_memory_op has the following check:
> 
>     /* Is size too large for us to encode a continuation? */
>     if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) )
>         return start_extent;
> 
> it would work as-is for ARM too.

Not afaict. For a 32-bit guest, but the above code executed in
a 64-bit hypervisor, the guest could pass in (theoretically)
UINT_MAX, which would pass this check, yet the eventual
continuation index would get truncated when stored in the
32-bit hypercall operation field.

Jan

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Stefano Stabellini]

On Wed, 8 Aug 2012, Jan Beulich wrote:
> >>> On 08.08.12 at 14:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> wrote:
> > On Tue, 7 Aug 2012, Jan Beulich wrote:
> >> > Considering that each field of a multicall_entry is usually passed as an
> >> > hypercall parameter, they should all remain unsigned long.
> >> 
> >> That'll give you subtle bugs I'm afraid: do_memory_op()'s
> >> encoding of a continuation start extent (into the 'cmd' value),
> >> for example, depends on being able to store the full value into
> >> the command field of the multicall structure. The limit checking
> >> of the permitted number of extents therefore is different
> >> between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and
> >> compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would
> >> neither find it very appealing to have do_memory_op() adjusted
> >> for dealing with this new special case, nor am I sure that's the
> >> only place your approach would cause problems if you excluded
> >> the multicall structure from the model change.
> > 
> > Given the way the continuation is implemented, the same problem can
> > also happen on x86.
> 
> No. The compat wrapper, as pointed out there has a different
> check on the maximum number of extents, and hence the
> continuation index can't overflow.

Right. I meant the same conceptual problem: the guest passing a number of
extents that is too big for the hypervisor to handle.


> > In fact, considering that we don't use any compat code, and that
> > do_memory_op has the following check:
> > 
> >     /* Is size too large for us to encode a continuation? */
> >     if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) )
> >         return start_extent;
> > 
> > it would work as-is for ARM too.
> 
> Not afaict. For a 32-bit guest, but the above code executed in
> a 64-bit hypervisor, the guest could pass in (theoretically)
> UINT_MAX, which would pass this check, yet the eventual
> continuation index would get truncated when stored in the
> 32-bit hypercall operation field.
 
Actually, like Ian wrote, I expect that using the upper 32 bit part of
the x0 register, only for continuations, would work just fine.

In any case we can make that check arch dependent and restrict the
maximum number of extents on aarch64 to the same limit we have on
aarch32.

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Jan Beulich]

>>> On 08.08.12 at 17:01, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> On Wed, 8 Aug 2012, Jan Beulich wrote:
>> >>> On 08.08.12 at 14:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> wrote:
>> > On Tue, 7 Aug 2012, Jan Beulich wrote:
>> >> > Considering that each field of a multicall_entry is usually passed as an
>> >> > hypercall parameter, they should all remain unsigned long.
>> >> 
>> >> That'll give you subtle bugs I'm afraid: do_memory_op()'s
>> >> encoding of a continuation start extent (into the 'cmd' value),
>> >> for example, depends on being able to store the full value into
>> >> the command field of the multicall structure. The limit checking
>> >> of the permitted number of extents therefore is different
>> >> between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and
>> >> compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would
>> >> neither find it very appealing to have do_memory_op() adjusted
>> >> for dealing with this new special case, nor am I sure that's the
>> >> only place your approach would cause problems if you excluded
>> >> the multicall structure from the model change.
>> > 
>> > Given the way the continuation is implemented, the same problem can
>> > also happen on x86.
>> 
>> No. The compat wrapper, as pointed out there has a different
>> check on the maximum number of extents, and hence the
>> continuation index can't overflow.
> 
> Right. I meant the same conceptual problem: the guest passing a number of
> extents that is too big for the hypervisor to handle.

I don't think the hypervisor has a problem handling such large
amounts.

>> > In fact, considering that we don't use any compat code, and that
>> > do_memory_op has the following check:
>> > 
>> >     /* Is size too large for us to encode a continuation? */
>> >     if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) )
>> >         return start_extent;
>> > 
>> > it would work as-is for ARM too.
>> 
>> Not afaict. For a 32-bit guest, but the above code executed in
>> a 64-bit hypervisor, the guest could pass in (theoretically)
>> UINT_MAX, which would pass this check, yet the eventual
>> continuation index would get truncated when stored in the
>> 32-bit hypercall operation field.
>  
> Actually, like Ian wrote, I expect that using the upper 32 bit part of
> the x0 register, only for continuations, would work just fine.
> 
> In any case we can make that check arch dependent and restrict the
> maximum number of extents on aarch64 to the same limit we have on
> aarch32.

We should even discuss lowering the limit universally to the
UINT_MAX based value. I don't think anyone is actively using
such insane numbers.

And don't forget that I pointed out this issue only as example
of possible problems with your intended approach to the
handling of multicalls.

Jan

Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions

[Stefano Stabellini]

> >> > do_memory_op has the following check:
> >> > 
> >> >     /* Is size too large for us to encode a continuation? */
> >> >     if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) )
> >> >         return start_extent;
> >> > 
> >> > it would work as-is for ARM too.
> >> 
> >> Not afaict. For a 32-bit guest, but the above code executed in
> >> a 64-bit hypervisor, the guest could pass in (theoretically)
> >> UINT_MAX, which would pass this check, yet the eventual
> >> continuation index would get truncated when stored in the
> >> 32-bit hypercall operation field.
> >  
> > Actually, like Ian wrote, I expect that using the upper 32 bit part of
> > the x0 register, only for continuations, would work just fine.
> > 
> > In any case we can make that check arch dependent and restrict the
> > maximum number of extents on aarch64 to the same limit we have on
> > aarch32.
> 
> We should even discuss lowering the limit universally to the
> UINT_MAX based value. I don't think anyone is actively using
> such insane numbers.

I am OK with that, it would make things easier for me.


> And don't forget that I pointed out this issue only as example
> of possible problems with your intended approach to the
> handling of multicalls.

I just went through the hypercall continuations in common code, and
do_memory_op is the only one that uses this "trick" of encoding the
start extent in another parameter.
I think we'll have to deal with these issues as we find them out.

[PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

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.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 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

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 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@eu.citrix.com>
> ---
>  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@lists.xen.org 
> http://lists.xen.org/xen-devel 

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Ian Campbell]

On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote:
> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 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

Well, on x86 we have the compat XLAT layer, which is a pretty complex
piece of code, so "got away without" is a bit strong... We'd really
rather not have to have a non-trivial compat layer on arm too by having
the struct layouts be the same on 32/64.

Ian.

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote:
>> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
> 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
> 
> Well, on x86 we have the compat XLAT layer, which is a pretty complex
> piece of code, so "got away without" is a bit strong...

Hmm, yes, that's a valid correction.

> We'd really
> rather not have to have a non-trivial compat layer on arm too by having
> the struct layouts be the same on 32/64.

And paying a penalty like this in the 32-bit half (if what is likely
to remain the much bigger portion for the next couple of years
can validly be called "half") is worth it? The more that the compat
layer is now reasonably mature (and should hence be easily
re-usable for ARM)?

Jan

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

On Mon, 6 Aug 2012, Jan Beulich wrote:
> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote:
> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
> > 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
> > 
> > Well, on x86 we have the compat XLAT layer, which is a pretty complex
> > piece of code, so "got away without" is a bit strong...
> 
> Hmm, yes, that's a valid correction.
> 
> > We'd really
> > rather not have to have a non-trivial compat layer on arm too by having
> > the struct layouts be the same on 32/64.
> 
> And paying a penalty like this in the 32-bit half (if what is likely
> to remain the much bigger portion for the next couple of years
> can validly be called "half") is worth it? The more that the compat
> layer is now reasonably mature (and should hence be easily
> re-usable for ARM)?

What penalty? The only penalty is the wasted space in the structs in
memory.

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 06.08.12 at 18:02, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote:
>> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
>> > 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
>> > 
>> > Well, on x86 we have the compat XLAT layer, which is a pretty complex
>> > piece of code, so "got away without" is a bit strong...
>> 
>> Hmm, yes, that's a valid correction.
>> 
>> > We'd really
>> > rather not have to have a non-trivial compat layer on arm too by having
>> > the struct layouts be the same on 32/64.
>> 
>> And paying a penalty like this in the 32-bit half (if what is likely
>> to remain the much bigger portion for the next couple of years
>> can validly be called "half") is worth it? The more that the compat
>> layer is now reasonably mature (and should hence be easily
>> re-usable for ARM)?
> 
> What penalty? The only penalty is the wasted space in the structs in
> memory.

No - the caller has to zero-initialize those extra 32 bits, and the
hypervisor has to check for them to be zero (the latter may be
implicit in the 64-bit one, but certainly needs to be explicit on the
32-bit side).

Jan

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

On Tue, 7 Aug 2012, Jan Beulich wrote:
> >>> On 06.08.12 at 18:02, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> wrote:
> > On Mon, 6 Aug 2012, Jan Beulich wrote:
> >> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote:
> >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
> >> > 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
> >> > 
> >> > Well, on x86 we have the compat XLAT layer, which is a pretty complex
> >> > piece of code, so "got away without" is a bit strong...
> >> 
> >> Hmm, yes, that's a valid correction.
> >> 
> >> > We'd really
> >> > rather not have to have a non-trivial compat layer on arm too by having
> >> > the struct layouts be the same on 32/64.
> >> 
> >> And paying a penalty like this in the 32-bit half (if what is likely
> >> to remain the much bigger portion for the next couple of years
> >> can validly be called "half") is worth it? The more that the compat
> >> layer is now reasonably mature (and should hence be easily
> >> re-usable for ARM)?
> > 
> > What penalty? The only penalty is the wasted space in the structs in
> > memory.
> 
> No - the caller has to zero-initialize those extra 32 bits, and the
> hypervisor has to check for them to be zero (the latter may be
> implicit in the 64-bit one, but certainly needs to be explicit on the
> 32-bit side).

You are saying that on a 32 bit hypervisor we should check that the
padding is zero? Why should we care about the value of the padding?

In any case fortunately accesses to guest_handles already go via macros,
so it should be easy to arrange if it comes down to it.

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Ian Campbell]

On Tue, 2012-08-07 at 13:35 +0100, Stefano Stabellini wrote:
> On Tue, 7 Aug 2012, Jan Beulich wrote:
> > >>> On 06.08.12 at 18:02, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > wrote:
> > > On Mon, 6 Aug 2012, Jan Beulich wrote:
> > >> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > >> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote:
> > >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
> > >> > 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
> > >> > 
> > >> > Well, on x86 we have the compat XLAT layer, which is a pretty complex
> > >> > piece of code, so "got away without" is a bit strong...
> > >> 
> > >> Hmm, yes, that's a valid correction.
> > >> 
> > >> > We'd really
> > >> > rather not have to have a non-trivial compat layer on arm too by having
> > >> > the struct layouts be the same on 32/64.
> > >> 
> > >> And paying a penalty like this in the 32-bit half (if what is likely
> > >> to remain the much bigger portion for the next couple of years
> > >> can validly be called "half") is worth it? The more that the compat
> > >> layer is now reasonably mature (and should hence be easily
> > >> re-usable for ARM)?
> > > 
> > > What penalty? The only penalty is the wasted space in the structs in
> > > memory.
> > 
> > No - the caller has to zero-initialize those extra 32 bits, and the
> > hypervisor has to check for them to be zero (the latter may be
> > implicit in the 64-bit one, but certainly needs to be explicit on the
> > 32-bit side).
> 
> You are saying that on a 32 bit hypervisor we should check that the
> padding is zero? Why should we care about the value of the padding?

The point is so that we can treat them as 64 bit values in a 64 bit
hypervisor, otherwise we would need a compat layer to translate (which
is what we want to avoid).

So the 32 bit guest definitely does need to zero them, and a debug build
of the 32 bit hypervisor probably ought to reject non-zero upper halves,
otherwise the chances of the guest doing it consistently is pretty
small.


> In any case fortunately accesses to guest_handles already go via macros,
> so it should be easy to arrange if it comes down to it.

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 07.08.12 at 14:35, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> On Tue, 7 Aug 2012, Jan Beulich wrote:
>> >>> On 06.08.12 at 18:02, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> wrote:
>> > On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> >> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote:
>> >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
> 
>> >> > 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
>> >> > 
>> >> > Well, on x86 we have the compat XLAT layer, which is a pretty complex
>> >> > piece of code, so "got away without" is a bit strong...
>> >> 
>> >> Hmm, yes, that's a valid correction.
>> >> 
>> >> > We'd really
>> >> > rather not have to have a non-trivial compat layer on arm too by having
>> >> > the struct layouts be the same on 32/64.
>> >> 
>> >> And paying a penalty like this in the 32-bit half (if what is likely
>> >> to remain the much bigger portion for the next couple of years
>> >> can validly be called "half") is worth it? The more that the compat
>> >> layer is now reasonably mature (and should hence be easily
>> >> re-usable for ARM)?
>> > 
>> > What penalty? The only penalty is the wasted space in the structs in
>> > memory.
>> 
>> No - the caller has to zero-initialize those extra 32 bits, and the
>> hypervisor has to check for them to be zero (the latter may be
>> implicit in the 64-bit one, but certainly needs to be explicit on the
>> 32-bit side).
> 
> You are saying that on a 32 bit hypervisor we should check that the
> padding is zero? Why should we care about the value of the padding?

Because otherwise the same 32-bit guest kernel's behavior on
32- and 64-bit hypervisor may unexpectedly differ. And even if
you didn't care to do the check, the guest would still be required
to put zeros there in order to run on a 64-bit hypervisor. (And
of course this costs you cache and TLB bandwidth. See how
x86-64 just recently got the ILP32 model [aka x32] added for
this very reason.)

Jan

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

On Tue, 7 Aug 2012, Jan Beulich wrote:
> >>> On 07.08.12 at 14:35, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> wrote:
> > On Tue, 7 Aug 2012, Jan Beulich wrote:
> >> >>> On 06.08.12 at 18:02, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >> wrote:
> >> > On Mon, 6 Aug 2012, Jan Beulich wrote:
> >> >> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> >> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote:
> >> >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> 
> > 
> >> >> > 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
> >> >> > 
> >> >> > Well, on x86 we have the compat XLAT layer, which is a pretty complex
> >> >> > piece of code, so "got away without" is a bit strong...
> >> >> 
> >> >> Hmm, yes, that's a valid correction.
> >> >> 
> >> >> > We'd really
> >> >> > rather not have to have a non-trivial compat layer on arm too by having
> >> >> > the struct layouts be the same on 32/64.
> >> >> 
> >> >> And paying a penalty like this in the 32-bit half (if what is likely
> >> >> to remain the much bigger portion for the next couple of years
> >> >> can validly be called "half") is worth it? The more that the compat
> >> >> layer is now reasonably mature (and should hence be easily
> >> >> re-usable for ARM)?
> >> > 
> >> > What penalty? The only penalty is the wasted space in the structs in
> >> > memory.
> >> 
> >> No - the caller has to zero-initialize those extra 32 bits, and the
> >> hypervisor has to check for them to be zero (the latter may be
> >> implicit in the 64-bit one, but certainly needs to be explicit on the
> >> 32-bit side).
> > 
> > You are saying that on a 32 bit hypervisor we should check that the
> > padding is zero? Why should we care about the value of the padding?
> 
> Because otherwise the same 32-bit guest kernel's behavior on
> 32- and 64-bit hypervisor may unexpectedly differ. And even if
> you didn't care to do the check, the guest would still be required
> to put zeros there in order to run on a 64-bit hypervisor. (And
> of course this costs you cache and TLB bandwidth. See how
> x86-64 just recently got the ILP32 model [aka x32] added for
> this very reason.)

Considering that no MMU hypercalls are required on ARM and that none of
the grant table and event channel ops on the hot path have any guest
handles, I think we are going to be fine.

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Ian Campbell]

On Tue, 2012-08-07 at 14:08 +0100, Jan Beulich wrote:
> See how
> x86-64 just recently got the ILP32 model [aka x32] added for
> this very reason.)

For userspace only though, with the kernel remaining a full 64-bit
kernel. I don't know what direction they eventually took but there was
also a contingent in favour of using the 64-bit syscall ABI for x32.

I don't think that the wastage at the hypercall interface is going to be
significant. Especially once you consider that many of the 32-bit fields
actually need to be 64-bit for a compat mode guest any, e.g. MFNs should
certainly be 64 bit regardless or else you impose limitations on 32-bit
guests which the 64-bit hypervisor doesn't itself have.

Ian.

Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 08.08.12 at 09:48, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> I don't think that the wastage at the hypercall interface is going to be
> significant. Especially once you consider that many of the 32-bit fields
> actually need to be 64-bit for a compat mode guest any, e.g. MFNs should
> certainly be 64 bit regardless or else you impose limitations on 32-bit
> guests which the 64-bit hypervisor doesn't itself have.

Hmm, MFNs are probably a bad example, because we already have
a separate xen_pfn_t to deal with precisely that case.

Jan

[PATCH 5/5] xen: replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when appropriate

[Stefano Stabellini]

Note: these changes don't make any difference on x86 and ia64.


Replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when it is used as
an hypercall argument.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain.c              |    2 +-
 xen/arch/arm/domctl.c              |    2 +-
 xen/arch/arm/hvm.c                 |    2 +-
 xen/arch/arm/mm.c                  |    2 +-
 xen/arch/arm/physdev.c             |    2 +-
 xen/arch/arm/sysctl.c              |    2 +-
 xen/arch/x86/cpu/mcheck/mce.c      |    2 +-
 xen/arch/x86/domain.c              |    2 +-
 xen/arch/x86/domctl.c              |    2 +-
 xen/arch/x86/efi/runtime.c         |    2 +-
 xen/arch/x86/hvm/hvm.c             |   26 +++++++++---------
 xen/arch/x86/microcode.c           |    2 +-
 xen/arch/x86/mm.c                  |   14 +++++-----
 xen/arch/x86/mm/hap/hap.c          |    2 +-
 xen/arch/x86/mm/mem_event.c        |    2 +-
 xen/arch/x86/mm/paging.c           |    2 +-
 xen/arch/x86/mm/shadow/common.c    |    2 +-
 xen/arch/x86/physdev.c             |    2 +-
 xen/arch/x86/platform_hypercall.c  |    2 +-
 xen/arch/x86/sysctl.c              |    2 +-
 xen/arch/x86/traps.c               |    2 +-
 xen/arch/x86/x86_32/mm.c           |    2 +-
 xen/arch/x86/x86_32/traps.c        |    2 +-
 xen/arch/x86/x86_64/compat/mm.c    |    8 +++---
 xen/arch/x86/x86_64/domain.c       |    2 +-
 xen/arch/x86/x86_64/mm.c           |    2 +-
 xen/arch/x86/x86_64/traps.c        |    2 +-
 xen/common/compat/domain.c         |    2 +-
 xen/common/compat/grant_table.c    |    2 +-
 xen/common/compat/memory.c         |    2 +-
 xen/common/domain.c                |    2 +-
 xen/common/domctl.c                |    2 +-
 xen/common/event_channel.c         |    2 +-
 xen/common/grant_table.c           |   36 ++++++++++++------------
 xen/common/kernel.c                |    4 +-
 xen/common/kexec.c                 |   16 +++++-----
 xen/common/memory.c                |    4 +-
 xen/common/multicall.c             |    2 +-
 xen/common/schedule.c              |    2 +-
 xen/common/sysctl.c                |    2 +-
 xen/common/xenoprof.c              |    8 +++---
 xen/drivers/acpi/pmstat.c          |    2 +-
 xen/drivers/char/console.c         |    6 ++--
 xen/drivers/passthrough/iommu.c    |    2 +-
 xen/include/asm-arm/guest_access.h |    2 +-
 xen/include/asm-arm/hypercall.h    |    2 +-
 xen/include/asm-arm/mm.h           |    2 +-
 xen/include/asm-x86/hap.h          |    2 +-
 xen/include/asm-x86/hypercall.h    |   24 ++++++++--------
 xen/include/asm-x86/mem_event.h    |    2 +-
 xen/include/asm-x86/mm.h           |    8 +++---
 xen/include/asm-x86/paging.h       |    2 +-
 xen/include/asm-x86/processor.h    |    2 +-
 xen/include/asm-x86/shadow.h       |    2 +-
 xen/include/asm-x86/xenoprof.h     |    6 ++--
 xen/include/xen/acpi.h             |    4 +-
 xen/include/xen/hypercall.h        |   52 ++++++++++++++++++------------------
 xen/include/xen/iommu.h            |    2 +-
 xen/include/xen/tmem_xen.h         |    2 +-
 xen/include/xsm/xsm.h              |    4 +-
 xen/xsm/dummy.c                    |    2 +-
 xen/xsm/flask/flask_op.c           |    4 +-
 xen/xsm/flask/hooks.c              |    2 +-
 xen/xsm/xsm_core.c                 |    2 +-
 64 files changed, 160 insertions(+), 160 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index ee58d68..07b50e2 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -515,7 +515,7 @@ void arch_dump_domain_info(struct domain *d)
 {
 }
 
-long arch_do_vcpu_op(int cmd, struct vcpu *v, XEN_GUEST_HANDLE(void) arg)
+long arch_do_vcpu_op(int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     return -ENOSYS;
 }
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 1a5f79f..cf16791 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -11,7 +11,7 @@
 #include <public/domctl.h>
 
 long arch_do_domctl(struct xen_domctl *domctl,
-                    XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
+                    XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 {
     return -ENOSYS;
 }
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index c11378d..40f519e 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -11,7 +11,7 @@
 
 #include <asm/hypercall.h>
 
-long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg)
+long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 
 {
     long rc = 0;
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 08bc55b..c9cc59f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -541,7 +541,7 @@ static int xenmem_add_to_physmap(struct domain *d,
     return xenmem_add_to_physmap_once(d, xatp);
 }
 
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     int rc;
 
diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
index bcf4337..0801e8c 100644
--- a/xen/arch/arm/physdev.c
+++ b/xen/arch/arm/physdev.c
@@ -11,7 +11,7 @@
 #include <asm/hypercall.h>
 
 
-int do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     printk("%s %d cmd=%d: not implemented yet\n", __func__, __LINE__, cmd);
     return -ENOSYS;
diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
index e8e1c0d..a286abe 100644
--- a/xen/arch/arm/sysctl.c
+++ b/xen/arch/arm/sysctl.c
@@ -13,7 +13,7 @@
 #include <public/sysctl.h>
 
 long arch_do_sysctl(struct xen_sysctl *sysctl,
-                    XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl)
+                    XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
 {
     return -ENOSYS;
 }
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index ed76131..4b2e0c7 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1359,7 +1359,7 @@ CHECK_mcinfo_recovery;
 #endif
 
 /* Machine Check Architecture Hypercall */
-long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc)
+long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
 {
     long ret = 0;
     struct xen_mc curop, *op = &curop;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 5bba4b9..13ff776 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1138,7 +1138,7 @@ map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset)
 
 long
 arch_do_vcpu_op(
-    int cmd, struct vcpu *v, XEN_GUEST_HANDLE(void) arg)
+    int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     long rc = 0;
 
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 135ea6e..663bfe4 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -48,7 +48,7 @@ static int gdbsx_guest_mem_io(
 
 long arch_do_domctl(
     struct xen_domctl *domctl,
-    XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
+    XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 {
     long ret = 0;
 
diff --git a/xen/arch/x86/efi/runtime.c b/xen/arch/x86/efi/runtime.c
index 1dbe2db..b2ff495 100644
--- a/xen/arch/x86/efi/runtime.c
+++ b/xen/arch/x86/efi/runtime.c
@@ -184,7 +184,7 @@ int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
     return 0;
 }
 
-static long gwstrlen(XEN_GUEST_HANDLE(CHAR16) str)
+static long gwstrlen(XEN_GUEST_HANDLE_PARAM(CHAR16) str)
 {
     unsigned long len;
 
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 22c136b..bf97aea 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3041,14 +3041,14 @@ static int grant_table_op_is_allowed(unsigned int cmd)
 }
 
 static long hvm_grant_table_op(
-    unsigned int cmd, XEN_GUEST_HANDLE(void) uop, unsigned int count)
+    unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
 {
     if ( !grant_table_op_is_allowed(cmd) )
         return -ENOSYS; /* all other commands need auditing */
     return do_grant_table_op(cmd, uop, count);
 }
 
-static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     long rc;
 
@@ -3066,7 +3066,7 @@ static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE(void) arg)
     return do_memory_op(cmd, arg);
 }
 
-static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     switch ( cmd )
     {
@@ -3082,7 +3082,7 @@ static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
 }
 
 static long hvm_vcpu_op(
-    int cmd, int vcpuid, XEN_GUEST_HANDLE(void) arg)
+    int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     long rc;
 
@@ -3131,7 +3131,7 @@ static hvm_hypercall_t *hvm_hypercall32_table[NR_hypercalls] = {
 #else /* defined(__x86_64__) */
 
 static long hvm_grant_table_op_compat32(unsigned int cmd,
-                                        XEN_GUEST_HANDLE(void) uop,
+                                        XEN_GUEST_HANDLE_PARAM(void) uop,
                                         unsigned int count)
 {
     if ( !grant_table_op_is_allowed(cmd) )
@@ -3139,7 +3139,7 @@ static long hvm_grant_table_op_compat32(unsigned int cmd,
     return compat_grant_table_op(cmd, uop, count);
 }
 
-static long hvm_memory_op_compat32(int cmd, XEN_GUEST_HANDLE(void) arg)
+static long hvm_memory_op_compat32(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     int rc;
 
@@ -3158,7 +3158,7 @@ static long hvm_memory_op_compat32(int cmd, XEN_GUEST_HANDLE(void) arg)
 }
 
 static long hvm_vcpu_op_compat32(
-    int cmd, int vcpuid, XEN_GUEST_HANDLE(void) arg)
+    int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     long rc;
 
@@ -3182,7 +3182,7 @@ static long hvm_vcpu_op_compat32(
 }
 
 static long hvm_physdev_op_compat32(
-    int cmd, XEN_GUEST_HANDLE(void) arg)
+    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     switch ( cmd )
     {
@@ -3354,7 +3354,7 @@ void hvm_hypercall_page_initialise(struct domain *d,
 }
 
 static int hvmop_set_pci_intx_level(
-    XEN_GUEST_HANDLE(xen_hvm_set_pci_intx_level_t) uop)
+    XEN_GUEST_HANDLE_PARAM(xen_hvm_set_pci_intx_level_t) uop)
 {
     struct xen_hvm_set_pci_intx_level op;
     struct domain *d;
@@ -3519,7 +3519,7 @@ static void hvm_s3_resume(struct domain *d)
 }
 
 static int hvmop_set_isa_irq_level(
-    XEN_GUEST_HANDLE(xen_hvm_set_isa_irq_level_t) uop)
+    XEN_GUEST_HANDLE_PARAM(xen_hvm_set_isa_irq_level_t) uop)
 {
     struct xen_hvm_set_isa_irq_level op;
     struct domain *d;
@@ -3563,7 +3563,7 @@ static int hvmop_set_isa_irq_level(
 }
 
 static int hvmop_set_pci_link_route(
-    XEN_GUEST_HANDLE(xen_hvm_set_pci_link_route_t) uop)
+    XEN_GUEST_HANDLE_PARAM(xen_hvm_set_pci_link_route_t) uop)
 {
     struct xen_hvm_set_pci_link_route op;
     struct domain *d;
@@ -3596,7 +3596,7 @@ static int hvmop_set_pci_link_route(
 }
 
 static int hvmop_inject_msi(
-    XEN_GUEST_HANDLE(xen_hvm_inject_msi_t) uop)
+    XEN_GUEST_HANDLE_PARAM(xen_hvm_inject_msi_t) uop)
 {
     struct xen_hvm_inject_msi op;
     struct domain *d;
@@ -3680,7 +3680,7 @@ static int hvm_replace_event_channel(struct vcpu *v, domid_t remote_domid,
     return 0;
 }
 
-long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg)
+long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 
 {
     struct domain *curr_d = current->domain;
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index bdda3f5..1477481 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -192,7 +192,7 @@ static long do_microcode_update(void *_info)
     return error;
 }
 
-int microcode_update(XEN_GUEST_HANDLE(const_void) buf, unsigned long len)
+int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
 {
     int ret;
     struct microcode_info *info;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 9f63974..fd1c890 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2914,7 +2914,7 @@ static void put_pg_owner(struct domain *pg_owner)
 }
 
 static inline int vcpumask_to_pcpumask(
-    struct domain *d, XEN_GUEST_HANDLE(const_void) bmap, cpumask_t *pmask)
+    struct domain *d, XEN_GUEST_HANDLE_PARAM(const_void) bmap, cpumask_t *pmask)
 {
     unsigned int vcpu_id, vcpu_bias, offs;
     unsigned long vmask;
@@ -2974,9 +2974,9 @@ static inline void fixunmap_domain_page(const void *ptr)
 #endif
 
 int do_mmuext_op(
-    XEN_GUEST_HANDLE(mmuext_op_t) uops,
+    XEN_GUEST_HANDLE_PARAM(mmuext_op_t) uops,
     unsigned int count,
-    XEN_GUEST_HANDLE(uint) pdone,
+    XEN_GUEST_HANDLE_PARAM(uint) pdone,
     unsigned int foreigndom)
 {
     struct mmuext_op op;
@@ -3438,9 +3438,9 @@ int do_mmuext_op(
 }
 
 int do_mmu_update(
-    XEN_GUEST_HANDLE(mmu_update_t) ureqs,
+    XEN_GUEST_HANDLE_PARAM(mmu_update_t) ureqs,
     unsigned int count,
-    XEN_GUEST_HANDLE(uint) pdone,
+    XEN_GUEST_HANDLE_PARAM(uint) pdone,
     unsigned int foreigndom)
 {
     struct mmu_update req;
@@ -4387,7 +4387,7 @@ long set_gdt(struct vcpu *v,
 }
 
 
-long do_set_gdt(XEN_GUEST_HANDLE(ulong) frame_list, unsigned int entries)
+long do_set_gdt(XEN_GUEST_HANDLE_PARAM(ulong) frame_list, unsigned int entries)
 {
     int nr_pages = (entries + 511) / 512;
     unsigned long frames[16];
@@ -4661,7 +4661,7 @@ static int xenmem_add_to_physmap(struct domain *d,
     return xenmem_add_to_physmap_once(d, xatp);
 }
 
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     int rc;
 
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 13b4be2..67e48a3 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -690,7 +690,7 @@ void hap_teardown(struct domain *d)
 }
 
 int hap_domctl(struct domain *d, xen_domctl_shadow_op_t *sc,
-               XEN_GUEST_HANDLE(void) u_domctl)
+               XEN_GUEST_HANDLE_PARAM(void) u_domctl)
 {
     int rc, preempted = 0;
 
diff --git a/xen/arch/x86/mm/mem_event.c b/xen/arch/x86/mm/mem_event.c
index d728889..d3dac14 100644
--- a/xen/arch/x86/mm/mem_event.c
+++ b/xen/arch/x86/mm/mem_event.c
@@ -512,7 +512,7 @@ void mem_event_cleanup(struct domain *d)
 }
 
 int mem_event_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
-                     XEN_GUEST_HANDLE(void) u_domctl)
+                     XEN_GUEST_HANDLE_PARAM(void) u_domctl)
 {
     int rc;
 
diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index ca879f9..ea44e39 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -654,7 +654,7 @@ void paging_vcpu_init(struct vcpu *v)
 
 
 int paging_domctl(struct domain *d, xen_domctl_shadow_op_t *sc,
-                  XEN_GUEST_HANDLE(void) u_domctl)
+                  XEN_GUEST_HANDLE_PARAM(void) u_domctl)
 {
     int rc;
 
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index dc245be..bd47f03 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -3786,7 +3786,7 @@ out:
 
 int shadow_domctl(struct domain *d, 
                   xen_domctl_shadow_op_t *sc,
-                  XEN_GUEST_HANDLE(void) u_domctl)
+                  XEN_GUEST_HANDLE_PARAM(void) u_domctl)
 {
     int rc, preempted = 0;
 
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index b0458fd..b6474ef 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -255,7 +255,7 @@ int physdev_unmap_pirq(domid_t domid, int pirq)
 }
 #endif /* COMPAT */
 
-ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     int irq;
     ret_t ret;
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 88880b0..a32e0a2 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -60,7 +60,7 @@ long cpu_down_helper(void *data);
 long core_parking_helper(void *data);
 uint32_t get_cur_idle_nums(void);
 
-ret_t do_platform_op(XEN_GUEST_HANDLE(xen_platform_op_t) u_xenpf_op)
+ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
 {
     ret_t ret = 0;
     struct xen_platform_op curop, *op = &curop;
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 379f071..b84dd34 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -58,7 +58,7 @@ long cpu_down_helper(void *data)
 }
 
 long arch_do_sysctl(
-    struct xen_sysctl *sysctl, XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl)
+    struct xen_sysctl *sysctl, XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
 {
     long ret = 0;
 
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 767be86..281d9e7 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3700,7 +3700,7 @@ int send_guest_trap(struct domain *d, uint16_t vcpuid, unsigned int trap_nr)
 }
 
 
-long do_set_trap_table(XEN_GUEST_HANDLE(const_trap_info_t) traps)
+long do_set_trap_table(XEN_GUEST_HANDLE_PARAM(const_trap_info_t) traps)
 {
     struct trap_info cur;
     struct vcpu *curr = current;
diff --git a/xen/arch/x86/x86_32/mm.c b/xen/arch/x86/x86_32/mm.c
index 37efa3c..f6448fb 100644
--- a/xen/arch/x86/x86_32/mm.c
+++ b/xen/arch/x86/x86_32/mm.c
@@ -203,7 +203,7 @@ void __init subarch_init_memory(void)
     }
 }
 
-long subarch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+long subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct xen_machphys_mfn_list xmml;
     unsigned long mfn, last_mfn;
diff --git a/xen/arch/x86/x86_32/traps.c b/xen/arch/x86/x86_32/traps.c
index 8f68808..0c7c860 100644
--- a/xen/arch/x86/x86_32/traps.c
+++ b/xen/arch/x86/x86_32/traps.c
@@ -492,7 +492,7 @@ static long unregister_guest_callback(struct callback_unregister *unreg)
 }
 
 
-long do_callback_op(int cmd, XEN_GUEST_HANDLE(const_void) arg)
+long do_callback_op(int cmd, XEN_GUEST_HANDLE_PARAM(const_void) arg)
 {
     long ret;
 
diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c
index f497503..88a07e8 100644
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -5,7 +5,7 @@
 #include <asm/mem_event.h>
 #include <asm/mem_sharing.h>
 
-int compat_set_gdt(XEN_GUEST_HANDLE(uint) frame_list, unsigned int entries)
+int compat_set_gdt(XEN_GUEST_HANDLE_PARAM(uint) frame_list, unsigned int entries)
 {
     unsigned int i, nr_pages = (entries + 511) / 512;
     unsigned long frames[16];
@@ -44,7 +44,7 @@ int compat_update_descriptor(u32 pa_lo, u32 pa_hi, u32 desc_lo, u32 desc_hi)
                                 desc_lo | ((u64)desc_hi << 32));
 }
 
-int compat_arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+int compat_arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct compat_machphys_mfn_list xmml;
     l2_pgentry_t l2e;
@@ -260,9 +260,9 @@ int compat_update_va_mapping_otherdomain(unsigned long va, u32 lo, u32 hi,
 
 DEFINE_XEN_GUEST_HANDLE(mmuext_op_compat_t);
 
-int compat_mmuext_op(XEN_GUEST_HANDLE(mmuext_op_compat_t) cmp_uops,
+int compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t) cmp_uops,
                      unsigned int count,
-                     XEN_GUEST_HANDLE(uint) pdone,
+                     XEN_GUEST_HANDLE_PARAM(uint) pdone,
                      unsigned int foreigndom)
 {
     unsigned int i, preempt_mask;
diff --git a/xen/arch/x86/x86_64/domain.c b/xen/arch/x86/x86_64/domain.c
index e746c89..144ca2d 100644
--- a/xen/arch/x86/x86_64/domain.c
+++ b/xen/arch/x86/x86_64/domain.c
@@ -23,7 +23,7 @@ CHECK_vcpu_get_physid;
 
 int
 arch_compat_vcpu_op(
-    int cmd, struct vcpu *v, XEN_GUEST_HANDLE(void) arg)
+    int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     int rc = -ENOSYS;
 
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 635a499..17c46a1 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1043,7 +1043,7 @@ void __init subarch_init_memory(void)
     }
 }
 
-long subarch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+long subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct xen_machphys_mfn_list xmml;
     l3_pgentry_t l3e;
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 806cf2e..6ead813 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -518,7 +518,7 @@ static long unregister_guest_callback(struct callback_unregister *unreg)
 }
 
 
-long do_callback_op(int cmd, XEN_GUEST_HANDLE(const_void) arg)
+long do_callback_op(int cmd, XEN_GUEST_HANDLE_PARAM(const_void) arg)
 {
     long ret;
 
diff --git a/xen/common/compat/domain.c b/xen/common/compat/domain.c
index 40a0287..e4c8ceb 100644
--- a/xen/common/compat/domain.c
+++ b/xen/common/compat/domain.c
@@ -15,7 +15,7 @@
 CHECK_vcpu_set_periodic_timer;
 #undef xen_vcpu_set_periodic_timer
 
-int compat_vcpu_op(int cmd, int vcpuid, XEN_GUEST_HANDLE(void) arg)
+int compat_vcpu_op(int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct domain *d = current->domain;
     struct vcpu *v;
diff --git a/xen/common/compat/grant_table.c b/xen/common/compat/grant_table.c
index edd20c6..74a4733 100644
--- a/xen/common/compat/grant_table.c
+++ b/xen/common/compat/grant_table.c
@@ -52,7 +52,7 @@ CHECK_gnttab_swap_grant_ref;
 #undef xen_gnttab_swap_grant_ref
 
 int compat_grant_table_op(unsigned int cmd,
-                          XEN_GUEST_HANDLE(void) cmp_uop,
+                          XEN_GUEST_HANDLE_PARAM(void) cmp_uop,
                           unsigned int count)
 {
     int rc = 0;
diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
index e7257cc..8e311ff 100644
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -13,7 +13,7 @@ CHECK_TYPE(domid);
 #undef compat_domid_t
 #undef xen_domid_t
 
-int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE(void) compat)
+int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat)
 {
     int rc, split, op = cmd & MEMOP_CMD_MASK;
     unsigned int start_extent = cmd >> MEMOP_EXTENT_SHIFT;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 4c5d241..d7cd135 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -804,7 +804,7 @@ void vcpu_reset(struct vcpu *v)
 }
 
 
-long do_vcpu_op(int cmd, int vcpuid, XEN_GUEST_HANDLE(void) arg)
+long do_vcpu_op(int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct domain *d = current->domain;
     struct vcpu *v;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 7ca6b08..527c5ad 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -238,7 +238,7 @@ void domctl_lock_release(void)
     spin_unlock(&current->domain->hypercall_deadlock_mutex);
 }
 
-long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
+long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 {
     long ret = 0;
     struct xen_domctl curop, *op = &curop;
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 53777f8..a80a0d1 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -970,7 +970,7 @@ out:
 }
 
 
-long do_event_channel_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+long do_event_channel_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     long rc;
 
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 9961e83..d780dc6 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -771,7 +771,7 @@ __gnttab_map_grant_ref(
 
 static long
 gnttab_map_grant_ref(
-    XEN_GUEST_HANDLE(gnttab_map_grant_ref_t) uop, unsigned int count)
+    XEN_GUEST_HANDLE_PARAM(gnttab_map_grant_ref_t) uop, unsigned int count)
 {
     int i;
     struct gnttab_map_grant_ref op;
@@ -1040,7 +1040,7 @@ __gnttab_unmap_grant_ref(
 
 static long
 gnttab_unmap_grant_ref(
-    XEN_GUEST_HANDLE(gnttab_unmap_grant_ref_t) uop, unsigned int count)
+    XEN_GUEST_HANDLE_PARAM(gnttab_unmap_grant_ref_t) uop, unsigned int count)
 {
     int i, c, partial_done, done = 0;
     struct gnttab_unmap_grant_ref op;
@@ -1102,7 +1102,7 @@ __gnttab_unmap_and_replace(
 
 static long
 gnttab_unmap_and_replace(
-    XEN_GUEST_HANDLE(gnttab_unmap_and_replace_t) uop, unsigned int count)
+    XEN_GUEST_HANDLE_PARAM(gnttab_unmap_and_replace_t) uop, unsigned int count)
 {
     int i, c, partial_done, done = 0;
     struct gnttab_unmap_and_replace op;
@@ -1254,7 +1254,7 @@ active_alloc_failed:
 
 static long 
 gnttab_setup_table(
-    XEN_GUEST_HANDLE(gnttab_setup_table_t) uop, unsigned int count)
+    XEN_GUEST_HANDLE_PARAM(gnttab_setup_table_t) uop, unsigned int count)
 {
     struct gnttab_setup_table op;
     struct domain *d;
@@ -1348,7 +1348,7 @@ gnttab_setup_table(
 
 static long 
 gnttab_query_size(
-    XEN_GUEST_HANDLE(gnttab_query_size_t) uop, unsigned int count)
+    XEN_GUEST_HANDLE_PARAM(gnttab_query_size_t) uop, unsigned int count)
 {
     struct gnttab_query_size op;
     struct domain *d;
@@ -1485,7 +1485,7 @@ gnttab_prepare_for_transfer(
 
 static long
 gnttab_transfer(
-    XEN_GUEST_HANDLE(gnttab_transfer_t) uop, unsigned int count)
+    XEN_GUEST_HANDLE_PARAM(gnttab_transfer_t) uop, unsigned int count)
 {
     struct domain *d = current->domain;
     struct domain *e;
@@ -2082,7 +2082,7 @@ __gnttab_copy(
 
 static long
 gnttab_copy(
-    XEN_GUEST_HANDLE(gnttab_copy_t) uop, unsigned int count)
+    XEN_GUEST_HANDLE_PARAM(gnttab_copy_t) uop, unsigned int count)
 {
     int i;
     struct gnttab_copy op;
@@ -2101,7 +2101,7 @@ gnttab_copy(
 }
 
 static long
-gnttab_set_version(XEN_GUEST_HANDLE(gnttab_set_version_t uop))
+gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t uop))
 {
     gnttab_set_version_t op;
     struct domain *d = current->domain;
@@ -2220,7 +2220,7 @@ out:
 }
 
 static long
-gnttab_get_status_frames(XEN_GUEST_HANDLE(gnttab_get_status_frames_t) uop,
+gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop,
                          int count)
 {
     gnttab_get_status_frames_t op;
@@ -2289,7 +2289,7 @@ out1:
 }
 
 static long
-gnttab_get_version(XEN_GUEST_HANDLE(gnttab_get_version_t uop))
+gnttab_get_version(XEN_GUEST_HANDLE_PARAM(gnttab_get_version_t uop))
 {
     gnttab_get_version_t op;
     struct domain *d;
@@ -2368,7 +2368,7 @@ out:
 }
 
 static long
-gnttab_swap_grant_ref(XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t uop),
+gnttab_swap_grant_ref(XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t uop),
                       unsigned int count)
 {
     int i;
@@ -2389,7 +2389,7 @@ gnttab_swap_grant_ref(XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t uop),
 
 long
 do_grant_table_op(
-    unsigned int cmd, XEN_GUEST_HANDLE(void) uop, unsigned int count)
+    unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
 {
     long rc;
     
@@ -2401,7 +2401,7 @@ do_grant_table_op(
     {
     case GNTTABOP_map_grant_ref:
     {
-        XEN_GUEST_HANDLE(gnttab_map_grant_ref_t) map =
+        XEN_GUEST_HANDLE_PARAM(gnttab_map_grant_ref_t) map =
             guest_handle_cast(uop, gnttab_map_grant_ref_t);
         if ( unlikely(!guest_handle_okay(map, count)) )
             goto out;
@@ -2415,7 +2415,7 @@ do_grant_table_op(
     }
     case GNTTABOP_unmap_grant_ref:
     {
-        XEN_GUEST_HANDLE(gnttab_unmap_grant_ref_t) unmap =
+        XEN_GUEST_HANDLE_PARAM(gnttab_unmap_grant_ref_t) unmap =
             guest_handle_cast(uop, gnttab_unmap_grant_ref_t);
         if ( unlikely(!guest_handle_okay(unmap, count)) )
             goto out;
@@ -2429,7 +2429,7 @@ do_grant_table_op(
     }
     case GNTTABOP_unmap_and_replace:
     {
-        XEN_GUEST_HANDLE(gnttab_unmap_and_replace_t) unmap =
+        XEN_GUEST_HANDLE_PARAM(gnttab_unmap_and_replace_t) unmap =
             guest_handle_cast(uop, gnttab_unmap_and_replace_t);
         if ( unlikely(!guest_handle_okay(unmap, count)) )
             goto out;
@@ -2453,7 +2453,7 @@ do_grant_table_op(
     }
     case GNTTABOP_transfer:
     {
-        XEN_GUEST_HANDLE(gnttab_transfer_t) transfer =
+        XEN_GUEST_HANDLE_PARAM(gnttab_transfer_t) transfer =
             guest_handle_cast(uop, gnttab_transfer_t);
         if ( unlikely(!guest_handle_okay(transfer, count)) )
             goto out;
@@ -2467,7 +2467,7 @@ do_grant_table_op(
     }
     case GNTTABOP_copy:
     {
-        XEN_GUEST_HANDLE(gnttab_copy_t) copy =
+        XEN_GUEST_HANDLE_PARAM(gnttab_copy_t) copy =
             guest_handle_cast(uop, gnttab_copy_t);
         if ( unlikely(!guest_handle_okay(copy, count)) )
             goto out;
@@ -2504,7 +2504,7 @@ do_grant_table_op(
     }
     case GNTTABOP_swap_grant_ref:
     {
-        XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t) swap =
+        XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t) swap =
             guest_handle_cast(uop, gnttab_swap_grant_ref_t);
         if ( unlikely(!guest_handle_okay(swap, count)) )
             goto out;
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index c915bbc..55caff6 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -204,7 +204,7 @@ void __init do_initcalls(void)
  * Simple hypercalls.
  */
 
-DO(xen_version)(int cmd, XEN_GUEST_HANDLE(void) arg)
+DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     switch ( cmd )
     {
@@ -332,7 +332,7 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE(void) arg)
     return -ENOSYS;
 }
 
-DO(nmi_op)(unsigned int cmd, XEN_GUEST_HANDLE(void) arg)
+DO(nmi_op)(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct xennmi_callback cb;
     long rc = 0;
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 09a5624..03389eb 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -613,7 +613,7 @@ static int kexec_get_range_internal(xen_kexec_range_t *range)
     return ret;
 }
 
-static int kexec_get_range(XEN_GUEST_HANDLE(void) uarg)
+static int kexec_get_range(XEN_GUEST_HANDLE_PARAM(void) uarg)
 {
     xen_kexec_range_t range;
     int ret = -EINVAL;
@@ -629,7 +629,7 @@ static int kexec_get_range(XEN_GUEST_HANDLE(void) uarg)
     return ret;
 }
 
-static int kexec_get_range_compat(XEN_GUEST_HANDLE(void) uarg)
+static int kexec_get_range_compat(XEN_GUEST_HANDLE_PARAM(void) uarg)
 {
 #ifdef CONFIG_COMPAT
     xen_kexec_range_t range;
@@ -777,7 +777,7 @@ static int kexec_load_unload_internal(unsigned long op, xen_kexec_load_t *load)
     return ret;
 }
 
-static int kexec_load_unload(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
+static int kexec_load_unload(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) uarg)
 {
     xen_kexec_load_t load;
 
@@ -788,7 +788,7 @@ static int kexec_load_unload(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
 }
 
 static int kexec_load_unload_compat(unsigned long op,
-                                    XEN_GUEST_HANDLE(void) uarg)
+                                    XEN_GUEST_HANDLE_PARAM(void) uarg)
 {
 #ifdef CONFIG_COMPAT
     compat_kexec_load_t compat_load;
@@ -813,7 +813,7 @@ static int kexec_load_unload_compat(unsigned long op,
 #endif /* CONFIG_COMPAT */
 }
 
-static int kexec_exec(XEN_GUEST_HANDLE(void) uarg)
+static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
 {
     xen_kexec_exec_t exec;
     xen_kexec_image_t *image;
@@ -845,7 +845,7 @@ static int kexec_exec(XEN_GUEST_HANDLE(void) uarg)
     return -EINVAL; /* never reached */
 }
 
-int do_kexec_op_internal(unsigned long op, XEN_GUEST_HANDLE(void) uarg,
+int do_kexec_op_internal(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) uarg,
                            int compat)
 {
     unsigned long flags;
@@ -886,13 +886,13 @@ int do_kexec_op_internal(unsigned long op, XEN_GUEST_HANDLE(void) uarg,
     return ret;
 }
 
-long do_kexec_op(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
+long do_kexec_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) uarg)
 {
     return do_kexec_op_internal(op, uarg, 0);
 }
 
 #ifdef CONFIG_COMPAT
-int compat_kexec_op(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
+int compat_kexec_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) uarg)
 {
     return do_kexec_op_internal(op, uarg, 1);
 }
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 5d64cb6..a126188 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -277,7 +277,7 @@ static void decrease_reservation(struct memop_args *a)
     a->nr_done = i;
 }
 
-static long memory_exchange(XEN_GUEST_HANDLE(xen_memory_exchange_t) arg)
+static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
 {
     struct xen_memory_exchange exch;
     PAGE_LIST_HEAD(in_chunk_list);
@@ -530,7 +530,7 @@ static long memory_exchange(XEN_GUEST_HANDLE(xen_memory_exchange_t) arg)
     return rc;
 }
 
-long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE(void) arg)
+long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct domain *d;
     int rc, op;
diff --git a/xen/common/multicall.c b/xen/common/multicall.c
index 6c1a9d7..5de5f8d 100644
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -21,7 +21,7 @@ typedef long ret_t;
 
 ret_t
 do_multicall(
-    XEN_GUEST_HANDLE(multicall_entry_t) call_list, unsigned int nr_calls)
+    XEN_GUEST_HANDLE_PARAM(multicall_entry_t) call_list, unsigned int nr_calls)
 {
     struct mc_state *mcs = &current->mc_state;
     unsigned int     i;
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 0854f55..c26eac4 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -836,7 +836,7 @@ typedef long ret_t;
 
 #endif /* !COMPAT */
 
-ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     ret_t ret = 0;
 
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index ea68278..47142f4 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -27,7 +27,7 @@
 #include <xsm/xsm.h>
 #include <xen/pmstat.h>
 
-long do_sysctl(XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl)
+long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
 {
     long ret = 0;
     struct xen_sysctl curop, *op = &curop;
diff --git a/xen/common/xenoprof.c b/xen/common/xenoprof.c
index e571fea..c001b38 100644
--- a/xen/common/xenoprof.c
+++ b/xen/common/xenoprof.c
@@ -404,7 +404,7 @@ static int add_active_list(domid_t domid)
     return 0;
 }
 
-static int add_passive_list(XEN_GUEST_HANDLE(void) arg)
+static int add_passive_list(XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct xenoprof_passive passive;
     struct domain *d;
@@ -585,7 +585,7 @@ void xenoprof_log_event(struct vcpu *vcpu, const struct cpu_user_regs *regs,
 
 
 
-static int xenoprof_op_init(XEN_GUEST_HANDLE(void) arg)
+static int xenoprof_op_init(XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct domain *d = current->domain;
     struct xenoprof_init xenoprof_init;
@@ -609,7 +609,7 @@ static int xenoprof_op_init(XEN_GUEST_HANDLE(void) arg)
 
 #endif /* !COMPAT */
 
-static int xenoprof_op_get_buffer(XEN_GUEST_HANDLE(void) arg)
+static int xenoprof_op_get_buffer(XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct xenoprof_get_buffer xenoprof_get_buffer;
     struct domain *d = current->domain;
@@ -660,7 +660,7 @@ static int xenoprof_op_get_buffer(XEN_GUEST_HANDLE(void) arg)
                       || (op == XENOPROF_disable_virq)  \
                       || (op == XENOPROF_get_buffer))
  
-int do_xenoprof_op(int op, XEN_GUEST_HANDLE(void) arg)
+int do_xenoprof_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     int ret = 0;
     
diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index 8788f01..2be1764 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -513,7 +513,7 @@ int do_pm_op(struct xen_sysctl_pm_op *op)
     return ret;
 }
 
-int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE(uint32) pdc)
+int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE_PARAM(uint32) pdc)
 {
     u32 bits[3];
     int ret;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index e10bed5..b0f2334 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -182,7 +182,7 @@ static void putchar_console_ring(int c)
 
 long read_console_ring(struct xen_sysctl_readconsole *op)
 {
-    XEN_GUEST_HANDLE(char) str;
+    XEN_GUEST_HANDLE_PARAM(char) str;
     uint32_t idx, len, max, sofar, c;
 
     str   = guest_handle_cast(op->buffer, char),
@@ -320,7 +320,7 @@ static void notify_dom0_con_ring(unsigned long unused)
 static DECLARE_SOFTIRQ_TASKLET(notify_dom0_con_ring_tasklet,
                                notify_dom0_con_ring, 0);
 
-static long guest_console_write(XEN_GUEST_HANDLE(char) buffer, int count)
+static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
 {
     char kbuf[128], *kptr;
     int kcount;
@@ -358,7 +358,7 @@ static long guest_console_write(XEN_GUEST_HANDLE(char) buffer, int count)
     return 0;
 }
 
-long do_console_io(int cmd, int count, XEN_GUEST_HANDLE(char) buffer)
+long do_console_io(int cmd, int count, XEN_GUEST_HANDLE_PARAM(char) buffer)
 {
     long rc;
     unsigned int idx, len;
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 64f5fd1..396461f 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -518,7 +518,7 @@ void iommu_crash_shutdown(void)
 
 int iommu_do_domctl(
     struct xen_domctl *domctl,
-    XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
+    XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
 {
     struct domain *d;
     u16 seg;
diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h
index 7a955cb..bf5005b 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_PARAM(type)) { _x };            \
 })
 
 #define guest_handle_from_ptr(ptr, type)        \
diff --git a/xen/include/asm-arm/hypercall.h b/xen/include/asm-arm/hypercall.h
index 454f02e..090e620 100644
--- a/xen/include/asm-arm/hypercall.h
+++ b/xen/include/asm-arm/hypercall.h
@@ -2,7 +2,7 @@
 #define __ASM_ARM_HYPERCALL_H__
 
 #include <public/domctl.h> /* for arch_do_domctl */
-int do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg);
+int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 #endif /* __ASM_ARM_HYPERCALL_H__ */
 /*
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index b37bd35..8bf45ba 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -267,7 +267,7 @@ static inline int relinquish_shared_pages(struct domain *d)
 
 
 /* Arch-specific portion of memory_op hypercall. */
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg);
+long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 int steal_page(
     struct domain *d, struct page_info *page, unsigned int memflags);
diff --git a/xen/include/asm-x86/hap.h b/xen/include/asm-x86/hap.h
index a2532a4..916a35b 100644
--- a/xen/include/asm-x86/hap.h
+++ b/xen/include/asm-x86/hap.h
@@ -51,7 +51,7 @@ hap_unmap_domain_page(void *p)
 /************************************************/
 void  hap_domain_init(struct domain *d);
 int   hap_domctl(struct domain *d, xen_domctl_shadow_op_t *sc,
-                 XEN_GUEST_HANDLE(void) u_domctl);
+                 XEN_GUEST_HANDLE_PARAM(void) u_domctl);
 int   hap_enable(struct domain *d, u32 mode);
 void  hap_final_teardown(struct domain *d);
 void  hap_teardown(struct domain *d);
diff --git a/xen/include/asm-x86/hypercall.h b/xen/include/asm-x86/hypercall.h
index 9e136c3..55b5ca2 100644
--- a/xen/include/asm-x86/hypercall.h
+++ b/xen/include/asm-x86/hypercall.h
@@ -18,22 +18,22 @@
 
 extern long
 do_event_channel_op_compat(
-    XEN_GUEST_HANDLE(evtchn_op_t) uop);
+    XEN_GUEST_HANDLE_PARAM(evtchn_op_t) uop);
 
 extern long
 do_set_trap_table(
-    XEN_GUEST_HANDLE(const_trap_info_t) traps);
+    XEN_GUEST_HANDLE_PARAM(const_trap_info_t) traps);
 
 extern int
 do_mmu_update(
-    XEN_GUEST_HANDLE(mmu_update_t) ureqs,
+    XEN_GUEST_HANDLE_PARAM(mmu_update_t) ureqs,
     unsigned int count,
-    XEN_GUEST_HANDLE(uint) pdone,
+    XEN_GUEST_HANDLE_PARAM(uint) pdone,
     unsigned int foreigndom);
 
 extern long
 do_set_gdt(
-    XEN_GUEST_HANDLE(ulong) frame_list,
+    XEN_GUEST_HANDLE_PARAM(ulong) frame_list,
     unsigned int entries);
 
 extern long
@@ -60,7 +60,7 @@ do_update_descriptor(
     u64 desc);
 
 extern long
-do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc);
+do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc);
 
 extern int
 do_update_va_mapping(
@@ -70,7 +70,7 @@ do_update_va_mapping(
 
 extern long
 do_physdev_op(
-    int cmd, XEN_GUEST_HANDLE(void) arg);
+    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern int
 do_update_va_mapping_otherdomain(
@@ -81,9 +81,9 @@ do_update_va_mapping_otherdomain(
 
 extern int
 do_mmuext_op(
-    XEN_GUEST_HANDLE(mmuext_op_t) uops,
+    XEN_GUEST_HANDLE_PARAM(mmuext_op_t) uops,
     unsigned int count,
-    XEN_GUEST_HANDLE(uint) pdone,
+    XEN_GUEST_HANDLE_PARAM(uint) pdone,
     unsigned int foreigndom);
 
 extern unsigned long
@@ -92,7 +92,7 @@ do_iret(
 
 extern int
 do_kexec(
-    unsigned long op, unsigned arg1, XEN_GUEST_HANDLE(void) uarg);
+    unsigned long op, unsigned arg1, XEN_GUEST_HANDLE_PARAM(void) uarg);
 
 #ifdef __x86_64__
 
@@ -110,11 +110,11 @@ do_set_segment_base(
 extern int
 compat_physdev_op(
     int cmd,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern int
 arch_compat_vcpu_op(
-    int cmd, struct vcpu *v, XEN_GUEST_HANDLE(void) arg);
+    int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 #else
 
diff --git a/xen/include/asm-x86/mem_event.h b/xen/include/asm-x86/mem_event.h
index 23d71c1..e17f36b 100644
--- a/xen/include/asm-x86/mem_event.h
+++ b/xen/include/asm-x86/mem_event.h
@@ -65,7 +65,7 @@ int mem_event_get_response(struct domain *d, struct mem_event_domain *med,
 struct domain *get_mem_event_op_target(uint32_t domain, int *rc);
 int do_mem_event_op(int op, uint32_t domain, void *arg);
 int mem_event_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
-                     XEN_GUEST_HANDLE(void) u_domctl);
+                     XEN_GUEST_HANDLE_PARAM(void) u_domctl);
 
 #endif /* __MEM_EVENT_H__ */
 
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index 4cba276..6373b3b 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -604,10 +604,10 @@ void *do_page_walk(struct vcpu *v, unsigned long addr);
 int __sync_local_execstate(void);
 
 /* Arch-specific portion of memory_op hypercall. */
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg);
-long subarch_memory_op(int op, XEN_GUEST_HANDLE(void) arg);
-int compat_arch_memory_op(int op, XEN_GUEST_HANDLE(void));
-int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE(void));
+long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
+long subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
+int compat_arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void));
+int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void));
 
 int steal_page(
     struct domain *d, struct page_info *page, unsigned int memflags);
diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index c432a97..1cd0e3f 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -215,7 +215,7 @@ int paging_domain_init(struct domain *d, unsigned int domcr_flags);
  * and disable ephemeral shadow modes (test mode and log-dirty mode) and
  * manipulate the log-dirty bitmap. */
 int paging_domctl(struct domain *d, xen_domctl_shadow_op_t *sc,
-                  XEN_GUEST_HANDLE(void) u_domctl);
+                  XEN_GUEST_HANDLE_PARAM(void) u_domctl);
 
 /* Call when destroying a domain */
 void paging_teardown(struct domain *d);
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 7164a50..efdbddd 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -598,7 +598,7 @@ int rdmsr_hypervisor_regs(uint32_t idx, uint64_t *val);
 int wrmsr_hypervisor_regs(uint32_t idx, uint64_t val);
 
 void microcode_set_module(unsigned int);
-int microcode_update(XEN_GUEST_HANDLE(const_void), unsigned long len);
+int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len);
 int microcode_resume_cpu(int cpu);
 
 unsigned long *get_x86_gpr(struct cpu_user_regs *regs, unsigned int modrm_reg);
diff --git a/xen/include/asm-x86/shadow.h b/xen/include/asm-x86/shadow.h
index 88a8cd2..2eb6efc 100644
--- a/xen/include/asm-x86/shadow.h
+++ b/xen/include/asm-x86/shadow.h
@@ -73,7 +73,7 @@ int shadow_track_dirty_vram(struct domain *d,
  * manipulate the log-dirty bitmap. */
 int shadow_domctl(struct domain *d, 
                   xen_domctl_shadow_op_t *sc,
-                  XEN_GUEST_HANDLE(void) u_domctl);
+                  XEN_GUEST_HANDLE_PARAM(void) u_domctl);
 
 /* Call when destroying a domain */
 void shadow_teardown(struct domain *d);
diff --git a/xen/include/asm-x86/xenoprof.h b/xen/include/asm-x86/xenoprof.h
index c03f8c8..3f5ea15 100644
--- a/xen/include/asm-x86/xenoprof.h
+++ b/xen/include/asm-x86/xenoprof.h
@@ -40,9 +40,9 @@ int xenoprof_arch_init(int *num_events, char *cpu_type);
 #define xenoprof_arch_disable_virq()            nmi_disable_virq()
 #define xenoprof_arch_release_counters()        nmi_release_counters()
 
-int xenoprof_arch_counter(XEN_GUEST_HANDLE(void) arg);
-int compat_oprof_arch_counter(XEN_GUEST_HANDLE(void) arg);
-int xenoprof_arch_ibs_counter(XEN_GUEST_HANDLE(void) arg);
+int xenoprof_arch_counter(XEN_GUEST_HANDLE_PARAM(void) arg);
+int compat_oprof_arch_counter(XEN_GUEST_HANDLE_PARAM(void) arg);
+int xenoprof_arch_ibs_counter(XEN_GUEST_HANDLE_PARAM(void) arg);
 
 struct vcpu;
 struct cpu_user_regs;
diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
index d7e2f94..8f3cdca 100644
--- a/xen/include/xen/acpi.h
+++ b/xen/include/xen/acpi.h
@@ -145,8 +145,8 @@ static inline unsigned int acpi_get_cstate_limit(void) { return 0; }
 static inline void acpi_set_cstate_limit(unsigned int new_limit) { return; }
 #endif
 
-#ifdef XEN_GUEST_HANDLE
-int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE(uint32));
+#ifdef XEN_GUEST_HANDLE_PARAM
+int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE_PARAM(uint32));
 #endif
 int arch_acpi_set_pdc_bits(u32 acpi_id, u32 *, u32 mask);
 
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index 73b1598..e335037 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -29,29 +29,29 @@ do_sched_op_compat(
 extern long
 do_sched_op(
     int cmd,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern long
 do_domctl(
-    XEN_GUEST_HANDLE(xen_domctl_t) u_domctl);
+    XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl);
 
 extern long
 arch_do_domctl(
     struct xen_domctl *domctl,
-    XEN_GUEST_HANDLE(xen_domctl_t) u_domctl);
+    XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl);
 
 extern long
 do_sysctl(
-    XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl);
+    XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl);
 
 extern long
 arch_do_sysctl(
     struct xen_sysctl *sysctl,
-    XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl);
+    XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl);
 
 extern long
 do_platform_op(
-    XEN_GUEST_HANDLE(xen_platform_op_t) u_xenpf_op);
+    XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
 
 /*
  * To allow safe resume of do_memory_op() after preemption, we need to know
@@ -64,11 +64,11 @@ do_platform_op(
 extern long
 do_memory_op(
     unsigned long cmd,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern long
 do_multicall(
-    XEN_GUEST_HANDLE(multicall_entry_t) call_list,
+    XEN_GUEST_HANDLE_PARAM(multicall_entry_t) call_list,
     unsigned int nr_calls);
 
 extern long
@@ -77,23 +77,23 @@ do_set_timer_op(
 
 extern long
 do_event_channel_op(
-    int cmd, XEN_GUEST_HANDLE(void) arg);
+    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern long
 do_xen_version(
     int cmd,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern long
 do_console_io(
     int cmd,
     int count,
-    XEN_GUEST_HANDLE(char) buffer);
+    XEN_GUEST_HANDLE_PARAM(char) buffer);
 
 extern long
 do_grant_table_op(
     unsigned int cmd,
-    XEN_GUEST_HANDLE(void) uop,
+    XEN_GUEST_HANDLE_PARAM(void) uop,
     unsigned int count);
 
 extern long
@@ -105,72 +105,72 @@ extern long
 do_vcpu_op(
     int cmd,
     int vcpuid,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 struct vcpu;
 extern long
 arch_do_vcpu_op(int cmd,
     struct vcpu *v,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern long
 do_nmi_op(
     unsigned int cmd,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern long
 do_hvm_op(
     unsigned long op,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern long
 do_kexec_op(
     unsigned long op,
     int arg1,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern long
 do_xsm_op(
-    XEN_GUEST_HANDLE(xsm_op_t) u_xsm_op);
+    XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_xsm_op);
 
 extern long
 do_tmem_op(
-    XEN_GUEST_HANDLE(tmem_op_t) uops);
+    XEN_GUEST_HANDLE_PARAM(tmem_op_t) uops);
 
 extern int
-do_xenoprof_op(int op, XEN_GUEST_HANDLE(void) arg);
+do_xenoprof_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 #ifdef CONFIG_COMPAT
 
 extern int
 compat_memory_op(
     unsigned int cmd,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern int
 compat_grant_table_op(
     unsigned int cmd,
-    XEN_GUEST_HANDLE(void) uop,
+    XEN_GUEST_HANDLE_PARAM(void) uop,
     unsigned int count);
 
 extern int
 compat_vcpu_op(
     int cmd,
     int vcpuid,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern int
-compat_xenoprof_op(int op, XEN_GUEST_HANDLE(void) arg);
+compat_xenoprof_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern int
 compat_xen_version(
     int cmd,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern int
 compat_sched_op(
     int cmd,
-    XEN_GUEST_HANDLE(void) arg);
+    XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern int
 compat_set_timer_op(
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 6f7fbf7..bd19e23 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -155,7 +155,7 @@ void iommu_crash_shutdown(void);
 void iommu_set_dom0_mapping(struct domain *d);
 void iommu_share_p2m_table(struct domain *d);
 
-int iommu_do_domctl(struct xen_domctl *, XEN_GUEST_HANDLE(xen_domctl_t));
+int iommu_do_domctl(struct xen_domctl *, XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
 
 void iommu_iotlb_flush(struct domain *d, unsigned long gfn, unsigned int page_count);
 void iommu_iotlb_flush_all(struct domain *d);
diff --git a/xen/include/xen/tmem_xen.h b/xen/include/xen/tmem_xen.h
index 4a35760..2e7199a 100644
--- a/xen/include/xen/tmem_xen.h
+++ b/xen/include/xen/tmem_xen.h
@@ -448,7 +448,7 @@ static inline void tmh_tze_copy_from_pfp(void *tva, pfp_t *pfp, pagesize_t len)
 typedef XEN_GUEST_HANDLE(void) cli_mfn_t;
 typedef XEN_GUEST_HANDLE(char) cli_va_t;
 */
-typedef XEN_GUEST_HANDLE(tmem_op_t) tmem_cli_op_t;
+typedef XEN_GUEST_HANDLE_PARAM(tmem_op_t) tmem_cli_op_t;
 
 static inline int tmh_get_tmemop_from_client(tmem_op_t *op, tmem_cli_op_t uops)
 {
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index bef79df..3e4a47f 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -139,7 +139,7 @@ struct xsm_operations {
     int (*cpupool_op)(void);
     int (*sched_op)(void);
 
-    long (*__do_xsm_op) (XEN_GUEST_HANDLE(xsm_op_t) op);
+    long (*__do_xsm_op) (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op);
 
 #ifdef CONFIG_X86
     int (*shadow_control) (struct domain *d, uint32_t op);
@@ -585,7 +585,7 @@ static inline int xsm_sched_op(void)
     return xsm_call(sched_op());
 }
 
-static inline long __do_xsm_op (XEN_GUEST_HANDLE(xsm_op_t) op)
+static inline long __do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
 {
 #ifdef XSM_ENABLE
     return xsm_ops->__do_xsm_op(op);
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 7027ee7..5ef6529 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -365,7 +365,7 @@ static int dummy_sched_op (void)
     return 0;
 }
 
-static long dummy___do_xsm_op(XEN_GUEST_HANDLE(xsm_op_t) op)
+static long dummy___do_xsm_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
 {
     return -ENOSYS;
 }
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index bd4db37..23e7d34 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -71,7 +71,7 @@ static int domain_has_security(struct domain *d, u32 perms)
                         perms, NULL);
 }
 
-static int flask_copyin_string(XEN_GUEST_HANDLE(char) u_buf, char **buf, uint32_t size)
+static int flask_copyin_string(XEN_GUEST_HANDLE_PARAM(char) u_buf, char **buf, uint32_t size)
 {
     char *tmp = xmalloc_bytes(size + 1);
     if ( !tmp )
@@ -573,7 +573,7 @@ static int flask_get_peer_sid(struct xen_flask_peersid *arg)
     return rv;
 }
 
-long do_flask_op(XEN_GUEST_HANDLE(xsm_op_t) u_flask_op)
+long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
 {
     xen_flask_op_t op;
     int rv;
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 23b84f3..0fc299c 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1553,7 +1553,7 @@ static int flask_vcpuextstate (struct domain *d, uint32_t cmd)
 }
 #endif
 
-long do_flask_op(XEN_GUEST_HANDLE(xsm_op_t) u_flask_op);
+long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
 
 static struct xsm_operations flask_ops = {
     .security_domaininfo = flask_security_domaininfo,
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 96c8669..46287cb 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -111,7 +111,7 @@ int unregister_xsm(struct xsm_operations *ops)
 
 #endif
 
-long do_xsm_op (XEN_GUEST_HANDLE(xsm_op_t) op)
+long do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
 {
     return __do_xsm_op(op);
 }
-- 
1.7.2.5

Re: [PATCH 0/5] ARM hypercall ABI: 64 bit ready

[David Vrabel]

On 06/08/12 15:11, Stefano Stabellini wrote:
> Hi all,
> this patch series makes the necessary changes to make sure that the
> current ARM hypercall ABI can be used as-is on 64 bit ARM platforms:
> 
> - it defines xen_ulong_t as uint64_t on ARM;
> - it introduces a new macro to handle guest pointers, called
> XEN_GUEST_HANDLE_PARAM (that has size 4 bytes on aarch and is going to
> have size 8 bytes on aarch64);
> - it replaces all the occurrences of XEN_GUEST_HANDLE in hypercall
> parameters with XEN_GUEST_HANDLE_PARAM.

This is a subtle (and undocumented!) distinction. I can see people
adding/modifying hypercall etc. getting this wrong and no one noticing
for a while (since it doesn't affect x86).

The xen_ulong_t parameters (when used for pointers) from an aarch guest
point of view are a uint32_t guest pointer and uint32_t of padding.  So
the guest handles will be the same size in hypercall parameters and
structure members.

David

> On x86 and ia64 things should stay exactly the same.
> 
> On ARM all the unsigned long and the guest pointers that are members of
> a struct become size 8 byte (both aarch and aarch64).
> However guest pointers that are passed as hypercall arguments in
> registers are going to be 4 bytes on aarch and 8 bytes on aarch64.
> 
> It is based on Ian's arm-for-4.3 branch. 
> 
> 
> Stefano Stabellini (5):
>       xen: improve changes to xen_add_to_physmap
>       xen/arm: introduce __lshrdi3 and __aeabi_llsr
>       xen: few more xen_ulong_t substitutions
>       xen: introduce XEN_GUEST_HANDLE_PARAM
>       xen: replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when appropriate
> 
> 
>  xen/arch/arm/domain.c              |    2 +-
>  xen/arch/arm/domctl.c              |    2 +-
>  xen/arch/arm/hvm.c                 |    2 +-
>  xen/arch/arm/lib/Makefile          |    2 +-
>  xen/arch/arm/lib/lshrdi3.S         |   54 ++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/mm.c                  |    2 +-
>  xen/arch/arm/physdev.c             |    2 +-
>  xen/arch/arm/sysctl.c              |    2 +-
>  xen/arch/x86/cpu/mcheck/mce.c      |    2 +-
>  xen/arch/x86/domain.c              |    2 +-
>  xen/arch/x86/domctl.c              |    2 +-
>  xen/arch/x86/efi/runtime.c         |    2 +-
>  xen/arch/x86/hvm/hvm.c             |   26 ++++++++--------
>  xen/arch/x86/microcode.c           |    2 +-
>  xen/arch/x86/mm.c                  |   14 ++++----
>  xen/arch/x86/mm/hap/hap.c          |    2 +-
>  xen/arch/x86/mm/mem_event.c        |    2 +-
>  xen/arch/x86/mm/paging.c           |    2 +-
>  xen/arch/x86/mm/shadow/common.c    |    2 +-
>  xen/arch/x86/physdev.c             |    2 +-
>  xen/arch/x86/platform_hypercall.c  |    2 +-
>  xen/arch/x86/sysctl.c              |    2 +-
>  xen/arch/x86/traps.c               |    2 +-
>  xen/arch/x86/x86_32/mm.c           |    2 +-
>  xen/arch/x86/x86_32/traps.c        |    2 +-
>  xen/arch/x86/x86_64/compat/mm.c    |    8 ++--
>  xen/arch/x86/x86_64/domain.c       |    2 +-
>  xen/arch/x86/x86_64/mm.c           |    2 +-
>  xen/arch/x86/x86_64/traps.c        |    2 +-
>  xen/common/compat/domain.c         |    2 +-
>  xen/common/compat/grant_table.c    |    2 +-
>  xen/common/compat/memory.c         |    2 +-
>  xen/common/domain.c                |    2 +-
>  xen/common/domctl.c                |    2 +-
>  xen/common/event_channel.c         |    2 +-
>  xen/common/grant_table.c           |   36 ++++++++++++------------
>  xen/common/kernel.c                |    4 +-
>  xen/common/kexec.c                 |   16 +++++-----
>  xen/common/memory.c                |    4 +-
>  xen/common/multicall.c             |    2 +-
>  xen/common/schedule.c              |    2 +-
>  xen/common/sysctl.c                |    2 +-
>  xen/common/xenoprof.c              |    8 ++--
>  xen/drivers/acpi/pmstat.c          |    2 +-
>  xen/drivers/char/console.c         |    6 ++--
>  xen/drivers/passthrough/iommu.c    |    2 +-
>  xen/include/asm-arm/guest_access.h |    2 +-
>  xen/include/asm-arm/hypercall.h    |    2 +-
>  xen/include/asm-arm/mm.h           |    2 +-
>  xen/include/asm-x86/hap.h          |    2 +-
>  xen/include/asm-x86/hypercall.h    |   24 ++++++++--------
>  xen/include/asm-x86/mem_event.h    |    2 +-
>  xen/include/asm-x86/mm.h           |    8 ++--
>  xen/include/asm-x86/paging.h       |    2 +-
>  xen/include/asm-x86/processor.h    |    2 +-
>  xen/include/asm-x86/shadow.h       |    2 +-
>  xen/include/asm-x86/xenoprof.h     |    6 ++--
>  xen/include/public/arch-arm.h      |   21 ++++++++++----
>  xen/include/public/arch-ia64.h     |    1 +
>  xen/include/public/arch-x86/xen.h  |    1 +
>  xen/include/public/memory.h        |   11 ++++--
>  xen/include/public/physdev.h       |    2 +-
>  xen/include/public/version.h       |    2 +-
>  xen/include/public/xen.h           |    4 +-
>  xen/include/xen/acpi.h             |    4 +-
>  xen/include/xen/hypercall.h        |   52 +++++++++++++++++-----------------
>  xen/include/xen/iommu.h            |    2 +-
>  xen/include/xen/tmem_xen.h         |    2 +-
>  xen/include/xsm/xsm.h              |    4 +-
>  xen/xsm/dummy.c                    |    2 +-
>  xen/xsm/flask/flask_op.c           |    4 +-
>  xen/xsm/flask/hooks.c              |    2 +-
>  xen/xsm/xsm_core.c                 |    2 +-
>  73 files changed, 243 insertions(+), 175 deletions(-)
> 
> 
> Cheers,
> 
> Stefano
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 0/5] ARM hypercall ABI: 64 bit ready

[Stefano Stabellini]

On Mon, 6 Aug 2012, David Vrabel wrote:
> On 06/08/12 15:11, Stefano Stabellini wrote:
> > Hi all,
> > this patch series makes the necessary changes to make sure that the
> > current ARM hypercall ABI can be used as-is on 64 bit ARM platforms:
> > 
> > - it defines xen_ulong_t as uint64_t on ARM;
> > - it introduces a new macro to handle guest pointers, called
> > XEN_GUEST_HANDLE_PARAM (that has size 4 bytes on aarch and is going to
> > have size 8 bytes on aarch64);
> > - it replaces all the occurrences of XEN_GUEST_HANDLE in hypercall
> > parameters with XEN_GUEST_HANDLE_PARAM.
> 
> This is a subtle (and undocumented!) distinction. I can see people
> adding/modifying hypercall etc. getting this wrong and no one noticing
> for a while (since it doesn't affect x86).

Where should I document this? I wrote it into the commit message but
maybe a doc under docs is better.


> The xen_ulong_t parameters (when used for pointers) from an aarch guest
> point of view are a uint32_t guest pointer and uint32_t of padding.  So
> the guest handles will be the same size in hypercall parameters and
> structure members.

I changed xen_ulong_t to be 64 bit on ARM

Re: [PATCH 0/5] ARM hypercall ABI: 64 bit ready

[Stefano Stabellini]

On Mon, 6 Aug 2012, Stefano Stabellini wrote:
> On Mon, 6 Aug 2012, David Vrabel wrote:
> > On 06/08/12 15:11, Stefano Stabellini wrote:
> > > Hi all,
> > > this patch series makes the necessary changes to make sure that the
> > > current ARM hypercall ABI can be used as-is on 64 bit ARM platforms:
> > > 
> > > - it defines xen_ulong_t as uint64_t on ARM;
> > > - it introduces a new macro to handle guest pointers, called
> > > XEN_GUEST_HANDLE_PARAM (that has size 4 bytes on aarch and is going to
> > > have size 8 bytes on aarch64);
> > > - it replaces all the occurrences of XEN_GUEST_HANDLE in hypercall
> > > parameters with XEN_GUEST_HANDLE_PARAM.
> > 
> > This is a subtle (and undocumented!) distinction. I can see people
> > adding/modifying hypercall etc. getting this wrong and no one noticing
> > for a while (since it doesn't affect x86).
> 
> Where should I document this? I wrote it into the commit message but
> maybe a doc under docs is better.
> 
> 
> > The xen_ulong_t parameters (when used for pointers) from an aarch guest
> > point of view are a uint32_t guest pointer and uint32_t of padding.  So
> > the guest handles will be the same size in hypercall parameters and
> > structure members.
> 
> I changed xen_ulong_t to be 64 bit on ARM
> 

Sorry, pressed "send" too soon.

I don't think there are any xen_ulong_t used for pointers in hypercall
parameters, at least there are none in the hypercalls we are using so
far.

Re: [PATCH 0/5] ARM hypercall ABI: 64 bit ready

[David Vrabel]

On 06/08/12 15:44, Stefano Stabellini wrote:
> On Mon, 6 Aug 2012, David Vrabel wrote:
>> On 06/08/12 15:11, Stefano Stabellini wrote:
>>> Hi all,
>>> this patch series makes the necessary changes to make sure that the
>>> current ARM hypercall ABI can be used as-is on 64 bit ARM platforms:
>>>
>>> - it defines xen_ulong_t as uint64_t on ARM;
>>> - it introduces a new macro to handle guest pointers, called
>>> XEN_GUEST_HANDLE_PARAM (that has size 4 bytes on aarch and is going to
>>> have size 8 bytes on aarch64);
>>> - it replaces all the occurrences of XEN_GUEST_HANDLE in hypercall
>>> parameters with XEN_GUEST_HANDLE_PARAM.
>>
>> This is a subtle (and undocumented!) distinction. I can see people
>> adding/modifying hypercall etc. getting this wrong and no one noticing
>> for a while (since it doesn't affect x86).
> 
> Where should I document this? I wrote it into the commit message but
> maybe a doc under docs is better.

A comment next to the #define of the two macros?

>> The xen_ulong_t parameters (when used for pointers) from an aarch guest
>> point of view are a uint32_t guest pointer and uint32_t of padding.  So
>> the guest handles will be the same size in hypercall parameters and
>> structure members.

Ignore this.  Dunno what I was thinking.

David