All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang
@ 2019-10-02 18:00 Julien Grall
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 1/6] xen/arm: fix get_cpu_info() when built " Julien Grall
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Julien Grall @ 2019-10-02 18:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Artem_Mygaiev, Volodymyr Babchuk, Julien Grall,
	Stefano Stabellini, Julien Grall

Hi all,

After this series, I am able to build Xen on Arm64 with clang 7.0. There
are still some issues when building Xen on Arm32 and also using lld.

Cross-compilation is left outside for now, but this is still a good start
for clang (and armclang).

Cheers,

Julien Grall (6):
  xen/arm: fix get_cpu_info() when built with clang
  xen/arm64: bitops: Match the register size with the value size in flsl
  xen/arm: cpuerrata: Match register size with value size in
    check_workaround_*
  xen/arm: cpufeature: Match register size with value size in
    cpus_have_const_cap
  xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused
  xen/arm: traps: Mark check_stack_alignment_constraints as unused

 xen/arch/arm/mm.c                  |  3 ++-
 xen/arch/arm/traps.c               |  3 ++-
 xen/include/asm-arm/arm64/bitops.h |  3 ++-
 xen/include/asm-arm/cpuerrata.h    |  4 ++--
 xen/include/asm-arm/cpufeature.h   |  4 ++--
 xen/include/asm-arm/current.h      | 10 +++++++++-
 6 files changed, 19 insertions(+), 8 deletions(-)

