[PATCH v05 54/72] include/uapi/xen/privcmd.h: fix compilation in userspace

Subsystems: arm port, the rest, xen hypervisor arm, xen hypervisor interface

STALE3582d

Revision v05 of 3 in this series.

5 messages, 3 authors, 2016-08-23 · open the first message on its own page

[PATCH v05 54/72] include/uapi/xen/privcmd.h: fix compilation in userspace

From: Mikko Rapeli <hidden>
Date: 2016-08-22 19:12:11

xen/interface/xen.h is not exported from kernel headers so remove the
dependency and provide needed defines for domid_t and xen_pfn_t if they
are not already defined by some other e.g. Xen specific headers.

Suggested by Andrew Cooper [off-list ref] on lkml message
[off-list ref].

The ifdef for ARM is ugly but did not find better solutions for it.

Fixes userspace compilation error:

xen/privcmd.h:38:31: fatal error: xen/interface/xen.h: No such file or directory

Signed-off-by: Mikko Rapeli <redacted>
Cc: David Vrabel <redacted>
---
 arch/arm/include/asm/xen/interface.h |  2 +-
 include/uapi/xen/privcmd.h           | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h
index 75d5968..6898ee1 100644
--- a/arch/arm/include/asm/xen/interface.h
+++ b/arch/arm/include/asm/xen/interface.h
@@ -38,7 +38,7 @@
  * fine since it simply wouldn't be able to create any sure pfns in
  * the first place.
  */
-typedef uint64_t xen_pfn_t;
+typedef __u64 xen_pfn_t;
 #define PRI_xen_pfn "llx"
 typedef uint64_t xen_ulong_t;
 #define PRI_xen_ulong "llx"
diff --git a/include/uapi/xen/privcmd.h b/include/uapi/xen/privcmd.h
index 7ddeeda..16c11f9 100644
--- a/include/uapi/xen/privcmd.h
+++ b/include/uapi/xen/privcmd.h
@@ -35,7 +35,17 @@
 
 #include <linux/types.h>
 #include <linux/compiler.h>
-#include <xen/interface/xen.h>
+
+/* Defined by include/xen/interface/xen.h, but it is not part of Linux uapi */
+#ifndef __XEN_PUBLIC_XEN_H__
+typedef __u16 domid_t;
+
+#if (defined __ARMEL__ || defined __ARMEB__)
+typedef __u64 xen_pfn_t;
+#else
+typedef unsigned long xen_pfn_t;
+#endif /* (defined __ARMEL__ || defined __ARMEB__) */
+#endif /* __XEN_PUBLIC_XEN_H__ */
 
 struct privcmd_hypercall {
 	__u64 op;
-- 
2.8.1

Re: [PATCH v05 54/72] include/uapi/xen/privcmd.h: fix compilation in userspace

From: Russell King - ARM Linux <linux@armlinux.org.uk>
Date: 2016-08-23 10:04:39

On Mon, Aug 22, 2016 at 08:33:11PM +0200, Mikko Rapeli wrote:
xen/interface/xen.h is not exported from kernel headers so remove the
dependency and provide needed defines for domid_t and xen_pfn_t if they
are not already defined by some other e.g. Xen specific headers.
I'm confused.  How did we end up with a 64-bit PFN number on ARM?  It's
insane - especially as the kernel uses "unsigned long" almost everywhere
for PFNs - we can't have physical addresses more than 44 bits (32 bit
pfn + 4k page size).
Suggested by Andrew Cooper [off-list ref] on lkml message
[off-list ref].

The ifdef for ARM is ugly but did not find better solutions for it.
#ifdef __arm__

maybe?  Even if not, the unsightly parens are not necessary.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v05 54/72] include/uapi/xen/privcmd.h: fix compilation in userspace

From: Stefano Stabellini <sstabellini@kernel.org>
Date: 2016-08-23 18:13:58

On Tue, 23 Aug 2016, Russell King - ARM Linux wrote:
On Mon, Aug 22, 2016 at 08:33:11PM +0200, Mikko Rapeli wrote:
quoted
xen/interface/xen.h is not exported from kernel headers so remove the
dependency and provide needed defines for domid_t and xen_pfn_t if they
are not already defined by some other e.g. Xen specific headers.
I'm confused.  How did we end up with a 64-bit PFN number on ARM?  It's
insane - especially as the kernel uses "unsigned long" almost everywhere
for PFNs - we can't have physical addresses more than 44 bits (32 bit
pfn + 4k page size).
That's because xen_pfn_t is the type used to store pfns in structures
passed to Xen via hypercalls. The Xen hypercall ABI is shared between
ARM and ARM64. On x86_32 and x86_64 we have different sizes for pfn
types in the hypercall ABI and it caused quite a bit of trouble in the
past as it is possible to run 32bit domains on a 64bit hypervisor.
Having a common type for pfns on ARM and ARM64 helped a lot in that
respect.

quoted
Suggested by Andrew Cooper [off-list ref] on lkml message
[off-list ref].

The ifdef for ARM is ugly but did not find better solutions for it.
#ifdef __arm__

maybe?  Even if not, the unsightly parens are not necessary.
Yes, I think it should be:

#if defined(__arm__) || defined(__aarch64__)

quoted hunk
diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h
index 75d5968..6898ee1 100644
--- a/arch/arm/include/asm/xen/interface.h
+++ b/arch/arm/include/asm/xen/interface.h
@@ -38,7 +38,7 @@
  * fine since it simply wouldn't be able to create any sure pfns in
  * the first place.
  */
