Building xtf on older distros

Building xtf on older distros

[Glenn Enright]

Hi there

When building the xen testing framework on CentOS6, I see the below 
issue. I see similar issues with CentOS7 and other distros with older 
tool chains. The only one that built cleanly for me was Ubuntu 1604.

Some workarounds (disabling the -W options) let it get a bit further but 
I hit other issues.

Andrew separately mentioned this may be related to C99 initialisation, 
which sounds about right.

At the moment I'm doing a static build and moving the binaries where I 
need to run the tests, which is fine, but it would be nice to have a 
native c6 build working to match the rest of my existing test infra.

Not a C dev, but happy to test any/all things.

Not list subscribed so please include in CC.

Regards, Glenn
http://rimuhosting.com

...
gcc -pipe -I/root/gle/xtf/include -I/root/gle/xtf/arch/x86/include -MMD 
-MP  -Wall -Wextra -Werror -std=gnu99 -Wstrict-prototypes -O3 -g 
-fno-common -fno-asynchronous-unwind-tables -fno-strict-aliasing 
-fno-stack-protector -fno-pic -ffreestanding -mno-red-zone -mno-sse 
-Wno-unused-parameter -Winline -m64  -DCONFIG_ENV_pv64 -include 
arch/config.h -c /root/gle/xtf/arch/x86/setup.c -o 
/root/gle/xtf/arch/x86/setup-pv64.o
cc1: warnings being treated as errors
/root/gle/xtf/arch/x86/grant_table.c: In function ‘arch_map_gnttab’:
/root/gle/xtf/arch/x86/grant_table.c:19: error: implicit declaration of 
function ‘_Static_assert’
make[1]: *** [/root/gle/xtf/arch/x86/grant_table-pv64.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: Leaving directory `/root/gle/xtf/tests/cpuid'
make: *** [all] Error 2

# gcc --version
gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-18)
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

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

Re: Building xtf on older distros

[Wei Liu]

On Thu, Sep 28, 2017 at 02:43:57PM +1300, Glenn Enright wrote:
> Hi there
> 
> When building the xen testing framework on CentOS6, I see the below issue. I
> see similar issues with CentOS7 and other distros with older tool chains.
> The only one that built cleanly for me was Ubuntu 1604.
> 
> Some workarounds (disabling the -W options) let it get a bit further but I
> hit other issues.
> 
> Andrew separately mentioned this may be related to C99 initialisation, which
> sounds about right.
> 
> At the moment I'm doing a static build and moving the binaries where I need
> to run the tests, which is fine, but it would be nice to have a native c6
> build working to match the rest of my existing test infra.
> 
> Not a C dev, but happy to test any/all things.
> 
> Not list subscribed so please include in CC.
> 
> Regards, Glenn
> http://rimuhosting.com
> 
> ...
> gcc -pipe -I/root/gle/xtf/include -I/root/gle/xtf/arch/x86/include -MMD -MP
> -Wall -Wextra -Werror -std=gnu99 -Wstrict-prototypes -O3 -g -fno-common
> -fno-asynchronous-unwind-tables -fno-strict-aliasing -fno-stack-protector
> -fno-pic -ffreestanding -mno-red-zone -mno-sse -Wno-unused-parameter
> -Winline -m64  -DCONFIG_ENV_pv64 -include arch/config.h -c
> /root/gle/xtf/arch/x86/setup.c -o /root/gle/xtf/arch/x86/setup-pv64.o
> cc1: warnings being treated as errors
> /root/gle/xtf/arch/x86/grant_table.c: In function ‘arch_map_gnttab’:
> /root/gle/xtf/arch/x86/grant_table.c:19: error: implicit declaration of
> function ‘_Static_assert’
> make[1]: *** [/root/gle/xtf/arch/x86/grant_table-pv64.o] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make[1]: Leaving directory `/root/gle/xtf/tests/cpuid'
> make: *** [all] Error 2
> 
> # gcc --version
> gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-18)
> Copyright (C) 2010 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 

Please try the following patch:

---8<---
From 7fd09585c1b511d1cd2c2b761d32bfcfb022d2d8 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Thu, 28 Sep 2017 09:43:12 +0100
Subject: [XTF PATCH] lib: provide BUILD_BUG_ON for older gcc's
Cc: andrew.cooper3@citrix.com

