All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] clang: fixes for the indirect thunk
@ 2018-01-24 15:48 Roger Pau Monne
  2018-01-24 15:48 ` [PATCH 1/2] x86/clang: fix build with indirect thunks Roger Pau Monne
  2018-01-24 15:48 ` [PATCH 2/2] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK Roger Pau Monne
  0 siblings, 2 replies; 11+ messages in thread
From: Roger Pau Monne @ 2018-01-24 15:48 UTC (permalink / raw)
  To: xen-devel; +Cc: Roger Pau Monne

Hello,

The following two patches fix building Xen with clang and the indirect
thunk.

Patch 2 is not strictly needed, because when building with clang we
still compile assembly only files with -no-integrated-as, but it's a
nice to have so that we don't regress more in assembly only code.

Thanks, Roger.

Roger Pau Monne (2):
  x86/clang: fix build with indirect thunks
  x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK

 xen/Rules.mk                           |  6 ++++--
 xen/arch/x86/extable.c                 |  3 ++-
 xen/arch/x86/x86_emulate/x86_emulate.c |  3 ++-
 xen/common/wait.c                      |  1 +
 xen/include/asm-x86/asm_defns.h        | 10 +++++++---
 5 files changed, 16 insertions(+), 7 deletions(-)

-- 
2.15.1


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

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

* [PATCH 1/2] x86/clang: fix build with indirect thunks
  2018-01-24 15:48 [PATCH 0/2] clang: fixes for the indirect thunk Roger Pau Monne
@ 2018-01-24 15:48 ` Roger Pau Monne
  2018-01-24 16:40   ` Jan Beulich
  2018-01-24 15:48 ` [PATCH 2/2] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK Roger Pau Monne
  1 sibling, 1 reply; 11+ messages in thread
From: Roger Pau Monne @ 2018-01-24 15:48 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, Jan Beulich, Roger Pau Monne

The build with clang is currently broken because clang requires asm
macros to be declared inside the same inline asm declaration where
they are used.

In order to fix this always include indirect_thunk_asm.h in the same
asm declaration where it's being used.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 xen/Rules.mk                           | 6 ++++--
 xen/arch/x86/extable.c                 | 3 ++-
 xen/arch/x86/x86_emulate/x86_emulate.c | 3 ++-
 xen/common/wait.c                      | 1 +
 xen/include/asm-x86/asm_defns.h        | 7 ++++---
 5 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 3cf40754a6..1bd3fa1773 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -66,8 +66,10 @@ endif
 
 AFLAGS-y                += -D__ASSEMBLY__
 
-# Clang's built-in assembler can't handle embedded .include's
-CFLAGS-$(clang)         += -no-integrated-as
+# Clang's built-in assembler doesn't understand assembler directives without
+# an absolute value:
+# https://bugs.llvm.org/show_bug.cgi?id=27369
+AFLAGS-$(clang)         += -no-integrated-as
 
 ALL_OBJS := $(ALL_OBJS-y)
 
diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c
index 72f30d9060..30893c3770 100644
--- a/xen/arch/x86/extable.c
+++ b/xen/arch/x86/extable.c
@@ -158,7 +158,8 @@ static int __init stub_selftest(void)
         memcpy(ptr, tests[i].opc, ARRAY_SIZE(tests[i].opc));
         unmap_domain_page(ptr);
 
-        asm volatile ( "INDIRECT_CALL %[stb]\n"
+        asm volatile ( INCLUDE_INDIRECT_THUNK
+                       "INDIRECT_CALL %[stb]\n"
                        ".Lret%=:\n\t"
                        ".pushsection .fixup,\"ax\"\n"
                        ".Lfix%=:\n\t"
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index ff0a003902..e525e6bb0d 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -867,7 +867,8 @@ static inline int mkec(uint8_t e, int32_t ec, ...)
 #ifdef __XEN__
 # define invoke_stub(pre, post, constraints...) do {                    \
     union stub_exception_token res_ = { .raw = ~0 };                    \
-    asm volatile ( pre "\n\tINDIRECT_CALL %[stub]\n\t" post "\n"        \
+    asm volatile ( INCLUDE_INDIRECT_THUNK                               \
+                   pre "\n\tINDIRECT_CALL %[stub]\n\t" post "\n"        \
                    ".Lret%=:\n\t"                                       \
                    ".pushsection .fixup,\"ax\"\n"                       \
                    ".Lfix%=:\n\t"                                       \
diff --git a/xen/common/wait.c b/xen/common/wait.c
index a57bc10d61..91dcd46342 100644
--- a/xen/common/wait.c
+++ b/xen/common/wait.c
@@ -207,6 +207,7 @@ void check_wakeup_from_wait(void)
      * restored from the stack, so are available for use here.
      */
     asm volatile (
+        INCLUDE_INDIRECT_THUNK
         "mov %1,%%"__OP"sp; INDIRECT_JMP %[ip]"
         : : "S" (wqv->stack), "D" (wqv->esp),
           "c" ((char *)get_cpu_info() - (char *)wqv->esp),
diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index d2d91ca1fa..627a544326 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -16,9 +16,10 @@
 #ifdef __ASSEMBLY__
 # include <asm/indirect_thunk_asm.h>
 #else
-asm ( "\t.equ CONFIG_INDIRECT_THUNK, "
-      __stringify(IS_ENABLED(CONFIG_INDIRECT_THUNK)) );
-asm ( "\t.include \"asm/indirect_thunk_asm.h\"" );
+#define INCLUDE_INDIRECT_THUNK                                  \
+    "\t.equ CONFIG_INDIRECT_THUNK, "                            \
+    __stringify(IS_ENABLED(CONFIG_INDIRECT_THUNK)) "\n"         \
+    "\t.include \"asm/indirect_thunk_asm.h\"\n"
 #endif
 
 #ifndef __ASSEMBLY__
-- 
2.15.1


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

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

* [PATCH 2/2] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
  2018-01-24 15:48 [PATCH 0/2] clang: fixes for the indirect thunk Roger Pau Monne
  2018-01-24 15:48 ` [PATCH 1/2] x86/clang: fix build with indirect thunks Roger Pau Monne
