Hi all,
this patch series makes the necessary changes to make sure that the
current ARM hypercall ABI can be used as-is on 64 bit ARM platforms:
- it defines xen_ulong_t as uint64_t on ARM;
- it introduces a new macro to handle guest pointers, called
XEN_GUEST_HANDLE_PARAM (that has size 4 bytes on aarch and is going to
have size 8 bytes on aarch64);
- it replaces all the occurrences of XEN_GUEST_HANDLE in hypercall
parameters with XEN_GUEST_HANDLE_PARAM.
On x86 and ia64 things should stay exactly the same.
On ARM all the unsigned long and the guest pointers that are members of
a struct become size 8 byte (both aarch and aarch64).
However guest pointers that are passed as hypercall arguments in
registers are going to be 4 bytes on aarch and 8 bytes on aarch64.
It is based on Ian''s arm-for-4.3 branch.
Stefano Stabellini (5):
xen: improve changes to xen_add_to_physmap
xen/arm: introduce __lshrdi3 and __aeabi_llsr
xen: few more xen_ulong_t substitutions
xen: introduce XEN_GUEST_HANDLE_PARAM
xen: replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when appropriate
xen/arch/arm/domain.c | 2 +-
xen/arch/arm/domctl.c | 2 +-
xen/arch/arm/hvm.c | 2 +-
xen/arch/arm/lib/Makefile | 2 +-
xen/arch/arm/lib/lshrdi3.S | 54 ++++++++++++++++++++++++++++++++++++
xen/arch/arm/mm.c | 2 +-
xen/arch/arm/physdev.c | 2 +-
xen/arch/arm/sysctl.c | 2 +-
xen/arch/x86/cpu/mcheck/mce.c | 2 +-
xen/arch/x86/domain.c | 2 +-
xen/arch/x86/domctl.c | 2 +-
xen/arch/x86/efi/runtime.c | 2 +-
xen/arch/x86/hvm/hvm.c | 26 ++++++++--------
xen/arch/x86/microcode.c | 2 +-
xen/arch/x86/mm.c | 14 ++++----
xen/arch/x86/mm/hap/hap.c | 2 +-
xen/arch/x86/mm/mem_event.c | 2 +-
xen/arch/x86/mm/paging.c | 2 +-
xen/arch/x86/mm/shadow/common.c | 2 +-
xen/arch/x86/physdev.c | 2 +-
xen/arch/x86/platform_hypercall.c | 2 +-
xen/arch/x86/sysctl.c | 2 +-
xen/arch/x86/traps.c | 2 +-
xen/arch/x86/x86_32/mm.c | 2 +-
xen/arch/x86/x86_32/traps.c | 2 +-
xen/arch/x86/x86_64/compat/mm.c | 8 ++--
xen/arch/x86/x86_64/domain.c | 2 +-
xen/arch/x86/x86_64/mm.c | 2 +-
xen/arch/x86/x86_64/traps.c | 2 +-
xen/common/compat/domain.c | 2 +-
xen/common/compat/grant_table.c | 2 +-
xen/common/compat/memory.c | 2 +-
xen/common/domain.c | 2 +-
xen/common/domctl.c | 2 +-
xen/common/event_channel.c | 2 +-
xen/common/grant_table.c | 36 ++++++++++++------------
xen/common/kernel.c | 4 +-
xen/common/kexec.c | 16 +++++-----
xen/common/memory.c | 4 +-
xen/common/multicall.c | 2 +-
xen/common/schedule.c | 2 +-
xen/common/sysctl.c | 2 +-
xen/common/xenoprof.c | 8 ++--
xen/drivers/acpi/pmstat.c | 2 +-
xen/drivers/char/console.c | 6 ++--
xen/drivers/passthrough/iommu.c | 2 +-
xen/include/asm-arm/guest_access.h | 2 +-
xen/include/asm-arm/hypercall.h | 2 +-
xen/include/asm-arm/mm.h | 2 +-
xen/include/asm-x86/hap.h | 2 +-
xen/include/asm-x86/hypercall.h | 24 ++++++++--------
xen/include/asm-x86/mem_event.h | 2 +-
xen/include/asm-x86/mm.h | 8 ++--
xen/include/asm-x86/paging.h | 2 +-
xen/include/asm-x86/processor.h | 2 +-
xen/include/asm-x86/shadow.h | 2 +-
xen/include/asm-x86/xenoprof.h | 6 ++--
xen/include/public/arch-arm.h | 21 ++++++++++----
xen/include/public/arch-ia64.h | 1 +
xen/include/public/arch-x86/xen.h | 1 +
xen/include/public/memory.h | 11 ++++--
xen/include/public/physdev.h | 2 +-
xen/include/public/version.h | 2 +-
xen/include/public/xen.h | 4 +-
xen/include/xen/acpi.h | 4 +-
xen/include/xen/hypercall.h | 52 +++++++++++++++++-----------------
xen/include/xen/iommu.h | 2 +-
xen/include/xen/tmem_xen.h | 2 +-
xen/include/xsm/xsm.h | 4 +-
xen/xsm/dummy.c | 2 +-
xen/xsm/flask/flask_op.c | 4 +-
xen/xsm/flask/hooks.c | 2 +-
xen/xsm/xsm_core.c | 2 +-
73 files changed, 243 insertions(+), 175 deletions(-)
Cheers,
Stefano
Stefano Stabellini
2012-Aug-06 14:12 UTC
[PATCH 1/5] xen: improve changes to xen_add_to_physmap
This is an incremental patch on top of
c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary
compatibility, it is better to introduce foreign_domid as part of a
union containing both size and foreign_domid.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/include/public/memory.h | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index b2adfbe..b0af2fd 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -208,8 +208,12 @@ struct xen_add_to_physmap {
/* Which domain to change the mapping for. */
domid_t domid;
- /* Number of pages to go through for gmfn_range */
- uint16_t size;
+ union {
+ /* Number of pages to go through for gmfn_range */
+ uint16_t size;
+ /* IFF gmfn_foreign */
+ domid_t foreign_domid;
+ };
/* Source mapping space. */
#define XENMAPSPACE_shared_info 0 /* shared info page */
@@ -217,8 +221,7 @@ struct xen_add_to_physmap {
#define XENMAPSPACE_gmfn 2 /* GMFN */
#define XENMAPSPACE_gmfn_range 3 /* GMFN range */
#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
- uint16_t space;
- domid_t foreign_domid; /* IFF gmfn_foreign */
+ unsigned int space;
#define XENMAPIDX_grant_table_status 0x80000000
--
1.7.2.5
Stefano Stabellini
2012-Aug-06 14:12 UTC
[PATCH 2/5] xen/arm: introduce __lshrdi3 and __aeabi_llsr
Taken from Linux. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- xen/arch/arm/lib/Makefile | 2 +- xen/arch/arm/lib/lshrdi3.S | 54 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletions(-) create mode 100644 xen/arch/arm/lib/lshrdi3.S diff --git a/xen/arch/arm/lib/Makefile b/xen/arch/arm/lib/Makefile index cbbed68..4cf41f4 100644 --- a/xen/arch/arm/lib/Makefile +++ b/xen/arch/arm/lib/Makefile @@ -2,4 +2,4 @@ obj-y += memcpy.o memmove.o memset.o memzero.o obj-y += findbit.o setbit.o obj-y += setbit.o clearbit.o changebit.o obj-y += testsetbit.o testclearbit.o testchangebit.o -obj-y += lib1funcs.o div64.o +obj-y += lib1funcs.o lshrdi3.o div64.o diff --git a/xen/arch/arm/lib/lshrdi3.S b/xen/arch/arm/lib/lshrdi3.S new file mode 100644 index 0000000..3e8887e --- /dev/null +++ b/xen/arch/arm/lib/lshrdi3.S @@ -0,0 +1,54 @@ +/* Copyright 1995, 1996, 1998, 1999, 2000, 2003, 2004, 2005 + Free Software Foundation, Inc. + +This file is free software; you can redistribute it and/or modify it +under the terms of the GNU General Public License as published by the +Free Software Foundation; either version 2, or (at your option) any +later version. + +In addition to the permissions in the GNU General Public License, the +Free Software Foundation gives you unlimited permission to link the +compiled version of this file into combinations with other programs, +and to distribute those combinations without any restriction coming +from the use of this file. (The General Public License restrictions +do apply in other respects; for example, they cover modification of +the file, and distribution when not linked into a combine +executable.) + +This file is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program; see the file COPYING. If not, write to +the Free Software Foundation, 51 Franklin Street, Fifth Floor, +Boston, MA 02110-1301, USA. */ + + +#include <xen/config.h> +#include "assembler.h" + +#ifdef __ARMEB__ +#define al r1 +#define ah r0 +#else +#define al r0 +#define ah r1 +#endif + +ENTRY(__lshrdi3) +ENTRY(__aeabi_llsr) + + subs r3, r2, #32 + rsb ip, r2, #32 + movmi al, al, lsr r2 + movpl al, ah, lsr r3 + ARM( orrmi al, al, ah, lsl ip ) + THUMB( lslmi r3, ah, ip ) + THUMB( orrmi al, al, r3 ) + mov ah, ah, lsr r2 + mov pc, lr + +ENDPROC(__lshrdi3) +ENDPROC(__aeabi_llsr) -- 1.7.2.5
Stefano Stabellini
2012-Aug-06 14:12 UTC
[PATCH 3/5] xen: few more xen_ulong_t substitutions
There are still few unsigned long in the xen public interface: replace
them with xen_ulong_t.
Also typedef xen_ulong_t to uint64_t on ARM.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/include/public/arch-arm.h | 4 ++--
xen/include/public/physdev.h | 2 +-
xen/include/public/version.h | 2 +-
xen/include/public/xen.h | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 14ad0ab..2ae6548 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -122,8 +122,8 @@ typedef uint64_t xen_pfn_t;
/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */
#define XEN_LEGACY_MAX_VCPUS 1
-typedef uint32_t xen_ulong_t;
-#define PRI_xen_ulong PRIx32
+typedef uint64_t xen_ulong_t;
+#define PRI_xen_ulong PRIx64
struct vcpu_guest_context {
#define _VGCF_online 0
diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
index b78eeba..a4cf6eb 100644
--- a/xen/include/public/physdev.h
+++ b/xen/include/public/physdev.h
@@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
#define PHYSDEVOP_apic_write 9
struct physdev_apic {
/* IN */
- unsigned long apic_physbase;
+ xen_ulong_t apic_physbase;
uint32_t reg;
/* IN or OUT */
uint32_t value;
diff --git a/xen/include/public/version.h b/xen/include/public/version.h
index 8742c2b..eb83eba 100644
--- a/xen/include/public/version.h
+++ b/xen/include/public/version.h
@@ -58,7 +58,7 @@ typedef char xen_changeset_info_t[64];
#define XENVER_platform_parameters 5
struct xen_platform_parameters {
- unsigned long virt_start;
+ xen_ulong_t virt_start;
};
typedef struct xen_platform_parameters xen_platform_parameters_t;
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index b2f6c50..d635bbf 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
* NB. The fields are natural register size for this architecture.
*/
struct multicall_entry {
- unsigned long op, result;
- unsigned long args[6];
+ xen_ulong_t op, result;
+ xen_ulong_t args[6];
};
typedef struct multicall_entry multicall_entry_t;
DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);
--
1.7.2.5
Stefano Stabellini
2012-Aug-06 14:12 UTC
[PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM
Note: this change does not make any difference on x86 and ia64.
XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers
stored in memory from guest pointers as hypercall parameters.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/include/asm-arm/guest_access.h | 2 +-
xen/include/public/arch-arm.h | 17 +++++++++++++----
xen/include/public/arch-ia64.h | 1 +
xen/include/public/arch-x86/xen.h | 1 +
4 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/xen/include/asm-arm/guest_access.h
b/xen/include/asm-arm/guest_access.h
index 0fceae6..7a955cb 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -30,7 +30,7 @@ unsigned long raw_clear_guest(void *to, unsigned len);
/* Cast a guest handle to the specified type of handle. */
#define guest_handle_cast(hnd, type) ({ \
type *_x = (hnd).p; \
- (XEN_GUEST_HANDLE(type)) { _x }; \
+ (XEN_GUEST_HANDLE(type)) { {_x } }; \
})
#define guest_handle_from_ptr(ptr, type) \
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 2ae6548..d17d645 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -51,18 +51,27 @@
#define XEN_HYPERCALL_TAG 0XEA1
+#define uint64_aligned_t uint64_t __attribute__((aligned(8)))
#ifndef __ASSEMBLY__
-#define ___DEFINE_XEN_GUEST_HANDLE(name, type) \
- typedef struct { type *p; } __guest_handle_ ## name
+#define ___DEFINE_XEN_GUEST_HANDLE(name, type) \
+ typedef struct { type *p; } \
+ __guest_handle_ ## name; \
+ typedef struct { union { type *p; uint64_aligned_t q; }; } \
+ __guest_handle_64_ ## name;
#define __DEFINE_XEN_GUEST_HANDLE(name, type) \
___DEFINE_XEN_GUEST_HANDLE(name, type); \
___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
#define DEFINE_XEN_GUEST_HANDLE(name) __DEFINE_XEN_GUEST_HANDLE(name, name)
-#define __XEN_GUEST_HANDLE(name) __guest_handle_ ## name
+#define __XEN_GUEST_HANDLE(name) __guest_handle_64_ ## name
#define XEN_GUEST_HANDLE(name) __XEN_GUEST_HANDLE(name)
-#define set_xen_guest_handle_raw(hnd, val) do { (hnd).p = val; } while (0)
+/* this is going to be changes on 64 bit */
+#define XEN_GUEST_HANDLE_PARAM(name) __guest_handle_ ## name
+#define set_xen_guest_handle_raw(hnd, val) \
+ do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0; \
+ (hnd).p = val; \
+ } while ( 0 )
#ifdef __XEN_TOOLS__
#define get_xen_guest_handle(val, hnd) do { val = (hnd).p; } while (0)
#endif
diff --git a/xen/include/public/arch-ia64.h b/xen/include/public/arch-ia64.h
index c9da5d4..97583ea 100644
--- a/xen/include/public/arch-ia64.h
+++ b/xen/include/public/arch-ia64.h
@@ -47,6 +47,7 @@
#define DEFINE_XEN_GUEST_HANDLE(name) __DEFINE_XEN_GUEST_HANDLE(name, name)
#define XEN_GUEST_HANDLE(name) __guest_handle_ ## name
+#define XEN_GUEST_HANDLE_PARAM(name) XEN_GUEST_HANDLE(name)
#define XEN_GUEST_HANDLE_64(name) XEN_GUEST_HANDLE(name)
#define uint64_aligned_t uint64_t
#define set_xen_guest_handle_raw(hnd, val) do { (hnd).p = val; } while (0)
diff --git a/xen/include/public/arch-x86/xen.h
b/xen/include/public/arch-x86/xen.h
index 1c186d7..8ee5437 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -44,6 +44,7 @@
#define DEFINE_XEN_GUEST_HANDLE(name) __DEFINE_XEN_GUEST_HANDLE(name, name)
#define __XEN_GUEST_HANDLE(name) __guest_handle_ ## name
#define XEN_GUEST_HANDLE(name) __XEN_GUEST_HANDLE(name)
+#define XEN_GUEST_HANDLE_PARAM(name) XEN_GUEST_HANDLE(name)
#define set_xen_guest_handle_raw(hnd, val) do { (hnd).p = val; } while (0)
#ifdef __XEN_TOOLS__
#define get_xen_guest_handle(val, hnd) do { val = (hnd).p; } while (0)
--
1.7.2.5
Stefano Stabellini
2012-Aug-06 14:12 UTC
[PATCH 5/5] xen: replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when appropriate
Note: these changes don''t make any difference on x86 and ia64.
Replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when it is used as
an hypercall argument.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/arch/arm/domain.c | 2 +-
xen/arch/arm/domctl.c | 2 +-
xen/arch/arm/hvm.c | 2 +-
xen/arch/arm/mm.c | 2 +-
xen/arch/arm/physdev.c | 2 +-
xen/arch/arm/sysctl.c | 2 +-
xen/arch/x86/cpu/mcheck/mce.c | 2 +-
xen/arch/x86/domain.c | 2 +-
xen/arch/x86/domctl.c | 2 +-
xen/arch/x86/efi/runtime.c | 2 +-
xen/arch/x86/hvm/hvm.c | 26 +++++++++---------
xen/arch/x86/microcode.c | 2 +-
xen/arch/x86/mm.c | 14 +++++-----
xen/arch/x86/mm/hap/hap.c | 2 +-
xen/arch/x86/mm/mem_event.c | 2 +-
xen/arch/x86/mm/paging.c | 2 +-
xen/arch/x86/mm/shadow/common.c | 2 +-
xen/arch/x86/physdev.c | 2 +-
xen/arch/x86/platform_hypercall.c | 2 +-
xen/arch/x86/sysctl.c | 2 +-
xen/arch/x86/traps.c | 2 +-
xen/arch/x86/x86_32/mm.c | 2 +-
xen/arch/x86/x86_32/traps.c | 2 +-
xen/arch/x86/x86_64/compat/mm.c | 8 +++---
xen/arch/x86/x86_64/domain.c | 2 +-
xen/arch/x86/x86_64/mm.c | 2 +-
xen/arch/x86/x86_64/traps.c | 2 +-
xen/common/compat/domain.c | 2 +-
xen/common/compat/grant_table.c | 2 +-
xen/common/compat/memory.c | 2 +-
xen/common/domain.c | 2 +-
xen/common/domctl.c | 2 +-
xen/common/event_channel.c | 2 +-
xen/common/grant_table.c | 36 ++++++++++++------------
xen/common/kernel.c | 4 +-
xen/common/kexec.c | 16 +++++-----
xen/common/memory.c | 4 +-
xen/common/multicall.c | 2 +-
xen/common/schedule.c | 2 +-
xen/common/sysctl.c | 2 +-
xen/common/xenoprof.c | 8 +++---
xen/drivers/acpi/pmstat.c | 2 +-
xen/drivers/char/console.c | 6 ++--
xen/drivers/passthrough/iommu.c | 2 +-
xen/include/asm-arm/guest_access.h | 2 +-
xen/include/asm-arm/hypercall.h | 2 +-
xen/include/asm-arm/mm.h | 2 +-
xen/include/asm-x86/hap.h | 2 +-
xen/include/asm-x86/hypercall.h | 24 ++++++++--------
xen/include/asm-x86/mem_event.h | 2 +-
xen/include/asm-x86/mm.h | 8 +++---
xen/include/asm-x86/paging.h | 2 +-
xen/include/asm-x86/processor.h | 2 +-
xen/include/asm-x86/shadow.h | 2 +-
xen/include/asm-x86/xenoprof.h | 6 ++--
xen/include/xen/acpi.h | 4 +-
xen/include/xen/hypercall.h | 52 ++++++++++++++++++------------------
xen/include/xen/iommu.h | 2 +-
xen/include/xen/tmem_xen.h | 2 +-
xen/include/xsm/xsm.h | 4 +-
xen/xsm/dummy.c | 2 +-
xen/xsm/flask/flask_op.c | 4 +-
xen/xsm/flask/hooks.c | 2 +-
xen/xsm/xsm_core.c | 2 +-
64 files changed, 160 insertions(+), 160 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index ee58d68..07b50e2 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -515,7 +515,7 @@ void arch_dump_domain_info(struct domain *d)
{
}
-long arch_do_vcpu_op(int cmd, struct vcpu *v, XEN_GUEST_HANDLE(void) arg)
+long arch_do_vcpu_op(int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
{
return -ENOSYS;
}
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 1a5f79f..cf16791 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -11,7 +11,7 @@
#include <public/domctl.h>
long arch_do_domctl(struct xen_domctl *domctl,
- XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
+ XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
{
return -ENOSYS;
}
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index c11378d..40f519e 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -11,7 +11,7 @@
#include <asm/hypercall.h>
-long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg)
+long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
{
long rc = 0;
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 08bc55b..c9cc59f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -541,7 +541,7 @@ static int xenmem_add_to_physmap(struct domain *d,
return xenmem_add_to_physmap_once(d, xatp);
}
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
{
int rc;
diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
index bcf4337..0801e8c 100644
--- a/xen/arch/arm/physdev.c
+++ b/xen/arch/arm/physdev.c
@@ -11,7 +11,7 @@
#include <asm/hypercall.h>
-int do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
printk("%s %d cmd=%d: not implemented yet\n", __func__, __LINE__,
cmd);
return -ENOSYS;
diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
index e8e1c0d..a286abe 100644
--- a/xen/arch/arm/sysctl.c
+++ b/xen/arch/arm/sysctl.c
@@ -13,7 +13,7 @@
#include <public/sysctl.h>
long arch_do_sysctl(struct xen_sysctl *sysctl,
- XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl)
+ XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
{
return -ENOSYS;
}
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index ed76131..4b2e0c7 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1359,7 +1359,7 @@ CHECK_mcinfo_recovery;
#endif
/* Machine Check Architecture Hypercall */
-long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc)
+long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
{
long ret = 0;
struct xen_mc curop, *op = &curop;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 5bba4b9..13ff776 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1138,7 +1138,7 @@ map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned
offset)
long
arch_do_vcpu_op(
- int cmd, struct vcpu *v, XEN_GUEST_HANDLE(void) arg)
+ int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
{
long rc = 0;
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 135ea6e..663bfe4 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -48,7 +48,7 @@ static int gdbsx_guest_mem_io(
long arch_do_domctl(
struct xen_domctl *domctl,
- XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
+ XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
{
long ret = 0;
diff --git a/xen/arch/x86/efi/runtime.c b/xen/arch/x86/efi/runtime.c
index 1dbe2db..b2ff495 100644
--- a/xen/arch/x86/efi/runtime.c
+++ b/xen/arch/x86/efi/runtime.c
@@ -184,7 +184,7 @@ int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
return 0;
}
-static long gwstrlen(XEN_GUEST_HANDLE(CHAR16) str)
+static long gwstrlen(XEN_GUEST_HANDLE_PARAM(CHAR16) str)
{
unsigned long len;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 22c136b..bf97aea 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3041,14 +3041,14 @@ static int grant_table_op_is_allowed(unsigned int cmd)
}
static long hvm_grant_table_op(
- unsigned int cmd, XEN_GUEST_HANDLE(void) uop, unsigned int count)
+ unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
{
if ( !grant_table_op_is_allowed(cmd) )
return -ENOSYS; /* all other commands need auditing */
return do_grant_table_op(cmd, uop, count);
}
-static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
long rc;
@@ -3066,7 +3066,7 @@ static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE(void)
arg)
return do_memory_op(cmd, arg);
}
-static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
switch ( cmd )
{
@@ -3082,7 +3082,7 @@ static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE(void)
arg)
}
static long hvm_vcpu_op(
- int cmd, int vcpuid, XEN_GUEST_HANDLE(void) arg)
+ int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
{
long rc;
@@ -3131,7 +3131,7 @@ static hvm_hypercall_t
*hvm_hypercall32_table[NR_hypercalls] = {
#else /* defined(__x86_64__) */
static long hvm_grant_table_op_compat32(unsigned int cmd,
- XEN_GUEST_HANDLE(void) uop,
+ XEN_GUEST_HANDLE_PARAM(void) uop,
unsigned int count)
{
if ( !grant_table_op_is_allowed(cmd) )
@@ -3139,7 +3139,7 @@ static long hvm_grant_table_op_compat32(unsigned int cmd,
return compat_grant_table_op(cmd, uop, count);
}
-static long hvm_memory_op_compat32(int cmd, XEN_GUEST_HANDLE(void) arg)
+static long hvm_memory_op_compat32(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
int rc;
@@ -3158,7 +3158,7 @@ static long hvm_memory_op_compat32(int cmd,
XEN_GUEST_HANDLE(void) arg)
}
static long hvm_vcpu_op_compat32(
- int cmd, int vcpuid, XEN_GUEST_HANDLE(void) arg)
+ int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
{
long rc;
@@ -3182,7 +3182,7 @@ static long hvm_vcpu_op_compat32(
}
static long hvm_physdev_op_compat32(
- int cmd, XEN_GUEST_HANDLE(void) arg)
+ int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
switch ( cmd )
{
@@ -3354,7 +3354,7 @@ void hvm_hypercall_page_initialise(struct domain *d,
}
static int hvmop_set_pci_intx_level(
- XEN_GUEST_HANDLE(xen_hvm_set_pci_intx_level_t) uop)
+ XEN_GUEST_HANDLE_PARAM(xen_hvm_set_pci_intx_level_t) uop)
{
struct xen_hvm_set_pci_intx_level op;
struct domain *d;
@@ -3519,7 +3519,7 @@ static void hvm_s3_resume(struct domain *d)
}
static int hvmop_set_isa_irq_level(
- XEN_GUEST_HANDLE(xen_hvm_set_isa_irq_level_t) uop)
+ XEN_GUEST_HANDLE_PARAM(xen_hvm_set_isa_irq_level_t) uop)
{
struct xen_hvm_set_isa_irq_level op;
struct domain *d;
@@ -3563,7 +3563,7 @@ static int hvmop_set_isa_irq_level(
}
static int hvmop_set_pci_link_route(
- XEN_GUEST_HANDLE(xen_hvm_set_pci_link_route_t) uop)
+ XEN_GUEST_HANDLE_PARAM(xen_hvm_set_pci_link_route_t) uop)
{
struct xen_hvm_set_pci_link_route op;
struct domain *d;
@@ -3596,7 +3596,7 @@ static int hvmop_set_pci_link_route(
}
static int hvmop_inject_msi(
- XEN_GUEST_HANDLE(xen_hvm_inject_msi_t) uop)
+ XEN_GUEST_HANDLE_PARAM(xen_hvm_inject_msi_t) uop)
{
struct xen_hvm_inject_msi op;
struct domain *d;
@@ -3680,7 +3680,7 @@ static int hvm_replace_event_channel(struct vcpu *v,
domid_t remote_domid,
return 0;
}
-long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg)
+long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct domain *curr_d = current->domain;
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index bdda3f5..1477481 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -192,7 +192,7 @@ static long do_microcode_update(void *_info)
return error;
}
-int microcode_update(XEN_GUEST_HANDLE(const_void) buf, unsigned long len)
+int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
{
int ret;
struct microcode_info *info;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 9f63974..fd1c890 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2914,7 +2914,7 @@ static void put_pg_owner(struct domain *pg_owner)
}
static inline int vcpumask_to_pcpumask(
- struct domain *d, XEN_GUEST_HANDLE(const_void) bmap, cpumask_t *pmask)
+ struct domain *d, XEN_GUEST_HANDLE_PARAM(const_void) bmap, cpumask_t
*pmask)
{
unsigned int vcpu_id, vcpu_bias, offs;
unsigned long vmask;
@@ -2974,9 +2974,9 @@ static inline void fixunmap_domain_page(const void *ptr)
#endif
int do_mmuext_op(
- XEN_GUEST_HANDLE(mmuext_op_t) uops,
+ XEN_GUEST_HANDLE_PARAM(mmuext_op_t) uops,
unsigned int count,
- XEN_GUEST_HANDLE(uint) pdone,
+ XEN_GUEST_HANDLE_PARAM(uint) pdone,
unsigned int foreigndom)
{
struct mmuext_op op;
@@ -3438,9 +3438,9 @@ int do_mmuext_op(
}
int do_mmu_update(
- XEN_GUEST_HANDLE(mmu_update_t) ureqs,
+ XEN_GUEST_HANDLE_PARAM(mmu_update_t) ureqs,
unsigned int count,
- XEN_GUEST_HANDLE(uint) pdone,
+ XEN_GUEST_HANDLE_PARAM(uint) pdone,
unsigned int foreigndom)
{
struct mmu_update req;
@@ -4387,7 +4387,7 @@ long set_gdt(struct vcpu *v,
}
-long do_set_gdt(XEN_GUEST_HANDLE(ulong) frame_list, unsigned int entries)
+long do_set_gdt(XEN_GUEST_HANDLE_PARAM(ulong) frame_list, unsigned int entries)
{
int nr_pages = (entries + 511) / 512;
unsigned long frames[16];
@@ -4661,7 +4661,7 @@ static int xenmem_add_to_physmap(struct domain *d,
return xenmem_add_to_physmap_once(d, xatp);
}
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
{
int rc;
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 13b4be2..67e48a3 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -690,7 +690,7 @@ void hap_teardown(struct domain *d)
}
int hap_domctl(struct domain *d, xen_domctl_shadow_op_t *sc,
- XEN_GUEST_HANDLE(void) u_domctl)
+ XEN_GUEST_HANDLE_PARAM(void) u_domctl)
{
int rc, preempted = 0;
diff --git a/xen/arch/x86/mm/mem_event.c b/xen/arch/x86/mm/mem_event.c
index d728889..d3dac14 100644
--- a/xen/arch/x86/mm/mem_event.c
+++ b/xen/arch/x86/mm/mem_event.c
@@ -512,7 +512,7 @@ void mem_event_cleanup(struct domain *d)
}
int mem_event_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
- XEN_GUEST_HANDLE(void) u_domctl)
+ XEN_GUEST_HANDLE_PARAM(void) u_domctl)
{
int rc;
diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index ca879f9..ea44e39 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -654,7 +654,7 @@ void paging_vcpu_init(struct vcpu *v)
int paging_domctl(struct domain *d, xen_domctl_shadow_op_t *sc,
- XEN_GUEST_HANDLE(void) u_domctl)
+ XEN_GUEST_HANDLE_PARAM(void) u_domctl)
{
int rc;
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index dc245be..bd47f03 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -3786,7 +3786,7 @@ out:
int shadow_domctl(struct domain *d,
xen_domctl_shadow_op_t *sc,
- XEN_GUEST_HANDLE(void) u_domctl)
+ XEN_GUEST_HANDLE_PARAM(void) u_domctl)
{
int rc, preempted = 0;
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index b0458fd..b6474ef 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -255,7 +255,7 @@ int physdev_unmap_pirq(domid_t domid, int pirq)
}
#endif /* COMPAT */
-ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
int irq;
ret_t ret;
diff --git a/xen/arch/x86/platform_hypercall.c
b/xen/arch/x86/platform_hypercall.c
index 88880b0..a32e0a2 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -60,7 +60,7 @@ long cpu_down_helper(void *data);
long core_parking_helper(void *data);
uint32_t get_cur_idle_nums(void);
-ret_t do_platform_op(XEN_GUEST_HANDLE(xen_platform_op_t) u_xenpf_op)
+ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
{
ret_t ret = 0;
struct xen_platform_op curop, *op = &curop;
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 379f071..b84dd34 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -58,7 +58,7 @@ long cpu_down_helper(void *data)
}
long arch_do_sysctl(
- struct xen_sysctl *sysctl, XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl)
+ struct xen_sysctl *sysctl, XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
{
long ret = 0;
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 767be86..281d9e7 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3700,7 +3700,7 @@ int send_guest_trap(struct domain *d, uint16_t vcpuid,
unsigned int trap_nr)
}
-long do_set_trap_table(XEN_GUEST_HANDLE(const_trap_info_t) traps)
+long do_set_trap_table(XEN_GUEST_HANDLE_PARAM(const_trap_info_t) traps)
{
struct trap_info cur;
struct vcpu *curr = current;
diff --git a/xen/arch/x86/x86_32/mm.c b/xen/arch/x86/x86_32/mm.c
index 37efa3c..f6448fb 100644
--- a/xen/arch/x86/x86_32/mm.c
+++ b/xen/arch/x86/x86_32/mm.c
@@ -203,7 +203,7 @@ void __init subarch_init_memory(void)
}
}
-long subarch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+long subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct xen_machphys_mfn_list xmml;
unsigned long mfn, last_mfn;
diff --git a/xen/arch/x86/x86_32/traps.c b/xen/arch/x86/x86_32/traps.c
index 8f68808..0c7c860 100644
--- a/xen/arch/x86/x86_32/traps.c
+++ b/xen/arch/x86/x86_32/traps.c
@@ -492,7 +492,7 @@ static long unregister_guest_callback(struct
callback_unregister *unreg)
}
-long do_callback_op(int cmd, XEN_GUEST_HANDLE(const_void) arg)
+long do_callback_op(int cmd, XEN_GUEST_HANDLE_PARAM(const_void) arg)
{
long ret;
diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c
index f497503..88a07e8 100644
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -5,7 +5,7 @@
#include <asm/mem_event.h>
#include <asm/mem_sharing.h>
-int compat_set_gdt(XEN_GUEST_HANDLE(uint) frame_list, unsigned int entries)
+int compat_set_gdt(XEN_GUEST_HANDLE_PARAM(uint) frame_list, unsigned int
entries)
{
unsigned int i, nr_pages = (entries + 511) / 512;
unsigned long frames[16];
@@ -44,7 +44,7 @@ int compat_update_descriptor(u32 pa_lo, u32 pa_hi, u32
desc_lo, u32 desc_hi)
desc_lo | ((u64)desc_hi << 32));
}
-int compat_arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+int compat_arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct compat_machphys_mfn_list xmml;
l2_pgentry_t l2e;
@@ -260,9 +260,9 @@ int compat_update_va_mapping_otherdomain(unsigned long va,
u32 lo, u32 hi,
DEFINE_XEN_GUEST_HANDLE(mmuext_op_compat_t);
-int compat_mmuext_op(XEN_GUEST_HANDLE(mmuext_op_compat_t) cmp_uops,
+int compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t) cmp_uops,
unsigned int count,
- XEN_GUEST_HANDLE(uint) pdone,
+ XEN_GUEST_HANDLE_PARAM(uint) pdone,
unsigned int foreigndom)
{
unsigned int i, preempt_mask;
diff --git a/xen/arch/x86/x86_64/domain.c b/xen/arch/x86/x86_64/domain.c
index e746c89..144ca2d 100644
--- a/xen/arch/x86/x86_64/domain.c
+++ b/xen/arch/x86/x86_64/domain.c
@@ -23,7 +23,7 @@ CHECK_vcpu_get_physid;
int
arch_compat_vcpu_op(
- int cmd, struct vcpu *v, XEN_GUEST_HANDLE(void) arg)
+ int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
{
int rc = -ENOSYS;
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 635a499..17c46a1 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1043,7 +1043,7 @@ void __init subarch_init_memory(void)
}
}
-long subarch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+long subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct xen_machphys_mfn_list xmml;
l3_pgentry_t l3e;
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 806cf2e..6ead813 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -518,7 +518,7 @@ static long unregister_guest_callback(struct
callback_unregister *unreg)
}
-long do_callback_op(int cmd, XEN_GUEST_HANDLE(const_void) arg)
+long do_callback_op(int cmd, XEN_GUEST_HANDLE_PARAM(const_void) arg)
{
long ret;
diff --git a/xen/common/compat/domain.c b/xen/common/compat/domain.c
index 40a0287..e4c8ceb 100644
--- a/xen/common/compat/domain.c
+++ b/xen/common/compat/domain.c
@@ -15,7 +15,7 @@
CHECK_vcpu_set_periodic_timer;
#undef xen_vcpu_set_periodic_timer
-int compat_vcpu_op(int cmd, int vcpuid, XEN_GUEST_HANDLE(void) arg)
+int compat_vcpu_op(int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct domain *d = current->domain;
struct vcpu *v;
diff --git a/xen/common/compat/grant_table.c b/xen/common/compat/grant_table.c
index edd20c6..74a4733 100644
--- a/xen/common/compat/grant_table.c
+++ b/xen/common/compat/grant_table.c
@@ -52,7 +52,7 @@ CHECK_gnttab_swap_grant_ref;
#undef xen_gnttab_swap_grant_ref
int compat_grant_table_op(unsigned int cmd,
- XEN_GUEST_HANDLE(void) cmp_uop,
+ XEN_GUEST_HANDLE_PARAM(void) cmp_uop,
unsigned int count)
{
int rc = 0;
diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
index e7257cc..8e311ff 100644
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -13,7 +13,7 @@ CHECK_TYPE(domid);
#undef compat_domid_t
#undef xen_domid_t
-int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE(void) compat)
+int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat)
{
int rc, split, op = cmd & MEMOP_CMD_MASK;
unsigned int start_extent = cmd >> MEMOP_EXTENT_SHIFT;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 4c5d241..d7cd135 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -804,7 +804,7 @@ void vcpu_reset(struct vcpu *v)
}
-long do_vcpu_op(int cmd, int vcpuid, XEN_GUEST_HANDLE(void) arg)
+long do_vcpu_op(int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct domain *d = current->domain;
struct vcpu *v;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 7ca6b08..527c5ad 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -238,7 +238,7 @@ void domctl_lock_release(void)
spin_unlock(¤t->domain->hypercall_deadlock_mutex);
}
-long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
+long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
{
long ret = 0;
struct xen_domctl curop, *op = &curop;
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 53777f8..a80a0d1 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -970,7 +970,7 @@ out:
}
-long do_event_channel_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+long do_event_channel_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
long rc;
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 9961e83..d780dc6 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -771,7 +771,7 @@ __gnttab_map_grant_ref(
static long
gnttab_map_grant_ref(
- XEN_GUEST_HANDLE(gnttab_map_grant_ref_t) uop, unsigned int count)
+ XEN_GUEST_HANDLE_PARAM(gnttab_map_grant_ref_t) uop, unsigned int count)
{
int i;
struct gnttab_map_grant_ref op;
@@ -1040,7 +1040,7 @@ __gnttab_unmap_grant_ref(
static long
gnttab_unmap_grant_ref(
- XEN_GUEST_HANDLE(gnttab_unmap_grant_ref_t) uop, unsigned int count)
+ XEN_GUEST_HANDLE_PARAM(gnttab_unmap_grant_ref_t) uop, unsigned int count)
{
int i, c, partial_done, done = 0;
struct gnttab_unmap_grant_ref op;
@@ -1102,7 +1102,7 @@ __gnttab_unmap_and_replace(
static long
gnttab_unmap_and_replace(
- XEN_GUEST_HANDLE(gnttab_unmap_and_replace_t) uop, unsigned int count)
+ XEN_GUEST_HANDLE_PARAM(gnttab_unmap_and_replace_t) uop, unsigned int count)
{
int i, c, partial_done, done = 0;
struct gnttab_unmap_and_replace op;
@@ -1254,7 +1254,7 @@ active_alloc_failed:
static long
gnttab_setup_table(
- XEN_GUEST_HANDLE(gnttab_setup_table_t) uop, unsigned int count)
+ XEN_GUEST_HANDLE_PARAM(gnttab_setup_table_t) uop, unsigned int count)
{
struct gnttab_setup_table op;
struct domain *d;
@@ -1348,7 +1348,7 @@ gnttab_setup_table(
static long
gnttab_query_size(
- XEN_GUEST_HANDLE(gnttab_query_size_t) uop, unsigned int count)
+ XEN_GUEST_HANDLE_PARAM(gnttab_query_size_t) uop, unsigned int count)
{
struct gnttab_query_size op;
struct domain *d;
@@ -1485,7 +1485,7 @@ gnttab_prepare_for_transfer(
static long
gnttab_transfer(
- XEN_GUEST_HANDLE(gnttab_transfer_t) uop, unsigned int count)
+ XEN_GUEST_HANDLE_PARAM(gnttab_transfer_t) uop, unsigned int count)
{
struct domain *d = current->domain;
struct domain *e;
@@ -2082,7 +2082,7 @@ __gnttab_copy(
static long
gnttab_copy(
- XEN_GUEST_HANDLE(gnttab_copy_t) uop, unsigned int count)
+ XEN_GUEST_HANDLE_PARAM(gnttab_copy_t) uop, unsigned int count)
{
int i;
struct gnttab_copy op;
@@ -2101,7 +2101,7 @@ gnttab_copy(
}
static long
-gnttab_set_version(XEN_GUEST_HANDLE(gnttab_set_version_t uop))
+gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t uop))
{
gnttab_set_version_t op;
struct domain *d = current->domain;
@@ -2220,7 +2220,7 @@ out:
}
static long
-gnttab_get_status_frames(XEN_GUEST_HANDLE(gnttab_get_status_frames_t) uop,
+gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t)
uop,
int count)
{
gnttab_get_status_frames_t op;
@@ -2289,7 +2289,7 @@ out1:
}
static long
-gnttab_get_version(XEN_GUEST_HANDLE(gnttab_get_version_t uop))
+gnttab_get_version(XEN_GUEST_HANDLE_PARAM(gnttab_get_version_t uop))
{
gnttab_get_version_t op;
struct domain *d;
@@ -2368,7 +2368,7 @@ out:
}
static long
-gnttab_swap_grant_ref(XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t uop),
+gnttab_swap_grant_ref(XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t uop),
unsigned int count)
{
int i;
@@ -2389,7 +2389,7 @@
gnttab_swap_grant_ref(XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t uop),
long
do_grant_table_op(
- unsigned int cmd, XEN_GUEST_HANDLE(void) uop, unsigned int count)
+ unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
{
long rc;
@@ -2401,7 +2401,7 @@ do_grant_table_op(
{
case GNTTABOP_map_grant_ref:
{
- XEN_GUEST_HANDLE(gnttab_map_grant_ref_t) map +
XEN_GUEST_HANDLE_PARAM(gnttab_map_grant_ref_t) map
guest_handle_cast(uop, gnttab_map_grant_ref_t);
if ( unlikely(!guest_handle_okay(map, count)) )
goto out;
@@ -2415,7 +2415,7 @@ do_grant_table_op(
}
case GNTTABOP_unmap_grant_ref:
{
- XEN_GUEST_HANDLE(gnttab_unmap_grant_ref_t) unmap +
XEN_GUEST_HANDLE_PARAM(gnttab_unmap_grant_ref_t) unmap
guest_handle_cast(uop, gnttab_unmap_grant_ref_t);
if ( unlikely(!guest_handle_okay(unmap, count)) )
goto out;
@@ -2429,7 +2429,7 @@ do_grant_table_op(
}
case GNTTABOP_unmap_and_replace:
{
- XEN_GUEST_HANDLE(gnttab_unmap_and_replace_t) unmap +
XEN_GUEST_HANDLE_PARAM(gnttab_unmap_and_replace_t) unmap
guest_handle_cast(uop, gnttab_unmap_and_replace_t);
if ( unlikely(!guest_handle_okay(unmap, count)) )
goto out;
@@ -2453,7 +2453,7 @@ do_grant_table_op(
}
case GNTTABOP_transfer:
{
- XEN_GUEST_HANDLE(gnttab_transfer_t) transfer +
XEN_GUEST_HANDLE_PARAM(gnttab_transfer_t) transfer
guest_handle_cast(uop, gnttab_transfer_t);
if ( unlikely(!guest_handle_okay(transfer, count)) )
goto out;
@@ -2467,7 +2467,7 @@ do_grant_table_op(
}
case GNTTABOP_copy:
{
- XEN_GUEST_HANDLE(gnttab_copy_t) copy +
XEN_GUEST_HANDLE_PARAM(gnttab_copy_t) copy guest_handle_cast(uop,
gnttab_copy_t);
if ( unlikely(!guest_handle_okay(copy, count)) )
goto out;
@@ -2504,7 +2504,7 @@ do_grant_table_op(
}
case GNTTABOP_swap_grant_ref:
{
- XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t) swap +
XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t) swap
guest_handle_cast(uop, gnttab_swap_grant_ref_t);
if ( unlikely(!guest_handle_okay(swap, count)) )
goto out;
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index c915bbc..55caff6 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -204,7 +204,7 @@ void __init do_initcalls(void)
* Simple hypercalls.
*/
-DO(xen_version)(int cmd, XEN_GUEST_HANDLE(void) arg)
+DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
switch ( cmd )
{
@@ -332,7 +332,7 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE(void) arg)
return -ENOSYS;
}
-DO(nmi_op)(unsigned int cmd, XEN_GUEST_HANDLE(void) arg)
+DO(nmi_op)(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct xennmi_callback cb;
long rc = 0;
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 09a5624..03389eb 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -613,7 +613,7 @@ static int kexec_get_range_internal(xen_kexec_range_t
*range)
return ret;
}
-static int kexec_get_range(XEN_GUEST_HANDLE(void) uarg)
+static int kexec_get_range(XEN_GUEST_HANDLE_PARAM(void) uarg)
{
xen_kexec_range_t range;
int ret = -EINVAL;
@@ -629,7 +629,7 @@ static int kexec_get_range(XEN_GUEST_HANDLE(void) uarg)
return ret;
}
-static int kexec_get_range_compat(XEN_GUEST_HANDLE(void) uarg)
+static int kexec_get_range_compat(XEN_GUEST_HANDLE_PARAM(void) uarg)
{
#ifdef CONFIG_COMPAT
xen_kexec_range_t range;
@@ -777,7 +777,7 @@ static int kexec_load_unload_internal(unsigned long op,
xen_kexec_load_t *load)
return ret;
}
-static int kexec_load_unload(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
+static int kexec_load_unload(unsigned long op, XEN_GUEST_HANDLE_PARAM(void)
uarg)
{
xen_kexec_load_t load;
@@ -788,7 +788,7 @@ static int kexec_load_unload(unsigned long op,
XEN_GUEST_HANDLE(void) uarg)
}
static int kexec_load_unload_compat(unsigned long op,
- XEN_GUEST_HANDLE(void) uarg)
+ XEN_GUEST_HANDLE_PARAM(void) uarg)
{
#ifdef CONFIG_COMPAT
compat_kexec_load_t compat_load;
@@ -813,7 +813,7 @@ static int kexec_load_unload_compat(unsigned long op,
#endif /* CONFIG_COMPAT */
}
-static int kexec_exec(XEN_GUEST_HANDLE(void) uarg)
+static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
{
xen_kexec_exec_t exec;
xen_kexec_image_t *image;
@@ -845,7 +845,7 @@ static int kexec_exec(XEN_GUEST_HANDLE(void) uarg)
return -EINVAL; /* never reached */
}
-int do_kexec_op_internal(unsigned long op, XEN_GUEST_HANDLE(void) uarg,
+int do_kexec_op_internal(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) uarg,
int compat)
{
unsigned long flags;
@@ -886,13 +886,13 @@ int do_kexec_op_internal(unsigned long op,
XEN_GUEST_HANDLE(void) uarg,
return ret;
}
-long do_kexec_op(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
+long do_kexec_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) uarg)
{
return do_kexec_op_internal(op, uarg, 0);
}
#ifdef CONFIG_COMPAT
-int compat_kexec_op(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
+int compat_kexec_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) uarg)
{
return do_kexec_op_internal(op, uarg, 1);
}
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 5d64cb6..a126188 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -277,7 +277,7 @@ static void decrease_reservation(struct memop_args *a)
a->nr_done = i;
}
-static long memory_exchange(XEN_GUEST_HANDLE(xen_memory_exchange_t) arg)
+static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
{
struct xen_memory_exchange exch;
PAGE_LIST_HEAD(in_chunk_list);
@@ -530,7 +530,7 @@ static long
memory_exchange(XEN_GUEST_HANDLE(xen_memory_exchange_t) arg)
return rc;
}
-long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE(void) arg)
+long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct domain *d;
int rc, op;
diff --git a/xen/common/multicall.c b/xen/common/multicall.c
index 6c1a9d7..5de5f8d 100644
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -21,7 +21,7 @@ typedef long ret_t;
ret_t
do_multicall(
- XEN_GUEST_HANDLE(multicall_entry_t) call_list, unsigned int nr_calls)
+ XEN_GUEST_HANDLE_PARAM(multicall_entry_t) call_list, unsigned int nr_calls)
{
struct mc_state *mcs = ¤t->mc_state;
unsigned int i;
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 0854f55..c26eac4 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -836,7 +836,7 @@ typedef long ret_t;
#endif /* !COMPAT */
-ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
ret_t ret = 0;
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index ea68278..47142f4 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -27,7 +27,7 @@
#include <xsm/xsm.h>
#include <xen/pmstat.h>
-long do_sysctl(XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl)
+long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
{
long ret = 0;
struct xen_sysctl curop, *op = &curop;
diff --git a/xen/common/xenoprof.c b/xen/common/xenoprof.c
index e571fea..c001b38 100644
--- a/xen/common/xenoprof.c
+++ b/xen/common/xenoprof.c
@@ -404,7 +404,7 @@ static int add_active_list(domid_t domid)
return 0;
}
-static int add_passive_list(XEN_GUEST_HANDLE(void) arg)
+static int add_passive_list(XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct xenoprof_passive passive;
struct domain *d;
@@ -585,7 +585,7 @@ void xenoprof_log_event(struct vcpu *vcpu, const struct
cpu_user_regs *regs,
-static int xenoprof_op_init(XEN_GUEST_HANDLE(void) arg)
+static int xenoprof_op_init(XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct domain *d = current->domain;
struct xenoprof_init xenoprof_init;
@@ -609,7 +609,7 @@ static int xenoprof_op_init(XEN_GUEST_HANDLE(void) arg)
#endif /* !COMPAT */
-static int xenoprof_op_get_buffer(XEN_GUEST_HANDLE(void) arg)
+static int xenoprof_op_get_buffer(XEN_GUEST_HANDLE_PARAM(void) arg)
{
struct xenoprof_get_buffer xenoprof_get_buffer;
struct domain *d = current->domain;
@@ -660,7 +660,7 @@ static int xenoprof_op_get_buffer(XEN_GUEST_HANDLE(void)
arg)
|| (op == XENOPROF_disable_virq) \
|| (op == XENOPROF_get_buffer))
-int do_xenoprof_op(int op, XEN_GUEST_HANDLE(void) arg)
+int do_xenoprof_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
{
int ret = 0;
diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index 8788f01..2be1764 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -513,7 +513,7 @@ int do_pm_op(struct xen_sysctl_pm_op *op)
return ret;
}
-int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE(uint32) pdc)
+int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE_PARAM(uint32) pdc)
{
u32 bits[3];
int ret;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index e10bed5..b0f2334 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -182,7 +182,7 @@ static void putchar_console_ring(int c)
long read_console_ring(struct xen_sysctl_readconsole *op)
{
- XEN_GUEST_HANDLE(char) str;
+ XEN_GUEST_HANDLE_PARAM(char) str;
uint32_t idx, len, max, sofar, c;
str = guest_handle_cast(op->buffer, char),
@@ -320,7 +320,7 @@ static void notify_dom0_con_ring(unsigned long unused)
static DECLARE_SOFTIRQ_TASKLET(notify_dom0_con_ring_tasklet,
notify_dom0_con_ring, 0);
-static long guest_console_write(XEN_GUEST_HANDLE(char) buffer, int count)
+static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
{
char kbuf[128], *kptr;
int kcount;
@@ -358,7 +358,7 @@ static long guest_console_write(XEN_GUEST_HANDLE(char)
buffer, int count)
return 0;
}
-long do_console_io(int cmd, int count, XEN_GUEST_HANDLE(char) buffer)
+long do_console_io(int cmd, int count, XEN_GUEST_HANDLE_PARAM(char) buffer)
{
long rc;
unsigned int idx, len;
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 64f5fd1..396461f 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -518,7 +518,7 @@ void iommu_crash_shutdown(void)
int iommu_do_domctl(
struct xen_domctl *domctl,
- XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
+ XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
{
struct domain *d;
u16 seg;
diff --git a/xen/include/asm-arm/guest_access.h
b/xen/include/asm-arm/guest_access.h
index 7a955cb..bf5005b 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -30,7 +30,7 @@ unsigned long raw_clear_guest(void *to, unsigned len);
/* Cast a guest handle to the specified type of handle. */
#define guest_handle_cast(hnd, type) ({ \
type *_x = (hnd).p; \
- (XEN_GUEST_HANDLE(type)) { {_x } }; \
+ (XEN_GUEST_HANDLE_PARAM(type)) { _x }; \
})
#define guest_handle_from_ptr(ptr, type) \
diff --git a/xen/include/asm-arm/hypercall.h b/xen/include/asm-arm/hypercall.h
index 454f02e..090e620 100644
--- a/xen/include/asm-arm/hypercall.h
+++ b/xen/include/asm-arm/hypercall.h
@@ -2,7 +2,7 @@
#define __ASM_ARM_HYPERCALL_H__
#include <public/domctl.h> /* for arch_do_domctl */
-int do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg);
+int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
#endif /* __ASM_ARM_HYPERCALL_H__ */
/*
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index b37bd35..8bf45ba 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -267,7 +267,7 @@ static inline int relinquish_shared_pages(struct domain *d)
/* Arch-specific portion of memory_op hypercall. */
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg);
+long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
int steal_page(
struct domain *d, struct page_info *page, unsigned int memflags);
diff --git a/xen/include/asm-x86/hap.h b/xen/include/asm-x86/hap.h
index a2532a4..916a35b 100644
--- a/xen/include/asm-x86/hap.h
+++ b/xen/include/asm-x86/hap.h
@@ -51,7 +51,7 @@ hap_unmap_domain_page(void *p)
/************************************************/
void hap_domain_init(struct domain *d);
int hap_domctl(struct domain *d, xen_domctl_shadow_op_t *sc,
- XEN_GUEST_HANDLE(void) u_domctl);
+ XEN_GUEST_HANDLE_PARAM(void) u_domctl);
int hap_enable(struct domain *d, u32 mode);
void hap_final_teardown(struct domain *d);
void hap_teardown(struct domain *d);
diff --git a/xen/include/asm-x86/hypercall.h b/xen/include/asm-x86/hypercall.h
index 9e136c3..55b5ca2 100644
--- a/xen/include/asm-x86/hypercall.h
+++ b/xen/include/asm-x86/hypercall.h
@@ -18,22 +18,22 @@
extern long
do_event_channel_op_compat(
- XEN_GUEST_HANDLE(evtchn_op_t) uop);
+ XEN_GUEST_HANDLE_PARAM(evtchn_op_t) uop);
extern long
do_set_trap_table(
- XEN_GUEST_HANDLE(const_trap_info_t) traps);
+ XEN_GUEST_HANDLE_PARAM(const_trap_info_t) traps);
extern int
do_mmu_update(
- XEN_GUEST_HANDLE(mmu_update_t) ureqs,
+ XEN_GUEST_HANDLE_PARAM(mmu_update_t) ureqs,
unsigned int count,
- XEN_GUEST_HANDLE(uint) pdone,
+ XEN_GUEST_HANDLE_PARAM(uint) pdone,
unsigned int foreigndom);
extern long
do_set_gdt(
- XEN_GUEST_HANDLE(ulong) frame_list,
+ XEN_GUEST_HANDLE_PARAM(ulong) frame_list,
unsigned int entries);
extern long
@@ -60,7 +60,7 @@ do_update_descriptor(
u64 desc);
extern long
-do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc);
+do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc);
extern int
do_update_va_mapping(
@@ -70,7 +70,7 @@ do_update_va_mapping(
extern long
do_physdev_op(
- int cmd, XEN_GUEST_HANDLE(void) arg);
+ int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
extern int
do_update_va_mapping_otherdomain(
@@ -81,9 +81,9 @@ do_update_va_mapping_otherdomain(
extern int
do_mmuext_op(
- XEN_GUEST_HANDLE(mmuext_op_t) uops,
+ XEN_GUEST_HANDLE_PARAM(mmuext_op_t) uops,
unsigned int count,
- XEN_GUEST_HANDLE(uint) pdone,
+ XEN_GUEST_HANDLE_PARAM(uint) pdone,
unsigned int foreigndom);
extern unsigned long
@@ -92,7 +92,7 @@ do_iret(
extern int
do_kexec(
- unsigned long op, unsigned arg1, XEN_GUEST_HANDLE(void) uarg);
+ unsigned long op, unsigned arg1, XEN_GUEST_HANDLE_PARAM(void) uarg);
#ifdef __x86_64__
@@ -110,11 +110,11 @@ do_set_segment_base(
extern int
compat_physdev_op(
int cmd,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern int
arch_compat_vcpu_op(
- int cmd, struct vcpu *v, XEN_GUEST_HANDLE(void) arg);
+ int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg);
#else
diff --git a/xen/include/asm-x86/mem_event.h b/xen/include/asm-x86/mem_event.h
index 23d71c1..e17f36b 100644
--- a/xen/include/asm-x86/mem_event.h
+++ b/xen/include/asm-x86/mem_event.h
@@ -65,7 +65,7 @@ int mem_event_get_response(struct domain *d, struct
mem_event_domain *med,
struct domain *get_mem_event_op_target(uint32_t domain, int *rc);
int do_mem_event_op(int op, uint32_t domain, void *arg);
int mem_event_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
- XEN_GUEST_HANDLE(void) u_domctl);
+ XEN_GUEST_HANDLE_PARAM(void) u_domctl);
#endif /* __MEM_EVENT_H__ */
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index 4cba276..6373b3b 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -604,10 +604,10 @@ void *do_page_walk(struct vcpu *v, unsigned long addr);
int __sync_local_execstate(void);
/* Arch-specific portion of memory_op hypercall. */
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg);
-long subarch_memory_op(int op, XEN_GUEST_HANDLE(void) arg);
-int compat_arch_memory_op(int op, XEN_GUEST_HANDLE(void));
-int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE(void));
+long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
+long subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
+int compat_arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void));
+int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void));
int steal_page(
struct domain *d, struct page_info *page, unsigned int memflags);
diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index c432a97..1cd0e3f 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -215,7 +215,7 @@ int paging_domain_init(struct domain *d, unsigned int
domcr_flags);
* and disable ephemeral shadow modes (test mode and log-dirty mode) and
* manipulate the log-dirty bitmap. */
int paging_domctl(struct domain *d, xen_domctl_shadow_op_t *sc,
- XEN_GUEST_HANDLE(void) u_domctl);
+ XEN_GUEST_HANDLE_PARAM(void) u_domctl);
/* Call when destroying a domain */
void paging_teardown(struct domain *d);
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 7164a50..efdbddd 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -598,7 +598,7 @@ int rdmsr_hypervisor_regs(uint32_t idx, uint64_t *val);
int wrmsr_hypervisor_regs(uint32_t idx, uint64_t val);
void microcode_set_module(unsigned int);
-int microcode_update(XEN_GUEST_HANDLE(const_void), unsigned long len);
+int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len);
int microcode_resume_cpu(int cpu);
unsigned long *get_x86_gpr(struct cpu_user_regs *regs, unsigned int modrm_reg);
diff --git a/xen/include/asm-x86/shadow.h b/xen/include/asm-x86/shadow.h
index 88a8cd2..2eb6efc 100644
--- a/xen/include/asm-x86/shadow.h
+++ b/xen/include/asm-x86/shadow.h
@@ -73,7 +73,7 @@ int shadow_track_dirty_vram(struct domain *d,
* manipulate the log-dirty bitmap. */
int shadow_domctl(struct domain *d,
xen_domctl_shadow_op_t *sc,
- XEN_GUEST_HANDLE(void) u_domctl);
+ XEN_GUEST_HANDLE_PARAM(void) u_domctl);
/* Call when destroying a domain */
void shadow_teardown(struct domain *d);
diff --git a/xen/include/asm-x86/xenoprof.h b/xen/include/asm-x86/xenoprof.h
index c03f8c8..3f5ea15 100644
--- a/xen/include/asm-x86/xenoprof.h
+++ b/xen/include/asm-x86/xenoprof.h
@@ -40,9 +40,9 @@ int xenoprof_arch_init(int *num_events, char *cpu_type);
#define xenoprof_arch_disable_virq() nmi_disable_virq()
#define xenoprof_arch_release_counters() nmi_release_counters()
-int xenoprof_arch_counter(XEN_GUEST_HANDLE(void) arg);
-int compat_oprof_arch_counter(XEN_GUEST_HANDLE(void) arg);
-int xenoprof_arch_ibs_counter(XEN_GUEST_HANDLE(void) arg);
+int xenoprof_arch_counter(XEN_GUEST_HANDLE_PARAM(void) arg);
+int compat_oprof_arch_counter(XEN_GUEST_HANDLE_PARAM(void) arg);
+int xenoprof_arch_ibs_counter(XEN_GUEST_HANDLE_PARAM(void) arg);
struct vcpu;
struct cpu_user_regs;
diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
index d7e2f94..8f3cdca 100644
--- a/xen/include/xen/acpi.h
+++ b/xen/include/xen/acpi.h
@@ -145,8 +145,8 @@ static inline unsigned int acpi_get_cstate_limit(void) {
return 0; }
static inline void acpi_set_cstate_limit(unsigned int new_limit) { return; }
#endif
-#ifdef XEN_GUEST_HANDLE
-int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE(uint32));
+#ifdef XEN_GUEST_HANDLE_PARAM
+int acpi_set_pdc_bits(u32 acpi_id, XEN_GUEST_HANDLE_PARAM(uint32));
#endif
int arch_acpi_set_pdc_bits(u32 acpi_id, u32 *, u32 mask);
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index 73b1598..e335037 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -29,29 +29,29 @@ do_sched_op_compat(
extern long
do_sched_op(
int cmd,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern long
do_domctl(
- XEN_GUEST_HANDLE(xen_domctl_t) u_domctl);
+ XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl);
extern long
arch_do_domctl(
struct xen_domctl *domctl,
- XEN_GUEST_HANDLE(xen_domctl_t) u_domctl);
+ XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl);
extern long
do_sysctl(
- XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl);
+ XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl);
extern long
arch_do_sysctl(
struct xen_sysctl *sysctl,
- XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl);
+ XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl);
extern long
do_platform_op(
- XEN_GUEST_HANDLE(xen_platform_op_t) u_xenpf_op);
+ XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
/*
* To allow safe resume of do_memory_op() after preemption, we need to know
@@ -64,11 +64,11 @@ do_platform_op(
extern long
do_memory_op(
unsigned long cmd,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern long
do_multicall(
- XEN_GUEST_HANDLE(multicall_entry_t) call_list,
+ XEN_GUEST_HANDLE_PARAM(multicall_entry_t) call_list,
unsigned int nr_calls);
extern long
@@ -77,23 +77,23 @@ do_set_timer_op(
extern long
do_event_channel_op(
- int cmd, XEN_GUEST_HANDLE(void) arg);
+ int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
extern long
do_xen_version(
int cmd,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern long
do_console_io(
int cmd,
int count,
- XEN_GUEST_HANDLE(char) buffer);
+ XEN_GUEST_HANDLE_PARAM(char) buffer);
extern long
do_grant_table_op(
unsigned int cmd,
- XEN_GUEST_HANDLE(void) uop,
+ XEN_GUEST_HANDLE_PARAM(void) uop,
unsigned int count);
extern long
@@ -105,72 +105,72 @@ extern long
do_vcpu_op(
int cmd,
int vcpuid,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
struct vcpu;
extern long
arch_do_vcpu_op(int cmd,
struct vcpu *v,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern long
do_nmi_op(
unsigned int cmd,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern long
do_hvm_op(
unsigned long op,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern long
do_kexec_op(
unsigned long op,
int arg1,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern long
do_xsm_op(
- XEN_GUEST_HANDLE(xsm_op_t) u_xsm_op);
+ XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_xsm_op);
extern long
do_tmem_op(
- XEN_GUEST_HANDLE(tmem_op_t) uops);
+ XEN_GUEST_HANDLE_PARAM(tmem_op_t) uops);
extern int
-do_xenoprof_op(int op, XEN_GUEST_HANDLE(void) arg);
+do_xenoprof_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
#ifdef CONFIG_COMPAT
extern int
compat_memory_op(
unsigned int cmd,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern int
compat_grant_table_op(
unsigned int cmd,
- XEN_GUEST_HANDLE(void) uop,
+ XEN_GUEST_HANDLE_PARAM(void) uop,
unsigned int count);
extern int
compat_vcpu_op(
int cmd,
int vcpuid,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern int
-compat_xenoprof_op(int op, XEN_GUEST_HANDLE(void) arg);
+compat_xenoprof_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
extern int
compat_xen_version(
int cmd,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern int
compat_sched_op(
int cmd,
- XEN_GUEST_HANDLE(void) arg);
+ XEN_GUEST_HANDLE_PARAM(void) arg);
extern int
compat_set_timer_op(
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 6f7fbf7..bd19e23 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -155,7 +155,7 @@ void iommu_crash_shutdown(void);
void iommu_set_dom0_mapping(struct domain *d);
void iommu_share_p2m_table(struct domain *d);
-int iommu_do_domctl(struct xen_domctl *, XEN_GUEST_HANDLE(xen_domctl_t));
+int iommu_do_domctl(struct xen_domctl *, XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
void iommu_iotlb_flush(struct domain *d, unsigned long gfn, unsigned int
page_count);
void iommu_iotlb_flush_all(struct domain *d);
diff --git a/xen/include/xen/tmem_xen.h b/xen/include/xen/tmem_xen.h
index 4a35760..2e7199a 100644
--- a/xen/include/xen/tmem_xen.h
+++ b/xen/include/xen/tmem_xen.h
@@ -448,7 +448,7 @@ static inline void tmh_tze_copy_from_pfp(void *tva, pfp_t
*pfp, pagesize_t len)
typedef XEN_GUEST_HANDLE(void) cli_mfn_t;
typedef XEN_GUEST_HANDLE(char) cli_va_t;
*/
-typedef XEN_GUEST_HANDLE(tmem_op_t) tmem_cli_op_t;
+typedef XEN_GUEST_HANDLE_PARAM(tmem_op_t) tmem_cli_op_t;
static inline int tmh_get_tmemop_from_client(tmem_op_t *op, tmem_cli_op_t uops)
{
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index bef79df..3e4a47f 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -139,7 +139,7 @@ struct xsm_operations {
int (*cpupool_op)(void);
int (*sched_op)(void);
- long (*__do_xsm_op) (XEN_GUEST_HANDLE(xsm_op_t) op);
+ long (*__do_xsm_op) (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op);
#ifdef CONFIG_X86
int (*shadow_control) (struct domain *d, uint32_t op);
@@ -585,7 +585,7 @@ static inline int xsm_sched_op(void)
return xsm_call(sched_op());
}
-static inline long __do_xsm_op (XEN_GUEST_HANDLE(xsm_op_t) op)
+static inline long __do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
{
#ifdef XSM_ENABLE
return xsm_ops->__do_xsm_op(op);
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 7027ee7..5ef6529 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -365,7 +365,7 @@ static int dummy_sched_op (void)
return 0;
}
-static long dummy___do_xsm_op(XEN_GUEST_HANDLE(xsm_op_t) op)
+static long dummy___do_xsm_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
{
return -ENOSYS;
}
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index bd4db37..23e7d34 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -71,7 +71,7 @@ static int domain_has_security(struct domain *d, u32 perms)
perms, NULL);
}
-static int flask_copyin_string(XEN_GUEST_HANDLE(char) u_buf, char **buf,
uint32_t size)
+static int flask_copyin_string(XEN_GUEST_HANDLE_PARAM(char) u_buf, char **buf,
uint32_t size)
{
char *tmp = xmalloc_bytes(size + 1);
if ( !tmp )
@@ -573,7 +573,7 @@ static int flask_get_peer_sid(struct xen_flask_peersid *arg)
return rv;
}
-long do_flask_op(XEN_GUEST_HANDLE(xsm_op_t) u_flask_op)
+long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
{
xen_flask_op_t op;
int rv;
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 23b84f3..0fc299c 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1553,7 +1553,7 @@ static int flask_vcpuextstate (struct domain *d, uint32_t
cmd)
}
#endif
-long do_flask_op(XEN_GUEST_HANDLE(xsm_op_t) u_flask_op);
+long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
static struct xsm_operations flask_ops = {
.security_domaininfo = flask_security_domaininfo,
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 96c8669..46287cb 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -111,7 +111,7 @@ int unregister_xsm(struct xsm_operations *ops)
#endif
-long do_xsm_op (XEN_GUEST_HANDLE(xsm_op_t) op)
+long do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
{
return __do_xsm_op(op);
}
--
1.7.2.5
Konrad Rzeszutek Wilk
2012-Aug-06 14:24 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On Mon, Aug 06, 2012 at 03:12:01PM +0100, Stefano Stabellini wrote:> This is an incremental patch on top of > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary > compatibility, it is better to introduce foreign_domid as part of a > union containing both size and foreign_domid. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/include/public/memory.h | 11 +++++++---- > 1 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > index b2adfbe..b0af2fd 100644 > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -208,8 +208,12 @@ struct xen_add_to_physmap { > /* Which domain to change the mapping for. */ > domid_t domid; > > - /* Number of pages to go through for gmfn_range */ > - uint16_t size; > + union { > + /* Number of pages to go through for gmfn_range */ > + uint16_t size; > + /* IFF gmfn_foreign */ > + domid_t foreign_domid; > + }; > > /* Source mapping space. */ > #define XENMAPSPACE_shared_info 0 /* shared info page */ > @@ -217,8 +221,7 @@ struct xen_add_to_physmap { > #define XENMAPSPACE_gmfn 2 /* GMFN */ > #define XENMAPSPACE_gmfn_range 3 /* GMFN range */ > #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */ > - uint16_t space; > - domid_t foreign_domid; /* IFF gmfn_foreign */ > + unsigned int space;Should this be of uint32... ?> > #define XENMAPIDX_grant_table_status 0x80000000 > > -- > 1.7.2.5 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Stefano Stabellini
2012-Aug-06 14:38 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On Mon, 6 Aug 2012, Konrad Rzeszutek Wilk wrote:> On Mon, Aug 06, 2012 at 03:12:01PM +0100, Stefano Stabellini wrote: > > This is an incremental patch on top of > > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary > > compatibility, it is better to introduce foreign_domid as part of a > > union containing both size and foreign_domid. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > xen/include/public/memory.h | 11 +++++++---- > > 1 files changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > > index b2adfbe..b0af2fd 100644 > > --- a/xen/include/public/memory.h > > +++ b/xen/include/public/memory.h > > @@ -208,8 +208,12 @@ struct xen_add_to_physmap { > > /* Which domain to change the mapping for. */ > > domid_t domid; > > > > - /* Number of pages to go through for gmfn_range */ > > - uint16_t size; > > + union { > > + /* Number of pages to go through for gmfn_range */ > > + uint16_t size; > > + /* IFF gmfn_foreign */ > > + domid_t foreign_domid; > > + }; > > > > /* Source mapping space. */ > > #define XENMAPSPACE_shared_info 0 /* shared info page */ > > @@ -217,8 +221,7 @@ struct xen_add_to_physmap { > > #define XENMAPSPACE_gmfn 2 /* GMFN */ > > #define XENMAPSPACE_gmfn_range 3 /* GMFN range */ > > #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */ > > - uint16_t space; > > - domid_t foreign_domid; /* IFF gmfn_foreign */ > > + unsigned int space; > > Should this be of uint32... ?Nope: this patch is a fix for: diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h index 86d02c8..b2adfbe 100644 --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -212,11 +212,13 @@ struct xen_add_to_physmap { uint16_t size; /* Source mapping space. */ -#define XENMAPSPACE_shared_info 0 /* shared info page */ -#define XENMAPSPACE_grant_table 1 /* grant table page */ -#define XENMAPSPACE_gmfn 2 /* GMFN */ -#define XENMAPSPACE_gmfn_range 3 /* GMFN range */ - unsigned int space; +#define XENMAPSPACE_shared_info 0 /* shared info page */ +#define XENMAPSPACE_grant_table 1 /* grant table page */ +#define XENMAPSPACE_gmfn 2 /* GMFN */ +#define XENMAPSPACE_gmfn_range 3 /* GMFN range */ +#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */ + uint16_t space; + domid_t foreign_domid; /* IFF gmfn_foreign */ #define XENMAPIDX_grant_table_status 0x80000000 --- this is not upstream yet. So it brings space back to its original self.
On 06/08/12 15:11, Stefano Stabellini wrote:> Hi all, > this patch series makes the necessary changes to make sure that the > current ARM hypercall ABI can be used as-is on 64 bit ARM platforms: > > - it defines xen_ulong_t as uint64_t on ARM; > - it introduces a new macro to handle guest pointers, called > XEN_GUEST_HANDLE_PARAM (that has size 4 bytes on aarch and is going to > have size 8 bytes on aarch64); > - it replaces all the occurrences of XEN_GUEST_HANDLE in hypercall > parameters with XEN_GUEST_HANDLE_PARAM.This is a subtle (and undocumented!) distinction. I can see people adding/modifying hypercall etc. getting this wrong and no one noticing for a while (since it doesn''t affect x86). The xen_ulong_t parameters (when used for pointers) from an aarch guest point of view are a uint32_t guest pointer and uint32_t of padding. So the guest handles will be the same size in hypercall parameters and structure members. David> On x86 and ia64 things should stay exactly the same. > > On ARM all the unsigned long and the guest pointers that are members of > a struct become size 8 byte (both aarch and aarch64). > However guest pointers that are passed as hypercall arguments in > registers are going to be 4 bytes on aarch and 8 bytes on aarch64. > > It is based on Ian''s arm-for-4.3 branch. > > > Stefano Stabellini (5): > xen: improve changes to xen_add_to_physmap > xen/arm: introduce __lshrdi3 and __aeabi_llsr > xen: few more xen_ulong_t substitutions > xen: introduce XEN_GUEST_HANDLE_PARAM > xen: replace XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when appropriate > > > xen/arch/arm/domain.c | 2 +- > xen/arch/arm/domctl.c | 2 +- > xen/arch/arm/hvm.c | 2 +- > xen/arch/arm/lib/Makefile | 2 +- > xen/arch/arm/lib/lshrdi3.S | 54 ++++++++++++++++++++++++++++++++++++ > xen/arch/arm/mm.c | 2 +- > xen/arch/arm/physdev.c | 2 +- > xen/arch/arm/sysctl.c | 2 +- > xen/arch/x86/cpu/mcheck/mce.c | 2 +- > xen/arch/x86/domain.c | 2 +- > xen/arch/x86/domctl.c | 2 +- > xen/arch/x86/efi/runtime.c | 2 +- > xen/arch/x86/hvm/hvm.c | 26 ++++++++-------- > xen/arch/x86/microcode.c | 2 +- > xen/arch/x86/mm.c | 14 ++++---- > xen/arch/x86/mm/hap/hap.c | 2 +- > xen/arch/x86/mm/mem_event.c | 2 +- > xen/arch/x86/mm/paging.c | 2 +- > xen/arch/x86/mm/shadow/common.c | 2 +- > xen/arch/x86/physdev.c | 2 +- > xen/arch/x86/platform_hypercall.c | 2 +- > xen/arch/x86/sysctl.c | 2 +- > xen/arch/x86/traps.c | 2 +- > xen/arch/x86/x86_32/mm.c | 2 +- > xen/arch/x86/x86_32/traps.c | 2 +- > xen/arch/x86/x86_64/compat/mm.c | 8 ++-- > xen/arch/x86/x86_64/domain.c | 2 +- > xen/arch/x86/x86_64/mm.c | 2 +- > xen/arch/x86/x86_64/traps.c | 2 +- > xen/common/compat/domain.c | 2 +- > xen/common/compat/grant_table.c | 2 +- > xen/common/compat/memory.c | 2 +- > xen/common/domain.c | 2 +- > xen/common/domctl.c | 2 +- > xen/common/event_channel.c | 2 +- > xen/common/grant_table.c | 36 ++++++++++++------------ > xen/common/kernel.c | 4 +- > xen/common/kexec.c | 16 +++++----- > xen/common/memory.c | 4 +- > xen/common/multicall.c | 2 +- > xen/common/schedule.c | 2 +- > xen/common/sysctl.c | 2 +- > xen/common/xenoprof.c | 8 ++-- > xen/drivers/acpi/pmstat.c | 2 +- > xen/drivers/char/console.c | 6 ++-- > xen/drivers/passthrough/iommu.c | 2 +- > xen/include/asm-arm/guest_access.h | 2 +- > xen/include/asm-arm/hypercall.h | 2 +- > xen/include/asm-arm/mm.h | 2 +- > xen/include/asm-x86/hap.h | 2 +- > xen/include/asm-x86/hypercall.h | 24 ++++++++-------- > xen/include/asm-x86/mem_event.h | 2 +- > xen/include/asm-x86/mm.h | 8 ++-- > xen/include/asm-x86/paging.h | 2 +- > xen/include/asm-x86/processor.h | 2 +- > xen/include/asm-x86/shadow.h | 2 +- > xen/include/asm-x86/xenoprof.h | 6 ++-- > xen/include/public/arch-arm.h | 21 ++++++++++---- > xen/include/public/arch-ia64.h | 1 + > xen/include/public/arch-x86/xen.h | 1 + > xen/include/public/memory.h | 11 ++++-- > xen/include/public/physdev.h | 2 +- > xen/include/public/version.h | 2 +- > xen/include/public/xen.h | 4 +- > xen/include/xen/acpi.h | 4 +- > xen/include/xen/hypercall.h | 52 +++++++++++++++++----------------- > xen/include/xen/iommu.h | 2 +- > xen/include/xen/tmem_xen.h | 2 +- > xen/include/xsm/xsm.h | 4 +- > xen/xsm/dummy.c | 2 +- > xen/xsm/flask/flask_op.c | 4 +- > xen/xsm/flask/hooks.c | 2 +- > xen/xsm/xsm_core.c | 2 +- > 73 files changed, 243 insertions(+), 175 deletions(-) > > > Cheers, > > Stefano > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Mon, 6 Aug 2012, David Vrabel wrote:> On 06/08/12 15:11, Stefano Stabellini wrote: > > Hi all, > > this patch series makes the necessary changes to make sure that the > > current ARM hypercall ABI can be used as-is on 64 bit ARM platforms: > > > > - it defines xen_ulong_t as uint64_t on ARM; > > - it introduces a new macro to handle guest pointers, called > > XEN_GUEST_HANDLE_PARAM (that has size 4 bytes on aarch and is going to > > have size 8 bytes on aarch64); > > - it replaces all the occurrences of XEN_GUEST_HANDLE in hypercall > > parameters with XEN_GUEST_HANDLE_PARAM. > > This is a subtle (and undocumented!) distinction. I can see people > adding/modifying hypercall etc. getting this wrong and no one noticing > for a while (since it doesn''t affect x86).Where should I document this? I wrote it into the commit message but maybe a doc under docs is better.> The xen_ulong_t parameters (when used for pointers) from an aarch guest > point of view are a uint32_t guest pointer and uint32_t of padding. So > the guest handles will be the same size in hypercall parameters and > structure members.I changed xen_ulong_t to be 64 bit on ARM
On Mon, 6 Aug 2012, Stefano Stabellini wrote:> On Mon, 6 Aug 2012, David Vrabel wrote: > > On 06/08/12 15:11, Stefano Stabellini wrote: > > > Hi all, > > > this patch series makes the necessary changes to make sure that the > > > current ARM hypercall ABI can be used as-is on 64 bit ARM platforms: > > > > > > - it defines xen_ulong_t as uint64_t on ARM; > > > - it introduces a new macro to handle guest pointers, called > > > XEN_GUEST_HANDLE_PARAM (that has size 4 bytes on aarch and is going to > > > have size 8 bytes on aarch64); > > > - it replaces all the occurrences of XEN_GUEST_HANDLE in hypercall > > > parameters with XEN_GUEST_HANDLE_PARAM. > > > > This is a subtle (and undocumented!) distinction. I can see people > > adding/modifying hypercall etc. getting this wrong and no one noticing > > for a while (since it doesn''t affect x86). > > Where should I document this? I wrote it into the commit message but > maybe a doc under docs is better. > > > > The xen_ulong_t parameters (when used for pointers) from an aarch guest > > point of view are a uint32_t guest pointer and uint32_t of padding. So > > the guest handles will be the same size in hypercall parameters and > > structure members. > > I changed xen_ulong_t to be 64 bit on ARM >Sorry, pressed "send" too soon. I don''t think there are any xen_ulong_t used for pointers in hypercall parameters, at least there are none in the hypercalls we are using so far.
On 06/08/12 15:44, Stefano Stabellini wrote:> On Mon, 6 Aug 2012, David Vrabel wrote: >> On 06/08/12 15:11, Stefano Stabellini wrote: >>> Hi all, >>> this patch series makes the necessary changes to make sure that the >>> current ARM hypercall ABI can be used as-is on 64 bit ARM platforms: >>> >>> - it defines xen_ulong_t as uint64_t on ARM; >>> - it introduces a new macro to handle guest pointers, called >>> XEN_GUEST_HANDLE_PARAM (that has size 4 bytes on aarch and is going to >>> have size 8 bytes on aarch64); >>> - it replaces all the occurrences of XEN_GUEST_HANDLE in hypercall >>> parameters with XEN_GUEST_HANDLE_PARAM. >> >> This is a subtle (and undocumented!) distinction. I can see people >> adding/modifying hypercall etc. getting this wrong and no one noticing >> for a while (since it doesn''t affect x86). > > Where should I document this? I wrote it into the commit message but > maybe a doc under docs is better.A comment next to the #define of the two macros?>> The xen_ulong_t parameters (when used for pointers) from an aarch guest >> point of view are a uint32_t guest pointer and uint32_t of padding. So >> the guest handles will be the same size in hypercall parameters and >> structure members.Ignore this. Dunno what I was thinking. David
Jan Beulich
2012-Aug-06 15:32 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
>>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>wrote:> This is an incremental patch on top of > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary > compatibility, it is better to introduce foreign_domid as part of a > union containing both size and foreign_domid. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/include/public/memory.h | 11 +++++++---- > 1 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > index b2adfbe..b0af2fd 100644 > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -208,8 +208,12 @@ struct xen_add_to_physmap { > /* Which domain to change the mapping for. */ > domid_t domid; > > - /* Number of pages to go through for gmfn_range */ > - uint16_t size; > + union { > + /* Number of pages to go through for gmfn_range */ > + uint16_t size; > + /* IFF gmfn_foreign */ > + domid_t foreign_domid; > + };But you''re clear that this isn''t standard C, and hence can''t go in this way? Jan> /* Source mapping space. */ > #define XENMAPSPACE_shared_info 0 /* shared info page */ > @@ -217,8 +221,7 @@ struct xen_add_to_physmap { > #define XENMAPSPACE_gmfn 2 /* GMFN */ > #define XENMAPSPACE_gmfn_range 3 /* GMFN range */ > #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */ > - uint16_t space; > - domid_t foreign_domid; /* IFF gmfn_foreign */ > + unsigned int space; > > #define XENMAPIDX_grant_table_status 0x80000000 > > -- > 1.7.2.5 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
>>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > There are still few unsigned long in the xen public interface: replace > them with xen_ulong_t. > > Also typedef xen_ulong_t to uint64_t on ARM.While this change by itself already looks suspicious to me, I don''t follow what the global replacement is going to be good for, in particular when done in places that ARM should be completely ignorant of, e.g.> --- a/xen/include/public/physdev.h > +++ b/xen/include/public/physdev.h > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t); > #define PHYSDEVOP_apic_write 9 > struct physdev_apic { > /* IN */ > - unsigned long apic_physbase; > + xen_ulong_t apic_physbase; > uint32_t reg; > /* IN or OUT */ > uint32_t value; >... > --- a/xen/include/public/xen.h > +++ b/xen/include/public/xen.h > @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t); > * NB. The fields are natural register size for this architecture. > */ > struct multicall_entry { > - unsigned long op, result; > - unsigned long args[6]; > + xen_ulong_t op, result; > + xen_ulong_t args[6];And here I really start to wonder - what use is it to put all 64-bit values here on a 32-bit arch? You certainly know a lot more about ARM than me, but this looks pretty inefficient, the more that you''ll have to deal with checking the full values when converting to e.g. pointers anyway, in order to avoid behavioral differences between running on a 32- or 64-bit host. Zero-extending from 32-bits when in a 64-bit hypervisor wouldn''t have this problem. Jan> }; > typedef struct multicall_entry multicall_entry_t; > DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);
Stefano Stabellini
2012-Aug-06 15:43 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On Mon, 6 Aug 2012, Jan Beulich wrote:> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: > > This is an incremental patch on top of > > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary > > compatibility, it is better to introduce foreign_domid as part of a > > union containing both size and foreign_domid. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > xen/include/public/memory.h | 11 +++++++---- > > 1 files changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > > index b2adfbe..b0af2fd 100644 > > --- a/xen/include/public/memory.h > > +++ b/xen/include/public/memory.h > > @@ -208,8 +208,12 @@ struct xen_add_to_physmap { > > /* Which domain to change the mapping for. */ > > domid_t domid; > > > > - /* Number of pages to go through for gmfn_range */ > > - uint16_t size; > > + union { > > + /* Number of pages to go through for gmfn_range */ > > + uint16_t size; > > + /* IFF gmfn_foreign */ > > + domid_t foreign_domid; > > + }; > > But you''re clear that this isn''t standard C, and hence can''t go > in this way? >Why? It is c11 if I am not mistaken.
>>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > Note: this change does not make any difference on x86 and ia64. > > > XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers > stored in memory from guest pointers as hypercall parameters.I have to admit that really dislike this, to a large part because of the follow up patch that clutters the corresponding function declarations even further. Plus I see no mechanism to convert between the two, yet I can''t see how - long term at least - you could get away without such conversion. Is it really a well thought through and settled upon decision to make guest handles 64 bits wide even on 32-bit ARM? After all, both x86 and PPC got away without doing so (and I think your xen_ulong_t swipe would have broken PPC quite badly). Jan> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/include/asm-arm/guest_access.h | 2 +- > xen/include/public/arch-arm.h | 17 +++++++++++++---- > xen/include/public/arch-ia64.h | 1 + > xen/include/public/arch-x86/xen.h | 1 + > 4 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/xen/include/asm-arm/guest_access.h > b/xen/include/asm-arm/guest_access.h > index 0fceae6..7a955cb 100644 > --- a/xen/include/asm-arm/guest_access.h > +++ b/xen/include/asm-arm/guest_access.h > @@ -30,7 +30,7 @@ unsigned long raw_clear_guest(void *to, unsigned len); > /* Cast a guest handle to the specified type of handle. */ > #define guest_handle_cast(hnd, type) ({ \ > type *_x = (hnd).p; \ > - (XEN_GUEST_HANDLE(type)) { _x }; \ > + (XEN_GUEST_HANDLE(type)) { {_x } }; \ > }) > > #define guest_handle_from_ptr(ptr, type) \ > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index 2ae6548..d17d645 100644 > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -51,18 +51,27 @@ > > #define XEN_HYPERCALL_TAG 0XEA1 > > +#define uint64_aligned_t uint64_t __attribute__((aligned(8))) > > #ifndef __ASSEMBLY__ > -#define ___DEFINE_XEN_GUEST_HANDLE(name, type) \ > - typedef struct { type *p; } __guest_handle_ ## name > +#define ___DEFINE_XEN_GUEST_HANDLE(name, type) \ > + typedef struct { type *p; } \ > + __guest_handle_ ## name; \ > + typedef struct { union { type *p; uint64_aligned_t q; }; } \ > + __guest_handle_64_ ## name; > > #define __DEFINE_XEN_GUEST_HANDLE(name, type) \ > ___DEFINE_XEN_GUEST_HANDLE(name, type); \ > ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type) > #define DEFINE_XEN_GUEST_HANDLE(name) __DEFINE_XEN_GUEST_HANDLE(name, > name) > -#define __XEN_GUEST_HANDLE(name) __guest_handle_ ## name > +#define __XEN_GUEST_HANDLE(name) __guest_handle_64_ ## name > #define XEN_GUEST_HANDLE(name) __XEN_GUEST_HANDLE(name) > -#define set_xen_guest_handle_raw(hnd, val) do { (hnd).p = val; } while (0) > +/* this is going to be changes on 64 bit */ > +#define XEN_GUEST_HANDLE_PARAM(name) __guest_handle_ ## name > +#define set_xen_guest_handle_raw(hnd, val) \ > + do { if ( sizeof(hnd) == 8 ) *(uint64_t *)&(hnd) = 0; \ > + (hnd).p = val; \ > + } while ( 0 ) > #ifdef __XEN_TOOLS__ > #define get_xen_guest_handle(val, hnd) do { val = (hnd).p; } while (0) > #endif > diff --git a/xen/include/public/arch-ia64.h b/xen/include/public/arch-ia64.h > index c9da5d4..97583ea 100644 > --- a/xen/include/public/arch-ia64.h > +++ b/xen/include/public/arch-ia64.h > @@ -47,6 +47,7 @@ > > #define DEFINE_XEN_GUEST_HANDLE(name) __DEFINE_XEN_GUEST_HANDLE(name, > name) > #define XEN_GUEST_HANDLE(name) __guest_handle_ ## name > +#define XEN_GUEST_HANDLE_PARAM(name) XEN_GUEST_HANDLE(name) > #define XEN_GUEST_HANDLE_64(name) XEN_GUEST_HANDLE(name) > #define uint64_aligned_t uint64_t > #define set_xen_guest_handle_raw(hnd, val) do { (hnd).p = val; } while (0) > diff --git a/xen/include/public/arch-x86/xen.h > b/xen/include/public/arch-x86/xen.h > index 1c186d7..8ee5437 100644 > --- a/xen/include/public/arch-x86/xen.h > +++ b/xen/include/public/arch-x86/xen.h > @@ -44,6 +44,7 @@ > #define DEFINE_XEN_GUEST_HANDLE(name) __DEFINE_XEN_GUEST_HANDLE(name, > name) > #define __XEN_GUEST_HANDLE(name) __guest_handle_ ## name > #define XEN_GUEST_HANDLE(name) __XEN_GUEST_HANDLE(name) > +#define XEN_GUEST_HANDLE_PARAM(name) XEN_GUEST_HANDLE(name) > #define set_xen_guest_handle_raw(hnd, val) do { (hnd).p = val; } while (0) > #ifdef __XEN_TOOLS__ > #define get_xen_guest_handle(val, hnd) do { val = (hnd).p; } while (0) > -- > 1.7.2.5 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote:> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > > Note: this change does not make any difference on x86 and ia64. > > > > > > XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers > > stored in memory from guest pointers as hypercall parameters. > > I have to admit that really dislike this, to a large part because of > the follow up patch that clutters the corresponding function > declarations even further. Plus I see no mechanism to convert > between the two, yet I can''t see how - long term at least - you > could get away without such conversion. > > Is it really a well thought through and settled upon decision to > make guest handles 64 bits wide even on 32-bit ARM? After all, > both x86 and PPC got away without doing soWell, on x86 we have the compat XLAT layer, which is a pretty complex piece of code, so "got away without" is a bit strong... We''d really rather not have to have a non-trivial compat layer on arm too by having the struct layouts be the same on 32/64. Ian.
Jan Beulich
2012-Aug-06 15:54 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
>>> On 06.08.12 at 17:43, Stefano Stabellini <stefano.stabellini@eu.citrix.com>wrote:> On Mon, 6 Aug 2012, Jan Beulich wrote: >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> wrote: >> > This is an incremental patch on top of >> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary >> > compatibility, it is better to introduce foreign_domid as part of a >> > union containing both size and foreign_domid. >> > >> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> > --- >> > xen/include/public/memory.h | 11 +++++++---- >> > 1 files changed, 7 insertions(+), 4 deletions(-) >> > >> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h >> > index b2adfbe..b0af2fd 100644 >> > --- a/xen/include/public/memory.h >> > +++ b/xen/include/public/memory.h >> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap { >> > /* Which domain to change the mapping for. */ >> > domid_t domid; >> > >> > - /* Number of pages to go through for gmfn_range */ >> > - uint16_t size; >> > + union { >> > + /* Number of pages to go through for gmfn_range */ >> > + uint16_t size; >> > + /* IFF gmfn_foreign */ >> > + domid_t foreign_domid; >> > + }; >> >> But you''re clear that this isn''t standard C, and hence can''t go >> in this way? >> > > Why? It is c11 if I am not mistaken.Yes. But the common baseline is C89. Jan
>>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote: >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: >> > Note: this change does not make any difference on x86 and ia64. >> > >> > >> > XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers >> > stored in memory from guest pointers as hypercall parameters. >> >> I have to admit that really dislike this, to a large part because of >> the follow up patch that clutters the corresponding function >> declarations even further. Plus I see no mechanism to convert >> between the two, yet I can''t see how - long term at least - you >> could get away without such conversion. >> >> Is it really a well thought through and settled upon decision to >> make guest handles 64 bits wide even on 32-bit ARM? After all, >> both x86 and PPC got away without doing so > > Well, on x86 we have the compat XLAT layer, which is a pretty complex > piece of code, so "got away without" is a bit strong...Hmm, yes, that''s a valid correction.> We''d really > rather not have to have a non-trivial compat layer on arm too by having > the struct layouts be the same on 32/64.And paying a penalty like this in the 32-bit half (if what is likely to remain the much bigger portion for the next couple of years can validly be called "half") is worth it? The more that the compat layer is now reasonably mature (and should hence be easily re-usable for ARM)? Jan
Stefano Stabellini
2012-Aug-06 16:02 UTC
Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM
On Mon, 6 Aug 2012, Jan Beulich wrote:> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote: > >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > wrote: > >> > Note: this change does not make any difference on x86 and ia64. > >> > > >> > > >> > XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers > >> > stored in memory from guest pointers as hypercall parameters. > >> > >> I have to admit that really dislike this, to a large part because of > >> the follow up patch that clutters the corresponding function > >> declarations even further. Plus I see no mechanism to convert > >> between the two, yet I can''t see how - long term at least - you > >> could get away without such conversion. > >> > >> Is it really a well thought through and settled upon decision to > >> make guest handles 64 bits wide even on 32-bit ARM? After all, > >> both x86 and PPC got away without doing so > > > > Well, on x86 we have the compat XLAT layer, which is a pretty complex > > piece of code, so "got away without" is a bit strong... > > Hmm, yes, that''s a valid correction. > > > We''d really > > rather not have to have a non-trivial compat layer on arm too by having > > the struct layouts be the same on 32/64. > > And paying a penalty like this in the 32-bit half (if what is likely > to remain the much bigger portion for the next couple of years > can validly be called "half") is worth it? The more that the compat > layer is now reasonably mature (and should hence be easily > re-usable for ARM)?What penalty? The only penalty is the wasted space in the structs in memory.
>>> On 06.08.12 at 18:02, Stefano Stabellini <stefano.stabellini@eu.citrix.com>wrote:> On Mon, 6 Aug 2012, Jan Beulich wrote: >> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote: >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> > wrote: >> >> > Note: this change does not make any difference on x86 and ia64. >> >> > >> >> > >> >> > XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers >> >> > stored in memory from guest pointers as hypercall parameters. >> >> >> >> I have to admit that really dislike this, to a large part because of >> >> the follow up patch that clutters the corresponding function >> >> declarations even further. Plus I see no mechanism to convert >> >> between the two, yet I can''t see how - long term at least - you >> >> could get away without such conversion. >> >> >> >> Is it really a well thought through and settled upon decision to >> >> make guest handles 64 bits wide even on 32-bit ARM? After all, >> >> both x86 and PPC got away without doing so >> > >> > Well, on x86 we have the compat XLAT layer, which is a pretty complex >> > piece of code, so "got away without" is a bit strong... >> >> Hmm, yes, that''s a valid correction. >> >> > We''d really >> > rather not have to have a non-trivial compat layer on arm too by having >> > the struct layouts be the same on 32/64. >> >> And paying a penalty like this in the 32-bit half (if what is likely >> to remain the much bigger portion for the next couple of years >> can validly be called "half") is worth it? The more that the compat >> layer is now reasonably mature (and should hence be easily >> re-usable for ARM)? > > What penalty? The only penalty is the wasted space in the structs in > memory.No - the caller has to zero-initialize those extra 32 bits, and the hypervisor has to check for them to be zero (the latter may be implicit in the 64-bit one, but certainly needs to be explicit on the 32-bit side). Jan
Stefano Stabellini
2012-Aug-07 12:08 UTC
Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions
On Mon, 6 Aug 2012, Jan Beulich wrote:> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > > There are still few unsigned long in the xen public interface: replace > > them with xen_ulong_t. > > > > Also typedef xen_ulong_t to uint64_t on ARM. > > While this change by itself already looks suspicious to me, I don''t > follow what the global replacement is going to be good for, in > particular when done in places that ARM should be completely > ignorant of, e.g.See below> > --- a/xen/include/public/physdev.h > > +++ b/xen/include/public/physdev.h > > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t); > > #define PHYSDEVOP_apic_write 9 > > struct physdev_apic { > > /* IN */ > > - unsigned long apic_physbase; > > + xen_ulong_t apic_physbase; > > uint32_t reg; > > /* IN or OUT */ > > uint32_t value; > >...This change is actually not required, considering that ARM doesn''t have an APIC. I changed apic_physbase to xen_ulong_t only for consistency, but it wouldn''t make any difference for ARM (or x86). If you think that it is better to keep it unsigned long, I''ll remove this chunk for the patch.> > --- a/xen/include/public/xen.h > > +++ b/xen/include/public/xen.h > > @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t); > > * NB. The fields are natural register size for this architecture. > > */ > > struct multicall_entry { > > - unsigned long op, result; > > - unsigned long args[6]; > > + xen_ulong_t op, result; > > + xen_ulong_t args[6]; > > And here I really start to wonder - what use is it to put all 64-bit > values here on a 32-bit arch? You certainly know a lot more about > ARM than me, but this looks pretty inefficient, the more that > you''ll have to deal with checking the full values when converting > to e.g. pointers anyway, in order to avoid behavioral differences > between running on a 32- or 64-bit host. Zero-extending from > 32-bits when in a 64-bit hypervisor wouldn''t have this problem.Actually the multicall_entry change is wrong, thanks for point it out. The idea is that pointers are always 8 bytes sized and 8 bytes aligned, except when they are passed as hypercall arguments, in which case a 32 bit guest would use 32 bit pointers and a 64 bit guest would use 64 bit pointers. Considering that each field of a multicall_entry is usually passed as an hypercall parameter, they should all remain unsigned long.
Stefano Stabellini
2012-Aug-07 12:27 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On Mon, 6 Aug 2012, Jan Beulich wrote:> >>> On 06.08.12 at 17:43, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: > > On Mon, 6 Aug 2012, Jan Beulich wrote: > >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> wrote: > >> > This is an incremental patch on top of > >> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary > >> > compatibility, it is better to introduce foreign_domid as part of a > >> > union containing both size and foreign_domid. > >> > > >> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> > --- > >> > xen/include/public/memory.h | 11 +++++++---- > >> > 1 files changed, 7 insertions(+), 4 deletions(-) > >> > > >> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > >> > index b2adfbe..b0af2fd 100644 > >> > --- a/xen/include/public/memory.h > >> > +++ b/xen/include/public/memory.h > >> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap { > >> > /* Which domain to change the mapping for. */ > >> > domid_t domid; > >> > > >> > - /* Number of pages to go through for gmfn_range */ > >> > - uint16_t size; > >> > + union { > >> > + /* Number of pages to go through for gmfn_range */ > >> > + uint16_t size; > >> > + /* IFF gmfn_foreign */ > >> > + domid_t foreign_domid; > >> > + }; > >> > >> But you''re clear that this isn''t standard C, and hence can''t go > >> in this way? > >> > > > > Why? It is c11 if I am not mistaken. > > Yes. But the common baseline is C89.Do we need to keep it C89? If I am not mistaken anonymous unions have been in GCC for more than 10 years now. If we do want to keep it C89, considering that size was introduced only recently, do you think that it would be OK for me to change the interface and just add size to a union? Like this: union { /* Number of pages to go through for gmfn_range */ uint16_t size; /* IFF gmfn_foreign */ domid_t foreign_domid; } u; Also CC''ing Jean.
Stefano Stabellini
2012-Aug-07 12:35 UTC
Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM
On Tue, 7 Aug 2012, Jan Beulich wrote:> >>> On 06.08.12 at 18:02, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: > > On Mon, 6 Aug 2012, Jan Beulich wrote: > >> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote: > >> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote: > >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> > wrote: > >> >> > Note: this change does not make any difference on x86 and ia64. > >> >> > > >> >> > > >> >> > XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers > >> >> > stored in memory from guest pointers as hypercall parameters. > >> >> > >> >> I have to admit that really dislike this, to a large part because of > >> >> the follow up patch that clutters the corresponding function > >> >> declarations even further. Plus I see no mechanism to convert > >> >> between the two, yet I can''t see how - long term at least - you > >> >> could get away without such conversion. > >> >> > >> >> Is it really a well thought through and settled upon decision to > >> >> make guest handles 64 bits wide even on 32-bit ARM? After all, > >> >> both x86 and PPC got away without doing so > >> > > >> > Well, on x86 we have the compat XLAT layer, which is a pretty complex > >> > piece of code, so "got away without" is a bit strong... > >> > >> Hmm, yes, that''s a valid correction. > >> > >> > We''d really > >> > rather not have to have a non-trivial compat layer on arm too by having > >> > the struct layouts be the same on 32/64. > >> > >> And paying a penalty like this in the 32-bit half (if what is likely > >> to remain the much bigger portion for the next couple of years > >> can validly be called "half") is worth it? The more that the compat > >> layer is now reasonably mature (and should hence be easily > >> re-usable for ARM)? > > > > What penalty? The only penalty is the wasted space in the structs in > > memory. > > No - the caller has to zero-initialize those extra 32 bits, and the > hypervisor has to check for them to be zero (the latter may be > implicit in the 64-bit one, but certainly needs to be explicit on the > 32-bit side).You are saying that on a 32 bit hypervisor we should check that the padding is zero? Why should we care about the value of the padding? In any case fortunately accesses to guest_handles already go via macros, so it should be easy to arrange if it comes down to it.
Ian Campbell
2012-Aug-07 12:36 UTC
Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions
On Tue, 2012-08-07 at 13:08 +0100, Stefano Stabellini wrote:> On Mon, 6 Aug 2012, Jan Beulich wrote: > > >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > > > There are still few unsigned long in the xen public interface: replace > > > them with xen_ulong_t. > > > > > > Also typedef xen_ulong_t to uint64_t on ARM. > > > > While this change by itself already looks suspicious to me, I don''t > > follow what the global replacement is going to be good for, in > > particular when done in places that ARM should be completely > > ignorant of, e.g. > > See below > > > > > --- a/xen/include/public/physdev.h > > > +++ b/xen/include/public/physdev.h > > > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t); > > > #define PHYSDEVOP_apic_write 9 > > > struct physdev_apic { > > > /* IN */ > > > - unsigned long apic_physbase; > > > + xen_ulong_t apic_physbase; > > > uint32_t reg; > > > /* IN or OUT */ > > > uint32_t value; > > >... > > This change is actually not required, considering that ARM doesn''t have > an APIC. I changed apic_physbase to xen_ulong_t only for consistency, > but it wouldn''t make any difference for ARM (or x86). > If you think that it is better to keep it unsigned long, I''ll remove > this chunk for the patch. > > > > > --- a/xen/include/public/xen.h > > > +++ b/xen/include/public/xen.h > > > @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t); > > > * NB. The fields are natural register size for this architecture. > > > */ > > > struct multicall_entry { > > > - unsigned long op, result; > > > - unsigned long args[6]; > > > + xen_ulong_t op, result; > > > + xen_ulong_t args[6]; > > > > And here I really start to wonder - what use is it to put all 64-bit > > values here on a 32-bit arch? You certainly know a lot more about > > ARM than me, but this looks pretty inefficient, the more that > > you''ll have to deal with checking the full values when converting > > to e.g. pointers anyway, in order to avoid behavioral differences > > between running on a 32- or 64-bit host. Zero-extending from > > 32-bits when in a 64-bit hypervisor wouldn''t have this problem. > > Actually the multicall_entry change is wrong, thanks for point it out. > > The idea is that pointers are always 8 bytes sized and 8 bytes aligned, > except when they are passed as hypercall arguments, in which case a 32 > bit guest would use 32 bit pointers and a 64 bit guest would use 64 bit > pointers. > > Considering that each field of a multicall_entry is usually passed as an > hypercall parameter, they should all remain unsigned long.If possible, please make them an explicitly sized type, even if it is now 32 bits. Ian.
On Tue, 2012-08-07 at 13:35 +0100, Stefano Stabellini wrote:> On Tue, 7 Aug 2012, Jan Beulich wrote: > > >>> On 06.08.12 at 18:02, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > wrote: > > > On Mon, 6 Aug 2012, Jan Beulich wrote: > > >> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > >> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote: > > >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > >> > wrote: > > >> >> > Note: this change does not make any difference on x86 and ia64. > > >> >> > > > >> >> > > > >> >> > XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers > > >> >> > stored in memory from guest pointers as hypercall parameters. > > >> >> > > >> >> I have to admit that really dislike this, to a large part because of > > >> >> the follow up patch that clutters the corresponding function > > >> >> declarations even further. Plus I see no mechanism to convert > > >> >> between the two, yet I can''t see how - long term at least - you > > >> >> could get away without such conversion. > > >> >> > > >> >> Is it really a well thought through and settled upon decision to > > >> >> make guest handles 64 bits wide even on 32-bit ARM? After all, > > >> >> both x86 and PPC got away without doing so > > >> > > > >> > Well, on x86 we have the compat XLAT layer, which is a pretty complex > > >> > piece of code, so "got away without" is a bit strong... > > >> > > >> Hmm, yes, that''s a valid correction. > > >> > > >> > We''d really > > >> > rather not have to have a non-trivial compat layer on arm too by having > > >> > the struct layouts be the same on 32/64. > > >> > > >> And paying a penalty like this in the 32-bit half (if what is likely > > >> to remain the much bigger portion for the next couple of years > > >> can validly be called "half") is worth it? The more that the compat > > >> layer is now reasonably mature (and should hence be easily > > >> re-usable for ARM)? > > > > > > What penalty? The only penalty is the wasted space in the structs in > > > memory. > > > > No - the caller has to zero-initialize those extra 32 bits, and the > > hypervisor has to check for them to be zero (the latter may be > > implicit in the 64-bit one, but certainly needs to be explicit on the > > 32-bit side). > > You are saying that on a 32 bit hypervisor we should check that the > padding is zero? Why should we care about the value of the padding?The point is so that we can treat them as 64 bit values in a 64 bit hypervisor, otherwise we would need a compat layer to translate (which is what we want to avoid). So the 32 bit guest definitely does need to zero them, and a debug build of the 32 bit hypervisor probably ought to reject non-zero upper halves, otherwise the chances of the guest doing it consistently is pretty small.> In any case fortunately accesses to guest_handles already go via macros, > so it should be easy to arrange if it comes down to it.
Jean Guyader
2012-Aug-07 12:40 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On 7 August 2012 13:27, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:> On Mon, 6 Aug 2012, Jan Beulich wrote: >> >>> On 06.08.12 at 17:43, Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> wrote: >> > On Mon, 6 Aug 2012, Jan Beulich wrote: >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> >> wrote: >> >> > This is an incremental patch on top of >> >> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary >> >> > compatibility, it is better to introduce foreign_domid as part of a >> >> > union containing both size and foreign_domid. >> >> > >> >> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> >> > --- >> >> > xen/include/public/memory.h | 11 +++++++---- >> >> > 1 files changed, 7 insertions(+), 4 deletions(-) >> >> > >> >> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h >> >> > index b2adfbe..b0af2fd 100644 >> >> > --- a/xen/include/public/memory.h >> >> > +++ b/xen/include/public/memory.h >> >> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap { >> >> > /* Which domain to change the mapping for. */ >> >> > domid_t domid; >> >> > >> >> > - /* Number of pages to go through for gmfn_range */ >> >> > - uint16_t size; >> >> > + union { >> >> > + /* Number of pages to go through for gmfn_range */ >> >> > + uint16_t size; >> >> > + /* IFF gmfn_foreign */ >> >> > + domid_t foreign_domid; >> >> > + }; >> >> >> >> But you''re clear that this isn''t standard C, and hence can''t go >> >> in this way? >> >> >> > >> > Why? It is c11 if I am not mistaken. >> >> Yes. But the common baseline is C89. > > Do we need to keep it C89? > If I am not mistaken anonymous unions have been in GCC for more than 10 > years now. >It''s a public header so you can''t assume that the only consumer will be GCC.> If we do want to keep it C89, considering that size was introduced only > recently, do you think that it would be OK for me to change the > interface and just add size to a union? > Like this: > > union { > /* Number of pages to go through for gmfn_range */ > uint16_t size; > /* IFF gmfn_foreign */ > domid_t foreign_domid; > } u; >You could probably do something like #ifdef __GNUC__ # define UNION_NAME #else # define UNION_NAME u #endif union { /* Number of pages to go through for gmfn_range */ uint16_t size; /* IFF gmfn_foreign */ domid_t foreign_domid; } UNION_NAME; It''s not ideal but this way you keep the binary compatibility and you also the code still cmopatible with GCC. Jean
>>> On 07.08.12 at 14:08, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > On Mon, 6 Aug 2012, Jan Beulich wrote: >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: >> > --- a/xen/include/public/physdev.h >> > +++ b/xen/include/public/physdev.h >> > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t); >> > #define PHYSDEVOP_apic_write 9 >> > struct physdev_apic { >> > /* IN */ >> > - unsigned long apic_physbase; >> > + xen_ulong_t apic_physbase; >> > uint32_t reg; >> > /* IN or OUT */ >> > uint32_t value; >> >... > > This change is actually not required, considering that ARM doesn''t have > an APIC. I changed apic_physbase to xen_ulong_t only for consistency, > but it wouldn''t make any difference for ARM (or x86). > If you think that it is better to keep it unsigned long, I''ll remove > this chunk for the patch.I''d prefer that. Also any others that you may not actually need to be converted. Also, seems I was misremembering how PPC dealt with this - they indeed had xen_ulong_t defined to be a 64-bit value too.> Considering that each field of a multicall_entry is usually passed as an > hypercall parameter, they should all remain unsigned long.That''ll give you subtle bugs I''m afraid: do_memory_op()''s encoding of a continuation start extent (into the ''cmd'' value), for example, depends on being able to store the full value into the command field of the multicall structure. The limit checking of the permitted number of extents therefore is different between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would neither find it very appealing to have do_memory_op() adjusted for dealing with this new special case, nor am I sure that''s the only place your approach would cause problems if you excluded the multicall structure from the model change. Jan
Jan Beulich
2012-Aug-07 13:02 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
>>> On 07.08.12 at 14:27, Stefano Stabellini <stefano.stabellini@eu.citrix.com>wrote:> On Mon, 6 Aug 2012, Jan Beulich wrote: >> >>> On 06.08.12 at 17:43, Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> wrote: >> > On Mon, 6 Aug 2012, Jan Beulich wrote: >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> >> wrote: >> >> > This is an incremental patch on top of >> >> > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary >> >> > compatibility, it is better to introduce foreign_domid as part of a >> >> > union containing both size and foreign_domid. >> >> > >> >> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> >> > --- >> >> > xen/include/public/memory.h | 11 +++++++---- >> >> > 1 files changed, 7 insertions(+), 4 deletions(-) >> >> > >> >> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h >> >> > index b2adfbe..b0af2fd 100644 >> >> > --- a/xen/include/public/memory.h >> >> > +++ b/xen/include/public/memory.h >> >> > @@ -208,8 +208,12 @@ struct xen_add_to_physmap { >> >> > /* Which domain to change the mapping for. */ >> >> > domid_t domid; >> >> > >> >> > - /* Number of pages to go through for gmfn_range */ >> >> > - uint16_t size; >> >> > + union { >> >> > + /* Number of pages to go through for gmfn_range */ >> >> > + uint16_t size; >> >> > + /* IFF gmfn_foreign */ >> >> > + domid_t foreign_domid; >> >> > + }; >> >> >> >> But you''re clear that this isn''t standard C, and hence can''t go >> >> in this way? >> >> >> > >> > Why? It is c11 if I am not mistaken. >> >> Yes. But the common baseline is C89. > > Do we need to keep it C89?Yes, I think so. That''s what we indeed can (and already do) expect of compilers used on these headers. Even C99 would be too much already.> If I am not mistaken anonymous unions have been in GCC for more than 10 > years now.With a few quirks here and there, yes. But gcc is not the measure here anyway, we need to settle on the lowest common standard that''s largely accepted/implemented industry wide.> If we do want to keep it C89, considering that size was introduced only > recently, do you think that it would be OK for me to change the > interface and just add size to a union? > Like this: > > union { > /* Number of pages to go through for gmfn_range */ > uint16_t size; > /* IFF gmfn_foreign */ > domid_t foreign_domid; > } u; > > Also CC''ing Jean.If that''s okay with Jean (and implying eventual other users of the interface - I hope he would know), then yes; the addition isn''t in anything that we have released so far. The only other reasonable alternative I see would be to bump __XEN_LATEST_INTERFACE_VERSION__ again, and have this coded as (beware, ugly!) #if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200 union { #endif /* Number of pages to go through for gmfn_range */ uint16_t size; #if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200 /* IFF gmfn_foreign */ domid_t foreign_domid; } u; #endif Jan
>>> On 07.08.12 at 14:35, Stefano Stabellini <stefano.stabellini@eu.citrix.com>wrote:> On Tue, 7 Aug 2012, Jan Beulich wrote: >> >>> On 06.08.12 at 18:02, Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> wrote: >> > On Mon, 6 Aug 2012, Jan Beulich wrote: >> >> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> >> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote: >> >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> >> > wrote: >> >> >> > Note: this change does not make any difference on x86 and ia64. >> >> >> > >> >> >> > >> >> >> > XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers >> >> >> > stored in memory from guest pointers as hypercall parameters. >> >> >> >> >> >> I have to admit that really dislike this, to a large part because of >> >> >> the follow up patch that clutters the corresponding function >> >> >> declarations even further. Plus I see no mechanism to convert >> >> >> between the two, yet I can''t see how - long term at least - you >> >> >> could get away without such conversion. >> >> >> >> >> >> Is it really a well thought through and settled upon decision to >> >> >> make guest handles 64 bits wide even on 32-bit ARM? After all, >> >> >> both x86 and PPC got away without doing so >> >> > >> >> > Well, on x86 we have the compat XLAT layer, which is a pretty complex >> >> > piece of code, so "got away without" is a bit strong... >> >> >> >> Hmm, yes, that''s a valid correction. >> >> >> >> > We''d really >> >> > rather not have to have a non-trivial compat layer on arm too by having >> >> > the struct layouts be the same on 32/64. >> >> >> >> And paying a penalty like this in the 32-bit half (if what is likely >> >> to remain the much bigger portion for the next couple of years >> >> can validly be called "half") is worth it? The more that the compat >> >> layer is now reasonably mature (and should hence be easily >> >> re-usable for ARM)? >> > >> > What penalty? The only penalty is the wasted space in the structs in >> > memory. >> >> No - the caller has to zero-initialize those extra 32 bits, and the >> hypervisor has to check for them to be zero (the latter may be >> implicit in the 64-bit one, but certainly needs to be explicit on the >> 32-bit side). > > You are saying that on a 32 bit hypervisor we should check that the > padding is zero? Why should we care about the value of the padding?Because otherwise the same 32-bit guest kernel''s behavior on 32- and 64-bit hypervisor may unexpectedly differ. And even if you didn''t care to do the check, the guest would still be required to put zeros there in order to run on a 64-bit hypervisor. (And of course this costs you cache and TLB bandwidth. See how x86-64 just recently got the ILP32 model [aka x32] added for this very reason.) Jan
>>> On 07.08.12 at 14:36, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Tue, 2012-08-07 at 13:08 +0100, Stefano Stabellini wrote: >> On Mon, 6 Aug 2012, Jan Beulich wrote: >> > >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: >> > > There are still few unsigned long in the xen public interface: replace >> > > them with xen_ulong_t. >> > > >> > > Also typedef xen_ulong_t to uint64_t on ARM. >> > >> > While this change by itself already looks suspicious to me, I don''t >> > follow what the global replacement is going to be good for, in >> > particular when done in places that ARM should be completely >> > ignorant of, e.g. >> >> See below >> >> >> > > --- a/xen/include/public/physdev.h >> > > +++ b/xen/include/public/physdev.h >> > > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t); >> > > #define PHYSDEVOP_apic_write 9 >> > > struct physdev_apic { >> > > /* IN */ >> > > - unsigned long apic_physbase; >> > > + xen_ulong_t apic_physbase; >> > > uint32_t reg; >> > > /* IN or OUT */ >> > > uint32_t value; >> > >... >> >> This change is actually not required, considering that ARM doesn''t have >> an APIC. I changed apic_physbase to xen_ulong_t only for consistency, >> but it wouldn''t make any difference for ARM (or x86). >> If you think that it is better to keep it unsigned long, I''ll remove >> this chunk for the patch. >> >> >> > > --- a/xen/include/public/xen.h >> > > +++ b/xen/include/public/xen.h >> > > @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t); >> > > * NB. The fields are natural register size for this architecture. >> > > */ >> > > struct multicall_entry { >> > > - unsigned long op, result; >> > > - unsigned long args[6]; >> > > + xen_ulong_t op, result; >> > > + xen_ulong_t args[6]; >> > >> > And here I really start to wonder - what use is it to put all 64-bit >> > values here on a 32-bit arch? You certainly know a lot more about >> > ARM than me, but this looks pretty inefficient, the more that >> > you''ll have to deal with checking the full values when converting >> > to e.g. pointers anyway, in order to avoid behavioral differences >> > between running on a 32- or 64-bit host. Zero-extending from >> > 32-bits when in a 64-bit hypervisor wouldn''t have this problem. >> >> Actually the multicall_entry change is wrong, thanks for point it out. >> >> The idea is that pointers are always 8 bytes sized and 8 bytes aligned, >> except when they are passed as hypercall arguments, in which case a 32 >> bit guest would use 32 bit pointers and a 64 bit guest would use 64 bit >> pointers. >> >> Considering that each field of a multicall_entry is usually passed as an >> hypercall parameter, they should all remain unsigned long. > > If possible, please make them an explicitly sized type, even if it is > now 32 bits.??? This structure is already shared between 32-bit and 64-bit implementations, and the fields are intentionally fitting both by using "unsigned long". Are you suggesting to add #ifdef-ery to public/xen.h? Jan
Jan Beulich
2012-Aug-07 13:18 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
>>> On 07.08.12 at 14:40, Jean Guyader <jean.guyader@gmail.com> wrote: > You could probably do something like > > #ifdef __GNUC__ > # define UNION_NAME > #else > # define UNION_NAME u > #endif > union { > /* Number of pages to go through for gmfn_range */ > uint16_t size; > /* IFF gmfn_foreign */ > domid_t foreign_domid; > } UNION_NAME; > > It''s not ideal but this way you keep the binary compatibility and you > also the code still cmopatible with GCC.Ah, yes, something along those lines might be okay; we already have similar things e.g. for __DECL_REG() (note the additional use of __STRICT_ANSI__ there, though). The most problematic thing here is the name space cluttering - UNION_NAME is certainly not good, but __DECL_REG really is too unspecific too (yet we got away with it so far). Jan
Ian Campbell
2012-Aug-07 13:30 UTC
Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions
On Tue, 2012-08-07 at 14:13 +0100, Jan Beulich wrote:> >>> On 07.08.12 at 14:36, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Tue, 2012-08-07 at 13:08 +0100, Stefano Stabellini wrote: > >> On Mon, 6 Aug 2012, Jan Beulich wrote: > >> > >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > wrote: > >> > > There are still few unsigned long in the xen public interface: replace > >> > > them with xen_ulong_t. > >> > > > >> > > Also typedef xen_ulong_t to uint64_t on ARM. > >> > > >> > While this change by itself already looks suspicious to me, I don''t > >> > follow what the global replacement is going to be good for, in > >> > particular when done in places that ARM should be completely > >> > ignorant of, e.g. > >> > >> See below > >> > >> > >> > > --- a/xen/include/public/physdev.h > >> > > +++ b/xen/include/public/physdev.h > >> > > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t); > >> > > #define PHYSDEVOP_apic_write 9 > >> > > struct physdev_apic { > >> > > /* IN */ > >> > > - unsigned long apic_physbase; > >> > > + xen_ulong_t apic_physbase; > >> > > uint32_t reg; > >> > > /* IN or OUT */ > >> > > uint32_t value; > >> > >... > >> > >> This change is actually not required, considering that ARM doesn''t have > >> an APIC. I changed apic_physbase to xen_ulong_t only for consistency, > >> but it wouldn''t make any difference for ARM (or x86). > >> If you think that it is better to keep it unsigned long, I''ll remove > >> this chunk for the patch. > >> > >> > >> > > --- a/xen/include/public/xen.h > >> > > +++ b/xen/include/public/xen.h > >> > > @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t); > >> > > * NB. The fields are natural register size for this architecture. > >> > > */ > >> > > struct multicall_entry { > >> > > - unsigned long op, result; > >> > > - unsigned long args[6]; > >> > > + xen_ulong_t op, result; > >> > > + xen_ulong_t args[6]; > >> > > >> > And here I really start to wonder - what use is it to put all 64-bit > >> > values here on a 32-bit arch? You certainly know a lot more about > >> > ARM than me, but this looks pretty inefficient, the more that > >> > you''ll have to deal with checking the full values when converting > >> > to e.g. pointers anyway, in order to avoid behavioral differences > >> > between running on a 32- or 64-bit host. Zero-extending from > >> > 32-bits when in a 64-bit hypervisor wouldn''t have this problem. > >> > >> Actually the multicall_entry change is wrong, thanks for point it out. > >> > >> The idea is that pointers are always 8 bytes sized and 8 bytes aligned, > >> except when they are passed as hypercall arguments, in which case a 32 > >> bit guest would use 32 bit pointers and a 64 bit guest would use 64 bit > >> pointers. > >> > >> Considering that each field of a multicall_entry is usually passed as an > >> hypercall parameter, they should all remain unsigned long. > > > > If possible, please make them an explicitly sized type, even if it is > > now 32 bits. > > ??? This structure is already shared between 32-bit and 64-bit > implementations, and the fields are intentionally fitting both by > using "unsigned long". Are you suggesting to add #ifdef-ery to > public/xen.h?Oh, I thought multicall_entry was in an arch header. In any case I would have expected a typedef xen_multicall_arg_t or something.> > Jan >
Ian Jackson
2012-Aug-07 15:24 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
Jan Beulich writes ("Re: [Xen-devel] [PATCH 1/5] xen: improve changes to
xen_add_to_physmap"):> #if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200
> union {
> #endif
> /* Number of pages to go through for gmfn_range */
> uint16_t size;
> #if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200
> /* IFF gmfn_foreign */
> domid_t foreign_domid;
> } u;
> #endif
Has someone checked that on all supported platforms the size and
alignment of this union is identical to that of the original
uint16_t ?
Ian.
Jan Beulich
2012-Aug-07 15:37 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
>>> On 07.08.12 at 17:24, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote: > Jan Beulich writes ("Re: [Xen-devel] [PATCH 1/5] xen: improve changes to > xen_add_to_physmap"): >> #if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200 >> union { >> #endif >> /* Number of pages to go through for gmfn_range */ >> uint16_t size; >> #if __XEN_LATEST_INTERFACE_VERSION__ > 0x040200 >> /* IFF gmfn_foreign */ >> domid_t foreign_domid; >> } u; >> #endif > > Has someone checked that on all supported platforms the size and > alignment of this union is identical to that of the original > uint16_t ?Since domid_t is uniformly a typedef of uint16_t, I don''t think there''s any doubt in that. Jan
Stefano Stabellini
2012-Aug-07 17:07 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On Tue, 7 Aug 2012, Jan Beulich wrote:> >>> On 07.08.12 at 14:40, Jean Guyader <jean.guyader@gmail.com> wrote: > > You could probably do something like > > > > #ifdef __GNUC__ > > # define UNION_NAME > > #else > > # define UNION_NAME u > > #endif > > union { > > /* Number of pages to go through for gmfn_range */ > > uint16_t size; > > /* IFF gmfn_foreign */ > > domid_t foreign_domid; > > } UNION_NAME; > > > > It''s not ideal but this way you keep the binary compatibility and you > > also the code still cmopatible with GCC. > > Ah, yes, something along those lines might be okay; we already > have similar things e.g. for __DECL_REG() (note the additional > use of __STRICT_ANSI__ there, though).But it is strict ANSI.> The most problematic > thing here is the name space cluttering - UNION_NAME is certainly > not good, but __DECL_REG really is too unspecific too (yet we > got away with it so far).OK, I''ll got for this strategy. Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD? #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >= 201112L) # define XEN_ADD_TO_PHYSMAP_ARG #else # define XEN_ADD_TO_PHYSMAP_ARG u #endif
Stefano Stabellini
2012-Aug-07 18:09 UTC
Re: [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM
On Tue, 7 Aug 2012, Jan Beulich wrote:> >>> On 07.08.12 at 14:35, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: > > On Tue, 7 Aug 2012, Jan Beulich wrote: > >> >>> On 06.08.12 at 18:02, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> wrote: > >> > On Mon, 6 Aug 2012, Jan Beulich wrote: > >> >> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@citrix.com> wrote: > >> >> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote: > >> >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > >> >> > wrote: > >> >> >> > Note: this change does not make any difference on x86 and ia64. > >> >> >> > > >> >> >> > > >> >> >> > XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest pointers > >> >> >> > stored in memory from guest pointers as hypercall parameters. > >> >> >> > >> >> >> I have to admit that really dislike this, to a large part because of > >> >> >> the follow up patch that clutters the corresponding function > >> >> >> declarations even further. Plus I see no mechanism to convert > >> >> >> between the two, yet I can''t see how - long term at least - you > >> >> >> could get away without such conversion. > >> >> >> > >> >> >> Is it really a well thought through and settled upon decision to > >> >> >> make guest handles 64 bits wide even on 32-bit ARM? After all, > >> >> >> both x86 and PPC got away without doing so > >> >> > > >> >> > Well, on x86 we have the compat XLAT layer, which is a pretty complex > >> >> > piece of code, so "got away without" is a bit strong... > >> >> > >> >> Hmm, yes, that''s a valid correction. > >> >> > >> >> > We''d really > >> >> > rather not have to have a non-trivial compat layer on arm too by having > >> >> > the struct layouts be the same on 32/64. > >> >> > >> >> And paying a penalty like this in the 32-bit half (if what is likely > >> >> to remain the much bigger portion for the next couple of years > >> >> can validly be called "half") is worth it? The more that the compat > >> >> layer is now reasonably mature (and should hence be easily > >> >> re-usable for ARM)? > >> > > >> > What penalty? The only penalty is the wasted space in the structs in > >> > memory. > >> > >> No - the caller has to zero-initialize those extra 32 bits, and the > >> hypervisor has to check for them to be zero (the latter may be > >> implicit in the 64-bit one, but certainly needs to be explicit on the > >> 32-bit side). > > > > You are saying that on a 32 bit hypervisor we should check that the > > padding is zero? Why should we care about the value of the padding? > > Because otherwise the same 32-bit guest kernel''s behavior on > 32- and 64-bit hypervisor may unexpectedly differ. And even if > you didn''t care to do the check, the guest would still be required > to put zeros there in order to run on a 64-bit hypervisor. (And > of course this costs you cache and TLB bandwidth. See how > x86-64 just recently got the ILP32 model [aka x32] added for > this very reason.)Considering that no MMU hypercalls are required on ARM and that none of the grant table and event channel ops on the hot path have any guest handles, I think we are going to be fine.
Jan Beulich
2012-Aug-08 07:14 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
>>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD?Sounds fine (and I like this better than the ..._ARG one you used below.> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >= 201112L)#if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \ (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) avoiding compilers to warn about the use of the possibly undefined __STDC_VERSION__, which only got introduced after C89 was already published (and which e.g. gcc indeed doesn''t define if the value would end up being below 199409L).> # define XEN_ADD_TO_PHYSMAP_ARG > #else > # define XEN_ADD_TO_PHYSMAP_ARG u > #endifAlso, please don''t forget to #undef it after use. Jan
Ian Campbell
2012-Aug-08 07:45 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On Wed, 2012-08-08 at 08:14 +0100, Jan Beulich wrote:> >>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD? > > Sounds fine (and I like this better than the ..._ARG one you used > below. > > > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >= 201112L) > > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \ > (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L)The downside of this is that users of this header might need to change their code depending on what compiler they actually build with today (or even what options). Is adding the ".u" throughout the Xen code base too intrusive?> avoiding compilers to warn about the use of the possibly > undefined __STDC_VERSION__, which only got introduced > after C89 was already published (and which e.g. gcc indeed > doesn''t define if the value would end up being below 199409L). > > > # define XEN_ADD_TO_PHYSMAP_ARG > > #else > > # define XEN_ADD_TO_PHYSMAP_ARG u > > #endif > > Also, please don''t forget to #undef it after use. > > Jan >
On Tue, 2012-08-07 at 14:08 +0100, Jan Beulich wrote:> See how > x86-64 just recently got the ILP32 model [aka x32] added for > this very reason.)For userspace only though, with the kernel remaining a full 64-bit kernel. I don''t know what direction they eventually took but there was also a contingent in favour of using the 64-bit syscall ABI for x32. I don''t think that the wastage at the hypercall interface is going to be significant. Especially once you consider that many of the 32-bit fields actually need to be 64-bit for a compat mode guest any, e.g. MFNs should certainly be 64 bit regardless or else you impose limitations on 32-bit guests which the 64-bit hypervisor doesn''t itself have. Ian.
Ian Campbell
2012-Aug-08 07:59 UTC
Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions
On Tue, 2012-08-07 at 13:54 +0100, Jan Beulich wrote:> >>> On 07.08.12 at 14:08, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > > On Mon, 6 Aug 2012, Jan Beulich wrote: > >> >>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > >> > --- a/xen/include/public/physdev.h > >> > +++ b/xen/include/public/physdev.h > >> > @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t); > >> > #define PHYSDEVOP_apic_write 9 > >> > struct physdev_apic { > >> > /* IN */ > >> > - unsigned long apic_physbase; > >> > + xen_ulong_t apic_physbase; > >> > uint32_t reg; > >> > /* IN or OUT */ > >> > uint32_t value; > >> >... > > > > This change is actually not required, considering that ARM doesn''t have > > an APIC. I changed apic_physbase to xen_ulong_t only for consistency, > > but it wouldn''t make any difference for ARM (or x86). > > If you think that it is better to keep it unsigned long, I''ll remove > > this chunk for the patch. > > I''d prefer that. Also any others that you may not actually need > to be converted. > > Also, seems I was misremembering how PPC dealt with this - they > indeed had xen_ulong_t defined to be a 64-bit value too. > > > Considering that each field of a multicall_entry is usually passed as an > > hypercall parameter, they should all remain unsigned long. > > That''ll give you subtle bugs I''m afraid: do_memory_op()''s > encoding of a continuation start extent (into the ''cmd'' value), > for example, depends on being able to store the full value into > the command field of the multicall structure. The limit checking > of the permitted number of extents therefore is different > between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and > compat (UINT_MAX >> MEMOP_EXTENT_SHIFT).On compat this would be 2^(32-6) == 67 and a bit million extents. I think we can live with this limit on 64bit ARM too. We could have lived with it on 64bit X86 too but by the time we noticed it was too late so we have to live with it.> I would > neither find it very appealing to have do_memory_op() adjusted > for dealing with this new special case, nor am I sure that''s the > only place your approach would cause problems if you excluded > the multicall structure from the model change.On the other hand it doesn''t seem right for the multicall args to be able to represent values which you couldn''t pass to the regular hypercall itself (since the 32 bit args are only 32 bit). Perhaps if use of the upper portions for 32 bit guests were restricted for the use of continuations only that might work -- it would rely on the top half of the 64-bit x0 register surviving a trip back to 32 bit mode and into hypervisor mode again. I think that can be expected (but I''d have to double check a AArch64 docs to be sure). On the other-other hand that would mean that a 32-on-32 guest would see different things to a 32-on-64 guest which is bad. I don''t expect we will be able to get away with no compat layer what so ever. At the minimum there will have to be compat hypercall entry points which correctly extend the 32 bit arguments to 64 bit, and do the same for the return value etc but if we can avoid the majority of the struct munging (ideally completely avoiding the need for a hypercall arguments translation area) then I think that is worth doing. Ian.
Jan Beulich
2012-Aug-08 08:49 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
>>> On 08.08.12 at 09:45, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Wed, 2012-08-08 at 08:14 +0100, Jan Beulich wrote: >> >>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: >> > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD? >> >> Sounds fine (and I like this better than the ..._ARG one you used >> below. >> >> > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >= > 201112L) >> >> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \ >> (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) > > The downside of this is that users of this header might need to change > their code depending on what compiler they actually build with today (or > even what options). > > Is adding the ".u" throughout the Xen code base too intrusive?I don''t know and didn''t check; I think the goal was to avoid having to change consumers that use gcc for compilation. Jan
>>> On 08.08.12 at 09:48, Ian Campbell <Ian.Campbell@citrix.com> wrote: > I don''t think that the wastage at the hypercall interface is going to be > significant. Especially once you consider that many of the 32-bit fields > actually need to be 64-bit for a compat mode guest any, e.g. MFNs should > certainly be 64 bit regardless or else you impose limitations on 32-bit > guests which the 64-bit hypervisor doesn''t itself have.Hmm, MFNs are probably a bad example, because we already have a separate xen_pfn_t to deal with precisely that case. Jan
Stefano Stabellini
2012-Aug-08 09:51 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On Wed, 8 Aug 2012, Jan Beulich wrote:> >>> On 08.08.12 at 09:45, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Wed, 2012-08-08 at 08:14 +0100, Jan Beulich wrote: > >> >>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > wrote: > >> > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD? > >> > >> Sounds fine (and I like this better than the ..._ARG one you used > >> below. > >> > >> > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >= > > 201112L) > >> > >> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \ > >> (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) > > > > The downside of this is that users of this header might need to change > > their code depending on what compiler they actually build with today (or > > even what options). > > > > Is adding the ".u" throughout the Xen code base too intrusive? > > I don''t know and didn''t check; I think the goal was to avoid having > to change consumers that use gcc for compilation.For ARM is not an issue, but the size parameter can be used by out of tree code (V4V?). That''s why I CC''ed Jean, I was hoping he was going to say that it is OK to add ".u".
Jean Guyader
2012-Aug-08 10:03 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On 8 August 2012 10:51, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:> On Wed, 8 Aug 2012, Jan Beulich wrote: >> >>> On 08.08.12 at 09:45, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> > On Wed, 2012-08-08 at 08:14 +0100, Jan Beulich wrote: >> >> >>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> > wrote: >> >> > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD? >> >> >> >> Sounds fine (and I like this better than the ..._ARG one you used >> >> below. >> >> >> >> > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >>> > 201112L) >> >> >> >> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \ >> >> (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) >> > >> > The downside of this is that users of this header might need to change >> > their code depending on what compiler they actually build with today (or >> > even what options). >> > >> > Is adding the ".u" throughout the Xen code base too intrusive? >> >> I don''t know and didn''t check; I think the goal was to avoid having >> to change consumers that use gcc for compilation. > > For ARM is not an issue, but the size parameter can be used by out of > tree code (V4V?). > That''s why I CC''ed Jean, I was hoping he was going to say that it is OK > to add ".u".I have introduced the size field for XENMAPSPACE_gmfn_range and that is used by hvmload when we want to relocated the memory because the pci hole needs to be bigger. Maybe something else use it by now, if not you could get away by just updating the Xen code to use .u. Jean
Stefano Stabellini
2012-Aug-08 10:08 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On Wed, 8 Aug 2012, Jean Guyader wrote:> On 8 August 2012 10:51, Stefano Stabellini > <stefano.stabellini@eu.citrix.com> wrote: > > On Wed, 8 Aug 2012, Jan Beulich wrote: > >> >>> On 08.08.12 at 09:45, Ian Campbell <Ian.Campbell@citrix.com> wrote: > >> > On Wed, 2012-08-08 at 08:14 +0100, Jan Beulich wrote: > >> >> >>> On 07.08.12 at 19:07, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> > wrote: > >> >> > Regarding the name, maybe it should be XEN_ADD_TO_PHYSMAP_FIELD? > >> >> > >> >> Sounds fine (and I like this better than the ..._ARG one you used > >> >> below. > >> >> > >> >> > #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || (__STDC_VERSION__ >> >> > 201112L) > >> >> > >> >> #if (defined(__GNUC__) && !defined(__STRICT_ANSI__)) || \ > >> >> (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) > >> > > >> > The downside of this is that users of this header might need to change > >> > their code depending on what compiler they actually build with today (or > >> > even what options). > >> > > >> > Is adding the ".u" throughout the Xen code base too intrusive? > >> > >> I don''t know and didn''t check; I think the goal was to avoid having > >> to change consumers that use gcc for compilation. > > > > For ARM is not an issue, but the size parameter can be used by out of > > tree code (V4V?). > > That''s why I CC''ed Jean, I was hoping he was going to say that it is OK > > to add ".u". > > I have introduced the size field for XENMAPSPACE_gmfn_range and that is used > by hvmload when we want to relocated the memory because the pci hole needs to be > bigger. > > Maybe something else use it by now, if not you could get away by just > updating the Xen > code to use .u.Great, I''ll do that then.
Stefano Stabellini
2012-Aug-08 12:12 UTC
Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions
On Tue, 7 Aug 2012, Jan Beulich wrote:> > Considering that each field of a multicall_entry is usually passed as an > > hypercall parameter, they should all remain unsigned long. > > That''ll give you subtle bugs I''m afraid: do_memory_op()''s > encoding of a continuation start extent (into the ''cmd'' value), > for example, depends on being able to store the full value into > the command field of the multicall structure. The limit checking > of the permitted number of extents therefore is different > between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and > compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would > neither find it very appealing to have do_memory_op() adjusted > for dealing with this new special case, nor am I sure that''s the > only place your approach would cause problems if you excluded > the multicall structure from the model change.Given the way the continuation is implemented, the same problem can also happen on x86. In fact, considering that we don''t use any compat code, and that do_memory_op has the following check: /* Is size too large for us to encode a continuation? */ if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) ) return start_extent; it would work as-is for ARM too.
Ian Campbell
2012-Aug-08 12:17 UTC
Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions
On Wed, 2012-08-08 at 13:12 +0100, Stefano Stabellini wrote:> On Tue, 7 Aug 2012, Jan Beulich wrote: > > > Considering that each field of a multicall_entry is usually passed as an > > > hypercall parameter, they should all remain unsigned long. > > > > That''ll give you subtle bugs I''m afraid: do_memory_op()''s > > encoding of a continuation start extent (into the ''cmd'' value), > > for example, depends on being able to store the full value into > > the command field of the multicall structure. The limit checking > > of the permitted number of extents therefore is different > > between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and > > compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would > > neither find it very appealing to have do_memory_op() adjusted > > for dealing with this new special case, nor am I sure that''s the > > only place your approach would cause problems if you excluded > > the multicall structure from the model change. > > Given the way the continuation is implemented, the same problem can > also happen on x86. > In fact, considering that we don''t use any compat code, and that > do_memory_op has the following check: > > /* Is size too large for us to encode a continuation? */ > if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) ) > return start_extent; > > it would work as-is for ARM too.For 32-on-32 ARM it''ll work. It''ll need changing for 32-on-64 ARM though. We might even consider making the limit the smaller number even for 64-on-64 ARM. We should make the limit MEMOP_MAX_EXTENTS and allow it to be per-arch, on x86 it''ll remain ULONG_MAX >> MEMOP_EXTENT_SHIFT. Ian.
>>> On 08.08.12 at 14:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com>wrote:> On Tue, 7 Aug 2012, Jan Beulich wrote: >> > Considering that each field of a multicall_entry is usually passed as an >> > hypercall parameter, they should all remain unsigned long. >> >> That''ll give you subtle bugs I''m afraid: do_memory_op()''s >> encoding of a continuation start extent (into the ''cmd'' value), >> for example, depends on being able to store the full value into >> the command field of the multicall structure. The limit checking >> of the permitted number of extents therefore is different >> between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and >> compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would >> neither find it very appealing to have do_memory_op() adjusted >> for dealing with this new special case, nor am I sure that''s the >> only place your approach would cause problems if you excluded >> the multicall structure from the model change. > > Given the way the continuation is implemented, the same problem can > also happen on x86.No. The compat wrapper, as pointed out there has a different check on the maximum number of extents, and hence the continuation index can''t overflow.> In fact, considering that we don''t use any compat code, and that > do_memory_op has the following check: > > /* Is size too large for us to encode a continuation? */ > if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) ) > return start_extent; > > it would work as-is for ARM too.Not afaict. For a 32-bit guest, but the above code executed in a 64-bit hypervisor, the guest could pass in (theoretically) UINT_MAX, which would pass this check, yet the eventual continuation index would get truncated when stored in the 32-bit hypercall operation field. Jan
David Vrabel
2012-Aug-08 14:20 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On 08/08/2012 11:03, Jean Guyader wrote:> > I have introduced the size field for XENMAPSPACE_gmfn_range and that is used > by hvmload when we want to relocated the memory because the pci hole needs to be > bigger.Why do this so late? Rather than creating the domain with the correctly sized PCI hole? David
Stefano Stabellini
2012-Aug-08 15:01 UTC
Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions
On Wed, 8 Aug 2012, Jan Beulich wrote:> >>> On 08.08.12 at 14:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: > > On Tue, 7 Aug 2012, Jan Beulich wrote: > >> > Considering that each field of a multicall_entry is usually passed as an > >> > hypercall parameter, they should all remain unsigned long. > >> > >> That''ll give you subtle bugs I''m afraid: do_memory_op()''s > >> encoding of a continuation start extent (into the ''cmd'' value), > >> for example, depends on being able to store the full value into > >> the command field of the multicall structure. The limit checking > >> of the permitted number of extents therefore is different > >> between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and > >> compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would > >> neither find it very appealing to have do_memory_op() adjusted > >> for dealing with this new special case, nor am I sure that''s the > >> only place your approach would cause problems if you excluded > >> the multicall structure from the model change. > > > > Given the way the continuation is implemented, the same problem can > > also happen on x86. > > No. The compat wrapper, as pointed out there has a different > check on the maximum number of extents, and hence the > continuation index can''t overflow.Right. I meant the same conceptual problem: the guest passing a number of extents that is too big for the hypervisor to handle.> > In fact, considering that we don''t use any compat code, and that > > do_memory_op has the following check: > > > > /* Is size too large for us to encode a continuation? */ > > if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) ) > > return start_extent; > > > > it would work as-is for ARM too. > > Not afaict. For a 32-bit guest, but the above code executed in > a 64-bit hypervisor, the guest could pass in (theoretically) > UINT_MAX, which would pass this check, yet the eventual > continuation index would get truncated when stored in the > 32-bit hypercall operation field.Actually, like Ian wrote, I expect that using the upper 32 bit part of the x0 register, only for continuations, would work just fine. In any case we can make that check arch dependent and restrict the maximum number of extents on aarch64 to the same limit we have on aarch32.
>>> On 08.08.12 at 17:01, Stefano Stabellini <stefano.stabellini@eu.citrix.com>wrote:> On Wed, 8 Aug 2012, Jan Beulich wrote: >> >>> On 08.08.12 at 14:12, Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> wrote: >> > On Tue, 7 Aug 2012, Jan Beulich wrote: >> >> > Considering that each field of a multicall_entry is usually passed as an >> >> > hypercall parameter, they should all remain unsigned long. >> >> >> >> That''ll give you subtle bugs I''m afraid: do_memory_op()''s >> >> encoding of a continuation start extent (into the ''cmd'' value), >> >> for example, depends on being able to store the full value into >> >> the command field of the multicall structure. The limit checking >> >> of the permitted number of extents therefore is different >> >> between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and >> >> compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would >> >> neither find it very appealing to have do_memory_op() adjusted >> >> for dealing with this new special case, nor am I sure that''s the >> >> only place your approach would cause problems if you excluded >> >> the multicall structure from the model change. >> > >> > Given the way the continuation is implemented, the same problem can >> > also happen on x86. >> >> No. The compat wrapper, as pointed out there has a different >> check on the maximum number of extents, and hence the >> continuation index can''t overflow. > > Right. I meant the same conceptual problem: the guest passing a number of > extents that is too big for the hypervisor to handle.I don''t think the hypervisor has a problem handling such large amounts.>> > In fact, considering that we don''t use any compat code, and that >> > do_memory_op has the following check: >> > >> > /* Is size too large for us to encode a continuation? */ >> > if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) ) >> > return start_extent; >> > >> > it would work as-is for ARM too. >> >> Not afaict. For a 32-bit guest, but the above code executed in >> a 64-bit hypervisor, the guest could pass in (theoretically) >> UINT_MAX, which would pass this check, yet the eventual >> continuation index would get truncated when stored in the >> 32-bit hypercall operation field. > > Actually, like Ian wrote, I expect that using the upper 32 bit part of > the x0 register, only for continuations, would work just fine. > > In any case we can make that check arch dependent and restrict the > maximum number of extents on aarch64 to the same limit we have on > aarch32.We should even discuss lowering the limit universally to the UINT_MAX based value. I don''t think anyone is actively using such insane numbers. And don''t forget that I pointed out this issue only as example of possible problems with your intended approach to the handling of multicalls. Jan
Stefano Stabellini
2012-Aug-08 15:55 UTC
Re: [PATCH 3/5] xen: few more xen_ulong_t substitutions
> >> > do_memory_op has the following check: > >> > > >> > /* Is size too large for us to encode a continuation? */ > >> > if ( reservation.nr_extents > (ULONG_MAX >> MEMOP_EXTENT_SHIFT) ) > >> > return start_extent; > >> > > >> > it would work as-is for ARM too. > >> > >> Not afaict. For a 32-bit guest, but the above code executed in > >> a 64-bit hypervisor, the guest could pass in (theoretically) > >> UINT_MAX, which would pass this check, yet the eventual > >> continuation index would get truncated when stored in the > >> 32-bit hypercall operation field. > > > > Actually, like Ian wrote, I expect that using the upper 32 bit part of > > the x0 register, only for continuations, would work just fine. > > > > In any case we can make that check arch dependent and restrict the > > maximum number of extents on aarch64 to the same limit we have on > > aarch32. > > We should even discuss lowering the limit universally to the > UINT_MAX based value. I don''t think anyone is actively using > such insane numbers.I am OK with that, it would make things easier for me.> And don''t forget that I pointed out this issue only as example > of possible problems with your intended approach to the > handling of multicalls.I just went through the hypercall continuations in common code, and do_memory_op is the only one that uses this "trick" of encoding the start extent in another parameter. I think we''ll have to deal with these issues as we find them out.
Jean Guyader
2012-Aug-08 19:33 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On 8 August 2012 15:20, David Vrabel <dvrabel@cantab.net> wrote:> On 08/08/2012 11:03, Jean Guyader wrote: >> >> >> I have introduced the size field for XENMAPSPACE_gmfn_range and that is >> used >> by hvmload when we want to relocated the memory because the pci hole needs >> to be >> bigger. > > > Why do this so late? Rather than creating the domain with the correctly > sized PCI hole? >The PCI hole isn''t fix in size, especially if you do passthrough. It''s not really easy to compute outside of the domain you will have to ask Qemu. I think it''s fine how it''s done today. Jean
Ian Campbell
2012-Aug-09 09:16 UTC
Re: [PATCH 2/5] xen/arm: introduce __lshrdi3 and __aeabi_llsr
On Mon, 2012-08-06 at 15:12 +0100, Stefano Stabellini wrote:> Taken from Linux. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>Any idea what lshrdi means? Anyway, given that this is presumably required by code which gcc generates and that it comes direct from Linux: Acked-by: Ian Campbell <ian.campbell@citrix.com> and pushed to my arm-for-4.3 branch.> --- > xen/arch/arm/lib/Makefile | 2 +- > xen/arch/arm/lib/lshrdi3.S | 54 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 55 insertions(+), 1 deletions(-) > create mode 100644 xen/arch/arm/lib/lshrdi3.S > > diff --git a/xen/arch/arm/lib/Makefile b/xen/arch/arm/lib/Makefile > index cbbed68..4cf41f4 100644 > --- a/xen/arch/arm/lib/Makefile > +++ b/xen/arch/arm/lib/Makefile > @@ -2,4 +2,4 @@ obj-y += memcpy.o memmove.o memset.o memzero.o > obj-y += findbit.o setbit.o > obj-y += setbit.o clearbit.o changebit.o > obj-y += testsetbit.o testclearbit.o testchangebit.o > -obj-y += lib1funcs.o div64.o > +obj-y += lib1funcs.o lshrdi3.o div64.o > diff --git a/xen/arch/arm/lib/lshrdi3.S b/xen/arch/arm/lib/lshrdi3.S > new file mode 100644 > index 0000000..3e8887e > --- /dev/null > +++ b/xen/arch/arm/lib/lshrdi3.S > @@ -0,0 +1,54 @@ > +/* Copyright 1995, 1996, 1998, 1999, 2000, 2003, 2004, 2005 > + Free Software Foundation, Inc. > + > +This file is free software; you can redistribute it and/or modify it > +under the terms of the GNU General Public License as published by the > +Free Software Foundation; either version 2, or (at your option) any > +later version. > + > +In addition to the permissions in the GNU General Public License, the > +Free Software Foundation gives you unlimited permission to link the > +compiled version of this file into combinations with other programs, > +and to distribute those combinations without any restriction coming > +from the use of this file. (The General Public License restrictions > +do apply in other respects; for example, they cover modification of > +the file, and distribution when not linked into a combine > +executable.) > + > +This file is distributed in the hope that it will be useful, but > +WITHOUT ANY WARRANTY; without even the implied warranty of > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +General Public License for more details. > + > +You should have received a copy of the GNU General Public License > +along with this program; see the file COPYING. If not, write to > +the Free Software Foundation, 51 Franklin Street, Fifth Floor, > +Boston, MA 02110-1301, USA. */ > + > + > +#include <xen/config.h> > +#include "assembler.h" > + > +#ifdef __ARMEB__ > +#define al r1 > +#define ah r0 > +#else > +#define al r0 > +#define ah r1 > +#endif > + > +ENTRY(__lshrdi3) > +ENTRY(__aeabi_llsr) > + > + subs r3, r2, #32 > + rsb ip, r2, #32 > + movmi al, al, lsr r2 > + movpl al, ah, lsr r3 > + ARM( orrmi al, al, ah, lsl ip ) > + THUMB( lslmi r3, ah, ip ) > + THUMB( orrmi al, al, r3 ) > + mov ah, ah, lsr r2 > + mov pc, lr > + > +ENDPROC(__lshrdi3) > +ENDPROC(__aeabi_llsr)
Stefano Stabellini
2012-Aug-09 09:43 UTC
Re: [PATCH 2/5] xen/arm: introduce __lshrdi3 and __aeabi_llsr
On Thu, 9 Aug 2012, Ian Campbell wrote:> On Mon, 2012-08-06 at 15:12 +0100, Stefano Stabellini wrote: > > Taken from Linux. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > Any idea what lshrdi means?I am not sure TBH, but it is one of the needed libgcc functions.> Anyway, given that this is presumably required by code which gcc > generates and that it comes direct from Linux: > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > and pushed to my arm-for-4.3 branch. > > > --- > > xen/arch/arm/lib/Makefile | 2 +- > > xen/arch/arm/lib/lshrdi3.S | 54 ++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 55 insertions(+), 1 deletions(-) > > create mode 100644 xen/arch/arm/lib/lshrdi3.S > > > > diff --git a/xen/arch/arm/lib/Makefile b/xen/arch/arm/lib/Makefile > > index cbbed68..4cf41f4 100644 > > --- a/xen/arch/arm/lib/Makefile > > +++ b/xen/arch/arm/lib/Makefile > > @@ -2,4 +2,4 @@ obj-y += memcpy.o memmove.o memset.o memzero.o > > obj-y += findbit.o setbit.o > > obj-y += setbit.o clearbit.o changebit.o > > obj-y += testsetbit.o testclearbit.o testchangebit.o > > -obj-y += lib1funcs.o div64.o > > +obj-y += lib1funcs.o lshrdi3.o div64.o > > diff --git a/xen/arch/arm/lib/lshrdi3.S b/xen/arch/arm/lib/lshrdi3.S > > new file mode 100644 > > index 0000000..3e8887e > > --- /dev/null > > +++ b/xen/arch/arm/lib/lshrdi3.S > > @@ -0,0 +1,54 @@ > > +/* Copyright 1995, 1996, 1998, 1999, 2000, 2003, 2004, 2005 > > + Free Software Foundation, Inc. > > + > > +This file is free software; you can redistribute it and/or modify it > > +under the terms of the GNU General Public License as published by the > > +Free Software Foundation; either version 2, or (at your option) any > > +later version. > > + > > +In addition to the permissions in the GNU General Public License, the > > +Free Software Foundation gives you unlimited permission to link the > > +compiled version of this file into combinations with other programs, > > +and to distribute those combinations without any restriction coming > > +from the use of this file. (The General Public License restrictions > > +do apply in other respects; for example, they cover modification of > > +the file, and distribution when not linked into a combine > > +executable.) > > + > > +This file is distributed in the hope that it will be useful, but > > +WITHOUT ANY WARRANTY; without even the implied warranty of > > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > +General Public License for more details. > > + > > +You should have received a copy of the GNU General Public License > > +along with this program; see the file COPYING. If not, write to > > +the Free Software Foundation, 51 Franklin Street, Fifth Floor, > > +Boston, MA 02110-1301, USA. */ > > + > > + > > +#include <xen/config.h> > > +#include "assembler.h" > > + > > +#ifdef __ARMEB__ > > +#define al r1 > > +#define ah r0 > > +#else > > +#define al r0 > > +#define ah r1 > > +#endif > > + > > +ENTRY(__lshrdi3) > > +ENTRY(__aeabi_llsr) > > + > > + subs r3, r2, #32 > > + rsb ip, r2, #32 > > + movmi al, al, lsr r2 > > + movpl al, ah, lsr r3 > > + ARM( orrmi al, al, ah, lsl ip ) > > + THUMB( lslmi r3, ah, ip ) > > + THUMB( orrmi al, al, r3 ) > > + mov ah, ah, lsr r2 > > + mov pc, lr > > + > > +ENDPROC(__lshrdi3) > > +ENDPROC(__aeabi_llsr) > > >
Mukesh Rathor
2012-Aug-11 01:33 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On Mon, 6 Aug 2012 15:12:01 +0100 Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:> This is an incremental patch on top of > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary > compatibility, it is better to introduce foreign_domid as part of a > union containing both size and foreign_domid. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/include/public/memory.h | 11 +++++++---- > 1 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > index b2adfbe..b0af2fd 100644 > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -208,8 +208,12 @@ struct xen_add_to_physmap { > /* Which domain to change the mapping for. */ > domid_t domid; > > - /* Number of pages to go through for gmfn_range */ > - uint16_t size; > + union { > + /* Number of pages to go through for gmfn_range */ > + uint16_t size; > + /* IFF gmfn_foreign */ > + domid_t foreign_domid; > + }; > > /* Source mapping space. */ > #define XENMAPSPACE_shared_info 0 /* shared info page */ > @@ -217,8 +221,7 @@ struct xen_add_to_physmap { > #define XENMAPSPACE_gmfn 2 /* GMFN */ > #define XENMAPSPACE_gmfn_range 3 /* GMFN range */ > #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */ > - uint16_t space; > - domid_t foreign_domid; /* IFF gmfn_foreign */ > + unsigned int space; > > #define XENMAPIDX_grant_table_status 0x80000000 >Is this the final version? I don''t see it in today''s xen unstable tree! I have this in my tree: struct xen_add_to_physmap { /* Which domain to change the mapping for. */ domid_t domid; /* Number of pages to go through for gmfn_range */ uint16_t size; /* Source mapping space. */ #define XENMAPSPACE_shared_info 0 /* shared info page */ #define XENMAPSPACE_grant_table 1 /* grant table page */ #define XENMAPSPACE_gmfn 2 /* GMFN */ #define XENMAPSPACE_gmfn_range 3 /* GMFN range */ #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */ uint16_t space; domid_t foreign_domid; /* IFF XENMAPSPACE_gmfn_foreign */ thanks, Mukesh
Stefano Stabellini
2012-Aug-13 10:43 UTC
Re: [PATCH 1/5] xen: improve changes to xen_add_to_physmap
On Sat, 11 Aug 2012, Mukesh Rathor wrote:> On Mon, 6 Aug 2012 15:12:01 +0100 > Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > > > This is an incremental patch on top of > > c0bc926083b5987a3e9944eec2c12ad0580100e2: in order to retain binary > > compatibility, it is better to introduce foreign_domid as part of a > > union containing both size and foreign_domid. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > xen/include/public/memory.h | 11 +++++++---- > > 1 files changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > > index b2adfbe..b0af2fd 100644 > > --- a/xen/include/public/memory.h > > +++ b/xen/include/public/memory.h > > @@ -208,8 +208,12 @@ struct xen_add_to_physmap { > > /* Which domain to change the mapping for. */ > > domid_t domid; > > > > - /* Number of pages to go through for gmfn_range */ > > - uint16_t size; > > + union { > > + /* Number of pages to go through for gmfn_range */ > > + uint16_t size; > > + /* IFF gmfn_foreign */ > > + domid_t foreign_domid; > > + }; > > > > /* Source mapping space. */ > > #define XENMAPSPACE_shared_info 0 /* shared info page */ > > @@ -217,8 +221,7 @@ struct xen_add_to_physmap { > > #define XENMAPSPACE_gmfn 2 /* GMFN */ > > #define XENMAPSPACE_gmfn_range 3 /* GMFN range */ > > #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */ > > - uint16_t space; > > - domid_t foreign_domid; /* IFF gmfn_foreign */ > > + unsigned int space; > > > > #define XENMAPIDX_grant_table_status 0x80000000 > > > > Is this the final version? I don''t see it in today''s xen unstable tree! > I have this in my tree: > > struct xen_add_to_physmap { > /* Which domain to change the mapping for. */ > domid_t domid; > > /* Number of pages to go through for gmfn_range */ > uint16_t size; > > /* Source mapping space. */ > #define XENMAPSPACE_shared_info 0 /* shared info page */ > #define XENMAPSPACE_grant_table 1 /* grant table page */ > #define XENMAPSPACE_gmfn 2 /* GMFN */ > #define XENMAPSPACE_gmfn_range 3 /* GMFN range */ > #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */ > uint16_t space; > domid_t foreign_domid; /* IFF XENMAPSPACE_gmfn_foreign */We are still discussing how the final version should actually look like but the last patch I sent is this one (for the for-4.3 tree): http://marc.info/?l=xen-devel&m=134460098420461