Reported-by: Glenn Enright <glenn@rimuhosting.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 include/xtf/lib.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/xtf/lib.h b/include/xtf/lib.h
index d792a8d..1d2a396 100644
--- a/include/xtf/lib.h
+++ b/include/xtf/lib.h
@@ -28,8 +28,12 @@ void __noreturn panic(const char *fmt, ...) __printf(1, 2);
                   #cond, __FILE__, __LINE__);           \
     } while ( 0 )
 
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
 #define BUILD_BUG_ON(cond)                              \
     _Static_assert(!(cond), "!(" #cond ")")
+#else
+#define BUILD_BUG_ON(cond) sizeof(struct { int:-!!(cond); })
+#endif
 
 #define min(a, b)                                       \
     ({                                                  \
-- 
2.11.0


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

Re: Building xtf on older distros

[Andrew Cooper]

On 28/09/2017 09:45, Wei Liu wrote:
> On Thu, Sep 28, 2017 at 02:43:57PM +1300, Glenn Enright wrote:
>> Hi there
>>
>> When building the xen testing framework on CentOS6, I see the below issue. I
>> see similar issues with CentOS7 and other distros with older tool chains.
>> The only one that built cleanly for me was Ubuntu 1604.
>>
>> Some workarounds (disabling the -W options) let it get a bit further but I
>> hit other issues.
>>
>> Andrew separately mentioned this may be related to C99 initialisation, which
>> sounds about right.
>>
>> At the moment I'm doing a static build and moving the binaries where I need
>> to run the tests, which is fine, but it would be nice to have a native c6
>> build working to match the rest of my existing test infra.
>>
>> Not a C dev, but happy to test any/all things.
>>
>> Not list subscribed so please include in CC.
>>
>> Regards, Glenn
>> http://rimuhosting.com
>>
>> ...
>> gcc -pipe -I/root/gle/xtf/include -I/root/gle/xtf/arch/x86/include -MMD -MP
>> -Wall -Wextra -Werror -std=gnu99 -Wstrict-prototypes -O3 -g -fno-common
>> -fno-asynchronous-unwind-tables -fno-strict-aliasing -fno-stack-protector
>> -fno-pic -ffreestanding -mno-red-zone -mno-sse -Wno-unused-parameter
>> -Winline -m64  -DCONFIG_ENV_pv64 -include arch/config.h -c
>> /root/gle/xtf/arch/x86/setup.c -o /root/gle/xtf/arch/x86/setup-pv64.o
>> cc1: warnings being treated as errors
>> /root/gle/xtf/arch/x86/grant_table.c: In function ‘arch_map_gnttab’:
>> /root/gle/xtf/arch/x86/grant_table.c:19: error: implicit declaration of
>> function ‘_Static_assert’
>> make[1]: *** [/root/gle/xtf/arch/x86/grant_table-pv64.o] Error 1
>> make[1]: *** Waiting for unfinished jobs....
>> make[1]: Leaving directory `/root/gle/xtf/tests/cpuid'
>> make: *** [all] Error 2
>>
>> # gcc --version
>> gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-18)
>> Copyright (C) 2010 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions.  There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>
> Please try the following patch:

;p

Sadly not.  There will be complaints of statements with no effect.

Beyond _Static_assert(), there are some anonymous union initialisation
problems, register scheduling problems, and binutils lack of
understanding of vmfunc.  I've a series in progress.

~Andrew

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

Re: Building xtf on older distros

[Wei Liu]

On Thu, Sep 28, 2017 at 09:51:20AM +0100, Andrew Cooper wrote:
> On 28/09/2017 09:45, Wei Liu wrote:
> > On Thu, Sep 28, 2017 at 02:43:57PM +1300, Glenn Enright wrote:
> >> Hi there
> >>
> >> When building the xen testing framework on CentOS6, I see the below issue. I
> >> see similar issues with CentOS7 and other distros with older tool chains.
> >> The only one that built cleanly for me was Ubuntu 1604.
> >>
> >> Some workarounds (disabling the -W options) let it get a bit further but I
> >> hit other issues.
> >>
> >> Andrew separately mentioned this may be related to C99 initialisation, which
> >> sounds about right.
> >>
> >> At the moment I'm doing a static build and moving the binaries where I need
> >> to run the tests, which is fine, but it would be nice to have a native c6
> >> build working to match the rest of my existing test infra.
> >>
> >> Not a C dev, but happy to test any/all things.
> >>
> >> Not list subscribed so please include in CC.
> >>
> >> Regards, Glenn
> >> http://rimuhosting.com
> >>
> >> ...
> >> gcc -pipe -I/root/gle/xtf/include -I/root/gle/xtf/arch/x86/include -MMD -MP
> >> -Wall -Wextra -Werror -std=gnu99 -Wstrict-prototypes -O3 -g -fno-common
> >> -fno-asynchronous-unwind-tables -fno-strict-aliasing -fno-stack-protector
> >> -fno-pic -ffreestanding -mno-red-zone -mno-sse -Wno-unused-parameter
> >> -Winline -m64  -DCONFIG_ENV_pv64 -include arch/config.h -c
> >> /root/gle/xtf/arch/x86/setup.c -o /root/gle/xtf/arch/x86/setup-pv64.o
> >> cc1: warnings being treated as errors
> >> /root/gle/xtf/arch/x86/grant_table.c: In function ‘arch_map_gnttab’:
> >> /root/gle/xtf/arch/x86/grant_table.c:19: error: implicit declaration of
> >> function ‘_Static_assert’
> >> make[1]: *** [/root/gle/xtf/arch/x86/grant_table-pv64.o] Error 1
> >> make[1]: *** Waiting for unfinished jobs....
> >> make[1]: Leaving directory `/root/gle/xtf/tests/cpuid'
> >> make: *** [all] Error 2
> >>
> >> # gcc --version
> >> gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-18)
> >> Copyright (C) 2010 Free Software Foundation, Inc.
> >> This is free software; see the source for copying conditions.  There is NO
> >> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> >>
> > Please try the following patch:
> 
> ;p
> 
> Sadly not.  There will be complaints of statements with no effect.
> 
> Beyond _Static_assert(), there are some anonymous union initialisation
> problems, register scheduling problems, and binutils lack of
> understanding of vmfunc.  I've a series in progress.

OK. I am more than happy to drop this from my list. :-)

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

[PATCH XTF 1/4] build: Support BUILD_BUG_ON() with compilers lacking _Static_assert()

[Andrew Cooper]

Implement enough compatibility so the code can use Clang's __has_extension()
logic when compiled with GCC.

Reported-by: Glenn Enright <glenn@rimuhosting.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 include/xtf/compiler-gcc.h | 31 +++++++++++++++++++++++++++++++
 include/xtf/compiler.h     |  4 ++++
 include/xtf/lib.h          |  7 ++++++-
 3 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 include/xtf/compiler-gcc.h

diff --git a/include/xtf/compiler-gcc.h b/include/xtf/compiler-gcc.h
new file mode 100644
index 0000000..2d3bc89
--- /dev/null
+++ b/include/xtf/compiler-gcc.h
@@ -0,0 +1,31 @@
+#ifndef XTF_COMPILER_GCC_H
+#define XTF_COMPILER_GCC_H
+
+#define GCC_VER (__GNUC__ * 10000 +		\
+		 __GNUC_MINOR__ * 100 +		\
+		 __GNUC_PATCHLEVEL__)
+
+/*
+ * The Clang __has_*() infrastructure is a very clean way to identify
+ * compiler support, without resorting to version checks.  Fake up
+ * enough support for XTF code to use, even on non-clang compilers.
+ */
+
+#ifndef __has_extension
+
+#define GCC_HAS_c_static_assert (GCC_VER >= 40600) /* _Static_assert() */
+
+#define __has_extension(x) GCC_HAS_ ## x
+#endif /* __has_extension */
+
+#endif /* XTF_COMPILER_GCC_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/include/xtf/compiler.h b/include/xtf/compiler.h
index 9dd6734..aa5fd6b 100644
--- a/include/xtf/compiler.h
+++ b/include/xtf/compiler.h
@@ -1,6 +1,10 @@
 #ifndef XTF_COMPILER_H
 #define XTF_COMPILER_H
 
