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

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

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 6690 bytes --]

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.

In this version of the patch series I have introduced conversion macros
to convert a XEN_GUEST_HANDLE_PARAM into a XEN_GUEST_HANDLE a vice
versa. Most of the problematic cases come from xen/arch/x86 code, in
order to spot them I wrote a simple debug patch that change the
definition of XEN_GUEST_HANDLE_PARAM to be different from
XEN_GUEST_HANDLE on x86 too. I am attaching the debug patch to this
email.



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


Changes in v3:
- default all the guest_handle_* conversion macros to
  XEN_GUEST_HANDLE_PARAM as return type;
- add two new guest_handle_to_param and guest_handle_from_param macros
  to do conversions.

Changes in v2:

- do not use an anonymous union in struct xen_add_to_physmap; 
- do not replace the unsigned long in x86 specific calls;
- do not replace the unsigned long in multicall_entry;
- add missing include "xen.h" in version.h;
- use proper printf flag for xen_ulong_t in python/xen/lowlevel/xc/xc;
- add 2 missing #define _XEN_GUEST_HANDLE_PARAM for the compilation of
the compat code;
- add a patch to limit the maximum number of extents handled by
do_memory_op;
- remove the patch "introduce __lshrdi3 and __aeabi_llsr" that is
already in the for-4.3 branch.



Stefano Stabellini (6):
      xen: improve changes to xen_add_to_physmap
      xen: xen_ulong_t substitution
      xen: change the limit of nr_extents to UINT_MAX >> MEMOP_EXTENT_SHIFT
      xen: introduce XEN_GUEST_HANDLE_PARAM
      xen: replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when appropriate
      xen: more substitutions

 tools/firmware/hvmloader/pci.c           |    2 +-
 tools/python/xen/lowlevel/xc/xc.c        |    2 +-
 xen/arch/arm/domain.c                    |    2 +-
 xen/arch/arm/domctl.c                    |    2 +-
 xen/arch/arm/hvm.c                       |    2 +-
 xen/arch/arm/mm.c                        |    4 +-
 xen/arch/arm/physdev.c                   |    2 +-
 xen/arch/arm/sysctl.c                    |    2 +-
 xen/arch/x86/compat.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                        |   36 ++++++++++++--------
 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/oprofile/backtrace.c        |    4 ++-
 xen/arch/x86/oprofile/xenoprof.c         |    6 ++--
 xen/arch/x86/physdev.c                   |    2 +-
 xen/arch/x86/platform_hypercall.c        |   10 ++++--
 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          |   16 ++++++---
 xen/arch/x86/x86_64/cpu_idle.c           |    4 ++-
 xen/arch/x86/x86_64/cpufreq.c            |    4 ++-
 xen/arch/x86/x86_64/domain.c             |    2 +-
 xen/arch/x86/x86_64/mm.c                 |    2 +-
 xen/arch/x86/x86_64/platform_hypercall.c |    1 +
 xen/arch/x86/x86_64/traps.c              |    2 +-
 xen/common/compat/domain.c               |    2 +-
 xen/common/compat/grant_table.c          |    8 ++--
 xen/common/compat/memory.c               |    4 +-
 xen/common/compat/multicall.c            |    1 +
 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                      |    6 ++--
 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       |   19 +++++++++--
 xen/include/asm-arm/hypercall.h          |    2 +-
 xen/include/asm-arm/mm.h                 |    2 +-
 xen/include/asm-x86/guest_access.h       |   19 +++++++++--
 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            |   30 +++++++++++++----
 xen/include/public/arch-ia64.h           |    9 +++++
 xen/include/public/arch-x86/xen.h        |    9 +++++
 xen/include/public/memory.h              |   11 ++++--
 xen/include/public/version.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 +-
 79 files changed, 292 insertions(+), 203 deletions(-)


Cheers,

Stefano