-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Xen-devel] [PATCH for-4.13 1/6] xen/arm: fix get_cpu_info() when built with clang
  2019-10-02 18:00 [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Julien Grall
@ 2019-10-02 18:00 ` Julien Grall
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 2/6] xen/arm64: bitops: Match the register size with the value size in flsl Julien Grall
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Julien Grall @ 2019-10-02 18:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Artem_Mygaiev, Volodymyr Babchuk, Julien Grall,
	Stefano Stabellini, Julien Grall

Clang understands the GCCism in use here, but still complains that sp is
unitialised. In such cases, resort to the older versions of this code,
which directly read sp into the temporary variable.

Note that GCCism is still kept in default because other compilers (e.g.
clang) may also define __GNUC__, so AFAIK there are no proper way to
detect properly GCC.

This means that in the event Xen is ported to a new compiler, the code
will need to be updated. But that likely not going to be the only place
where Xen will need to be adapted...

This is based on the x86 counterpart.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---
    Changes in v2:
        - Update the commit message to explain the ordering in the code.
        - Add Stefano's acked-by
---
 xen/include/asm-arm/current.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
index 1653e89d30..80503578cf 100644
--- a/xen/include/asm-arm/current.h
+++ b/xen/include/asm-arm/current.h
@@ -28,8 +28,16 @@ struct cpu_info {
 
 static inline struct cpu_info *get_cpu_info(void)
 {
+#ifdef __clang__
+    unsigned long sp;
+
+    asm ("mov %0, sp" : "=r" (sp));
+#else
     register unsigned long sp asm ("sp");
-    return (struct cpu_info *)((sp & ~(STACK_SIZE - 1)) + STACK_SIZE - sizeof(struct cpu_info));
+#endif
+
+    return (struct cpu_info *)((sp & ~(STACK_SIZE - 1)) +
+                               STACK_SIZE - sizeof(struct cpu_info));
 }
 
 #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
-- 
2.11.0


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Xen-devel] [PATCH for-4.13 2/6] xen/arm64: bitops: Match the register size with the value size in flsl
  2019-10-02 18:00 [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Julien Grall
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 1/6] xen/arm: fix get_cpu_info() when built " Julien Grall
@ 2019-10-02 18:00 ` Julien Grall
  2019-10-02 22:26   ` Stefano Stabellini
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 3/6] xen/arm: cpuerrata: Match register size with value size in check_workaround_* Julien Grall
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Julien Grall @ 2019-10-02 18:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Artem_Mygaiev, Volodymyr Babchuk, Julien Grall,
	Stefano Stabellini, Julien Grall

Clang is pickier than GCC for the register size in asm statement. It expects
the register size to match the value size.

The instruction clz is expecting the two operands to be the same size
(i.e 32-bit or 64-bit). As the flsl function is dealing with 64-bit
value, we need to make the destination variable 64-bit as well.

While at it, add a newline before the return statement.

Note that the return type of flsl is not updated because the result will
always be smaller than 64 and therefore fit in 32-bit.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Update the commit message to explain why the return type is
        not modified
---
 xen/include/asm-arm/arm64/bitops.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/include/asm-arm/arm64/bitops.h b/xen/include/asm-arm/arm64/bitops.h
index 6cc224ad13..d85a49bca4 100644
--- a/xen/include/asm-arm/arm64/bitops.h
+++ b/xen/include/asm-arm/arm64/bitops.h
@@ -24,12 +24,13 @@ static /*__*/always_inline unsigned long __ffs(unsigned long word)
 
 static inline int flsl(unsigned long x)
 {
-        int ret;
+        uint64_t ret;
 
         if (__builtin_constant_p(x))
                return generic_flsl(x);
 
         asm("clz\t%0, %1" : "=r" (ret) : "r" (x));
+
         return BITS_PER_LONG - ret;
 }
 
-- 
2.11.0


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Xen-devel] [PATCH for-4.13 3/6] xen/arm: cpuerrata: Match register size with value size in check_workaround_*
  2019-10-02 18:00 [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Julien Grall
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 1/6] xen/arm: fix get_cpu_info() when built " Julien Grall
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 2/6] xen/arm64: bitops: Match the register size with the value size in flsl Julien Grall
@ 2019-10-02 18:00 ` Julien Grall
  2019-10-02 18:42   ` Andrew Cooper
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 4/6] xen/arm: cpufeature: Match register size with value size in cpus_have_const_cap Julien Grall
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Julien Grall @ 2019-10-02 18:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Artem_Mygaiev, Volodymyr Babchuk, Julien Grall,
	Stefano Stabellini, Julien Grall

Clang is pickier than GCC for the register size in asm statement. It
expects the register size to match the value size.

The asm statement expects a 32-bit (resp. 64-bit) value on Arm32
(resp. Arm64) whereas the value is a boolean (Clang consider to be
32-bit).

It would be possible to impose 32-bit register for both architecture
but this require the code to use __OP32. However, it does not really
improve the assembly generated. Instead, replace switch the variable
to use register_t.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Use !! per Stefano's request
---
 xen/include/asm-arm/cpuerrata.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index 55ddfda272..0896fe6e25 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -14,14 +14,14 @@ static inline bool check_workaround_##erratum(void)             \
         return false;                                           \
     else                                                        \
     {                                                           \
-        bool ret;                                               \
+        register_t ret;                                         \
                                                                 \
         asm volatile (ALTERNATIVE("mov %0, #0",                 \
                                   "mov %0, #1",                 \
                                   feature)                      \
                       : "=r" (ret));                            \
                                                                 \
-        return unlikely(ret);                                   \
+        return unlikely(!!ret);                                 \
     }                                                           \
 }
 
-- 
2.11.0


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Xen-devel] [PATCH for-4.13 4/6] xen/arm: cpufeature: Match register size with value size in cpus_have_const_cap
  2019-10-02 18:00 [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Julien Grall
                   ` (2 preceding siblings ...)
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 3/6] xen/arm: cpuerrata: Match register size with value size in check_workaround_* Julien Grall
@ 2019-10-02 18:00 ` Julien Grall
  2019-10-02 22:26   ` Stefano Stabellini
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 5/6] xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused Julien Grall
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Julien Grall @ 2019-10-02 18:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Artem_Mygaiev, Volodymyr Babchuk, Julien Grall,
	Stefano Stabellini, Julien Grall

Clang is pickier than GCC for the register size in asm statement. It
expects the register size to match the value size.

