From: Arvind Sankar <email@example.com> To: Nick Desaulniers <firstname.lastname@example.org> Cc: Thomas Gleixner <email@example.com>, Ingo Molnar <firstname.lastname@example.org>, Arnd Bergmann <email@example.com>, Borislav Petkov <firstname.lastname@example.org>, "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <email@example.com>, "H. Peter Anvin" <firstname.lastname@example.org>, "Kirill A. Shutemov" <email@example.com>, Zhenzhong Duan <firstname.lastname@example.org>, Kees Cook <email@example.com>, Peter Zijlstra <firstname.lastname@example.org>, Juergen Gross <email@example.com>, Andy Lutomirski <firstname.lastname@example.org>, Andrew Cooper <email@example.com>, LKML <firstname.lastname@example.org>, clang-built-linux <email@example.com> Subject: Re: [PATCH] x86: work around clang IAS bug referencing __force_order Date: Thu, 13 Aug 2020 13:20:34 -0400 Message-ID: <20200813172034.GA500410@rani.riverdale.lan> (raw) In-Reply-To: <CAKwvOdnOh3H3ga2qpTktywvcgfXW5QJaB7r4XMhigmDzLhDNeA@mail.gmail.com> On Wed, Aug 12, 2020 at 05:12:34PM -0700, Nick Desaulniers wrote: > On Thu, Aug 6, 2020 at 3:11 PM Thomas Gleixner <firstname.lastname@example.org> wrote: > > > > Arnd Bergmann <email@example.com> writes: > > > When using the clang integrated assembler, we get a reference > > > to __force_order that should normally get ignored in a few > > > rare cases: > > > > > > ERROR: modpost: "__force_order" [drivers/cpufreq/powernow-k6.ko] undefined! > > > > > > Add a 'static' definition so any file in which this happens can > > > have a local copy. > > > > That's a horrible hack. > > Agreed. And static means everyone gets their own copy, rather than > sharing one memory address. I guess no one actually writes to it, so > it doesn't really matter, but __force_order just seems so strange to > me. > > > And the only reason why it does not trigger -Wunused-variable warnings > > all over the place is because it's "referenced" in unused inline > > functions and then optimized out along with the unused inlines. > > > > > * It is not referenced from the code, but GCC < 5 with -fPIE would fail > > > * due to an undefined symbol. Define it to make these ancient GCCs > > > work. > > > > Bah, we really should have moved straight to GCC5 instead of upping it > > just to 4.9 > > > > > + * > > > + * Clang sometimes fails to kill the reference to the dummy variable, so > > > + * provide an actual copy. > > > > Can that compiler be fixed instead? > > I don't think so. The logic in the compiler whether to emit an > "address is significant" assembler directive is based on whether the > variable is "used." The "use" of `__force_order` is as output of all > of these control register read/write functions' inline asm, even > though the inline asm doesn't actually write to them. We'd have to > peek inside of the inline asm and build "use/def chains" for the > inline asm, to see that you don't actually use the output variable. > Best we can do is see it listed as an output to the inline asm > statement. And if you reference an `extern` variable, it should be no > wonder that you can get undefined symbol linkage failures. > > I'd much rather remove all of __force_order. > > > > > Aside of that is there a reason to make this 'static' thing wrapped in > > #ifdeffery? A quick check with GCC8.3 just works. But maybe 4.9 gets > > unhappy. Can't say due to: -ENOANCIENTCOMPILER :) > > >From the comment in arch/x86/boot/compressed/pgtable_64.c, there's a > hint that maybe gcc < 5 and -pie (CONFIG_RANDOMIZE_BASE?) would fail > due to undefined symbol, though I'm not sure which symbol the comment > is referring to. If it's __force_order, then removing outright likely > fixes that issue. Yes, it's __force_order. Compressed kernel is always -fPIE, and gcc <5 and clang will generate mov instructions with GOTPCREL relocations to load the address of __force_order into a register for use by the inline asm. gcc-5+ works because it doesn't use GOTPCREL for global variables, instead relying on the linker inserting copy relocations if necessary. > > Not sure about the comment in arch/x86/include/asm/special_insns.h > either; smells fishy like a bug with a compiler from a long time ago. > It looks like it was introduced in: > commit d3ca901f94b32 ("x86: unify paravirt parts of system.h") > Lore has this thread: > https://lore.kernel.org/lkml/4755A809.firstname.lastname@example.org/ > Patch 4: https://email@example.com/ > It seems like there was a discussion about %cr8, but no one asked > "what's going on here with __force_order, is that right?" > Latest GCC release on December 4 2007 would have been GCC 4.2.2 according to: > https://gcc.gnu.org/releases.html > > Quick boot test of the below works for me, though I should probably > test hosting a virtualized guest since d3ca901f94b32 refers to > paravirt. Thoughts? It's unclear if there was a real problem this fixes, but if there was I'd expect it on native, not paravirt, given it's native that has this __force_order hack? gcc's documentation of volatile extended asm includes a caveat. https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile Near the end of 126.96.36.199: "Note that the compiler can move even volatile asm instructions relative to other code, including across jump instructions." and it provides an example of unexpected code motion, with the fix being adding an artificial dependency to the asm. So it might do something silly like reversing the order of two %crn writes, maybe?
next prev parent reply index Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-27 13:53 Arnd Bergmann 2020-08-01 11:50 ` Sedat Dilek 2020-08-06 22:13 ` Thomas Gleixner 2020-08-07 7:03 ` Sedat Dilek 2020-08-04 0:09 ` Nick Desaulniers 2020-08-14 17:29 ` Sedat Dilek 2020-08-14 21:19 ` Sedat Dilek 2020-08-14 22:57 ` Nick Desaulniers 2020-08-15 0:26 ` Nick Desaulniers 2020-08-15 3:28 ` Sedat Dilek 2020-08-15 8:23 ` Sedat Dilek 2020-08-15 10:46 ` Sedat Dilek 2020-08-15 14:39 ` Sedat Dilek 2020-08-16 9:37 ` Sedat Dilek 2020-08-06 22:11 ` Thomas Gleixner 2020-08-13 0:12 ` Nick Desaulniers 2020-08-13 8:49 ` David Laight 2020-08-13 17:20 ` Arvind Sankar [this message] 2020-08-13 17:28 ` Thomas Gleixner 2020-08-13 17:37 ` Paul E. McKenney 2020-08-13 18:09 ` Arvind Sankar 2020-08-13 18:20 ` Paul E. McKenney 2020-08-20 10:44 ` Thomas Gleixner 2020-08-20 13:06 ` Arvind Sankar 2020-08-21 0:37 ` Thomas Gleixner 2020-08-21 23:04 ` Arvind Sankar 2020-08-21 23:16 ` Nick Desaulniers 2020-08-21 23:25 ` Arvind Sankar 2020-08-22 0:43 ` Thomas Gleixner 2020-08-22 3:55 ` Arvind Sankar 2020-08-22 8:41 ` Segher Boessenkool 2020-08-22 9:23 ` Sedat Dilek 2020-08-22 9:51 ` Sedat Dilek 2020-08-22 10:26 ` Segher Boessenkool 2020-08-22 10:35 ` Arnd Bergmann 2020-08-22 18:17 ` Miguel Ojeda 2020-08-22 21:08 ` Linus Torvalds 2020-08-22 23:10 ` Arvind Sankar 2020-08-23 0:10 ` Linus Torvalds 2020-08-23 1:16 ` Arvind Sankar 2020-08-23 21:25 ` [PATCH] x86/asm: Replace __force_order with memory clobber Arvind Sankar 2020-08-24 17:50 ` Nathan Chancellor 2020-08-24 19:13 ` Miguel Ojeda 2020-08-25 15:19 ` Arvind Sankar 2020-08-25 15:21 ` Sedat Dilek 2020-09-02 15:33 ` [PATCH v2] " Arvind Sankar 2020-09-02 15:58 ` David Laight 2020-09-02 16:14 ` Arvind Sankar 2020-09-02 16:08 ` Arvind Sankar 2020-09-02 20:26 ` David Laight 2020-09-02 17:16 ` Segher Boessenkool 2020-09-02 17:36 ` Arvind Sankar 2020-09-02 18:19 ` Miguel Ojeda 2020-09-02 18:24 ` Arvind Sankar 2020-09-02 23:21 ` [PATCH v3] " Arvind Sankar 2020-09-03 2:17 ` Kees Cook 2020-09-03 5:34 ` Miguel Ojeda 2020-09-30 20:50 ` Kees Cook 2020-10-01 10:12 ` [tip: x86/asm] x86/asm: Replace __force_order with a " tip-bot2 for Arvind Sankar 2020-10-13 9:30 ` tip-bot2 for Arvind Sankar 2020-08-22 21:17 ` [PATCH] x86: work around clang IAS bug referencing __force_order Arvind Sankar 2020-08-23 13:31 ` David Laight 2020-09-08 22:25 ` Pavel Machek
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200813172034.GA500410@rani.riverdale.lan \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
LKML Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \ email@example.com public-inbox-index lkml Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git