[-- Attachment #2: Type: text/plain, Size: 1203 bytes --]

diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index 0e10260..08a788e 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -32,7 +32,8 @@
 /* Structural guest handles introduced in 0x00030201. */
 #if __XEN_INTERFACE_VERSION__ >= 0x00030201
 #define ___DEFINE_XEN_GUEST_HANDLE(name, type) \
-    typedef struct { type *p; } __guest_handle_ ## name
+    typedef struct { type *p; } __guest_handle_ ## name; \
+    typedef struct { type *p; } __guest_handle_param_ ## name
 #else
 #define ___DEFINE_XEN_GUEST_HANDLE(name, type) \
     typedef type * __guest_handle_ ## name
@@ -52,7 +53,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 XEN_GUEST_HANDLE_PARAM(name)    __guest_handle_param_ ## 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)

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

[PATCH v3 1/6] 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.

Changes in v2:

- do not use an anonymous union.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/firmware/hvmloader/pci.c  |    2 +-
 xen/arch/arm/mm.c               |    2 +-
 xen/arch/x86/mm.c               |   10 +++++-----
 xen/arch/x86/x86_64/compat/mm.c |    6 ++++++
 xen/include/public/memory.h     |   11 +++++++----
 5 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
index fd56e50..6375989 100644
--- a/tools/firmware/hvmloader/pci.c
+++ b/tools/firmware/hvmloader/pci.c
@@ -212,7 +212,7 @@ void pci_setup(void)
         xatp.space = XENMAPSPACE_gmfn_range;
         xatp.idx   = hvm_info->low_mem_pgend;
         xatp.gpfn  = hvm_info->high_mem_pgend;
-        xatp.size  = nr_pages;
+        xatp.u.size  = nr_pages;
         if ( hypercall_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
             BUG();
         hvm_info->high_mem_pgend += nr_pages;
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 08bc55b..2400e1c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -506,7 +506,7 @@ static int xenmem_add_to_physmap_once(
         paddr_t maddr;
         struct domain *od;
 
-        rc = rcu_lock_target_domain_by_id(xatp->foreign_domid, &od);
+        rc = rcu_lock_target_domain_by_id(xatp->u.foreign_domid, &od);
         if ( rc < 0 )
             return rc;
         maddr = p2m_lookup(od, xatp->idx << PAGE_SHIFT);
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 9f63974..f5c704e 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4630,7 +4630,7 @@ static int xenmem_add_to_physmap(struct domain *d,
             this_cpu(iommu_dont_flush_iotlb) = 1;
 
         start_xatp = *xatp;
-        while ( xatp->size > 0 )
+        while ( xatp->u.size > 0 )
         {
             rc = xenmem_add_to_physmap_once(d, xatp);
             if ( rc < 0 )
@@ -4638,10 +4638,10 @@ static int xenmem_add_to_physmap(struct domain *d,
 
             xatp->idx++;
             xatp->gpfn++;
-            xatp->size--;
+            xatp->u.size--;
 
             /* Check for continuation if it's not the last interation */
-            if ( xatp->size > 0 && hypercall_preempt_check() )
+            if ( xatp->u.size > 0 && hypercall_preempt_check() )
             {
                 rc = -EAGAIN;
                 break;
@@ -4651,8 +4651,8 @@ static int xenmem_add_to_physmap(struct domain *d,
         if ( need_iommu(d) )
         {
             this_cpu(iommu_dont_flush_iotlb) = 0;
-            iommu_iotlb_flush(d, start_xatp.idx, start_xatp.size - xatp->size);
-            iommu_iotlb_flush(d, start_xatp.gpfn, start_xatp.size - xatp->size);
+            iommu_iotlb_flush(d, start_xatp.idx, start_xatp.u.size - xatp->u.size);
+            iommu_iotlb_flush(d, start_xatp.gpfn, start_xatp.u.size - xatp->u.size);
         }
 
         return rc;
diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c
index f497503..5bcd2fd 100644
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -59,10 +59,16 @@ int compat_arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
     {
         struct compat_add_to_physmap cmp;
         struct xen_add_to_physmap *nat = COMPAT_ARG_XLAT_VIRT_BASE;
+        enum XLAT_add_to_physmap_u u;
 
         if ( copy_from_guest(&cmp, arg, 1) )
             return -EFAULT;
 
+        if ( cmp.space == XENMAPSPACE_gmfn_range )
+            u = XLAT_add_to_physmap_u_size;
+        if ( cmp.space == XENMAPSPACE_gmfn_foreign )
+            u = XLAT_add_to_physmap_u_foreign_domid;
+
         XLAT_add_to_physmap(nat, &cmp);
         rc = arch_memory_op(op, guest_handle_from_ptr(nat, void));
 
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index b2adfbe..7d4ee26 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;
+    } u;
 
     /* 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

[PATCH v3 2/6] xen: xen_ulong_t substitution

[Stefano Stabellini]

There is still an unwanted unsigned long in the xen public interface:
replace it with xen_ulong_t.

Also typedef xen_ulong_t to uint64_t on ARM.

Changes in v2:

- do not replace the unsigned long in x86 specific calls;
- do not replace the unsigned long in multicall_entry;
- add missing include "xen.h" in version.h;
- use proper printf flag for xen_ulong_t.

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

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 7c89756..e220f68 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -1439,7 +1439,7 @@ static PyObject *pyxc_xeninfo(XcObject *self)
     if ( xc_version(self->xc_handle, XENVER_commandline, &xen_commandline) != 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
-    snprintf(str, sizeof(str), "virt_start=0x%lx", p_parms.virt_start);
+    snprintf(str, sizeof(str), "virt_start=0x%"PRI_xen_ulong, p_parms.virt_start);
 
     xen_pagesize = xc_version(self->xc_handle, XENVER_pagesize, NULL);
     if (xen_pagesize < 0 )
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/version.h b/xen/include/public/version.h
index 8742c2b..c7e6f8c 100644
--- a/xen/include/public/version.h
+++ b/xen/include/public/version.h
@@ -28,6 +28,8 @@
 #ifndef __XEN_PUBLIC_VERSION_H__
 #define __XEN_PUBLIC_VERSION_H__
 
+#include "xen.h"
+
 /* NB. All ops return zero on success, except XENVER_{version,pagesize} */
 
 /* arg == NULL; returns major:minor (16:16). */
@@ -58,7 +60,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;
 
-- 
1.7.2.5

[PATCH v3 3/6] xen: change the limit of nr_extents to UINT_MAX >> MEMOP_EXTENT_SHIFT

[Stefano Stabellini]

Currently do_memory_op has a different maximum limit for nr_extents on
32 bit and 64 bit.
Change the limit to UINT_MAX >> MEMOP_EXTENT_SHIFT, so that it is the
same in both cases.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/common/memory.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 5d64cb6..7e58cc4 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -553,7 +553,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE(void) arg)
             return start_extent;
 
         /* Is size too large for us to encode a continuation? */
-        if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) )
+        if ( reservation.nr_extents > (UINT_MAX >> MEMOP_EXTENT_SHIFT) )
             return start_extent;
 
         if ( unlikely(start_extent >= reservation.nr_extents) )
-- 
1.7.2.5

[PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers
stored in memory from guest pointers as hypercall parameters.

guest_handle_* macros default to XEN_GUEST_HANDLE_PARAM as return type.
Two new guest_handle_to_param and guest_handle_from_param macros are
introduced to do conversions.

Changes in v2:

- add 2 missing #define _XEN_GUEST_HANDLE_PARAM for the compilation of
the compat code.

Changes in v3:

- move the guest_handle_cast change into this patch;
- add a clear comment on top of guest_handle_cast;
- also s/XEN_GUEST_HANDLE/XEN_GUEST_HANDLE_PARAM in
  guest_handle_from_ptr and const_guest_handle_from_ptr;
- introduce guest_handle_from_param and guest_handle_to_param.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/include/asm-arm/guest_access.h |   19 ++++++++++++++++---
 xen/include/asm-x86/guest_access.h |   19 ++++++++++++++++---
 xen/include/public/arch-arm.h      |   24 ++++++++++++++++++++----
 xen/include/public/arch-ia64.h     |    9 +++++++++
 xen/include/public/arch-x86/xen.h  |    9 +++++++++
 5 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h
index 0fceae6..28a4b3f 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -27,16 +27,29 @@ unsigned long raw_clear_guest(void *to, unsigned len);
 #define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
 #define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
 
-/* Cast a guest handle to the specified type of handle. */
+/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
+ * to the specified type of XEN_GUEST_HANDLE_PARAM. */
 #define guest_handle_cast(hnd, type) ({         \
     type *_x = (hnd).p;                         \
+    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
+})
+
+/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
+#define guest_handle_to_param(hnd, type) ({                  \
+    typeof((hnd).p) _x = (hnd).p;                         \
+    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
+})
+
+/* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
+#define guest_handle_from_param(hnd, type) ({                  \
+    typeof((hnd).p) _x = (hnd).p;                         \
     (XEN_GUEST_HANDLE(type)) { _x };            \
 })
 
 #define guest_handle_from_ptr(ptr, type)        \
-    ((XEN_GUEST_HANDLE(type)) { (type *)ptr })
+    ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr })
 #define const_guest_handle_from_ptr(ptr, type)  \
-    ((XEN_GUEST_HANDLE(const_##type)) { (const type *)ptr })
+    ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr })
 
 /*
  * Copy an array of objects to guest context via a guest handle,
diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h
index 2b429c2..6d35104 100644
--- a/xen/include/asm-x86/guest_access.h
+++ b/xen/include/asm-x86/guest_access.h
@@ -45,16 +45,29 @@
 #define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
 #define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
 
-/* Cast a guest handle to the specified type of handle. */
+/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
+ * to the specified type of XEN_GUEST_HANDLE_PARAM. */
 #define guest_handle_cast(hnd, type) ({         \
     type *_x = (hnd).p;                         \
+    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
+})
+
+/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
+#define guest_handle_to_param(hnd, type) ({                  \
+    typeof((hnd).p) _x = (hnd).p;                         \
+    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
+})
+
+/* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
+#define guest_handle_from_param(hnd, type) ({                  \
+    typeof((hnd).p) _x = (hnd).p;                         \
     (XEN_GUEST_HANDLE(type)) { _x };            \
 })
 
 #define guest_handle_from_ptr(ptr, type)        \
-    ((XEN_GUEST_HANDLE(type)) { (type *)ptr })
+    ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr })
 #define const_guest_handle_from_ptr(ptr, type)  \
-    ((XEN_GUEST_HANDLE(const_##type)) { (const type *)ptr })
+    ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr })
 
 /*
  * Copy an array of objects to guest context via a guest handle,
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 2ae6548..9db3c81 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -51,18 +51,34 @@
 
 #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;
 
+/*
+ * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field
+ * in a struct in memory. On ARM is always 8 bytes sizes and 8 bytes
+ * aligned.
+ * XEN_GUEST_HANDLE_PARAM represent a guest pointer, when passed as an
+ * hypercall argument. It is 4 bytes on aarch and 8 bytes on aarch64.
+ */
 #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..e4e9688 100644
--- a/xen/include/public/arch-ia64.h
+++ b/xen/include/public/arch-ia64.h
@@ -45,8 +45,17 @@
     ___DEFINE_XEN_GUEST_HANDLE(name, type);   \
     ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
 
+/*
+ * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field
+ * in a struct in memory.
+ * XEN_GUEST_HANDLE_PARAM represent a guest pointer, when passed as an
+ * hypercall argument.
+ * XEN_GUEST_HANDLE_PARAM and XEN_GUEST_HANDLE are the same on ia64 but
+ * they might not be on other architectures.
+ */
 #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..0e10260 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -38,12 +38,21 @@
     typedef type * __guest_handle_ ## name
 #endif
 
+/*
+ * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field
+ * in a struct in memory.
+ * XEN_GUEST_HANDLE_PARAM represent a guest pointer, when passed as an
+ * hypercall argument.
+ * XEN_GUEST_HANDLE_PARAM and XEN_GUEST_HANDLE are the same on X86 but
+ * they might not be on other architectures.
+ */
 #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)          __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 v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 16.08.12 at 16:50, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -51,18 +51,34 @@
>  
>  #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;

Why struct { union { ... } } instead of just union { ... }?

>  
> +/*
> + * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field
> + * in a struct in memory. On ARM is always 8 bytes sizes and 8 bytes
> + * aligned.
> + * XEN_GUEST_HANDLE_PARAM represent a guest pointer, when passed as an
> + * hypercall argument. It is 4 bytes on aarch and 8 bytes on aarch64.
> + */
>  #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;   \

If you made the "normal" handle a union too, you could avoid
the explicit cast (which e.g. gcc, when not passed
-fno-strict-aliasing, will choke on) and instead use (hnd).q (and
at once avoid the double initialization of the low half).

Also, the condition to do this could be "sizeof(hnd) > sizeof((hnd).p)",
usable at once for 64-bit avoiding a full double initialization there.

> +         (hnd).p = val;                                     \

In a public header you certainly want to avoid evaluating a
macro argument twice.

> +    } while ( 0 )
>  #ifdef __XEN_TOOLS__
>  #define get_xen_guest_handle(val, hnd)  do { val = (hnd).p; } while (0)
>  #endif

Seeing the patch I btw realized that there's no easy way to
avoid having the type as a second argument in the conversion
macros. Nevertheless I still don't like the explicitly specified type
there.

Jan

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
> Seeing the patch I btw realized that there's no easy way to
> avoid having the type as a second argument in the conversion
> macros. Nevertheless I still don't like the explicitly specified type
> there.

Btw - on the architecture(s) where the two handles are identical
I would prefer you to make the conversion functions trivial (and
thus avoid making use of the "type" parameter), thus allowing
the type checking to occur that you currently circumvent.

Jan

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