The asm statement expects a 32-bit (resp. 64-bit) value on Arm32
(resp. Arm64) whereas the value is a boolean (Clang consider to be
32-bit).

It would be possible to impose 32-bit register for both architecture
but this require the code to use __OP32. However, it does no really
improve the assembly generated. Instead, replace switch the variable to
use register_t.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Use !! per Stefano's request
---
 xen/include/asm-arm/cpufeature.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index c2c8f3417c..4c5ff6e8ac 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -67,14 +67,14 @@ static inline bool cpus_have_cap(unsigned int num)
 
 /* System capability check for constant cap */
 #define cpus_have_const_cap(num) ({                 \
-        bool __ret;                                 \
+        register_t __ret;                           \
                                                     \
         asm volatile (ALTERNATIVE("mov %0, #0",     \
                                   "mov %0, #1",     \
                                   num)              \
                       : "=r" (__ret));              \
                                                     \
-        unlikely(__ret);                            \
+        unlikely(!!__ret);                          \
         })
 
 static inline void cpus_set_cap(unsigned int num)
-- 
2.11.0


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Xen-devel] [PATCH for-4.13 5/6] xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused
  2019-10-02 18:00 [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Julien Grall
                   ` (3 preceding siblings ...)
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 4/6] xen/arm: cpufeature: Match register size with value size in cpus_have_const_cap Julien Grall
@ 2019-10-02 18:00 ` Julien Grall
  2019-10-02 22:26   ` Stefano Stabellini
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 6/6] xen/arm: traps: Mark check_stack_alignment_constraints " Julien Grall
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Julien Grall @ 2019-10-02 18:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Artem_Mygaiev, Volodymyr Babchuk, Julien Grall,
	Stefano Stabellini, Julien Grall

Clang will throw an error if a function is unused unless you tell
to ignore it. This can be done using __maybe_unused.

While modifying the declaration, update it to match prototype of similar
functions (see build_assertions). This helps to understand that the sole
purpose of the function is to hold BUILD_BUG_ON().

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Update the prototype to match style of other functions holding
        on build assertions.
---
 xen/arch/arm/mm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 9e0fdc39f9..be23acfe26 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -190,7 +190,8 @@ unsigned long total_pages;
 extern char __init_begin[], __init_end[];
 
 /* Checking VA memory layout alignment. */
-static inline void check_memory_layout_alignment_constraints(void) {
+static void __init __maybe_unused build_assertions(void)
+{
     /* 2MB aligned regions */
     BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
     BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);
-- 
2.11.0


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Xen-devel] [PATCH for-4.13 6/6] xen/arm: traps: Mark check_stack_alignment_constraints as unused
  2019-10-02 18:00 [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Julien Grall
                   ` (4 preceding siblings ...)
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 5/6] xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused Julien Grall
@ 2019-10-02 18:00 ` Julien Grall
  2019-10-02 22:26   ` Stefano Stabellini
  2019-10-03  7:37 ` [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Jürgen Groß
  2019-10-03  8:31 ` Artem Mygaiev
  7 siblings, 1 reply; 18+ messages in thread
From: Julien Grall @ 2019-10-02 18:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Artem_Mygaiev, Volodymyr Babchuk, Julien Grall,
	Stefano Stabellini, Julien Grall

Clang will throw an error if a function is unused unless you tell
to ignore it. This can be done using __maybe_unused.

While modifying the declaration, update it to match prototype of similar
functions (see build_assertions). This helps to understand that the sole
purpose of the function is to hold BUILD_BUG_ON().

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Update the prototype to match style of other functions holding
        on build assertions.
---
 xen/arch/arm/traps.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index a3b961bd06..a3deb59372 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -54,7 +54,8 @@
  * that both the kernel half of struct cpu_user_regs (which is pushed in
  * entry.S) and struct cpu_info (which lives at the bottom of a Xen
  * stack) must be doubleword-aligned in size.  */