@ 2018-01-24 15:48 ` Roger Pau Monne
  2018-01-24 16:23   ` Jan Beulich
  1 sibling, 1 reply; 11+ messages in thread
From: Roger Pau Monne @ 2018-01-24 15:48 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monne

When indirect_thunk_asm.h is instantiated directly into assembly files
CONFIG_INDIRECT_THUNK might not be defined, and thus using .if against
it is wrong.

Add a check to define CONFIG_INDIRECT_THUNK to 0 if not defined, so
that using .if CONFIG_INDIRECT_THUNK is always correct.

This suppresses the following clang error:

<instantiation>:8:9: error: expected absolute expression
    .if CONFIG_INDIRECT_THUNK == 1
        ^
<instantiation>:1:1: note: while in macro instantiation
INDIRECT_BRANCH call %rdx
^
entry.S:589:9: note: while in macro instantiation
        INDIRECT_CALL %rdx
        ^

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/include/asm-x86/asm_defns.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index 627a544326..f7e53627ff 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -14,6 +14,9 @@
 #include <asm/alternative.h>
 
 #ifdef __ASSEMBLY__
+# ifndef CONFIG_INDIRECT_THUNK
+#  define CONFIG_INDIRECT_THUNK 0
+# endif
 # include <asm/indirect_thunk_asm.h>
 #else
 #define INCLUDE_INDIRECT_THUNK                                  \
-- 
2.15.1


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

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