On Thu, 16 Aug 2012, Jan Beulich wrote:
> >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
> > Seeing the patch I btw realized that there's no easy way to
> > avoid having the type as a second argument in the conversion
> > macros. Nevertheless I still don't like the explicitly specified type
> > there.
> 
> Btw - on the architecture(s) where the two handles are identical
> I would prefer you to make the conversion functions trivial (and
> thus avoid making use of the "type" parameter), thus allowing
> the type checking to occur that you currently circumvent.

OK, I can do that.

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Ian Campbell]

On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
> On Thu, 16 Aug 2012, Jan Beulich wrote:
> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
> > > Seeing the patch I btw realized that there's no easy way to
> > > avoid having the type as a second argument in the conversion
> > > macros. Nevertheless I still don't like the explicitly specified type
> > > there.
> > 
> > Btw - on the architecture(s) where the two handles are identical
> > I would prefer you to make the conversion functions trivial (and
> > thus avoid making use of the "type" parameter), thus allowing
> > the type checking to occur that you currently circumvent.
> 
> OK, I can do that.

Will this result in the type parameter potentially becoming stale?

Adding a redundant pointer compare is a good way to get the compiler to
catch this. Smth like;

        /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
        #define guest_handle_from_param(hnd, type) ({
            typeof((hnd).p) _x = (hnd).p;
            XEN_GUEST_HANDLE(type) _y;
            &_y == &_x;
            hnd;
         })

I'm not sure which two pointers of members of the various structs need
to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
idea...

Ian.

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
>> On Thu, 16 Aug 2012, Jan Beulich wrote:
>> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
>> > > Seeing the patch I btw realized that there's no easy way to
>> > > avoid having the type as a second argument in the conversion
>> > > macros. Nevertheless I still don't like the explicitly specified type
>> > > there.
>> > 
>> > Btw - on the architecture(s) where the two handles are identical
>> > I would prefer you to make the conversion functions trivial (and
>> > thus avoid making use of the "type" parameter), thus allowing
>> > the type checking to occur that you currently circumvent.
>> 
>> OK, I can do that.
> 
> Will this result in the type parameter potentially becoming stale?
> 
> Adding a redundant pointer compare is a good way to get the compiler to
> catch this. Smth like;
> 
>         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
>         #define guest_handle_from_param(hnd, type) ({
>             typeof((hnd).p) _x = (hnd).p;
>             XEN_GUEST_HANDLE(type) _y;
>             &_y == &_x;
>             hnd;
>          })

Ah yes, that's a good suggestion.

> I'm not sure which two pointers of members of the various structs need
> to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
> idea...

Right, comparing (hnd).p with _y.p would be the right thing; no
need for _x, but some other (mechanical) adjustments would be
necessary.

Jan

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

On Fri, 17 Aug 2012, Jan Beulich wrote:
> >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
> >> On Thu, 16 Aug 2012, Jan Beulich wrote:
> >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
> >> > > Seeing the patch I btw realized that there's no easy way to
> >> > > avoid having the type as a second argument in the conversion
> >> > > macros. Nevertheless I still don't like the explicitly specified type
> >> > > there.
> >> > 
> >> > Btw - on the architecture(s) where the two handles are identical
> >> > I would prefer you to make the conversion functions trivial (and
> >> > thus avoid making use of the "type" parameter), thus allowing
> >> > the type checking to occur that you currently circumvent.
> >> 
> >> OK, I can do that.
> > 
> > Will this result in the type parameter potentially becoming stale?
> > 
> > Adding a redundant pointer compare is a good way to get the compiler to
> > catch this. Smth like;
> > 
> >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
> >         #define guest_handle_from_param(hnd, type) ({
> >             typeof((hnd).p) _x = (hnd).p;
> >             XEN_GUEST_HANDLE(type) _y;
> >             &_y == &_x;
> >             hnd;
> >          })
> 
> Ah yes, that's a good suggestion.
> 
> > I'm not sure which two pointers of members of the various structs need
> > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
> > idea...
> 
> Right, comparing (hnd).p with _y.p would be the right thing; no
> need for _x, but some other (mechanical) adjustments would be
> necessary.

The _x variable is still useful to avoid multiple evaluations of hnd,
even though I know that this is not a public header.

What about the following:

/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
#define guest_handle_to_param(hnd, type) ({                \
    typeof((hnd).p) _x = (hnd).p;                          \
    XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
    if (&_x != &_y.p) BUG();                               \
    _y;                                                    \
})

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Ian Campbell]

On Fri, 2012-08-17 at 14:47 +0100, Stefano Stabellini wrote:
> On Fri, 17 Aug 2012, Jan Beulich wrote:
> > >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
> > >> On Thu, 16 Aug 2012, Jan Beulich wrote:
> > >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
> > >> > > Seeing the patch I btw realized that there's no easy way to
> > >> > > avoid having the type as a second argument in the conversion
> > >> > > macros. Nevertheless I still don't like the explicitly specified type
> > >> > > there.
> > >> > 
> > >> > Btw - on the architecture(s) where the two handles are identical
> > >> > I would prefer you to make the conversion functions trivial (and
> > >> > thus avoid making use of the "type" parameter), thus allowing
> > >> > the type checking to occur that you currently circumvent.
> > >> 
> > >> OK, I can do that.
> > > 
> > > Will this result in the type parameter potentially becoming stale?
> > > 
> > > Adding a redundant pointer compare is a good way to get the compiler to
> > > catch this. Smth like;
> > > 
> > >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
> > >         #define guest_handle_from_param(hnd, type) ({
> > >             typeof((hnd).p) _x = (hnd).p;
> > >             XEN_GUEST_HANDLE(type) _y;
> > >             &_y == &_x;
> > >             hnd;
> > >          })
> > 
> > Ah yes, that's a good suggestion.
> > 
> > > I'm not sure which two pointers of members of the various structs need
> > > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
> > > idea...
> > 
> > Right, comparing (hnd).p with _y.p would be the right thing; no
> > need for _x, but some other (mechanical) adjustments would be
> > necessary.
> 
> The _x variable is still useful to avoid multiple evaluations of hnd,
> even though I know that this is not a public header.
> 
> What about the following:
> 
> /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> #define guest_handle_to_param(hnd, type) ({                \
>     typeof((hnd).p) _x = (hnd).p;                          \
>     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
>     if (&_x != &_y.p) BUG();                               \

&_x and &_y.p will always be different => this will always BUG().

You just need "(&_x == &_y.p)" if the types of _x and _y.p are different
then the compiler will error out due to the comparison of differently
typed pointers.

Ian.

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

On Fri, 17 Aug 2012, Ian Campbell wrote:
> On Fri, 2012-08-17 at 14:47 +0100, Stefano Stabellini wrote:
> > On Fri, 17 Aug 2012, Jan Beulich wrote:
> > > >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > > > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
> > > >> On Thu, 16 Aug 2012, Jan Beulich wrote:
> > > >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
> > > >> > > Seeing the patch I btw realized that there's no easy way to
> > > >> > > avoid having the type as a second argument in the conversion
> > > >> > > macros. Nevertheless I still don't like the explicitly specified type
> > > >> > > there.
> > > >> > 
> > > >> > Btw - on the architecture(s) where the two handles are identical
> > > >> > I would prefer you to make the conversion functions trivial (and
> > > >> > thus avoid making use of the "type" parameter), thus allowing
> > > >> > the type checking to occur that you currently circumvent.
> > > >> 
> > > >> OK, I can do that.
> > > > 
> > > > Will this result in the type parameter potentially becoming stale?
> > > > 
> > > > Adding a redundant pointer compare is a good way to get the compiler to
> > > > catch this. Smth like;
> > > > 
> > > >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
> > > >         #define guest_handle_from_param(hnd, type) ({
> > > >             typeof((hnd).p) _x = (hnd).p;
> > > >             XEN_GUEST_HANDLE(type) _y;
> > > >             &_y == &_x;
> > > >             hnd;
> > > >          })
> > > 
> > > Ah yes, that's a good suggestion.
> > > 
> > > > I'm not sure which two pointers of members of the various structs need
> > > > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
> > > > idea...
> > > 
> > > Right, comparing (hnd).p with _y.p would be the right thing; no
> > > need for _x, but some other (mechanical) adjustments would be
> > > necessary.
> > 
> > The _x variable is still useful to avoid multiple evaluations of hnd,
> > even though I know that this is not a public header.
> > 
> > What about the following:
> > 
> > /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> > #define guest_handle_to_param(hnd, type) ({                \
> >     typeof((hnd).p) _x = (hnd).p;                          \
> >     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
> >     if (&_x != &_y.p) BUG();                               \
> 
> &_x and &_y.p will always be different => this will always BUG().
>
> You just need "(&_x == &_y.p)" if the types of _x and _y.p are different
> then the compiler will error out due to the comparison of differently
> typed pointers.

I know what you mean, but we cannot do that because the compiler will
complain with "statement has no effects".
So we have to do something like:

if ((&_x == &_y.p) && 0) BUG();

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 17.08.12 at 15:55, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> On Fri, 17 Aug 2012, Ian Campbell wrote:
>> On Fri, 2012-08-17 at 14:47 +0100, Stefano Stabellini wrote:
>> > On Fri, 17 Aug 2012, Jan Beulich wrote:
>> > > >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > > > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
>> > > >> On Thu, 16 Aug 2012, Jan Beulich wrote:
>> > > >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
>> > > >> > > Seeing the patch I btw realized that there's no easy way to
>> > > >> > > avoid having the type as a second argument in the conversion
>> > > >> > > macros. Nevertheless I still don't like the explicitly specified type
>> > > >> > > there.
>> > > >> > 
>> > > >> > Btw - on the architecture(s) where the two handles are identical
>> > > >> > I would prefer you to make the conversion functions trivial (and
>> > > >> > thus avoid making use of the "type" parameter), thus allowing
>> > > >> > the type checking to occur that you currently circumvent.
>> > > >> 
>> > > >> OK, I can do that.
>> > > > 
>> > > > Will this result in the type parameter potentially becoming stale?
>> > > > 
>> > > > Adding a redundant pointer compare is a good way to get the compiler to
>> > > > catch this. Smth like;
>> > > > 
>> > > >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
>> > > >         #define guest_handle_from_param(hnd, type) ({
>> > > >             typeof((hnd).p) _x = (hnd).p;
>> > > >             XEN_GUEST_HANDLE(type) _y;
>> > > >             &_y == &_x;
>> > > >             hnd;
>> > > >          })
>> > > 
>> > > Ah yes, that's a good suggestion.
>> > > 
>> > > > I'm not sure which two pointers of members of the various structs need
>> > > > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
>> > > > idea...
>> > > 
>> > > Right, comparing (hnd).p with _y.p would be the right thing; no
>> > > need for _x, but some other (mechanical) adjustments would be
>> > > necessary.
>> > 
>> > The _x variable is still useful to avoid multiple evaluations of hnd,
>> > even though I know that this is not a public header.
>> > 
>> > What about the following:
>> > 
>> > /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
>> > #define guest_handle_to_param(hnd, type) ({                \
>> >     typeof((hnd).p) _x = (hnd).p;                          \
>> >     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
>> >     if (&_x != &_y.p) BUG();                               \
>> 
>> &_x and &_y.p will always be different => this will always BUG().
>>
>> You just need "(&_x == &_y.p)" if the types of _x and _y.p are different
>> then the compiler will error out due to the comparison of differently
>> typed pointers.
> 
> I know what you mean, but we cannot do that because the compiler will
> complain with "statement has no effects".
> So we have to do something like:
> 
> if ((&_x == &_y.p) && 0) BUG();

As done in my other mail - simply cast to void.

Jan

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 17.08.12 at 15:47, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> On Fri, 17 Aug 2012, Jan Beulich wrote:
>> >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
>> >> On Thu, 16 Aug 2012, Jan Beulich wrote:
>> >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
>> >> > > Seeing the patch I btw realized that there's no easy way to
>> >> > > avoid having the type as a second argument in the conversion
>> >> > > macros. Nevertheless I still don't like the explicitly specified type
>> >> > > there.
>> >> > 
>> >> > Btw - on the architecture(s) where the two handles are identical
>> >> > I would prefer you to make the conversion functions trivial (and
>> >> > thus avoid making use of the "type" parameter), thus allowing
>> >> > the type checking to occur that you currently circumvent.
>> >> 
>> >> OK, I can do that.
>> > 
>> > Will this result in the type parameter potentially becoming stale?
>> > 
>> > Adding a redundant pointer compare is a good way to get the compiler to
>> > catch this. Smth like;
>> > 
>> >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
>> >         #define guest_handle_from_param(hnd, type) ({
>> >             typeof((hnd).p) _x = (hnd).p;
>> >             XEN_GUEST_HANDLE(type) _y;
>> >             &_y == &_x;
>> >             hnd;
>> >          })
>> 
>> Ah yes, that's a good suggestion.
>> 
>> > I'm not sure which two pointers of members of the various structs need
>> > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
>> > idea...
>> 
>> Right, comparing (hnd).p with _y.p would be the right thing; no
>> need for _x, but some other (mechanical) adjustments would be
>> necessary.
> 
> The _x variable is still useful to avoid multiple evaluations of hnd,
> even though I know that this is not a public header.

But we had settled on returning hnd unmodified when both
handle types are the same.

> What about the following:
> 
> /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> #define guest_handle_to_param(hnd, type) ({                \
>     typeof((hnd).p) _x = (hnd).p;                          \
>     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
>     if (&_x != &_y.p) BUG();                               \
>     _y;                                                    \
> })

Since this is not a public header, something like this (untested,
so may not compile as is)

#define guest_handle_to_param(hnd, type) ({                \
    (void)(typeof((hnd).p)0 == (XEN_GUEST_HANDLE_PARAM(type){}).p); \
    (hnd);                                                    \
})

is what I was thinking of.

Jan

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

On Fri, 17 Aug 2012, Jan Beulich wrote:
> >>> On 17.08.12 at 15:47, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> wrote:
> > On Fri, 17 Aug 2012, Jan Beulich wrote:
> >> >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
> >> >> On Thu, 16 Aug 2012, Jan Beulich wrote:
> >> >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
> >> >> > > Seeing the patch I btw realized that there's no easy way to
> >> >> > > avoid having the type as a second argument in the conversion
> >> >> > > macros. Nevertheless I still don't like the explicitly specified type
> >> >> > > there.
> >> >> > 
> >> >> > Btw - on the architecture(s) where the two handles are identical
> >> >> > I would prefer you to make the conversion functions trivial (and
> >> >> > thus avoid making use of the "type" parameter), thus allowing
> >> >> > the type checking to occur that you currently circumvent.
> >> >> 
> >> >> OK, I can do that.
> >> > 
> >> > Will this result in the type parameter potentially becoming stale?
> >> > 
> >> > Adding a redundant pointer compare is a good way to get the compiler to
> >> > catch this. Smth like;
> >> > 
> >> >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
> >> >         #define guest_handle_from_param(hnd, type) ({
> >> >             typeof((hnd).p) _x = (hnd).p;
> >> >             XEN_GUEST_HANDLE(type) _y;
> >> >             &_y == &_x;
> >> >             hnd;
> >> >          })
> >> 
> >> Ah yes, that's a good suggestion.
> >> 
> >> > I'm not sure which two pointers of members of the various structs need
> >> > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
> >> > idea...
> >> 
> >> Right, comparing (hnd).p with _y.p would be the right thing; no
> >> need for _x, but some other (mechanical) adjustments would be
> >> necessary.
> > 
> > The _x variable is still useful to avoid multiple evaluations of hnd,
> > even though I know that this is not a public header.
> 
> But we had settled on returning hnd unmodified when both
> handle types are the same.
> 
> > What about the following:
> > 
> > /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> > #define guest_handle_to_param(hnd, type) ({                \
> >     typeof((hnd).p) _x = (hnd).p;                          \
> >     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
> >     if (&_x != &_y.p) BUG();                               \
> >     _y;                                                    \
> > })
> 
> Since this is not a public header, something like this (untested,
> so may not compile as is)
> 
> #define guest_handle_to_param(hnd, type) ({                \
>     (void)(typeof((hnd).p)0 == (XEN_GUEST_HANDLE_PARAM(type){}).p); \
>     (hnd);                                                    \
> })
> 
> is what I was thinking of.
> 

this is how it would look like:

#define guest_handle_to_param(hnd, type) ({                  \
    /* type checking: make sure that the pointers inside     \
     * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
     * the same type, than return hnd */                     \
    (void)((typeof(&(hnd).p)) 0 ==                           \
        (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \
    (hnd);                                                   \
})


Honestly I have very rarely seen anything less readable, but at least is
very compact.
For ARM I was going to go with the following, that is only slightly more
readable:

#define guest_handle_to_param(hnd, type) ({                  \
    typeof((hnd).p) _x = (hnd).p;                            \
    XEN_GUEST_HANDLE_PARAM(type) _y = { _x };                \
    /* type checking: make sure that the pointers inside     \
     * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
     * the same type, than return hnd */                     \
    (void)(&_x == &_y.p);                                    \
    _y;                                                      \
})

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 17.08.12 at 16:45, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> On Fri, 17 Aug 2012, Jan Beulich wrote:
>> >>> On 17.08.12 at 15:47, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> wrote:
>> > On Fri, 17 Aug 2012, Jan Beulich wrote:
>> >> >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> >> > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
>> >> >> On Thu, 16 Aug 2012, Jan Beulich wrote:
>> >> >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
>> >> >> > > Seeing the patch I btw realized that there's no easy way to
>> >> >> > > avoid having the type as a second argument in the conversion
>> >> >> > > macros. Nevertheless I still don't like the explicitly specified type
>> >> >> > > there.
>> >> >> > 
>> >> >> > Btw - on the architecture(s) where the two handles are identical
>> >> >> > I would prefer you to make the conversion functions trivial (and
>> >> >> > thus avoid making use of the "type" parameter), thus allowing
>> >> >> > the type checking to occur that you currently circumvent.
>> >> >> 
>> >> >> OK, I can do that.
>> >> > 
>> >> > Will this result in the type parameter potentially becoming stale?
>> >> > 
>> >> > Adding a redundant pointer compare is a good way to get the compiler to
>> >> > catch this. Smth like;
>> >> > 
>> >> >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
>> >> >         #define guest_handle_from_param(hnd, type) ({
>> >> >             typeof((hnd).p) _x = (hnd).p;
>> >> >             XEN_GUEST_HANDLE(type) _y;
>> >> >             &_y == &_x;
>> >> >             hnd;
>> >> >          })
>> >> 
>> >> Ah yes, that's a good suggestion.
>> >> 
>> >> > I'm not sure which two pointers of members of the various structs need
>> >> > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
>> >> > idea...
>> >> 
>> >> Right, comparing (hnd).p with _y.p would be the right thing; no
>> >> need for _x, but some other (mechanical) adjustments would be
>> >> necessary.
>> > 
>> > The _x variable is still useful to avoid multiple evaluations of hnd,
>> > even though I know that this is not a public header.
>> 
>> But we had settled on returning hnd unmodified when both
>> handle types are the same.
>> 
>> > What about the following:
>> > 
>> > /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
>> > #define guest_handle_to_param(hnd, type) ({                \
>> >     typeof((hnd).p) _x = (hnd).p;                          \
>> >     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
>> >     if (&_x != &_y.p) BUG();                               \
>> >     _y;                                                    \
>> > })
>> 
>> Since this is not a public header, something like this (untested,
>> so may not compile as is)
>> 
>> #define guest_handle_to_param(hnd, type) ({                \
>>     (void)(typeof((hnd).p)0 == (XEN_GUEST_HANDLE_PARAM(type){}).p); \
>>     (hnd);                                                    \
>> })
>> 
>> is what I was thinking of.
>> 
> 
> this is how it would look like:
> 
> #define guest_handle_to_param(hnd, type) ({                  \
>     /* type checking: make sure that the pointers inside     \
>      * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
>      * the same type, than return hnd */                     \
>     (void)((typeof(&(hnd).p)) 0 ==                           \
>         (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \
>     (hnd);                                                   \
> })
> 
> 
> Honestly I have very rarely seen anything less readable, but at least is
> very compact.
> For ARM I was going to go with the following, that is only slightly more
> readable:
> 
> #define guest_handle_to_param(hnd, type) ({                  \
>     typeof((hnd).p) _x = (hnd).p;                            \
>     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };                \
>     /* type checking: make sure that the pointers inside     \
>      * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
>      * the same type, than return hnd */                     \
>     (void)(&_x == &_y.p);                                    \
>     _y;                                                      \
> })

Yes, this looks good now to me (perhaps correct the minor
spelling mistake - should be "then", not "than" I think).

Jan

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Ian Campbell]

On Fri, 2012-08-17 at 14:58 +0100, Jan Beulich wrote:
> >>> On 17.08.12 at 15:47, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> wrote:
> > On Fri, 17 Aug 2012, Jan Beulich wrote:
> >> >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
> >> >> On Thu, 16 Aug 2012, Jan Beulich wrote:
> >> >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
> >> >> > > Seeing the patch I btw realized that there's no easy way to
> >> >> > > avoid having the type as a second argument in the conversion
> >> >> > > macros. Nevertheless I still don't like the explicitly specified type
> >> >> > > there.
> >> >> > 
> >> >> > Btw - on the architecture(s) where the two handles are identical
> >> >> > I would prefer you to make the conversion functions trivial (and
> >> >> > thus avoid making use of the "type" parameter), thus allowing
> >> >> > the type checking to occur that you currently circumvent.
> >> >> 
> >> >> OK, I can do that.
> >> > 
> >> > Will this result in the type parameter potentially becoming stale?
> >> > 
> >> > Adding a redundant pointer compare is a good way to get the compiler to
> >> > catch this. Smth like;
> >> > 
> >> >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
> >> >         #define guest_handle_from_param(hnd, type) ({
> >> >             typeof((hnd).p) _x = (hnd).p;
> >> >             XEN_GUEST_HANDLE(type) _y;
> >> >             &_y == &_x;
> >> >             hnd;
> >> >          })
> >> 
> >> Ah yes, that's a good suggestion.
> >> 
> >> > I'm not sure which two pointers of members of the various structs need
> >> > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
> >> > idea...
> >> 
> >> Right, comparing (hnd).p with _y.p would be the right thing; no
> >> need for _x, but some other (mechanical) adjustments would be
> >> necessary.
> > 
> > The _x variable is still useful to avoid multiple evaluations of hnd,
> > even though I know that this is not a public header.
> 
> But we had settled on returning hnd unmodified when both
> handle types are the same.
> 
> > What about the following:
> > 
> > /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> > #define guest_handle_to_param(hnd, type) ({                \
> >     typeof((hnd).p) _x = (hnd).p;                          \
> >     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
> >     if (&_x != &_y.p) BUG();                               \
> >     _y;                                                    \
> > })
> 
> Since this is not a public header, something like this (untested,
> so may not compile as is)
> 
> #define guest_handle_to_param(hnd, type) ({                \
>     (void)(typeof((hnd).p)0 == (XEN_GUEST_HANDLE_PARAM(type){}).p); \
>     (hnd);                                                    \
> })
> 
> is what I was thinking of.

This evaluates hnd twice, or do we only care about that in public
headers for some reason? (personally I think principal of least surprise
suggests avoiding it wherever possible)


> 
> Jan
> 

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

On Fri, 17 Aug 2012, Ian Campbell wrote:
> On Fri, 2012-08-17 at 14:58 +0100, Jan Beulich wrote:
> > >>> On 17.08.12 at 15:47, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > wrote:
> > > On Fri, 17 Aug 2012, Jan Beulich wrote:
> > >> >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > >> > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
> > >> >> On Thu, 16 Aug 2012, Jan Beulich wrote:
> > >> >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
> > >> >> > > Seeing the patch I btw realized that there's no easy way to
> > >> >> > > avoid having the type as a second argument in the conversion
> > >> >> > > macros. Nevertheless I still don't like the explicitly specified type
> > >> >> > > there.
> > >> >> > 
> > >> >> > Btw - on the architecture(s) where the two handles are identical
> > >> >> > I would prefer you to make the conversion functions trivial (and
> > >> >> > thus avoid making use of the "type" parameter), thus allowing
> > >> >> > the type checking to occur that you currently circumvent.
> > >> >> 
> > >> >> OK, I can do that.
> > >> > 
> > >> > Will this result in the type parameter potentially becoming stale?
> > >> > 
> > >> > Adding a redundant pointer compare is a good way to get the compiler to
> > >> > catch this. Smth like;
> > >> > 
> > >> >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
> > >> >         #define guest_handle_from_param(hnd, type) ({
> > >> >             typeof((hnd).p) _x = (hnd).p;
> > >> >             XEN_GUEST_HANDLE(type) _y;
> > >> >             &_y == &_x;
> > >> >             hnd;
> > >> >          })
> > >> 
> > >> Ah yes, that's a good suggestion.
> > >> 
> > >> > I'm not sure which two pointers of members of the various structs need
> > >> > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
> > >> > idea...
> > >> 
> > >> Right, comparing (hnd).p with _y.p would be the right thing; no
> > >> need for _x, but some other (mechanical) adjustments would be
> > >> necessary.
> > > 
> > > The _x variable is still useful to avoid multiple evaluations of hnd,
> > > even though I know that this is not a public header.
> > 
> > But we had settled on returning hnd unmodified when both
> > handle types are the same.
> > 
> > > What about the following:
> > > 
> > > /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> > > #define guest_handle_to_param(hnd, type) ({                \
> > >     typeof((hnd).p) _x = (hnd).p;                          \
> > >     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
> > >     if (&_x != &_y.p) BUG();                               \
> > >     _y;                                                    \
> > > })
> > 
> > Since this is not a public header, something like this (untested,
> > so may not compile as is)
> > 
> > #define guest_handle_to_param(hnd, type) ({                \
> >     (void)(typeof((hnd).p)0 == (XEN_GUEST_HANDLE_PARAM(type){}).p); \
> >     (hnd);                                                    \
> > })
> > 
> > is what I was thinking of.
> 
> This evaluates hnd twice, or do we only care about that in public
> headers for some reason? (personally I think principal of least surprise
> suggests avoiding it wherever possible)