-typedef uint64_t xen_pfn_t;
+typedef __u64 xen_pfn_t;
 #define PRI_xen_pfn "llx"
 typedef uint64_t xen_ulong_t;
 #define PRI_xen_ulong "llx"
Why this change?

Re: [PATCH v05 54/72] include/uapi/xen/privcmd.h: fix compilation in userspace

From: Mikko Rapeli <hidden>
Date: 2016-08-23 20:38:24

On Tue, Aug 23, 2016 at 11:13:52AM -0700, Stefano Stabellini wrote:
On Tue, 23 Aug 2016, Russell King - ARM Linux wrote:
quoted
On Mon, Aug 22, 2016 at 08:33:11PM +0200, Mikko Rapeli wrote:
quoted
xen/interface/xen.h is not exported from kernel headers so remove the
dependency and provide needed defines for domid_t and xen_pfn_t if they
are not already defined by some other e.g. Xen specific headers.
I'm confused.  How did we end up with a 64-bit PFN number on ARM?  It's
insane - especially as the kernel uses "unsigned long" almost everywhere
for PFNs - we can't have physical addresses more than 44 bits (32 bit
pfn + 4k page size).
That's because xen_pfn_t is the type used to store pfns in structures
passed to Xen via hypercalls. The Xen hypercall ABI is shared between
ARM and ARM64. On x86_32 and x86_64 we have different sizes for pfn
types in the hypercall ABI and it caused quite a bit of trouble in the
past as it is possible to run 32bit domains on a 64bit hypervisor.
Having a common type for pfns on ARM and ARM64 helped a lot in that
respect.

quoted
quoted
Suggested by Andrew Cooper [off-list ref] on lkml message
[off-list ref].

The ifdef for ARM is ugly but did not find better solutions for it.
#ifdef __arm__

maybe?  Even if not, the unsightly parens are not necessary.
Yes, I think it should be:

#if defined(__arm__) || defined(__aarch64__)
Thanks, I will send a new version with this change.
 
quoted
diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h
index 75d5968..6898ee1 100644
--- a/arch/arm/include/asm/xen/interface.h
+++ b/arch/arm/include/asm/xen/interface.h
@@ -38,7 +38,7 @@
  * fine since it simply wouldn't be able to create any sure pfns in
  * the first place.
  */
-typedef uint64_t xen_pfn_t;
+typedef __u64 xen_pfn_t;
 #define PRI_xen_pfn "llx"
 typedef uint64_t xen_ulong_t;
 #define PRI_xen_ulong "llx"
Why this change?
I will double check but I think the other fix exposed then the common
"<stdint.h> definitions not available in userspace <linux/types.h>" and this
was needed to please the compiler. If so, I'll add this to commit message.

If you would prefer to include libc's <stdint.h> in userspace, well, join the
club. drm and fuse maintainers want the same but my patches with those fixes got
rejected in the past. Example: https://lkml.org/lkml/2015/6/1/160

-Mikko

Re: [PATCH v05 54/72] include/uapi/xen/privcmd.h: fix compilation in userspace

From: Stefano Stabellini <sstabellini@kernel.org>
Date: 2016-08-23 21:16:19

On Tue, 23 Aug 2016, Mikko Rapeli wrote:
On Tue, Aug 23, 2016 at 11:13:52AM -0700, Stefano Stabellini wrote:
quoted
On Tue, 23 Aug 2016, Russell King - ARM Linux wrote:
quoted
On Mon, Aug 22, 2016 at 08:33:11PM +0200, Mikko Rapeli wrote:
quoted
xen/interface/xen.h is not exported from kernel headers so remove the
dependency and provide needed defines for domid_t and xen_pfn_t if they
are not already defined by some other e.g. Xen specific headers.
I'm confused.  How did we end up with a 64-bit PFN number on ARM?  It's
insane - especially as the kernel uses "unsigned long" almost everywhere
for PFNs - we can't have physical addresses more than 44 bits (32 bit
pfn + 4k page size).
That's because xen_pfn_t is the type used to store pfns in structures
passed to Xen via hypercalls. The Xen hypercall ABI is shared between
ARM and ARM64. On x86_32 and x86_64 we have different sizes for pfn
types in the hypercall ABI and it caused quite a bit of trouble in the
past as it is possible to run 32bit domains on a 64bit hypervisor.
Having a common type for pfns on ARM and ARM64 helped a lot in that
respect.

quoted
quoted
Suggested by Andrew Cooper [off-list ref] on lkml message
[off-list ref].

The ifdef for ARM is ugly but did not find better solutions for it.
#ifdef __arm__

maybe?  Even if not, the unsightly parens are not necessary.
Yes, I think it should be:

#if defined(__arm__) || defined(__aarch64__)
Thanks, I will send a new version with this change.
 
quoted
quoted
diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h
index 75d5968..6898ee1 100644
--- a/arch/arm/include/asm/xen/interface.h
+++ b/arch/arm/include/asm/xen/interface.h
@@ -38,7 +38,7 @@
  * fine since it simply wouldn't be able to create any sure pfns in
  * the first place.
  */
-typedef uint64_t xen_pfn_t;
+typedef __u64 xen_pfn_t;
 #define PRI_xen_pfn "llx"
 typedef uint64_t xen_ulong_t;
 #define PRI_xen_ulong "llx"
Why this change?
I will double check but I think the other fix exposed then the common
"<stdint.h> definitions not available in userspace <linux/types.h>" and this
was needed to please the compiler. If so, I'll add this to commit message.
That would be strange, because I don't think
arch/arm/include/asm/xen/interface.h is exposed to userspace. If it
was, we would need to replace the other definitions there too.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help