* Re: [PATCH 2/2] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
  2018-01-24 15:48 ` [PATCH 2/2] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK Roger Pau Monne
@ 2018-01-24 16:23   ` Jan Beulich
  2018-01-24 16:52     ` Roger Pau Monné
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2018-01-24 16:23 UTC (permalink / raw)
  To: Roger Pau Monne; +Cc: Andrew Cooper, xen-devel

>>> On 24.01.18 at 16:48, <roger.pau@citrix.com> wrote:
> When indirect_thunk_asm.h is instantiated directly into assembly files
> CONFIG_INDIRECT_THUNK might not be defined, and thus using .if against
> it is wrong.
> 
> Add a check to define CONFIG_INDIRECT_THUNK to 0 if not defined, so
> that using .if CONFIG_INDIRECT_THUNK is always correct.
> 
> This suppresses the following clang error:
> 
> <instantiation>:8:9: error: expected absolute expression
>     .if CONFIG_INDIRECT_THUNK == 1
>         ^
> <instantiation>:1:1: note: while in macro instantiation
> INDIRECT_BRANCH call %rdx
> ^
> entry.S:589:9: note: while in macro instantiation
>         INDIRECT_CALL %rdx
>         ^

Why is the same no problem with gas? It wants constant
expressions with .if too, after all.

Jan


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

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

* Re: [PATCH 1/2] x86/clang: fix build with indirect thunks
  2018-01-24 15:48 ` [PATCH 1/2] x86/clang: fix build with indirect thunks Roger Pau Monne
@ 2018-01-24 16:40   ` Jan Beulich
  2018-01-24 17:06     ` Roger Pau Monné
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2018-01-24 16:40 UTC (permalink / raw)
  To: Roger Pau Monne
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	IanJackson, Tim Deegan, xen-devel

>>> On 24.01.18 at 16:48, <roger.pau@citrix.com> wrote:
> The build with clang is currently broken because clang requires asm
> macros to be declared inside the same inline asm declaration where
> they are used.

I don't understand this: What if I have two asm()-s needing it? Does
this need to be done in each one? I'd expect this to result in duplicate
definitions on gas then (which may or may not be benign).

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -66,8 +66,10 @@ endif
>  
>  AFLAGS-y                += -D__ASSEMBLY__
>  
> -# Clang's built-in assembler can't handle embedded .include's
> -CFLAGS-$(clang)         += -no-integrated-as
> +# Clang's built-in assembler doesn't understand assembler directives without
> +# an absolute value:
> +# https://bugs.llvm.org/show_bug.cgi?id=27369 
> +AFLAGS-$(clang)         += -no-integrated-as

I also don't understand this - when you switch (back) to AFLAGS, you
don't affect C files. Furthermore without using its integrated assembler
for C files at present - how is the build broken? Is the description of
the change perhaps in need of some re-work (and maybe the title as
well)?

Nor am I of the opinion that the comment above is really correct -
I'm sure there are directives where their assembler supports non-
constant values (.include being the obvious first case in the context
here).

And finally, if you switch back to use AFLAGS here, you should
either restore the original comment as well, or explain in the
description why it isn't applicable anymore.