the ARM version evaluates hnd only once and would work for x86 too

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 17.08.12 at 16:50, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2012-08-17 at 14:58 +0100, Jan Beulich wrote:
>> >>> On 17.08.12 at 15:47, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> wrote:
>> > On Fri, 17 Aug 2012, Jan Beulich wrote:
>> >> >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> >> > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
>> >> >> On Thu, 16 Aug 2012, Jan Beulich wrote:
>> >> >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
>> >> >> > > Seeing the patch I btw realized that there's no easy way to
>> >> >> > > avoid having the type as a second argument in the conversion
>> >> >> > > macros. Nevertheless I still don't like the explicitly specified type
>> >> >> > > there.
>> >> >> > 
>> >> >> > Btw - on the architecture(s) where the two handles are identical
>> >> >> > I would prefer you to make the conversion functions trivial (and
>> >> >> > thus avoid making use of the "type" parameter), thus allowing
>> >> >> > the type checking to occur that you currently circumvent.
>> >> >> 
>> >> >> OK, I can do that.
>> >> > 
>> >> > Will this result in the type parameter potentially becoming stale?
>> >> > 
>> >> > Adding a redundant pointer compare is a good way to get the compiler to
>> >> > catch this. Smth like;
>> >> > 
>> >> >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
>> >> >         #define guest_handle_from_param(hnd, type) ({
>> >> >             typeof((hnd).p) _x = (hnd).p;
>> >> >             XEN_GUEST_HANDLE(type) _y;
>> >> >             &_y == &_x;
>> >> >             hnd;
>> >> >          })
>> >> 
>> >> Ah yes, that's a good suggestion.
>> >> 
>> >> > I'm not sure which two pointers of members of the various structs need
>> >> > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
>> >> > idea...
>> >> 
>> >> Right, comparing (hnd).p with _y.p would be the right thing; no
>> >> need for _x, but some other (mechanical) adjustments would be
>> >> necessary.
>> > 
>> > The _x variable is still useful to avoid multiple evaluations of hnd,
>> > even though I know that this is not a public header.
>> 
>> But we had settled on returning hnd unmodified when both
>> handle types are the same.
>> 
>> > What about the following:
>> > 
>> > /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
>> > #define guest_handle_to_param(hnd, type) ({                \
>> >     typeof((hnd).p) _x = (hnd).p;                          \
>> >     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
>> >     if (&_x != &_y.p) BUG();                               \
>> >     _y;                                                    \
>> > })
>> 
>> Since this is not a public header, something like this (untested,
>> so may not compile as is)
>> 
>> #define guest_handle_to_param(hnd, type) ({                \
>>     (void)(typeof((hnd).p)0 == (XEN_GUEST_HANDLE_PARAM(type){}).p); \
>>     (hnd);                                                    \
>> })
>> 
>> is what I was thinking of.
> 
> This evaluates hnd twice, or do we only care about that in public
> headers for some reason? (personally I think principal of least surprise
> suggests avoiding it wherever possible)

No, it doesn't - like sizeof(), typeof() doesn't evaluate its
argument.

Jan

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Ian Campbell]

On Fri, 2012-08-17 at 15:57 +0100, Jan Beulich wrote:
> >>> On 17.08.12 at 16:50, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2012-08-17 at 14:58 +0100, Jan Beulich wrote:
> >> >>> On 17.08.12 at 15:47, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >> wrote:
> >> > On Fri, 17 Aug 2012, Jan Beulich wrote:
> >> >> >>> On 17.08.12 at 10:02, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> >> > On Thu, 2012-08-16 at 18:10 +0100, Stefano Stabellini wrote:
> >> >> >> On Thu, 16 Aug 2012, Jan Beulich wrote:
> >> >> >> > >>> On 16.08.12 at 17:54, "Jan Beulich" <JBeulich@suse.com> wrote:
> >> >> >> > > Seeing the patch I btw realized that there's no easy way to
> >> >> >> > > avoid having the type as a second argument in the conversion
> >> >> >> > > macros. Nevertheless I still don't like the explicitly specified type
> >> >> >> > > there.
> >> >> >> > 
> >> >> >> > Btw - on the architecture(s) where the two handles are identical
> >> >> >> > I would prefer you to make the conversion functions trivial (and
> >> >> >> > thus avoid making use of the "type" parameter), thus allowing
> >> >> >> > the type checking to occur that you currently circumvent.
> >> >> >> 
> >> >> >> OK, I can do that.
> >> >> > 
> >> >> > Will this result in the type parameter potentially becoming stale?
> >> >> > 
> >> >> > Adding a redundant pointer compare is a good way to get the compiler to
> >> >> > catch this. Smth like;
> >> >> > 
> >> >> >         /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */
> >> >> >         #define guest_handle_from_param(hnd, type) ({
> >> >> >             typeof((hnd).p) _x = (hnd).p;
> >> >> >             XEN_GUEST_HANDLE(type) _y;
> >> >> >             &_y == &_x;
> >> >> >             hnd;
> >> >> >          })
> >> >> 
> >> >> Ah yes, that's a good suggestion.
> >> >> 
> >> >> > I'm not sure which two pointers of members of the various structs need
> >> >> > to be compared, maybe it's actually &_y.p and &hnd.p, but you get the
> >> >> > idea...
> >> >> 
> >> >> Right, comparing (hnd).p with _y.p would be the right thing; no
> >> >> need for _x, but some other (mechanical) adjustments would be
> >> >> necessary.
> >> > 
> >> > The _x variable is still useful to avoid multiple evaluations of hnd,
> >> > even though I know that this is not a public header.
> >> 
> >> But we had settled on returning hnd unmodified when both
> >> handle types are the same.
> >> 
> >> > What about the following:
> >> > 
> >> > /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> >> > #define guest_handle_to_param(hnd, type) ({                \
> >> >     typeof((hnd).p) _x = (hnd).p;                          \
> >> >     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };              \
> >> >     if (&_x != &_y.p) BUG();                               \
> >> >     _y;                                                    \
> >> > })
> >> 
> >> Since this is not a public header, something like this (untested,
> >> so may not compile as is)
> >> 
> >> #define guest_handle_to_param(hnd, type) ({                \
> >>     (void)(typeof((hnd).p)0 == (XEN_GUEST_HANDLE_PARAM(type){}).p); \
> >>     (hnd);                                                    \
> >> })
> >> 
> >> is what I was thinking of.
> > 
> > This evaluates hnd twice, or do we only care about that in public
> > headers for some reason? (personally I think principal of least surprise
> > suggests avoiding it wherever possible)
> 
> No, it doesn't - like sizeof(), typeof() doesn't evaluate its
> argument.

Right, of course, silly me.

Ian.

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

On Thu, 16 Aug 2012, Jan Beulich wrote:
> >>> On 16.08.12 at 16:50, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> > --- a/xen/include/public/arch-arm.h
> > +++ b/xen/include/public/arch-arm.h
> > @@ -51,18 +51,34 @@
> >  
> >  #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;
> 
> Why struct { union { ... } } instead of just union { ... }?

good point :)


> > +/*
> > + * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field
> > + * in a struct in memory. On ARM is always 8 bytes sizes and 8 bytes
> > + * aligned.
> > + * XEN_GUEST_HANDLE_PARAM represent a guest pointer, when passed as an
> > + * hypercall argument. It is 4 bytes on aarch and 8 bytes on aarch64.
> > + */
> >  #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;   \
> 
> If you made the "normal" handle a union too, you could avoid
> the explicit cast (which e.g. gcc, when not passed
> -fno-strict-aliasing, will choke on) and instead use (hnd).q (and
> at once avoid the double initialization of the low half).
> 
> Also, the condition to do this could be "sizeof(hnd) > sizeof((hnd).p)",
> usable at once for 64-bit avoiding a full double initialization there.
> 
> > +         (hnd).p = val;                                     \
> 
> In a public header you certainly want to avoid evaluating a
> macro argument twice.

That's a really good suggestion.
I am going to make both handles unions and therefore
set_xen_guest_handle_raw becomes:

#define set_xen_guest_handle_raw(hnd, val)                  \
    do { (hnd).q = 0;                                       \
         (hnd).p = val;                                     \
    } while ( 0 )


> > +    } while ( 0 )
> >  #ifdef __XEN_TOOLS__
> >  #define get_xen_guest_handle(val, hnd)  do { val = (hnd).p; } while (0)
> >  #endif
> 
> Seeing the patch I btw realized that there's no easy way to
> avoid having the type as a second argument in the conversion
> macros. Nevertheless I still don't like the explicitly specified type
> there.

I agree, I don't like it either but I couldn't find anything better.

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Jan Beulich]