-static inline void check_stack_alignment_constraints(void) {
+static void __init __maybe_unused build_assertions(void)
+{
 #ifdef CONFIG_ARM_64
     BUILD_BUG_ON((sizeof (struct cpu_user_regs)) & 0xf);
     BUILD_BUG_ON((offsetof(struct cpu_user_regs, spsr_el1)) & 0xf);
-- 
2.11.0


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 3/6] xen/arm: cpuerrata: Match register size with value size in check_workaround_*
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 3/6] xen/arm: cpuerrata: Match register size with value size in check_workaround_* Julien Grall
@ 2019-10-02 18:42   ` Andrew Cooper
  2019-10-02 19:17     ` Julien Grall
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2019-10-02 18:42 UTC (permalink / raw)
  To: Julien Grall, xen-devel
  Cc: Artem_Mygaiev, Julien Grall, Stefano Stabellini, Volodymyr Babchuk

On 02/10/2019 19:00, Julien Grall wrote:
> Clang is pickier than GCC for the register size in asm statement. It
> expects the register size to match the value size.
>
> The asm statement expects a 32-bit (resp. 64-bit) value on Arm32
> (resp. Arm64) whereas the value is a boolean (Clang consider to be
> 32-bit).
>
> It would be possible to impose 32-bit register for both architecture
> but this require the code to use __OP32. However, it does not really
> improve the assembly generated. Instead, replace switch the variable
> to use register_t.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> ---
>     Changes in v2:
>         - Use !! per Stefano's request

You are aware that unlikley(), deliberately, has an embedded !! ?

include/xen/compiler.h:11:#define unlikely(x)   __builtin_expect(!!(x),0)

~Andrew

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 3/6] xen/arm: cpuerrata: Match register size with value size in check_workaround_*
  2019-10-02 18:42   ` Andrew Cooper
@ 2019-10-02 19:17     ` Julien Grall
  2019-10-02 22:27       ` Stefano Stabellini
  0 siblings, 1 reply; 18+ messages in thread
From: Julien Grall @ 2019-10-02 19:17 UTC (permalink / raw)
  To: Andrew Cooper, xen-devel
  Cc: Artem_Mygaiev, Julien Grall, Stefano Stabellini, Volodymyr Babchuk

Hi Andrew,

On 10/2/19 7:42 PM, Andrew Cooper wrote:
> On 02/10/2019 19:00, Julien Grall wrote:
>> Clang is pickier than GCC for the register size in asm statement. It
>> expects the register size to match the value size.
>>
>> The asm statement expects a 32-bit (resp. 64-bit) value on Arm32
>> (resp. Arm64) whereas the value is a boolean (Clang consider to be
>> 32-bit).
>>
>> It would be possible to impose 32-bit register for both architecture
>> but this require the code to use __OP32. However, it does not really
>> improve the assembly generated. Instead, replace switch the variable
>> to use register_t.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> ---
>>      Changes in v2:
>>          - Use !! per Stefano's request
> 
> You are aware that unlikley(), deliberately, has an embedded !! ?

I forgot it, sorry about that :/. Stefano are you happy if I revert to v1?

> 
> include/xen/compiler.h:11:#define unlikely(x)   __builtin_expect(!!(x),0)
> 
> ~Andrew
> 

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 2/6] xen/arm64: bitops: Match the register size with the value size in flsl
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 2/6] xen/arm64: bitops: Match the register size with the value size in flsl Julien Grall
@ 2019-10-02 22:26   ` Stefano Stabellini
  0 siblings, 0 replies; 18+ messages in thread
From: Stefano Stabellini @ 2019-10-02 22:26 UTC (permalink / raw)
  To: Julien Grall
  Cc: Artem_Mygaiev, xen-devel, Stefano Stabellini, Julien Grall,
	Volodymyr Babchuk

