Re: [Xen-devel] [PATCH v3 5/5] x86: remove usage of .skip with non-absolute expressions
On Mon, Jan 29, 2018 at 09:50:56AM -0700, Jan Beulich wrote: > >>> On 29.01.18 at 13:26, wrote: > > Clang assembler doesn't support using .skip with non-absolute > > expressions: > > > > entry.S:109:15: error: expected absolute expression > > .skip .Lcr4_alt_end - .Lcr4_alt, 0x90 > > ^ > > > > This usage of .skip was to fill code sections with NOPs in order for > > them to be patched at run time if required by the alternatives > > framework. Instead of using .skip use the appropriate number of NOPs > > to match the size of the alternative code. > > NAK - I've voiced a number of times my opposition to Andrew adding > the various ASM_NOP in his Spectre v2 series, and I'm planning > to eliminate them in due course (by using - you guess it - .skip). It > is pretty much unacceptable to me to have to encode the size of > certain instructions in a second place, risking them to go out of sync > with what they shadow. > > If ugliness like this is needed to support clang's integrated assembler, > then I see no way other than avoiding its use. Right, I agree this is ugly and prone to error. This seems to be fixed in clang thunk, so I've asked for a backport of the fix to 6.0. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] x86: remove usage of .skip with non-absolute expressions
On 29/01/18 16:50, Jan Beulich wrote: On 29.01.18 at 13:26, wrote: >> Clang assembler doesn't support using .skip with non-absolute >> expressions: >> >> entry.S:109:15: error: expected absolute expression >> .skip .Lcr4_alt_end - .Lcr4_alt, 0x90 >> ^ >> >> This usage of .skip was to fill code sections with NOPs in order for >> them to be patched at run time if required by the alternatives >> framework. Instead of using .skip use the appropriate number of NOPs >> to match the size of the alternative code. > NAK - I've voiced a number of times my opposition to Andrew adding > the various ASM_NOP in his Spectre v2 series, and I'm planning > to eliminate them in due course (by using - you guess it - .skip). It > is pretty much unacceptable to me to have to encode the size of > certain instructions in a second place, risking them to go out of sync > with what they shadow. And FTR, the Spectre v2 series explicit sizes were only because I tried fixing Xen's ability to do sensible things with nops in alternative, but got far too bogged down and decided that the Spectre work was more important. I will be dusting the series off in due course and removing the explicit sizes. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] x86: remove usage of .skip with non-absolute expressions
>>> On 29.01.18 at 13:26, wrote: > Clang assembler doesn't support using .skip with non-absolute > expressions: > > entry.S:109:15: error: expected absolute expression > .skip .Lcr4_alt_end - .Lcr4_alt, 0x90 > ^ > > This usage of .skip was to fill code sections with NOPs in order for > them to be patched at run time if required by the alternatives > framework. Instead of using .skip use the appropriate number of NOPs > to match the size of the alternative code. NAK - I've voiced a number of times my opposition to Andrew adding the various ASM_NOP in his Spectre v2 series, and I'm planning to eliminate them in due course (by using - you guess it - .skip). It is pretty much unacceptable to me to have to encode the size of certain instructions in a second place, risking them to go out of sync with what they shadow. If ugliness like this is needed to support clang's integrated assembler, then I see no way other than avoiding its use. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] x86: remove usage of .skip with non-absolute expressions
>>> On 29.01.18 at 14:05, wrote: > On 29/01/18 13:02, Jan Beulich wrote: > On 29.01.18 at 13:43, wrote: >>> On Mon, Jan 29, 2018 at 12:26:43PM +, Roger Pau Monne wrote: Clang assembler doesn't support using .skip with non-absolute expressions: >>> But so is GNU as. From its manual for .skip: >>> >>> "This directive emits size bytes, each of value fill. Both size and fill >>> are absolute expressions." >>> entry.S:109:15: error: expected absolute expression .skip .Lcr4_alt_end - .Lcr4_alt, 0x90 ^ >>> OOI what makes .Lcr4_alt_end - .Lcr4_alt non-absolute? >> I guess they expect to be able to calculate the value right at the >> point they evaluate .skip's arguments. Whereas gas either records >> a fragment with variable size, or (less likely, as that could end up >> wrong) evaluates the expression right away (iirc the .skip sits after >> the definition of both symbols, and iirc further gas [at least some >> versions] has problems if that wasn't the case). > > One thing I've been experimenting with, along with trying to organise > the alternatives into a mergeable section, is to first write the > alternatives into .discard section, which allows calculations like this > to be completed immediately, rather than being deferred. How would such calculations be completed immediately? It doesn't matter what section you put things in - final addresses can be known only once all input has been consumed by the assembler. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] x86: remove usage of .skip with non-absolute expressions
On 29/01/18 13:02, Jan Beulich wrote: On 29.01.18 at 13:43, wrote: >> On Mon, Jan 29, 2018 at 12:26:43PM +, Roger Pau Monne wrote: >>> Clang assembler doesn't support using .skip with non-absolute >>> expressions: >>> >> But so is GNU as. From its manual for .skip: >> >> "This directive emits size bytes, each of value fill. Both size and fill >> are absolute expressions." >> >>> entry.S:109:15: error: expected absolute expression >>> .skip .Lcr4_alt_end - .Lcr4_alt, 0x90 >>> ^ >>> >> OOI what makes .Lcr4_alt_end - .Lcr4_alt non-absolute? > I guess they expect to be able to calculate the value right at the > point they evaluate .skip's arguments. Whereas gas either records > a fragment with variable size, or (less likely, as that could end up > wrong) evaluates the expression right away (iirc the .skip sits after > the definition of both symbols, and iirc further gas [at least some > versions] has problems if that wasn't the case). One thing I've been experimenting with, along with trying to organise the alternatives into a mergeable section, is to first write the alternatives into .discard section, which allows calculations like this to be completed immediately, rather than being deferred. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] x86: remove usage of .skip with non-absolute expressions
>>> On 29.01.18 at 13:43, wrote: > On Mon, Jan 29, 2018 at 12:26:43PM +, Roger Pau Monne wrote: >> Clang assembler doesn't support using .skip with non-absolute >> expressions: >> > > But so is GNU as. From its manual for .skip: > > "This directive emits size bytes, each of value fill. Both size and fill > are absolute expressions." > >> entry.S:109:15: error: expected absolute expression >> .skip .Lcr4_alt_end - .Lcr4_alt, 0x90 >> ^ >> > > OOI what makes .Lcr4_alt_end - .Lcr4_alt non-absolute? I guess they expect to be able to calculate the value right at the point they evaluate .skip's arguments. Whereas gas either records a fragment with variable size, or (less likely, as that could end up wrong) evaluates the expression right away (iirc the .skip sits after the definition of both symbols, and iirc further gas [at least some versions] has problems if that wasn't the case). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] x86: remove usage of .skip with non-absolute expressions
On Mon, Jan 29, 2018 at 12:43:10PM +, Wei Liu wrote: > On Mon, Jan 29, 2018 at 12:26:43PM +, Roger Pau Monne wrote: > > Clang assembler doesn't support using .skip with non-absolute > > expressions: > > > > But so is GNU as. From its manual for .skip: > > "This directive emits size bytes, each of value fill. Both size and fill > are absolute expressions." I guess clang and as have different interpretations of what's an absolute expression. Sadly I don't seem to be able to find neither definitions. I've already reported this to upstream clang long time ago, but it doesn't seem to make any progress: https://bugs.llvm.org/show_bug.cgi?id=27369 And I don't think I can hack this myself. > > entry.S:109:15: error: expected absolute expression > > .skip .Lcr4_alt_end - .Lcr4_alt, 0x90 > > ^ > > > > OOI what makes .Lcr4_alt_end - .Lcr4_alt non-absolute? Clang doesn't consider symbols as absolute expressions, at least in the context of .skip. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] x86: remove usage of .skip with non-absolute expressions
On Mon, Jan 29, 2018 at 12:26:43PM +, Roger Pau Monne wrote: > Clang assembler doesn't support using .skip with non-absolute > expressions: > But so is GNU as. From its manual for .skip: "This directive emits size bytes, each of value fill. Both size and fill are absolute expressions." > entry.S:109:15: error: expected absolute expression > .skip .Lcr4_alt_end - .Lcr4_alt, 0x90 > ^ > OOI what makes .Lcr4_alt_end - .Lcr4_alt non-absolute? Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 5/5] x86: remove usage of .skip with non-absolute expressions
Clang assembler doesn't support using .skip with non-absolute expressions: entry.S:109:15: error: expected absolute expression .skip .Lcr4_alt_end - .Lcr4_alt, 0x90 ^ This usage of .skip was to fill code sections with NOPs in order for them to be patched at run time if required by the alternatives framework. Instead of using .skip use the appropriate number of NOPs to match the size of the alternative code. Signed-off-by: Roger Pau Monné --- Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu --- xen/Rules.mk | 5 - xen/arch/x86/x86_64/compat/entry.S | 9 - xen/arch/x86/x86_64/entry.S| 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/xen/Rules.mk b/xen/Rules.mk index 205f0aff30..51bcd5804c 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -66,11 +66,6 @@ endif AFLAGS-y+= -D__ASSEMBLY__ -# Clang's built-in assembler doesn't understand .skip or .rept 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) # Get gcc to generate the dependencies for us. diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S index e668f00c36..d94220b6b6 100644 --- a/xen/arch/x86/x86_64/compat/entry.S +++ b/xen/arch/x86/x86_64/compat/entry.S @@ -106,7 +106,14 @@ ENTRY(compat_restore_all_guest) mov $~(X86_EFLAGS_IOPL|X86_EFLAGS_NT|X86_EFLAGS_VM),%r11d and UREGS_eflags(%rsp),%r11d .Lcr4_orig: -.skip .Lcr4_alt_end - .Lcr4_alt, 0x90 +ASM_NOP8 /* testb $3,UREGS_cs(%rsp) */ +ASM_NOP2 /* jpe .Lcr4_alt_end */ +ASM_NOP8 /* mov CPUINFO_cr4...(%rsp), %rax */ +ASM_NOP6 /* and $..., %rax */ +ASM_NOP8 /* mov %rax, CPUINFO_cr4...(%rsp) */ +ASM_NOP3 /* mov %rax, %cr4 */ +ASM_NOP8 /* mov %rax, CPUINFO_cr4...(%rsp) */ +ASM_NOP2 /* jne 1b */ .Lcr4_orig_end: .pushsection .altinstr_replacement, "ax" .Lcr4_alt: diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S index af703f6c06..d9dce0e421 100644 --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -529,7 +529,7 @@ handle_exception_saved: .Lcr4_pv32_orig: jmp .Lcr4_pv32_done -.skip (.Lcr4_pv32_alt_end - .Lcr4_pv32_alt) - (. - .Lcr4_pv32_orig), 0xcc +ASM_NOP2 /* jmp is 2 bytes, the alternative mov is 4 bytes. */ .pushsection .altinstr_replacement, "ax" .Lcr4_pv32_alt: mov VCPU_domain(%rbx),%rax -- 2.15.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