>>> On 16.08.12 at 19:08, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> On Thu, 16 Aug 2012, Jan Beulich wrote:
>> >>> On 16.08.12 at 16:50, Stefano Stabellini <stefano.stabellini@eu.citrix.com>  wrote:
>> > +#define set_xen_guest_handle_raw(hnd, val)                  \
>> > +    do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0;   \
>> 
>> If you made the "normal" handle a union too, you could avoid
>> the explicit cast (which e.g. gcc, when not passed
>> -fno-strict-aliasing, will choke on) and instead use (hnd).q (and
>> at once avoid the double initialization of the low half).
>> 
>> Also, the condition to do this could be "sizeof(hnd) > sizeof((hnd).p)",
>> usable at once for 64-bit avoiding a full double initialization there.
>> 
>> > +         (hnd).p = val;                                     \
>> 
>> In a public header you certainly want to avoid evaluating a
>> macro argument twice.
> 
> That's a really good suggestion.
> I am going to make both handles unions and therefore
> set_xen_guest_handle_raw becomes:
> 
> #define set_xen_guest_handle_raw(hnd, val)                  \
>     do { (hnd).q = 0;                                       \
>          (hnd).p = val;                                     \
>     } while ( 0 )

But that still doesn't eliminate the double evaluation of "hnd".

Jan

Re: [PATCH v3 4/6] xen: introduce XEN_GUEST_HANDLE_PARAM

[Stefano Stabellini]

On Fri, 17 Aug 2012, Jan Beulich wrote:
> >>> On 16.08.12 at 19:08, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> > On Thu, 16 Aug 2012, Jan Beulich wrote:
> >> >>> On 16.08.12 at 16:50, Stefano Stabellini <stefano.stabellini@eu.citrix.com>  wrote:
> >> > +#define set_xen_guest_handle_raw(hnd, val)                  \
> >> > +    do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0;   \
> >> 
> >> If you made the "normal" handle a union too, you could avoid
> >> the explicit cast (which e.g. gcc, when not passed
> >> -fno-strict-aliasing, will choke on) and instead use (hnd).q (and
> >> at once avoid the double initialization of the low half).
> >> 
> >> Also, the condition to do this could be "sizeof(hnd) > sizeof((hnd).p)",
> >> usable at once for 64-bit avoiding a full double initialization there.
> >> 
> >> > +         (hnd).p = val;                                     \
> >> 
> >> In a public header you certainly want to avoid evaluating a
> >> macro argument twice.
> > 
> > That's a really good suggestion.
> > I am going to make both handles unions and therefore
> > set_xen_guest_handle_raw becomes:
> > 
> > #define set_xen_guest_handle_raw(hnd, val)                  \
> >     do { (hnd).q = 0;                                       \
> >          (hnd).p = val;                                     \
> >     } while ( 0 )
> 
> But that still doesn't eliminate the double evaluation of "hnd".

Yes, you are right. I'll use a temporary pointer.

[PATCH v3 5/6] 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/compat.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/oprofile/xenoprof.c  |    6 ++--
 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   |   10 +++---
 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   |    8 +++---
 xen/common/compat/memory.c        |    4 +-
 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/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 +-
 65 files changed, 168 insertions(+), 168 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 2400e1c..3e8b6cc 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/compat.c b/xen/arch/x86/compat.c
index a4fda06..2d05867 100644
--- a/xen/arch/x86/compat.c
+++ b/xen/arch/x86/compat.c
@@ -27,7 +27,7 @@ ret_t do_physdev_op_compat(XEN_GUEST_HANDLE(physdev_op_t) uop)
 #ifndef COMPAT
 
 /* Legacy hypercall (as of 0x00030202). */
-long do_event_channel_op_compat(XEN_GUEST_HANDLE(evtchn_op_t) uop)
+long do_event_channel_op_compat(XEN_GUEST_HANDLE_PARAM(evtchn_op_t) uop)
 {
     struct evtchn_op op;
 
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index a89df6d..0f122b3 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1357,7 +1357,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 7f8a025c..e2bf831 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3047,14 +3047,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;
 
@@ -3072,7 +3072,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 )
     {
@@ -3088,7 +3088,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;
 
@@ -3137,7 +3137,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) )
@@ -3145,7 +3145,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;
 
@@ -3164,7 +3164,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;
 