On Wed, 2 Oct 2019, Julien Grall wrote:
> Clang is pickier than GCC for the register size in asm statement. It expects
> the register size to match the value size.
> 
> The instruction clz is expecting the two operands to be the same size
> (i.e 32-bit or 64-bit). As the flsl function is dealing with 64-bit
> value, we need to make the destination variable 64-bit as well.
> 
> While at it, add a newline before the return statement.
> 
> Note that the return type of flsl is not updated because the result will
> always be smaller than 64 and therefore fit in 32-bit.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Update the commit message to explain why the return type is
>         not modified
> ---
>  xen/include/asm-arm/arm64/bitops.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/include/asm-arm/arm64/bitops.h b/xen/include/asm-arm/arm64/bitops.h
> index 6cc224ad13..d85a49bca4 100644
> --- a/xen/include/asm-arm/arm64/bitops.h
> +++ b/xen/include/asm-arm/arm64/bitops.h
> @@ -24,12 +24,13 @@ static /*__*/always_inline unsigned long __ffs(unsigned long word)
>  
>  static inline int flsl(unsigned long x)
>  {
> -        int ret;
> +        uint64_t ret;
>  
>          if (__builtin_constant_p(x))
>                 return generic_flsl(x);
>  
>          asm("clz\t%0, %1" : "=r" (ret) : "r" (x));
> +
>          return BITS_PER_LONG - ret;
>  }
>  
> -- 
> 2.11.0
> 

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 4/6] xen/arm: cpufeature: Match register size with value size in cpus_have_const_cap
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 4/6] xen/arm: cpufeature: Match register size with value size in cpus_have_const_cap Julien Grall
@ 2019-10-02 22:26   ` Stefano Stabellini
  0 siblings, 0 replies; 18+ messages in thread
From: Stefano Stabellini @ 2019-10-02 22:26 UTC (permalink / raw)
  To: Julien Grall
  Cc: Artem_Mygaiev, xen-devel, Stefano Stabellini, Julien Grall,
	Volodymyr Babchuk

On Wed, 2 Oct 2019, Julien Grall wrote:
> Clang is pickier than GCC for the register size in asm statement. It
> expects the register size to match the value size.
> 
> The asm statement expects a 32-bit (resp. 64-bit) value on Arm32
> (resp. Arm64) whereas the value is a boolean (Clang consider to be
> 32-bit).
> 
> It would be possible to impose 32-bit register for both architecture
> but this require the code to use __OP32. However, it does no really
> improve the assembly generated. Instead, replace switch the variable to
> use register_t.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>


With or without !! given that it's part of unlikely.

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Use !! per Stefano's request
> ---
>  xen/include/asm-arm/cpufeature.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index c2c8f3417c..4c5ff6e8ac 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -67,14 +67,14 @@ static inline bool cpus_have_cap(unsigned int num)
>  
>  /* System capability check for constant cap */
>  #define cpus_have_const_cap(num) ({                 \
> -        bool __ret;                                 \
> +        register_t __ret;                           \
>                                                      \
>          asm volatile (ALTERNATIVE("mov %0, #0",     \
>                                    "mov %0, #1",     \
>                                    num)              \
>                        : "=r" (__ret));              \
>                                                      \
> -        unlikely(__ret);                            \
> +        unlikely(!!__ret);                          \
>          })
>  
>  static inline void cpus_set_cap(unsigned int num)
> -- 
> 2.11.0
> 

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 5/6] xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 5/6] xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused Julien Grall
@ 2019-10-02 22:26   ` Stefano Stabellini
  2019-10-03 10:41     ` Julien Grall
  0 siblings, 1 reply; 18+ messages in thread
From: Stefano Stabellini @ 2019-10-02 22:26 UTC (permalink / raw)
  To: Julien Grall
  Cc: Artem_Mygaiev, xen-devel, Stefano Stabellini, Julien Grall,
	Volodymyr Babchuk

On Wed, 2 Oct 2019, Julien Grall wrote:
> Clang will throw an error if a function is unused unless you tell
> to ignore it. This can be done using __maybe_unused.
> 
> While modifying the declaration, update it to match prototype of similar
> functions (see build_assertions). This helps to understand that the sole
> purpose of the function is to hold BUILD_BUG_ON().
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>

I'd like something like "Note that the function is now marked as __init"
to the commit message, but in any case:

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Update the prototype to match style of other functions holding
>         on build assertions.
> ---
>  xen/arch/arm/mm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 9e0fdc39f9..be23acfe26 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -190,7 +190,8 @@ unsigned long total_pages;
>  extern char __init_begin[], __init_end[];
>  
>  /* Checking VA memory layout alignment. */
> -static inline void check_memory_layout_alignment_constraints(void) {
> +static void __init __maybe_unused build_assertions(void)
> +{
>      /* 2MB aligned regions */
>      BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
>      BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);
> -- 
> 2.11.0
> 

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 6/6] xen/arm: traps: Mark check_stack_alignment_constraints as unused
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 6/6] xen/arm: traps: Mark check_stack_alignment_constraints " Julien Grall
@ 2019-10-02 22:26   ` Stefano Stabellini
  2019-10-03 10:44     ` Julien Grall
  0 siblings, 1 reply; 18+ messages in thread
From: Stefano Stabellini @ 2019-10-02 22:26 UTC (permalink / raw)
  To: Julien Grall
  Cc: Artem_Mygaiev, xen-devel, Stefano Stabellini, Julien Grall,
	Volodymyr Babchuk

On Wed, 2 Oct 2019, Julien Grall wrote:
> Clang will throw an error if a function is unused unless you tell
> to ignore it. This can be done using __maybe_unused.
> 
> While modifying the declaration, update it to match prototype of similar
> functions (see build_assertions). This helps to understand that the sole
> purpose of the function is to hold BUILD_BUG_ON().
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Same small note about build_assertions becoming __init.

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Update the prototype to match style of other functions holding
>         on build assertions.
> ---
>  xen/arch/arm/traps.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index a3b961bd06..a3deb59372 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -54,7 +54,8 @@
>   * that both the kernel half of struct cpu_user_regs (which is pushed in
>   * entry.S) and struct cpu_info (which lives at the bottom of a Xen
>   * stack) must be doubleword-aligned in size.  */
> -static inline void check_stack_alignment_constraints(void) {
> +static void __init __maybe_unused build_assertions(void)
> +{
>  #ifdef CONFIG_ARM_64
>      BUILD_BUG_ON((sizeof (struct cpu_user_regs)) & 0xf);
>      BUILD_BUG_ON((offsetof(struct cpu_user_regs, spsr_el1)) & 0xf);
> -- 
> 2.11.0
> 

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 3/6] xen/arm: cpuerrata: Match register size with value size in check_workaround_*
  2019-10-02 19:17     ` Julien Grall
