Re: [Xen-devel] [PATCH v4 3/4] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
On Tue, Feb 20, 2018 at 05:53:08AM -0700, Jan Beulich wrote: > >>> On 20.02.18 at 13:42, wrote: > > On Tue, Feb 20, 2018 at 05:32:17AM -0700, Jan Beulich wrote: > >> >>> On 20.02.18 at 12:22, wrote: > >> > On Tue, Feb 20, 2018 at 04:13:42AM -0700, Jan Beulich wrote: > >> >> >>> On 20.02.18 at 12:01, wrote: > >> >> > --- a/xen/include/asm-x86/asm_defns.h > >> >> > +++ b/xen/include/asm-x86/asm_defns.h > >> >> > @@ -15,6 +15,9 @@ > >> >> > #include > >> >> > > >> >> > #ifdef __ASSEMBLY__ > >> >> > +#ifndef CONFIG_INDIRECT_THUNK > >> >> > +.equ CONFIG_INDIRECT_THUNK, 0 > >> >> > +#endif > >> >> > # include > >> >> > #else > >> >> > asm ( "\t.equ CONFIG_INDIRECT_THUNK, " > >> >> > > >> >> > It's fairly similar to my first approach, but at least "#ifdef > >> >> > CONFIG_INDIRECT_THUNK" will still work as expected after this fix. > >> >> > >> >> I've used something similar in backports to old versions, so I > >> >> wonder whether what you quote above is right: Assembly files > >> >> don't get handed to clang anyway iirc, so the change would > >> >> seem to be needed in the #else part of the conditional. > >> > > >> > Assembly files do get handed to clang, from xen/Rules.mk: > >> > > >> > %.o: %.S Makefile > >> > $(CC) $(AFLAGS) -c $< -o $@ > >> > > >> > Xen doesn't call as directly to compile those, or am I missing > >> > something? > >> > >> Oh, I was referring to the -mno-integrated-as (or whatever it's > >> called) option. "Handed to clang" wasn't the best way to put it, > >> I agree. But it's clang's integrated assembler which the problem > >> is with. > > > > Oh, I see. You are right, this is just a preparatory change for patch > > 4, which does enable clang's integrated assembler if the right > > features are present. > > > > Let me know if you think the above chunk is acceptable. > > It is certainly acceptable if it helps; you now apparently agreeing > I'm more confused though by this question. Iirc you don't enable > the integrated assembler for assembly sources, but only for C ones. So at this point the integrated assembler is only enabled for C sources. In patch 4 I enable the integrated assembler for everything if possible, but in order to enable it I need to solve this issue first. I can merge patches 3 and 4 if it's going to be clearer. Hope this help shed some light. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/4] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
>>> On 20.02.18 at 13:42, wrote: > On Tue, Feb 20, 2018 at 05:32:17AM -0700, Jan Beulich wrote: >> >>> On 20.02.18 at 12:22, wrote: >> > On Tue, Feb 20, 2018 at 04:13:42AM -0700, Jan Beulich wrote: >> >> >>> On 20.02.18 at 12:01, wrote: >> >> > --- a/xen/include/asm-x86/asm_defns.h >> >> > +++ b/xen/include/asm-x86/asm_defns.h >> >> > @@ -15,6 +15,9 @@ >> >> > #include >> >> > >> >> > #ifdef __ASSEMBLY__ >> >> > +#ifndef CONFIG_INDIRECT_THUNK >> >> > +.equ CONFIG_INDIRECT_THUNK, 0 >> >> > +#endif >> >> > # include >> >> > #else >> >> > asm ( "\t.equ CONFIG_INDIRECT_THUNK, " >> >> > >> >> > It's fairly similar to my first approach, but at least "#ifdef >> >> > CONFIG_INDIRECT_THUNK" will still work as expected after this fix. >> >> >> >> I've used something similar in backports to old versions, so I >> >> wonder whether what you quote above is right: Assembly files >> >> don't get handed to clang anyway iirc, so the change would >> >> seem to be needed in the #else part of the conditional. >> > >> > Assembly files do get handed to clang, from xen/Rules.mk: >> > >> > %.o: %.S Makefile >> >$(CC) $(AFLAGS) -c $< -o $@ >> > >> > Xen doesn't call as directly to compile those, or am I missing >> > something? >> >> Oh, I was referring to the -mno-integrated-as (or whatever it's >> called) option. "Handed to clang" wasn't the best way to put it, >> I agree. But it's clang's integrated assembler which the problem >> is with. > > Oh, I see. You are right, this is just a preparatory change for patch > 4, which does enable clang's integrated assembler if the right > features are present. > > Let me know if you think the above chunk is acceptable. It is certainly acceptable if it helps; you now apparently agreeing I'm more confused though by this question. Iirc you don't enable the integrated assembler for assembly sources, but only for C ones. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/4] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
On Tue, Feb 20, 2018 at 05:32:17AM -0700, Jan Beulich wrote: > >>> On 20.02.18 at 12:22, wrote: > > On Tue, Feb 20, 2018 at 04:13:42AM -0700, Jan Beulich wrote: > >> >>> On 20.02.18 at 12:01, wrote: > >> > --- a/xen/include/asm-x86/asm_defns.h > >> > +++ b/xen/include/asm-x86/asm_defns.h > >> > @@ -15,6 +15,9 @@ > >> > #include > >> > > >> > #ifdef __ASSEMBLY__ > >> > +#ifndef CONFIG_INDIRECT_THUNK > >> > +.equ CONFIG_INDIRECT_THUNK, 0 > >> > +#endif > >> > # include > >> > #else > >> > asm ( "\t.equ CONFIG_INDIRECT_THUNK, " > >> > > >> > It's fairly similar to my first approach, but at least "#ifdef > >> > CONFIG_INDIRECT_THUNK" will still work as expected after this fix. > >> > >> I've used something similar in backports to old versions, so I > >> wonder whether what you quote above is right: Assembly files > >> don't get handed to clang anyway iirc, so the change would > >> seem to be needed in the #else part of the conditional. > > > > Assembly files do get handed to clang, from xen/Rules.mk: > > > > %.o: %.S Makefile > > $(CC) $(AFLAGS) -c $< -o $@ > > > > Xen doesn't call as directly to compile those, or am I missing > > something? > > Oh, I was referring to the -mno-integrated-as (or whatever it's > called) option. "Handed to clang" wasn't the best way to put it, > I agree. But it's clang's integrated assembler which the problem > is with. Oh, I see. You are right, this is just a preparatory change for patch 4, which does enable clang's integrated assembler if the right features are present. Let me know if you think the above chunk is acceptable. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/4] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
>>> On 20.02.18 at 12:22, wrote: > On Tue, Feb 20, 2018 at 04:13:42AM -0700, Jan Beulich wrote: >> >>> On 20.02.18 at 12:01, wrote: >> > --- a/xen/include/asm-x86/asm_defns.h >> > +++ b/xen/include/asm-x86/asm_defns.h >> > @@ -15,6 +15,9 @@ >> > #include >> > >> > #ifdef __ASSEMBLY__ >> > +#ifndef CONFIG_INDIRECT_THUNK >> > +.equ CONFIG_INDIRECT_THUNK, 0 >> > +#endif >> > # include >> > #else >> > asm ( "\t.equ CONFIG_INDIRECT_THUNK, " >> > >> > It's fairly similar to my first approach, but at least "#ifdef >> > CONFIG_INDIRECT_THUNK" will still work as expected after this fix. >> >> I've used something similar in backports to old versions, so I >> wonder whether what you quote above is right: Assembly files >> don't get handed to clang anyway iirc, so the change would >> seem to be needed in the #else part of the conditional. > > Assembly files do get handed to clang, from xen/Rules.mk: > > %.o: %.S Makefile > $(CC) $(AFLAGS) -c $< -o $@ > > Xen doesn't call as directly to compile those, or am I missing > something? Oh, I was referring to the -mno-integrated-as (or whatever it's called) option. "Handed to clang" wasn't the best way to put it, I agree. But it's clang's integrated assembler which the problem is with. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/4] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
On Tue, Feb 20, 2018 at 04:13:42AM -0700, Jan Beulich wrote: > >>> On 20.02.18 at 12:01, wrote: > > --- a/xen/include/asm-x86/asm_defns.h > > +++ b/xen/include/asm-x86/asm_defns.h > > @@ -15,6 +15,9 @@ > > #include > > > > #ifdef __ASSEMBLY__ > > +#ifndef CONFIG_INDIRECT_THUNK > > +.equ CONFIG_INDIRECT_THUNK, 0 > > +#endif > > # include > > #else > > asm ( "\t.equ CONFIG_INDIRECT_THUNK, " > > > > It's fairly similar to my first approach, but at least "#ifdef > > CONFIG_INDIRECT_THUNK" will still work as expected after this fix. > > I've used something similar in backports to old versions, so I > wonder whether what you quote above is right: Assembly files > don't get handed to clang anyway iirc, so the change would > seem to be needed in the #else part of the conditional. Assembly files do get handed to clang, from xen/Rules.mk: %.o: %.S Makefile $(CC) $(AFLAGS) -c $< -o $@ Xen doesn't call as directly to compile those, or am I missing something? Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/4] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
>>> On 20.02.18 at 12:01, wrote: > --- a/xen/include/asm-x86/asm_defns.h > +++ b/xen/include/asm-x86/asm_defns.h > @@ -15,6 +15,9 @@ > #include > > #ifdef __ASSEMBLY__ > +#ifndef CONFIG_INDIRECT_THUNK > +.equ CONFIG_INDIRECT_THUNK, 0 > +#endif > # include > #else > asm ( "\t.equ CONFIG_INDIRECT_THUNK, " > > It's fairly similar to my first approach, but at least "#ifdef > CONFIG_INDIRECT_THUNK" will still work as expected after this fix. I've used something similar in backports to old versions, so I wonder whether what you quote above is right: Assembly files don't get handed to clang anyway iirc, so the change would seem to be needed in the #else part of the conditional. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/4] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
On Tue, Feb 20, 2018 at 10:19:16AM +, Roger Pau Monné wrote: > On Tue, Feb 20, 2018 at 02:15:24AM -0700, Jan Beulich wrote: > > >>> On 19.02.18 at 15:16, wrote: > > > --- a/xen/include/asm-x86/indirect_thunk_asm.h > > > +++ b/xen/include/asm-x86/indirect_thunk_asm.h > > > @@ -3,6 +3,10 @@ > > > * usual #ifdef'ary to turn into comments. > > > */ > > > > > > +.ifndef CONFIG_INDIRECT_THUNK > > > +.equ CONFIG_INDIRECT_THUNK, 0 > > > +.endif > > > > I have to withdraw my R-b: This fails to build (I'm getting an error > > on the .ifndef above for more than one assembler source). I assume > > that's because with indirect thunks enabled the constant gets > > replaced by literal 1 by the C preprocessor. > > Right, I've just installed gcc7-devel and I'm seeing the same. Thanks > for noticing. Let me try to solve this in another way... So far the best way I've found to solve this issue that works with both gcc (with and without indirect thunk support) and clang is the following: diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h index 6fc13d39d8..ebd2c88a1f 100644 --- a/xen/include/asm-x86/asm_defns.h +++ b/xen/include/asm-x86/asm_defns.h @@ -15,6 +15,9 @@ #include #ifdef __ASSEMBLY__ +#ifndef CONFIG_INDIRECT_THUNK +.equ CONFIG_INDIRECT_THUNK, 0 +#endif # include #else asm ( "\t.equ CONFIG_INDIRECT_THUNK, " It's fairly similar to my first approach, but at least "#ifdef CONFIG_INDIRECT_THUNK" will still work as expected after this fix. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/4] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
On Tue, Feb 20, 2018 at 02:15:24AM -0700, Jan Beulich wrote: > >>> On 19.02.18 at 15:16, wrote: > > --- a/xen/include/asm-x86/indirect_thunk_asm.h > > +++ b/xen/include/asm-x86/indirect_thunk_asm.h > > @@ -3,6 +3,10 @@ > > * usual #ifdef'ary to turn into comments. > > */ > > > > +.ifndef CONFIG_INDIRECT_THUNK > > +.equ CONFIG_INDIRECT_THUNK, 0 > > +.endif > > I have to withdraw my R-b: This fails to build (I'm getting an error > on the .ifndef above for more than one assembler source). I assume > that's because with indirect thunks enabled the constant gets > replaced by literal 1 by the C preprocessor. Right, I've just installed gcc7-devel and I'm seeing the same. Thanks for noticing. Let me try to solve this in another way... Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/4] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
>>> On 19.02.18 at 15:16, wrote: > --- a/xen/include/asm-x86/indirect_thunk_asm.h > +++ b/xen/include/asm-x86/indirect_thunk_asm.h > @@ -3,6 +3,10 @@ > * usual #ifdef'ary to turn into comments. > */ > > +.ifndef CONFIG_INDIRECT_THUNK > +.equ CONFIG_INDIRECT_THUNK, 0 > +.endif I have to withdraw my R-b: This fails to build (I'm getting an error on the .ifndef above for more than one assembler source). I assume that's because with indirect thunks enabled the constant gets replaced by literal 1 by the C preprocessor. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/4] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
>>> On 19.02.18 at 15:16, 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: > > :8:9: error: expected absolute expression > .if CONFIG_INDIRECT_THUNK == 1 > ^ > :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é As said previously I'm not overly happy about this, but still Acked-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