@@ -3188,7 +3188,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 )
     {
@@ -3360,7 +3360,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;
@@ -3525,7 +3525,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;
@@ -3569,7 +3569,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;
@@ -3602,7 +3602,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;
@@ -3686,7 +3686,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 f5c704e..4d72700 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/oprofile/xenoprof.c b/xen/arch/x86/oprofile/xenoprof.c
index 71f00ef..5d286a2 100644
--- a/xen/arch/x86/oprofile/xenoprof.c
+++ b/xen/arch/x86/oprofile/xenoprof.c
@@ -19,7 +19,7 @@
 
 #include "op_counter.h"
 
-int xenoprof_arch_counter(XEN_GUEST_HANDLE(void) arg)
+int xenoprof_arch_counter(XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct xenoprof_counter counter;
 
@@ -39,7 +39,7 @@ int xenoprof_arch_counter(XEN_GUEST_HANDLE(void) arg)
     return 0;
 }
 
-int xenoprof_arch_ibs_counter(XEN_GUEST_HANDLE(void) arg)
+int xenoprof_arch_ibs_counter(XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct xenoprof_ibs_counter ibs_counter;
 
@@ -57,7 +57,7 @@ int xenoprof_arch_ibs_counter(XEN_GUEST_HANDLE(void) arg)
 }
 
 #ifdef CONFIG_COMPAT
-int compat_oprof_arch_counter(XEN_GUEST_HANDLE(void) arg)
+int compat_oprof_arch_counter(XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct compat_oprof_counter counter;
 
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 5bcd2fd..1de93b7 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;
@@ -266,14 +266,14 @@ 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;
     int rc = 0;
-    XEN_GUEST_HANDLE(mmuext_op_t) nat_ops;
+    XEN_GUEST_HANDLE_PARAM(mmuext_op_t) nat_ops;
 
     preempt_mask = count & MMU_UPDATE_PREEMPTED;
     count ^= 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..b524955 100644
--- a/xen/common/compat/grant_table.c
+++ b/xen/common/compat/grant_table.c
@@ -52,12 +52,12 @@ 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;
     unsigned int i;
-    XEN_GUEST_HANDLE(void) cnt_uop;
+    XEN_GUEST_HANDLE_PARAM(void) cnt_uop;
 
     set_xen_guest_handle(cnt_uop, NULL);
     switch ( cmd )
@@ -206,7 +206,7 @@ int compat_grant_table_op(unsigned int cmd,
             }
             if ( rc >= 0 )
             {
-                XEN_GUEST_HANDLE(gnttab_transfer_compat_t) xfer;
+                XEN_GUEST_HANDLE_PARAM(gnttab_transfer_compat_t) xfer;
 
                 xfer = guest_handle_cast(cmp_uop, gnttab_transfer_compat_t);
                 guest_handle_add_offset(xfer, i);
@@ -251,7 +251,7 @@ int compat_grant_table_op(unsigned int cmd,
             }
             if ( rc >= 0 )
             {
-                XEN_GUEST_HANDLE(gnttab_copy_compat_t) copy;
+                XEN_GUEST_HANDLE_PARAM(gnttab_copy_compat_t) copy;
 
                 copy = guest_handle_cast(cmp_uop, gnttab_copy_compat_t);
                 guest_handle_add_offset(copy, i);
diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
index e7257cc..996151c 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;
@@ -22,7 +22,7 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE(void) compat)
     {
         unsigned int i, end_extent = 0;
         union {
-            XEN_GUEST_HANDLE(void) hnd;
+            XEN_GUEST_HANDLE_PARAM(void) hnd;
             struct xen_memory_reservation *rsrv;
             struct xen_memory_exchange *xchg;
             struct xen_remove_from_physmap *xrfp;
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 7e58cc4..a683954 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 698711e..f8d62f2 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -515,7 +515,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/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

[PATCH v3 6/6] xen: more substitutions

[Stefano Stabellini]

More substitutions in this patch, not as obvious as the ones in the
previous patch.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/x86/mm.c                        |   12 +++++++++---
 xen/arch/x86/oprofile/backtrace.c        |    4 +++-
 xen/arch/x86/platform_hypercall.c        |    8 ++++++--
 xen/arch/x86/x86_64/cpu_idle.c           |    4 +++-
 xen/arch/x86/x86_64/cpufreq.c            |    4 +++-
 xen/arch/x86/x86_64/platform_hypercall.c |    1 +
 xen/common/compat/multicall.c            |    1 +
 7 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 4d72700..088db11 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3198,7 +3198,9 @@ int do_mmuext_op(
         {
             cpumask_t pmask;
 
-            if ( unlikely(vcpumask_to_pcpumask(d, op.arg2.vcpumask, &pmask)) )
+            if ( unlikely(vcpumask_to_pcpumask(d,
+                            guest_handle_to_param(op.arg2.vcpumask, const_void),
+                            &pmask)) )
             {
                 okay = 0;
                 break;
@@ -4484,6 +4486,7 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p)
     if ( s > ctxt->s )
     {
         e820entry_t ent;
+        XEN_GUEST_HANDLE_PARAM(e820entry_t) buffer_t;
         XEN_GUEST_HANDLE(e820entry_t) buffer;
 
         if ( ctxt->n + 1 >= ctxt->map.nr_entries )
@@ -4491,7 +4494,8 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p)
         ent.addr = (uint64_t)ctxt->s << PAGE_SHIFT;
         ent.size = (uint64_t)(s - ctxt->s) << PAGE_SHIFT;
         ent.type = E820_RESERVED;
-        buffer = guest_handle_cast(ctxt->map.buffer, e820entry_t);
+        buffer_t = guest_handle_cast(ctxt->map.buffer, e820entry_t);
+        buffer = guest_handle_from_param(buffer_t, e820entry_t);
         if ( __copy_to_guest_offset(buffer, ctxt->n, &ent, 1) )
             return -EFAULT;
         ctxt->n++;
@@ -4790,6 +4794,7 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
     {
         struct memory_map_context ctxt;
         XEN_GUEST_HANDLE(e820entry_t) buffer;
+        XEN_GUEST_HANDLE_PARAM(e820entry_t) buffer_t;
         unsigned int i;
 
         if ( !IS_PRIV(current->domain) )
@@ -4804,7 +4809,8 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( ctxt.map.nr_entries < e820.nr_map + 1 )
             return -EINVAL;
 
-        buffer = guest_handle_cast(ctxt.map.buffer, e820entry_t);
+        buffer_t = guest_handle_cast(ctxt.map.buffer, e820entry_t);
+        buffer = guest_handle_from_param(buffer_t, e820entry_t);
         if ( !guest_handle_okay(buffer, ctxt.map.nr_entries) )
             return -EFAULT;
 
diff --git a/xen/arch/x86/oprofile/backtrace.c b/xen/arch/x86/oprofile/backtrace.c
index 33fd142..699cd28 100644
--- a/xen/arch/x86/oprofile/backtrace.c
+++ b/xen/arch/x86/oprofile/backtrace.c
@@ -80,8 +80,10 @@ dump_guest_backtrace(struct vcpu *vcpu, const struct frame_head *head,
     else
 #endif
     {
-        XEN_GUEST_HANDLE(const_frame_head_t) guest_head =
+        XEN_GUEST_HANDLE(const_frame_head_t) guest_head;
+        XEN_GUEST_HANDLE_PARAM(const_frame_head_t) guest_head_t =
             const_guest_handle_from_ptr(head, frame_head_t);
+        guest_head = guest_handle_from_param(guest_head_t, const_frame_head_t);
 
         /* Also check accessibility of one struct frame_head beyond */
         if (!guest_handle_okay(guest_head, 2))
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index a32e0a2..2994b12 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -185,7 +185,9 @@ ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
             }
         }
 
-        ret = microcode_update(data, op->u.microcode.length);
+        ret = microcode_update(
+                guest_handle_to_param(data, const_void),
+                op->u.microcode.length);
         spin_unlock(&vcpu_alloc_lock);
     }
     break;
@@ -448,7 +450,9 @@ ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
             XEN_GUEST_HANDLE(uint32) pdc;
 
             guest_from_compat_handle(pdc, op->u.set_pminfo.u.pdc);
-            ret = acpi_set_pdc_bits(op->u.set_pminfo.id, pdc);
+            ret = acpi_set_pdc_bits(
+                    op->u.set_pminfo.id,
+                    guest_handle_to_param(pdc, uint32));
         }
         break;
 
diff --git a/xen/arch/x86/x86_64/cpu_idle.c b/xen/arch/x86/x86_64/cpu_idle.c
index 3e7422f..1cdaf96 100644
--- a/xen/arch/x86/x86_64/cpu_idle.c
+++ b/xen/arch/x86/x86_64/cpu_idle.c
@@ -57,10 +57,12 @@ static int copy_from_compat_state(xen_processor_cx_t *xen_state,
 {
 #define XLAT_processor_cx_HNDL_dp(_d_, _s_) do { \
     XEN_GUEST_HANDLE(compat_processor_csd_t) dps; \
+    XEN_GUEST_HANDLE_PARAM(xen_processor_csd_t) dps_t; \
     if ( unlikely(!compat_handle_okay((_s_)->dp, (_s_)->dpcnt)) ) \
             return -EFAULT; \
     guest_from_compat_handle(dps, (_s_)->dp); \
-    (_d_)->dp = guest_handle_cast(dps, xen_processor_csd_t); \
+    dps_t = guest_handle_cast(dps, xen_processor_csd_t); \
+    (_d_)->dp = guest_handle_from_param(dps_t, xen_processor_csd_t); \
 } while (0)
     XLAT_processor_cx(xen_state, state);
 #undef XLAT_processor_cx_HNDL_dp
diff --git a/xen/arch/x86/x86_64/cpufreq.c b/xen/arch/x86/x86_64/cpufreq.c
index ce9864e..1956777 100644
--- a/xen/arch/x86/x86_64/cpufreq.c
+++ b/xen/arch/x86/x86_64/cpufreq.c
@@ -45,10 +45,12 @@ compat_set_px_pminfo(uint32_t cpu, struct compat_processor_performance *perf)
 
 #define XLAT_processor_performance_HNDL_states(_d_, _s_) do { \
     XEN_GUEST_HANDLE(compat_processor_px_t) states; \
+    XEN_GUEST_HANDLE_PARAM(xen_processor_px_t) states_t; \
     if ( unlikely(!compat_handle_okay((_s_)->states, (_s_)->state_count)) ) \
         return -EFAULT; \
     guest_from_compat_handle(states, (_s_)->states); \
-    (_d_)->states = guest_handle_cast(states, xen_processor_px_t); \
+    states_t = guest_handle_cast(states, xen_processor_px_t); \
+    (_d_)->states = guest_handle_from_param(states_t, xen_processor_px_t); \
 } while (0)
 
     XLAT_processor_performance(xen_perf, perf);
diff --git a/xen/arch/x86/x86_64/platform_hypercall.c b/xen/arch/x86/x86_64/platform_hypercall.c
index 188aa37..f577761 100644
--- a/xen/arch/x86/x86_64/platform_hypercall.c
+++ b/xen/arch/x86/x86_64/platform_hypercall.c
@@ -38,6 +38,7 @@ CHECK_pf_pcpu_version;
 
 #define COMPAT
 #define _XEN_GUEST_HANDLE(t) XEN_GUEST_HANDLE(t)
+#define _XEN_GUEST_HANDLE_PARAM(t) XEN_GUEST_HANDLE(t)
 typedef int ret_t;
 
 #include "../platform_hypercall.c"
diff --git a/xen/common/compat/multicall.c b/xen/common/compat/multicall.c
index 0eb1212..72db213 100644
--- a/xen/common/compat/multicall.c
+++ b/xen/common/compat/multicall.c
@@ -24,6 +24,7 @@ DEFINE_XEN_GUEST_HANDLE(multicall_entry_compat_t);
 #define call                 compat_call
 #define do_multicall(l, n)   compat_multicall(_##l, n)
 #define _XEN_GUEST_HANDLE(t) XEN_GUEST_HANDLE(t)
+#define _XEN_GUEST_HANDLE_PARAM(t) XEN_GUEST_HANDLE(t)
 
 #include "../multicall.c"
 
-- 
1.7.2.5

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

[Ian Campbell]

On Thu, 2012-08-16 at 15:48 +0100, Stefano Stabellini wrote:
> In this version of the patch series I have introduced conversion macros
> to convert a XEN_GUEST_HANDLE_PARAM into a XEN_GUEST_HANDLE a vice
> versa. Most of the problematic cases come from xen/arch/x86 code, in
> order to spot them I wrote a simple debug patch that change the
> definition of XEN_GUEST_HANDLE_PARAM to be different from
> XEN_GUEST_HANDLE on x86 too. I am attaching the debug patch to this
> email. 

This (quoted below) seems like a useful patch from the PoV of catching
these sorts of things early on x86 before they break ARM.

It doesn't seem like it should have any impact on the generated code,
should we perhaps apply it?

I needed the addition of the following to actually make it work though.

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


> diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
> index 0e10260..08a788e 100644
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -32,7 +32,8 @@
>  /* Structural guest handles introduced in 0x00030201. */
>  #if __XEN_INTERFACE_VERSION__ >= 0x00030201
>  #define ___DEFINE_XEN_GUEST_HANDLE(name, type) \
> -    typedef struct { type *p; } __guest_handle_ ## name
> +    typedef struct { type *p; } __guest_handle_ ## name; \
> +    typedef struct { type *p; } __guest_handle_param_ ## name
>  #else
>  #define ___DEFINE_XEN_GUEST_HANDLE(name, type) \
>      typedef type * __guest_handle_ ## name
> @@ -52,7 +53,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 XEN_GUEST_HANDLE_PARAM(name)    __guest_handle_param_ ## 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)
> 

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

[Stefano Stabellini]

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

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


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

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