> --- a/xen/arch/x86/extable.c
> +++ b/xen/arch/x86/extable.c
> @@ -158,7 +158,8 @@ static int __init stub_selftest(void)
>          memcpy(ptr, tests[i].opc, ARRAY_SIZE(tests[i].opc));
>          unmap_domain_page(ptr);
>  
> -        asm volatile ( "INDIRECT_CALL %[stb]\n"
> +        asm volatile ( INCLUDE_INDIRECT_THUNK
> +                       "INDIRECT_CALL %[stb]\n"

Besides this being somewhat ugly, having to remember to add
this going forward, should any new indirect calls be added, is
surely prone to be forgotten.

Jan


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

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

* Re: [PATCH 2/2] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
  2018-01-24 16:23   ` Jan Beulich
@ 2018-01-24 16:52     ` Roger Pau Monné
  2018-01-25 10:13       ` Jan Beulich
  2018-01-25 10:15       ` Roger Pau Monné
  0 siblings, 2 replies; 11+ messages in thread
From: Roger Pau Monné @ 2018-01-24 16:52 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, xen-devel

On Wed, Jan 24, 2018 at 09:23:37AM -0700, Jan Beulich wrote:
> >>> On 24.01.18 at 16:48, <roger.pau@citrix.com> wrote:
> > When indirect_thunk_asm.h is instantiated directly into assembly files
> > CONFIG_INDIRECT_THUNK might not be defined, and thus using .if against
> > it is wrong.
> > 
> > Add a check to define CONFIG_INDIRECT_THUNK to 0 if not defined, so
> > that using .if CONFIG_INDIRECT_THUNK is always correct.
> > 
> > This suppresses the following clang error:
> > 
> > <instantiation>:8:9: error: expected absolute expression
> >     .if CONFIG_INDIRECT_THUNK == 1
> >         ^
> > <instantiation>:1:1: note: while in macro instantiation
> > INDIRECT_BRANCH call %rdx
> > ^
> > entry.S:589:9: note: while in macro instantiation
> >         INDIRECT_CALL %rdx
> >         ^
> 
> Why is the same no problem with gas? It wants constant
> expressions with .if too, after all.

Right, and I cannot figure out why it works when using
-no-integrated-as and not when not using it. The more that I cannot
see CONFIG_INDIRECT_THUNK being unconditionally defined to either 0 or
1. I guess I'm missing something.

Roger.

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

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

* Re: [PATCH 1/2] x86/clang: fix build with indirect thunks
  2018-01-24 16:40   ` Jan Beulich
@ 2018-01-24 17:06     ` Roger Pau Monné
  2018-01-25  9:39       ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: Roger Pau Monné @ 2018-01-24 17:06 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	IanJackson, Tim Deegan, xen-devel

On Wed, Jan 24, 2018 at 09:40:40AM -0700, Jan Beulich wrote:
> >>> On 24.01.18 at 16:48, <roger.pau@citrix.com> wrote:
> > The build with clang is currently broken because clang requires asm
> > macros to be declared inside the same inline asm declaration where
> > they are used.
> 
> I don't understand this: What if I have two asm()-s needing it? Does
> this need to be done in each one? I'd expect this to result in duplicate
> definitions on gas then (which may or may not be benign).

It's quite fun, this approach works fine with clang regardless of the
number of asm()-s needing it. It doesn't complain about duplicate
macros or anything. OTOH gcc complains with "Error: Macro `foo' was
already defined".

One option might be to guard indirect_thunk_asm.h with:

.ifndef INDIRECT_THUNK_ASM
.equ INDIRECT_THUNK_ASM, 1

...

.endif

Not the best, but should do the trick I guess...

> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -66,8 +66,10 @@ endif
> >  
> >  AFLAGS-y                += -D__ASSEMBLY__
> >  
> > -# Clang's built-in assembler can't handle embedded .include's
> > -CFLAGS-$(clang)         += -no-integrated-as
> > +# Clang's built-in assembler doesn't understand assembler directives without
> > +# an absolute value:
> > +# https://bugs.llvm.org/show_bug.cgi?id=27369 
> > +AFLAGS-$(clang)         += -no-integrated-as
> 
> I also don't understand this - when you switch (back) to AFLAGS, you
> don't affect C files.

Yes, it affects C files because now they are assembled using the
integrated as, not the external one.

> Furthermore without using its integrated assembler
> for C files at present - how is the build broken?

Not using the integrated as is a workaround for using the indirect
thunk. If we can manage to get the indirect thunk to work with the
integrated as there's no need to use the external one for C files.

> Is the description of
> the change perhaps in need of some re-work (and maybe the title as
> well)?
> 
> Nor am I of the opinion that the comment above is really correct -
> I'm sure there are directives where their assembler supports non-
> constant values (.include being the obvious first case in the context
> here).

Right, clang only accepts constant values for assembler directives
like .rept and .skip.

> And finally, if you switch back to use AFLAGS here, you should
> either restore the original comment as well, or explain in the
> description why it isn't applicable anymore.

I will check, but I'm fairly sure that all the clang versions that Xen
supports (>= 3.5) support .code16/.code32/.code64.

> > --- a/xen/arch/x86/extable.c
> > +++ b/xen/arch/x86/extable.c
> > @@ -158,7 +158,8 @@ static int __init stub_selftest(void)
> >          memcpy(ptr, tests[i].opc, ARRAY_SIZE(tests[i].opc));
> >          unmap_domain_page(ptr);
> >  
> > -        asm volatile ( "INDIRECT_CALL %[stb]\n"
> > +        asm volatile ( INCLUDE_INDIRECT_THUNK
> > +                       "INDIRECT_CALL %[stb]\n"
> 
> Besides this being somewhat ugly, having to remember to add
> this going forward, should any new indirect calls be added, is
> surely prone to be forgotten.

I don't have a better solution ATM I'm afraid.

Roger.

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

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

* Re: [PATCH 1/2] x86/clang: fix build with indirect thunks
  2018-01-24 17:06     ` Roger Pau Monné
@ 2018-01-25  9:39       ` Jan Beulich
  2018-01-25  9:50         ` Roger Pau Monné
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2018-01-25  9:39 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
	IanJackson, Tim Deegan, xen-devel

>>> On 24.01.18 at 18:06, <roger.pau@citrix.com> wrote:
> On Wed, Jan 24, 2018 at 09:40:40AM -0700, Jan Beulich wrote:
>> >>> On 24.01.18 at 16:48, <roger.pau@citrix.com> wrote:
>> > The build with clang is currently broken because clang requires asm
>> > macros to be declared inside the same inline asm declaration where
>> > they are used.
>> 
>> I don't understand this: What if I have two asm()-s needing it? Does
>> this need to be done in each one? I'd expect this to result in duplicate
>> definitions on gas then (which may or may not be benign).
> 
> It's quite fun, this approach works fine with clang regardless of the
> number of asm()-s needing it. It doesn't complain about duplicate
> macros or anything. OTOH gcc complains with "Error: Macro `foo' was
> already defined".

And does clang need it in each asm()? Albeit - since the order
functions are being output is undefined, it probably should go into
each one. Hence the answer would only affect whether ...

> One option might be to guard indirect_thunk_asm.h with:
> 
> .ifndef INDIRECT_THUNK_ASM
> .equ INDIRECT_THUNK_ASM, 1
> 
> ...
> 
> .endif

... this would be needed conditionally for gas, or unconditionally.

>> > --- a/xen/Rules.mk
>> > +++ b/xen/Rules.mk
>> > @@ -66,8 +66,10 @@ endif
>> >  
>> >  AFLAGS-y                += -D__ASSEMBLY__
>> >  
>> > -# Clang's built-in assembler can't handle embedded .include's
>> > -CFLAGS-$(clang)         += -no-integrated-as
>> > +# Clang's built-in assembler doesn't understand assembler directives without
>> > +# an absolute value:
>> > +# https://bugs.llvm.org/show_bug.cgi?id=27369 
>> > +AFLAGS-$(clang)         += -no-integrated-as
>> 
>> I also don't understand this - when you switch (back) to AFLAGS, you
>> don't affect C files.
> 
> Yes, it affects C files because now they are assembled using the
> integrated as, not the external one.
> 
>> Furthermore without using its integrated assembler
>> for C files at present - how is the build broken?
> 
> Not using the integrated as is a workaround for using the indirect
> thunk. If we can manage to get the indirect thunk to work with the
> integrated as there's no need to use the external one for C files.

Well, that'll hopefully become more clear with a more precise
description.

Jan


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

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

* Re: [PATCH 1/2] x86/clang: fix build with indirect thunks
  2018-01-25  9:39       ` Jan Beulich
@ 2018-01-25  9:50         ` Roger Pau Monné
  0 siblings, 0 replies; 11+ messages in thread
From: Roger Pau Monné @ 2018-01-25  9:50 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
	IanJackson, Tim Deegan, xen-devel

On Thu, Jan 25, 2018 at 02:39:36AM -0700, Jan Beulich wrote:
> >>> On 24.01.18 at 18:06, <roger.pau@citrix.com> wrote:
> > On Wed, Jan 24, 2018 at 09:40:40AM -0700, Jan Beulich wrote:
> >> >>> On 24.01.18 at 16:48, <roger.pau@citrix.com> wrote:
> >> > The build with clang is currently broken because clang requires asm
> >> > macros to be declared inside the same inline asm declaration where
> >> > they are used.
> >> 
> >> I don't understand this: What if I have two asm()-s needing it? Does
> >> this need to be done in each one? I'd expect this to result in duplicate
> >> definitions on gas then (which may or may not be benign).
> > 
> > It's quite fun, this approach works fine with clang regardless of the
> > number of asm()-s needing it. It doesn't complain about duplicate
> > macros or anything. OTOH gcc complains with "Error: Macro `foo' was
> > already defined".
> 
> And does clang need it in each asm()?

Yes. AFAICT .includes inside of asm()-s in clang are only valid inside
the same asm().

> Albeit - since the order
> functions are being output is undefined, it probably should go into
> each one. Hence the answer would only affect whether ...
> 
> > One option might be to guard indirect_thunk_asm.h with:
> > 
> > .ifndef INDIRECT_THUNK_ASM
> > .equ INDIRECT_THUNK_ASM, 1
> > 
> > ...
> > 
> > .endif
> 
> ... this would be needed conditionally for gas, or unconditionally.

The guard is needed unconditionally for gcc, or else having two
instances of the asm() in the same file is not going to work.

For gcc we could also avoid defining INCLUDE_INDIRECT_THUNK and keep
the same approach that's used currently, but I would rather have gcc
and clang using the same approach if possible.

Thanks, Roger.

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

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

* Re: [PATCH 2/2] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
  2018-01-24 16:52     ` Roger Pau Monné
@ 2018-01-25 10:13       ` Jan Beulich
  2018-01-25 10:15       ` Roger Pau Monné
  1 sibling, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2018-01-25 10:13 UTC (permalink / raw)
  To: Roger Pau Monné; +Cc: Andrew Cooper, xen-devel

>>> On 24.01.18 at 17:52, <roger.pau@citrix.com> wrote:
> On Wed, Jan 24, 2018 at 09:23:37AM -0700, Jan Beulich wrote:
>> >>> On 24.01.18 at 16:48, <roger.pau@citrix.com> wrote:
>> > When indirect_thunk_asm.h is instantiated directly into assembly files
>> > CONFIG_INDIRECT_THUNK might not be defined, and thus using .if against
>> > it is wrong.
>> > 
>> > Add a check to define CONFIG_INDIRECT_THUNK to 0 if not defined, so
>> > that using .if CONFIG_INDIRECT_THUNK is always correct.
>> > 
>> > This suppresses the following clang error:
>> > 
>> > <instantiation>:8:9: error: expected absolute expression
>> >     .if CONFIG_INDIRECT_THUNK == 1
>> >         ^
>> > <instantiation>:1:1: note: while in macro instantiation
>> > INDIRECT_BRANCH call %rdx
>> > ^
>> > entry.S:589:9: note: while in macro instantiation
>> >         INDIRECT_CALL %rdx
>> >         ^
>> 
>> Why is the same no problem with gas? It wants constant
>> expressions with .if too, after all.
> 
> Right, and I cannot figure out why it works when using
> -no-integrated-as and not when not using it. The more that I cannot
> see CONFIG_INDIRECT_THUNK being unconditionally defined to either 0 or
> 1. I guess I'm missing something.

Hmm, it looks to be (half way?) intended behavior for gas to
consider "<symbol> == <constant>" always false if <symbol>
is undefined (and similarly "<symbol> != <constant>" always
true). I'll inquire on the binutils list if this is really meant to
remain that way forever, as it's not spelled out that way in the
docs.

Jan


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

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

* Re: [PATCH 2/2] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
  2018-01-24 16:52     ` Roger Pau Monné
  2018-01-25 10:13       ` Jan Beulich
@ 2018-01-25 10:15       ` Roger Pau Monné
  1 sibling, 0 replies; 11+ messages in thread
From: Roger Pau Monné @ 2018-01-25 10:15 UTC (permalink / raw)
  To: Roger Pau Monné; +Cc: Andrew Cooper, Jan Beulich, xen-devel

On Wed, Jan 24, 2018 at 04:52:17PM +0000, Roger Pau Monné wrote:
> On Wed, Jan 24, 2018 at 09:23:37AM -0700, Jan Beulich wrote:
> > >>> On 24.01.18 at 16:48, <roger.pau@citrix.com> wrote:
> > > When indirect_thunk_asm.h is instantiated directly into assembly files
> > > CONFIG_INDIRECT_THUNK might not be defined, and thus using .if against
> > > it is wrong.
> > > 
> > > Add a check to define CONFIG_INDIRECT_THUNK to 0 if not defined, so
> > > that using .if CONFIG_INDIRECT_THUNK is always correct.
> > > 
> > > This suppresses the following clang error:
> > > 
> > > <instantiation>:8:9: error: expected absolute expression
> > >     .if CONFIG_INDIRECT_THUNK == 1
> > >         ^
> > > <instantiation>:1:1: note: while in macro instantiation
> > > INDIRECT_BRANCH call %rdx
> > > ^
> > > entry.S:589:9: note: while in macro instantiation
> > >         INDIRECT_CALL %rdx
> > >         ^
> > 
> > Why is the same no problem with gas? It wants constant
> > expressions with .if too, after all.
> 
> Right, and I cannot figure out why it works when using
> -no-integrated-as and not when not using it. The more that I cannot
> see CONFIG_INDIRECT_THUNK being unconditionally defined to either 0 or
> 1. I guess I'm missing something.

So given the following asm snippet:

# cat snip.S
.if FOO == 1
.error "FOO"
.endif

Attempting to compile it with different compilers/assemblers:

# clang -o snip.o -c snip.S
snip.S:1:5: error: expected absolute expression
.if FOO == 1
    ^
snip.S:2:1: error: FOO
.error "FOO"
^
# gcc -o snip.o -c snip.S
# as -o snip.o snip.S

So there's clearly something different in how as/gcc handle .if
directives when they reference undefined symbols versus clang.

I can expand the commit message to reflect/include the above, but I
think the change still stands.

Thanks, Roger.

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

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

end of thread, other threads:[~2018-01-25 10:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-24 15:48 [PATCH 0/2] clang: fixes for the indirect thunk Roger Pau Monne
2018-01-24 15:48 ` [PATCH 1/2] x86/clang: fix build with indirect thunks Roger Pau Monne
2018-01-24 16:40   ` Jan Beulich
2018-01-24 17:06     ` Roger Pau Monné
2018-01-25  9:39       ` Jan Beulich
2018-01-25  9:50         ` Roger Pau Monné
2018-01-24 15:48 ` [PATCH 2/2] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK Roger Pau Monne
2018-01-24 16:23   ` Jan Beulich
2018-01-24 16:52     ` Roger Pau Monné
2018-01-25 10:13       ` Jan Beulich
2018-01-25 10:15       ` Roger Pau Monné

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.