+#ifdef __GNUC__
+#include <xtf/compiler-gcc.h>
+#endif
+
 #define __alias(x)            __attribute__((__alias__(x)))
 #define __aligned(x)          __attribute__((__aligned__(x)))
 #define __noreturn            __attribute__((__noreturn__))
diff --git a/include/xtf/lib.h b/include/xtf/lib.h
index d792a8d..abf8f25 100644
--- a/include/xtf/lib.h
+++ b/include/xtf/lib.h
@@ -28,8 +28,13 @@ void __noreturn panic(const char *fmt, ...) __printf(1, 2);
                   #cond, __FILE__, __LINE__);           \
     } while ( 0 )
 
-#define BUILD_BUG_ON(cond)                              \
+#if __has_extension(c_static_assert)
+# define BUILD_BUG_ON(cond)                             \
     _Static_assert(!(cond), "!(" #cond ")")
+#else
+# define BUILD_BUG_ON(cond)                             \
+    ((void)sizeof(struct { char: -!!(cond); }))
+#endif
 
 #define min(a, b)                                       \
     ({                                                  \
-- 
2.1.4


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

[PATCH XTF 2/4] build: Drop unnecessary register clobbers

[Andrew Cooper]

The code in question is executing __HYPERVISOR_multicall which is 2-parameter
hypercall, which means that args 3-6 are preserved (as opposed to the
arguments in the multicall_entry_t list, which are clobbered).

GCC 4.4 in CentOS 6 can't cope with the ebp clobber.

Reported-by: Glenn Enright <glenn@rimuhosting.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tests/xsa-213/main.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/tests/xsa-213/main.c b/tests/xsa-213/main.c
index 34eac06..6ecaf11 100644
--- a/tests/xsa-213/main.c
+++ b/tests/xsa-213/main.c
@@ -96,12 +96,6 @@ static long multi_iret_call(multicall_entry_t *multi, size_t nr)
 #else
                     "D" (multi), "S" (nr)
 #endif
-                  :
-#ifdef __i386__
-                  "edx", "esi", "edi", "ebp"
-#else
-                  "rdx", "r10", "r8", "r9"
-#endif
         );
 
     return rc;
-- 
2.1.4


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

[PATCH XTF 3/4] build: Opencode vmfunc as bytes

[Andrew Cooper]

Binutils 2.20 of CentOS 6 vintage doesn't understand the mnemonic.  The
instruction doesn't encode any operands, so we don't lose any flexibility.

Reported-by: Glenn Enright <glenn@rimuhosting.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tests/xsa-203/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/xsa-203/main.c b/tests/xsa-203/main.c
index 01b5bca..e786577 100644
--- a/tests/xsa-203/main.c
+++ b/tests/xsa-203/main.c
@@ -22,7 +22,9 @@ bool test_needs_fep = true;
 void test_main(void)
 {
     asm volatile (_ASM_XEN_FEP
-                  "1: vmfunc; 2:"
+                  "1:"
+                  ".byte 0x0f, 0x01, 0xd4;" /* VMFUNC */
+                  "2:"
                   _ASM_EXTABLE(1b, 2b) /* Ignore #UD on older versions. */
                   :: "a" (0));
 
-- 
2.1.4


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

[PATCH XTF 4/4] build: Avoid using initialisers for anonymous unions

[Andrew Cooper]

GCC 4.4 of CentOS 6 vintage can't cope.

Reported-by: Glenn Enright <glenn@rimuhosting.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tests/livepatch-priv-check/main.c | 75 ++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 49 deletions(-)

diff --git a/tests/livepatch-priv-check/main.c b/tests/livepatch-priv-check/main.c
index e51ba64..9f9e778 100644
--- a/tests/livepatch-priv-check/main.c
+++ b/tests/livepatch-priv-check/main.c
@@ -38,23 +38,17 @@ static void check_ret(const char *test, int rc)
 static void test_upload(void)
 {
     static uint8_t payload[PAGE_SIZE];
-    xen_sysctl_t op =
-    {
+    xen_sysctl_t op = {
         .cmd = XEN_SYSCTL_livepatch_op,
         .interface_version = sysctl_interface_version,
-        .u.livepatch = {
-            .cmd = XEN_SYSCTL_LIVEPATCH_UPLOAD,
-            .u.upload = {
-                .name = {
-                    .name.p = TEST_NAME,
-                    .size = sizeof(TEST_NAME),
-                },
-                .size = PAGE_SIZE,
-                .payload.p = payload,
-            },
-        },
     };
 
+    op.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_UPLOAD;
+    op.u.livepatch.u.upload.name.name.p = TEST_NAME;
+    op.u.livepatch.u.upload.name.size = sizeof(TEST_NAME);
+    op.u.livepatch.u.upload.size = PAGE_SIZE;
+    op.u.livepatch.u.upload.payload.p = payload;
+
     check_ret(__func__, hypercall_sysctl(&op));
 }
 
@@ -64,64 +58,47 @@ static void test_list(void)
 {
     char names[NR_PAYLOADS * XEN_LIVEPATCH_NAME_SIZE];
     uint32_t lengths[NR_PAYLOADS];
-
-    xen_sysctl_t op =
-    {
+    xen_sysctl_t op = {
         .cmd = XEN_SYSCTL_livepatch_op,
         .interface_version = sysctl_interface_version,
-        .u.livepatch = {
-            .cmd = XEN_SYSCTL_LIVEPATCH_LIST,
-            .u.list = {
-                .idx = 0,
-                .nr = NR_PAYLOADS,
-                .name.p = names,
-                .len.p = lengths,
-            },
-        },
     };
 
+    op.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_LIST;
+    op.u.livepatch.u.list.idx = 0;
+    op.u.livepatch.u.list.nr = NR_PAYLOADS;
+    op.u.livepatch.u.list.name.p = names;
+    op.u.livepatch.u.list.len.p = lengths;
+
     check_ret(__func__, hypercall_sysctl(&op));
 }
 
 static void test_get(void)
 {
-    xen_sysctl_t op =
-    {
+    xen_sysctl_t op = {
         .cmd = XEN_SYSCTL_livepatch_op,
         .interface_version = sysctl_interface_version,
-        .u.livepatch = {
-            .cmd = XEN_SYSCTL_LIVEPATCH_GET,
-            .u.get = {
-                .name = {
-                    .name.p = TEST_NAME,
-                    .size = sizeof(TEST_NAME),
-                },
-            },
-        },
     };
 
+    op.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_GET;
+    op.u.livepatch.u.get.name.name.p = TEST_NAME;
+    op.u.livepatch.u.get.name.size = sizeof(TEST_NAME);
+
     check_ret(__func__, hypercall_sysctl(&op));
 }
 
 static void test_action(uint32_t action)
 {
-    xen_sysctl_t op =
-    {
+    xen_sysctl_t op = {
         .cmd = XEN_SYSCTL_livepatch_op,
         .interface_version = sysctl_interface_version,
-        .u.livepatch = {
-            .cmd = XEN_SYSCTL_LIVEPATCH_ACTION,
-            .u.action = {
-                .name = {
-                    .name.p = TEST_NAME,
-                    .size = sizeof(TEST_NAME),
-                },
-                .cmd = action,
-                .timeout = 0,
-            },
-        },
     };
 
+    op.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_ACTION;
+    op.u.livepatch.u.action.name.name.p = TEST_NAME;
+    op.u.livepatch.u.action.name.size = sizeof(TEST_NAME);
+    op.u.livepatch.u.action.cmd = action;
+    op.u.livepatch.u.action.timeout = 0;
+
     check_ret(__func__, hypercall_sysctl(&op));
 }
 
-- 
2.1.4


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

Re: [PATCH XTF 1/4] build: Support BUILD_BUG_ON() with compilers lacking _Static_assert()

[Jan Beulich]

>>> On 28.09.17 at 12:59, <andrew.cooper3@citrix.com> wrote:
> --- /dev/null
> +++ b/include/xtf/compiler-gcc.h
> @@ -0,0 +1,31 @@
> +#ifndef XTF_COMPILER_GCC_H
> +#define XTF_COMPILER_GCC_H
> +
> +#define GCC_VER (__GNUC__ * 10000 +		\
> +		 __GNUC_MINOR__ * 100 +		\
> +		 __GNUC_PATCHLEVEL__)
> +
> +/*
> + * The Clang __has_*() infrastructure is a very clean way to identify
> + * compiler support, without resorting to version checks.  Fake up
> + * enough support for XTF code to use, even on non-clang compilers.
> + */
> +
> +#ifndef __has_extension
> +
> +#define GCC_HAS_c_static_assert (GCC_VER >= 40600) /* _Static_assert() */
> +
> +#define __has_extension(x) GCC_HAS_ ## x

As always I'd recommend against underscore prefixed names.

Jan


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

Re: [PATCH XTF 2/4] build: Drop unnecessary register clobbers

[Jan Beulich]

>>> On 28.09.17 at 12:59, <andrew.cooper3@citrix.com> wrote:
> The code in question is executing __HYPERVISOR_multicall which is 2-parameter
> hypercall, which means that args 3-6 are preserved (as opposed to the
> arguments in the multicall_entry_t list, which are clobbered).
> 
> GCC 4.4 in CentOS 6 can't cope with the ebp clobber.
> 
> Reported-by: Glenn Enright <glenn@rimuhosting.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>



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

Re: [PATCH XTF 3/4] build: Opencode vmfunc as bytes

[Jan Beulich]

>>> On 28.09.17 at 12:59, <andrew.cooper3@citrix.com> wrote:
> Binutils 2.20 of CentOS 6 vintage doesn't understand the mnemonic.  The
> instruction doesn't encode any operands, so we don't lose any flexibility.
> 
> Reported-by: Glenn Enright <glenn@rimuhosting.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

FWIW
Reviewed-by: Jan Beulich <jbeulich@suse.com>



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

Re: [PATCH XTF 1/4] build: Support BUILD_BUG_ON() with compilers lacking _Static_assert()

[Andrew Cooper]

On 28/09/17 13:32, Jan Beulich wrote:
>>>> On 28.09.17 at 12:59, <andrew.cooper3@citrix.com> wrote:
>> --- /dev/null
>> +++ b/include/xtf/compiler-gcc.h
>> @@ -0,0 +1,31 @@
>> +#ifndef XTF_COMPILER_GCC_H
>> +#define XTF_COMPILER_GCC_H
>> +
>> +#define GCC_VER (__GNUC__ * 10000 +		\
>> +		 __GNUC_MINOR__ * 100 +		\
>> +		 __GNUC_PATCHLEVEL__)
>> +
>> +/*
>> + * The Clang __has_*() infrastructure is a very clean way to identify
>> + * compiler support, without resorting to version checks.  Fake up
>> + * enough support for XTF code to use, even on non-clang compilers.
>> + */
>> +
>> +#ifndef __has_extension
>> +
>> +#define GCC_HAS_c_static_assert (GCC_VER >= 40600) /* _Static_assert() */
>> +
>> +#define __has_extension(x) GCC_HAS_ ## x
> As always I'd recommend against underscore prefixed names.

In this case, its a Clang feature which I'm deliberately trying to use.

https://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros

~Andrew

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

Re: [PATCH XTF 4/4] build: Avoid using initialisers for anonymous unions

[Jan Beulich]

>>> On 28.09.17 at 12:59, <andrew.cooper3@citrix.com> wrote:
> --- a/tests/livepatch-priv-check/main.c
> +++ b/tests/livepatch-priv-check/main.c
> @@ -38,23 +38,17 @@ static void check_ret(const char *test, int rc)
>  static void test_upload(void)
>  {
>      static uint8_t payload[PAGE_SIZE];
> -    xen_sysctl_t op =
> -    {
> +    xen_sysctl_t op = {
>          .cmd = XEN_SYSCTL_livepatch_op,
>          .interface_version = sysctl_interface_version,
> -        .u.livepatch = {
> -            .cmd = XEN_SYSCTL_LIVEPATCH_UPLOAD,
> -            .u.upload = {
> -                .name = {
> -                    .name.p = TEST_NAME,
> -                    .size = sizeof(TEST_NAME),
> -                },
> -                .size = PAGE_SIZE,
> -                .payload.p = payload,
> -            },
> -        },
>      };
>  
> +    op.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_UPLOAD;
> +    op.u.livepatch.u.upload.name.name.p = TEST_NAME;
> +    op.u.livepatch.u.upload.name.size = sizeof(TEST_NAME);
> +    op.u.livepatch.u.upload.size = PAGE_SIZE;
> +    op.u.livepatch.u.upload.payload.p = payload;

So where's the anonymous field here? Same for the other changes
further down.

Jan


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

Re: [PATCH XTF 1/4] build: Support BUILD_BUG_ON() with compilers lacking _Static_assert()

[Jan Beulich]

>>> On 28.09.17 at 14:34, <andrew.cooper3@citrix.com> wrote:
> On 28/09/17 13:32, Jan Beulich wrote:
>>>>> On 28.09.17 at 12:59, <andrew.cooper3@citrix.com> wrote:
>>> --- /dev/null
>>> +++ b/include/xtf/compiler-gcc.h
>>> @@ -0,0 +1,31 @@
>>> +#ifndef XTF_COMPILER_GCC_H
>>> +#define XTF_COMPILER_GCC_H
>>> +
>>> +#define GCC_VER (__GNUC__ * 10000 +		\
>>> +		 __GNUC_MINOR__ * 100 +		\
>>> +		 __GNUC_PATCHLEVEL__)
>>> +
>>> +/*
>>> + * The Clang __has_*() infrastructure is a very clean way to identify
>>> + * compiler support, without resorting to version checks.  Fake up
>>> + * enough support for XTF code to use, even on non-clang compilers.
>>> + */
>>> +
>>> +#ifndef __has_extension
>>> +
>>> +#define GCC_HAS_c_static_assert (GCC_VER >= 40600) /* _Static_assert() */
>>> +
>>> +#define __has_extension(x) GCC_HAS_ ## x
>> As always I'd recommend against underscore prefixed names.
> 
> In this case, its a Clang feature which I'm deliberately trying to use.
> 
> https://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros 

Oh, sorry, should have paid closer attention to the comment.
But wouldn't it still be better to define has_extension(), simply
evaluating to _has_extension() for clang?

Jan


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

Re: [PATCH XTF 4/4] build: Avoid using initialisers for anonymous unions

[Andrew Cooper]

On 28/09/17 13:37, Jan Beulich wrote:
>>>> On 28.09.17 at 12:59, <andrew.cooper3@citrix.com> wrote:
>> --- a/tests/livepatch-priv-check/main.c
>> +++ b/tests/livepatch-priv-check/main.c
>> @@ -38,23 +38,17 @@ static void check_ret(const char *test, int rc)
>>  static void test_upload(void)
>>  {
>>      static uint8_t payload[PAGE_SIZE];
>> -    xen_sysctl_t op =
>> -    {
>> +    xen_sysctl_t op = {
>>          .cmd = XEN_SYSCTL_livepatch_op,
>>          .interface_version = sysctl_interface_version,
>> -        .u.livepatch = {
>> -            .cmd = XEN_SYSCTL_LIVEPATCH_UPLOAD,
>> -            .u.upload = {
>> -                .name = {
>> -                    .name.p = TEST_NAME,
>> -                    .size = sizeof(TEST_NAME),
>> -                },
>> -                .size = PAGE_SIZE,
>> -                .payload.p = payload,
>> -            },
>> -        },
>>      };
>>  
>> +    op.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_UPLOAD;
>> +    op.u.livepatch.u.upload.name.name.p = TEST_NAME;
>> +    op.u.livepatch.u.upload.name.size = sizeof(TEST_NAME);
>> +    op.u.livepatch.u.upload.size = PAGE_SIZE;
>> +    op.u.livepatch.u.upload.payload.p = payload;
> So where's the anonymous field here? Same for the other changes
> further down.

main.c: In function ‘test_upload’:
main.c:49: error: unknown field ‘p’ specified in initializer
cc1: warnings being treated as errors
main.c:49: error: missing braces around initializer
main.c:49: error: (near initialization for
‘op.u.livepatch.u.upload.name.name.<anonymous>’)
main.c:53: error: unknown field ‘p’ specified in initializer
main.c: In function ‘test_upload’:
main.c:49: error: unknown field ‘p’ specified in initializer
cc1: warnings being treated as errors

although this does give me an idea to simplify the patch...

~Andrew

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

[PATCH XTF v2 4/4] build: Avoid using initialisers for anonymous unions

[Andrew Cooper]

GCC 4.4 of CentOS 6 vintage can't cope.

Reported-by: Glenn Enright <glenn@rimuhosting.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v2: Reduce to just the anonymous parts of the union
---
 tests/livepatch-priv-check/main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/livepatch-priv-check/main.c b/tests/livepatch-priv-check/main.c
index e51ba64..8a96ad0 100644
--- a/tests/livepatch-priv-check/main.c
+++ b/tests/livepatch-priv-check/main.c
@@ -46,11 +46,11 @@ static void test_upload(void)
             .cmd = XEN_SYSCTL_LIVEPATCH_UPLOAD,
             .u.upload = {
                 .name = {
-                    .name.p = TEST_NAME,
+                    .name = {{ TEST_NAME }},
                     .size = sizeof(TEST_NAME),
                 },
                 .size = PAGE_SIZE,
-                .payload.p = payload,
+                .payload = {{ payload }},
             },
         },
     };
@@ -74,8 +74,8 @@ static void test_list(void)
             .u.list = {
                 .idx = 0,
                 .nr = NR_PAYLOADS,
-                .name.p = names,
-                .len.p = lengths,
+                .name = {{ names }},
+                .len = {{ lengths }},
             },
         },
     };
@@ -93,7 +93,7 @@ static void test_get(void)
             .cmd = XEN_SYSCTL_LIVEPATCH_GET,
             .u.get = {
                 .name = {
-                    .name.p = TEST_NAME,
+                    .name = {{ TEST_NAME }},
                     .size = sizeof(TEST_NAME),
                 },
             },
@@ -113,7 +113,7 @@ static void test_action(uint32_t action)
             .cmd = XEN_SYSCTL_LIVEPATCH_ACTION,
             .u.action = {
                 .name = {
-                    .name.p = TEST_NAME,
+                    .name = {{ TEST_NAME }},
                     .size = sizeof(TEST_NAME),
                 },
                 .cmd = action,
-- 
2.1.4


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

Re: [PATCH XTF v2 4/4] build: Avoid using initialisers for anonymous unions

[Glenn Enright]

On 29/09/17 06:10, Andrew Cooper wrote:
> GCC 4.4 of CentOS 6 vintage can't cope.
> 
> Reported-by: Glenn Enright <glenn@rimuhosting.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> v2: Reduce to just the anonymous parts of the union
> ---
>   tests/livepatch-priv-check/main.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/livepatch-priv-check/main.c b/tests/livepatch-priv-check/main.c
> index e51ba64..8a96ad0 100644
> --- a/tests/livepatch-priv-check/main.c
> +++ b/tests/livepatch-priv-check/main.c
> @@ -46,11 +46,11 @@ static void test_upload(void)
>               .cmd = XEN_SYSCTL_LIVEPATCH_UPLOAD,
>               .u.upload = {
>                   .name = {
> -                    .name.p = TEST_NAME,
> +                    .name = {{ TEST_NAME }},
>                       .size = sizeof(TEST_NAME),
>                   },
>                   .size = PAGE_SIZE,
> -                .payload.p = payload,
> +                .payload = {{ payload }},
>               },
>           },
>       };
> @@ -74,8 +74,8 @@ static void test_list(void)
>               .u.list = {
>                   .idx = 0,
>                   .nr = NR_PAYLOADS,
> -                .name.p = names,
> -                .len.p = lengths,
> +                .name = {{ names }},
> +                .len = {{ lengths }},
>               },
>           },
>       };
> @@ -93,7 +93,7 @@ static void test_get(void)
>               .cmd = XEN_SYSCTL_LIVEPATCH_GET,
>               .u.get = {
>                   .name = {
> -                    .name.p = TEST_NAME,
> +                    .name = {{ TEST_NAME }},
>                       .size = sizeof(TEST_NAME),
>                   },
>               },
> @@ -113,7 +113,7 @@ static void test_action(uint32_t action)
>               .cmd = XEN_SYSCTL_LIVEPATCH_ACTION,
>               .u.action = {
>                   .name = {
> -                    .name.p = TEST_NAME,
> +                    .name = {{ TEST_NAME }},
>                       .size = sizeof(TEST_NAME),
>                   },
>                   .cmd = action,
> 

With this and other recent patches in the repo, it now builds cleanly 
for me on c6 and elsewhere, thanks so much!

Regards, Glenn
http://rimuhosting.com
See more on our other services at http://ri.mu

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