@ 2019-10-02 22:27       ` Stefano Stabellini
  0 siblings, 0 replies; 18+ messages in thread
From: Stefano Stabellini @ 2019-10-02 22:27 UTC (permalink / raw)
  To: Julien Grall
  Cc: Artem_Mygaiev, Stefano Stabellini, Julien Grall, Andrew Cooper,
	xen-devel, Volodymyr Babchuk

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

On Wed, 2 Oct 2019, Julien Grall wrote:
> Hi Andrew,
> 
> On 10/2/19 7:42 PM, Andrew Cooper wrote:
> > On 02/10/2019 19:00, Julien Grall wrote:
> > > Clang is pickier than GCC for the register size in asm statement. It
> > > expects the register size to match the value size.
> > > 
> > > The asm statement expects a 32-bit (resp. 64-bit) value on Arm32
> > > (resp. Arm64) whereas the value is a boolean (Clang consider to be
> > > 32-bit).
> > > 
> > > It would be possible to impose 32-bit register for both architecture
> > > but this require the code to use __OP32. However, it does not really
> > > improve the assembly generated. Instead, replace switch the variable
> > > to use register_t.
> > > 
> > > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > > 
> > > ---
> > >      Changes in v2:
> > >          - Use !! per Stefano's request
> > 
> > You are aware that unlikley(), deliberately, has an embedded !! ?
> 
> I forgot it, sorry about that :/. Stefano are you happy if I revert to v1?

I forgot about that too. Yes, that's fine and add my acked-by.


> > 
> > include/xen/compiler.h:11:#define unlikely(x)   __builtin_expect(!!(x),0)
> > 
> > ~Andrew

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

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang
  2019-10-02 18:00 [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Julien Grall
                   ` (5 preceding siblings ...)
  2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 6/6] xen/arm: traps: Mark check_stack_alignment_constraints " Julien Grall
@ 2019-10-03  7:37 ` Jürgen Groß
  2019-10-03  8:31 ` Artem Mygaiev
  7 siblings, 0 replies; 18+ messages in thread
From: Jürgen Groß @ 2019-10-03  7:37 UTC (permalink / raw)
  To: Julien Grall, xen-devel
  Cc: Artem_Mygaiev, Julien Grall, Stefano Stabellini, Volodymyr Babchuk

On 02.10.19 20:00, Julien Grall wrote:
> Hi all,
> 
> After this series, I am able to build Xen on Arm64 with clang 7.0. There
> are still some issues when building Xen on Arm32 and also using lld.
> 
> Cross-compilation is left outside for now, but this is still a good start
> for clang (and armclang).
> 
> Cheers,
> 
> Julien Grall (6):
>    xen/arm: fix get_cpu_info() when built with clang
>    xen/arm64: bitops: Match the register size with the value size in flsl
>    xen/arm: cpuerrata: Match register size with value size in
>      check_workaround_*
>    xen/arm: cpufeature: Match register size with value size in
>      cpus_have_const_cap
>    xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused
>    xen/arm: traps: Mark check_stack_alignment_constraints as unused

For the series:

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang
  2019-10-02 18:00 [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Julien Grall
                   ` (6 preceding siblings ...)
  2019-10-03  7:37 ` [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Jürgen Groß
@ 2019-10-03  8:31 ` Artem Mygaiev
  7 siblings, 0 replies; 18+ messages in thread
From: Artem Mygaiev @ 2019-10-03  8:31 UTC (permalink / raw)
  To: julien.grall, xen-devel; +Cc: julien, sstabellini, Volodymyr Babchuk

Hi Julien

Just to confirm - with this series, we are able to run xen master
(4.13-unstable) on R-Car H3:
 * built using clang
 * built using clang-based arm compiler (with further modifications
needed for armlink)

Note we didn't perform full testing, just start xen on its own.

 -- Artem

On Wed, 2019-10-02 at 19:00 +0100, Julien Grall wrote:
> Hi all,
> 
> After this series, I am able to build Xen on Arm64 with clang 7.0.
> There
> are still some issues when building Xen on Arm32 and also using lld.
> 
> Cross-compilation is left outside for now, but this is still a good
> start
> for clang (and armclang).
> 
> Cheers,
> 
> Julien Grall (6):
>   xen/arm: fix get_cpu_info() when built with clang
>   xen/arm64: bitops: Match the register size with the value size in
> flsl
>   xen/arm: cpuerrata: Match register size with value size in
>     check_workaround_*
>   xen/arm: cpufeature: Match register size with value size in
>     cpus_have_const_cap
>   xen/arm: mm: Mark check_memory_layout_alignment_constraints as
> unused
>   xen/arm: traps: Mark check_stack_alignment_constraints as unused
> 
>  xen/arch/arm/mm.c                  |  3 ++-
>  xen/arch/arm/traps.c               |  3 ++-
>  xen/include/asm-arm/arm64/bitops.h |  3 ++-
>  xen/include/asm-arm/cpuerrata.h    |  4 ++--
>  xen/include/asm-arm/cpufeature.h   |  4 ++--
>  xen/include/asm-arm/current.h      | 10 +++++++++-
>  6 files changed, 19 insertions(+), 8 deletions(-)
> 
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 5/6] xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused
  2019-10-02 22:26   ` Stefano Stabellini
@ 2019-10-03 10:41     ` Julien Grall
  0 siblings, 0 replies; 18+ messages in thread
From: Julien Grall @ 2019-10-03 10:41 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Artem_Mygaiev, xen-devel, Julien Grall, Volodymyr Babchuk



On 02/10/2019 23:26, Stefano Stabellini wrote:
> On Wed, 2 Oct 2019, Julien Grall wrote:
>> Clang will throw an error if a function is unused unless you tell
>> to ignore it. This can be done using __maybe_unused.
>>
>> While modifying the declaration, update it to match prototype of similar
>> functions (see build_assertions). This helps to understand that the sole
>> purpose of the function is to hold BUILD_BUG_ON().
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> I'd like something like "Note that the function is now marked as __init"
> to the commit message, but in any case:

This is already implied with "update it to match prototype of similar functions".

42sh> grep "build_assertions"

> 
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> 
>> ---
>>      Changes in v2:
>>          - Update the prototype to match style of other functions holding
>>          on build assertions.
>> ---
>>   xen/arch/arm/mm.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index 9e0fdc39f9..be23acfe26 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -190,7 +190,8 @@ unsigned long total_pages;
>>   extern char __init_begin[], __init_end[];
>>   
>>   /* Checking VA memory layout alignment. */
>> -static inline void check_memory_layout_alignment_constraints(void) {
>> +static void __init __maybe_unused build_assertions(void)
>> +{
>>       /* 2MB aligned regions */
>>       BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
>>       BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);
>> -- 
>> 2.11.0
>>

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Xen-devel] [PATCH for-4.13 6/6] xen/arm: traps: Mark check_stack_alignment_constraints as unused
  2019-10-02 22:26   ` Stefano Stabellini
@ 2019-10-03 10:44     ` Julien Grall
  0 siblings, 0 replies; 18+ messages in thread
From: Julien Grall @ 2019-10-03 10:44 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Artem_Mygaiev, xen-devel, Julien Grall, Volodymyr Babchuk

Hi Stefano,

On 02/10/2019 23:26, Stefano Stabellini wrote:
> On Wed, 2 Oct 2019, Julien Grall wrote:
>> Clang will throw an error if a function is unused unless you tell
>> to ignore it. This can be done using __maybe_unused.
>>
>> While modifying the declaration, update it to match prototype of similar
>> functions (see build_assertions). This helps to understand that the sole
>> purpose of the function is to hold BUILD_BUG_ON().
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> Same small note about build_assertions becoming __init.

Similar to the previous version this is already implied by "update it to match 
prototype of similar functions".

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-10-03 10:45 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 18:00 [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Julien Grall
2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 1/6] xen/arm: fix get_cpu_info() when built " Julien Grall
2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 2/6] xen/arm64: bitops: Match the register size with the value size in flsl Julien Grall
2019-10-02 22:26   ` Stefano Stabellini
2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 3/6] xen/arm: cpuerrata: Match register size with value size in check_workaround_* Julien Grall
2019-10-02 18:42   ` Andrew Cooper
2019-10-02 19:17     ` Julien Grall
2019-10-02 22:27       ` Stefano Stabellini
2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 4/6] xen/arm: cpufeature: Match register size with value size in cpus_have_const_cap Julien Grall
2019-10-02 22:26   ` Stefano Stabellini
2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 5/6] xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused Julien Grall
2019-10-02 22:26   ` Stefano Stabellini
2019-10-03 10:41     ` Julien Grall
2019-10-02 18:00 ` [Xen-devel] [PATCH for-4.13 6/6] xen/arm: traps: Mark check_stack_alignment_constraints " Julien Grall
2019-10-02 22:26   ` Stefano Stabellini
2019-10-03 10:44     ` Julien Grall
2019-10-03  7:37 ` [Xen-devel] [PATCH for-4.13 0/6] xen/arm: Add support to build with clang Jürgen Groß
2019-10-03  8:31 ` Artem Mygaiev

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.