| Message ID | 20200323020844.17064-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | x86: remove always-defined CONFIG_AS_* options | expand |
Hey Masahrio,
Thanks for this series. I'll rebase my recent RFC on top of these
changes, which makes the work I was doing slightly easier, as there
are now fewer flags to deal with.
Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>
Jason
Hi again, I've consolidated your patches and rebased mine on top, and incorporated your useful binutils comments. The result lives here: https://git.zx2c4.com/linux-dev/log/?h=jd/kconfig-assembler-support I can submit all of those to the list, if you want, or maybe you can just pull them out of there, include them in your v2, and put them in your tree for 5.7? However you want is fine with me. Jason
Hi Jason, On Mon, Mar 23, 2020 at 1:28 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi again, > > I've consolidated your patches and rebased mine on top, and > incorporated your useful binutils comments. The result lives here: > > https://git.zx2c4.com/linux-dev/log/?h=jd/kconfig-assembler-support > > I can submit all of those to the list, if you want, or maybe you can > just pull them out of there, include them in your v2, and put them in > your tree for 5.7? However you want is fine with me. Your series does not work correctly. I will comment why later.
On Mon, Mar 23, 2020 at 12:36 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Hi Jason, > > On Mon, Mar 23, 2020 at 1:28 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > Hi again, > > > > I've consolidated your patches and rebased mine on top, and > > incorporated your useful binutils comments. The result lives here: > > > > https://git.zx2c4.com/linux-dev/log/?h=jd/kconfig-assembler-support > > > > I can submit all of those to the list, if you want, or maybe you can > > just pull them out of there, include them in your v2, and put them in > > your tree for 5.7? However you want is fine with me. > > > Your series does not work correctly. > > I will comment why later. Bummer, okay. Looking forward to learning what's up. Also, if some parts of it are useful (like the resorting and organizing of arch/x86/crypto/Makefile), feel free to cannibalize it, keeping what's useful and discarding what's not. Jason
On Mon, Mar 23, 2020 at 7:53 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > On Mon, Mar 23, 2020 at 12:36 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > Hi Jason, > > > > On Mon, Mar 23, 2020 at 1:28 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > > > Hi again, > > > > > > I've consolidated your patches and rebased mine on top, and > > > incorporated your useful binutils comments. The result lives here: > > > > > > https://git.zx2c4.com/linux-dev/log/?h=jd/kconfig-assembler-support > > > > > > I can submit all of those to the list, if you want, or maybe you can > > > just pull them out of there, include them in your v2, and put them in > > > your tree for 5.7? However you want is fine with me. > > > > > > Your series does not work correctly. > > > > I will comment why later. > > Bummer, okay. Looking forward to learning what's up. Also, if some > parts of it are useful (like the resorting and organizing of > arch/x86/crypto/Makefile), feel free to cannibalize it, keeping what's > useful and discarding what's not. > Hi Jason, I have your patches on my radar. I have not checked your Kconfig changes are really working, especially I looked at [2] and comment on this. I would have expected your arch/x86/Kconfig.assembler file as arch/x86/crypto/Kconfig (source include needs to be adapted in arch/x86/Kconfig). What about a commit subject like "x86: crypto: Probe assembler options via Kconfig instead of makefile"? Not sure if the commit message needs some massage? Maybe this is all irrelevant when Masahiro has commented. Thanks. Regards, - Sedat - [1] https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/linux.git/log/?h=jd/kconfig-assembler-support [2] https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/linux.git/commit/?h=jd/kconfig-assembler-support&id=ac483ff6fb4c785cd0b10d9756b71696829cd117
On Sun, Mar 22, 2020 at 7:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > arch/x86/Makefile tests instruction code by $(call as-instr, ...) > > Some of them are very old. > For example, the check for CONFIG_AS_CFI dates back to 2006. > > We raise GCC versions from time to time, and we clean old code away. > The same policy applied to binutils. > > The current minimal supported version of binutils is 2.21 > > This is new enough to recognize the instruction in most of > as-instr calls. I'm quite happy to see this series; a few weekends ago I was playing around with adding dwarf-5 support to the Linux kernel, and was looking at these noticing there was quite a bit of cruft. Unfortunately, I got detoured filing bugs against GNU as for dwarf-5 bugs, but the developers were very responsive and fixed them all. I should go find and dust off that patchset. In the meantime, I'll try to help review these patches. Thank you for sending them. > > > > Masahiro Yamada (7): > x86: remove unneeded defined(__ASSEMBLY__) check from asm/dwarf2.h > x86: remove always-defined CONFIG_AS_CFI > x86: remove always-defined CONFIG_AS_CFI_SIGNAL_FRAME > x86: remove always-defined CONFIG_AS_CFI_SECTIONS > x86: remove always-defined CONFIG_AS_SSSE3 > x86: remove always-defined CONFIG_AS_AVX > x86: add comments about the binutils version to support code in > as-instr > > arch/x86/Makefile | 21 +++------ > arch/x86/crypto/Makefile | 32 ++++++-------- > arch/x86/crypto/aesni-intel_avx-x86_64.S | 3 -- > arch/x86/crypto/aesni-intel_glue.c | 14 +----- > arch/x86/crypto/blake2s-core.S | 2 - > arch/x86/crypto/poly1305-x86_64-cryptogams.pl | 8 ---- > arch/x86/crypto/poly1305_glue.c | 6 +-- > arch/x86/crypto/sha1_ssse3_asm.S | 4 -- > arch/x86/crypto/sha1_ssse3_glue.c | 9 +--- > arch/x86/crypto/sha256-avx-asm.S | 3 -- > arch/x86/crypto/sha256_ssse3_glue.c | 8 +--- > arch/x86/crypto/sha512-avx-asm.S | 2 - > arch/x86/crypto/sha512_ssse3_glue.c | 7 +-- > arch/x86/include/asm/dwarf2.h | 43 ------------------- > arch/x86/include/asm/xor_avx.h | 9 ---- > lib/raid6/algos.c | 2 - > lib/raid6/recov_ssse3.c | 6 --- > lib/raid6/test/Makefile | 3 -- > 18 files changed, 26 insertions(+), 156 deletions(-) > > -- > 2.17.1 > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200323020844.17064-1-masahiroy%40kernel.org.
On Mon, Mar 23, 2020 at 3:53 AM Sedat Dilek <sedat.dilek@gmail.com> wrote: > Hi Jason, > I have not checked your Kconfig changes are really working, especially > I looked at [2] and comment on this. > > I would have expected your arch/x86/Kconfig.assembler file as > arch/x86/crypto/Kconfig (source include needs to be adapted in > arch/x86/Kconfig). CONFIG_AS_* is required for more than just the crypto. > What about a commit subject like "x86: crypto: Probe assembler options > via Kconfig instead of makefile"? Thanks. Will fix silly verbiage and update branch. Jason
Hi Jason, On Mon, Mar 23, 2020 at 3:53 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > On Mon, Mar 23, 2020 at 12:36 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > Hi Jason, > > > > On Mon, Mar 23, 2020 at 1:28 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > > > Hi again, > > > > > > I've consolidated your patches and rebased mine on top, and > > > incorporated your useful binutils comments. The result lives here: > > > > > > https://git.zx2c4.com/linux-dev/log/?h=jd/kconfig-assembler-support > > > > > > I can submit all of those to the list, if you want, or maybe you can > > > just pull them out of there, include them in your v2, and put them in > > > your tree for 5.7? However you want is fine with me. > > > > > > Your series does not work correctly. > > > > I will comment why later. > > Bummer, okay. Looking forward to learning what's up. Also, if some > parts of it are useful (like the resorting and organizing of > arch/x86/crypto/Makefile), feel free to cannibalize it, keeping what's > useful and discarding what's not. > The answer is mostly in my previous reply to Linus: https://lkml.org/lkml/2020/3/13/27 I think this problem would happen for CONFIG_AS_CFI and CONFIG_AS_ADX since the register in instruction code is machine-bit dependent. The former is OK wince we are planning to remove it. We need to handle -m64 for the latter. Otherwise, a problem like commit 3a7c733165a4799fa1 would happen. So, I think we should merge this https://lore.kernel.org/patchwork/patch/1214332/ then, fix-up CONFIG_AS_ADX on top of it. (Or, if we do not need to rush, we can delete CONFIG_AS_ADX entirely after we bump the binutils version to 2.23) Thanks.
On Mon, Mar 23, 2020 at 4:04 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Hi Jason, > > On Mon, Mar 23, 2020 at 3:53 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > On Mon, Mar 23, 2020 at 12:36 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > Hi Jason, > > > > > > On Mon, Mar 23, 2020 at 1:28 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > > > > > Hi again, > > > > > > > > I've consolidated your patches and rebased mine on top, and > > > > incorporated your useful binutils comments. The result lives here: > > > > > > > > https://git.zx2c4.com/linux-dev/log/?h=jd/kconfig-assembler-support > > > > > > > > I can submit all of those to the list, if you want, or maybe you can > > > > just pull them out of there, include them in your v2, and put them in > > > > your tree for 5.7? However you want is fine with me. > > > > > > > > > Your series does not work correctly. > > > > > > I will comment why later. > > > > Bummer, okay. Looking forward to learning what's up. Also, if some > > parts of it are useful (like the resorting and organizing of > > arch/x86/crypto/Makefile), feel free to cannibalize it, keeping what's > > useful and discarding what's not. > > > > > The answer is mostly in my previous reply to Linus: > https://lkml.org/lkml/2020/3/13/27 > > > I think this problem would happen > for CONFIG_AS_CFI and CONFIG_AS_ADX > since the register in instruction code > is machine-bit dependent. > > The former is OK wince we are planning to > remove it. > > We need to handle -m64 for the latter. > Otherwise, a problem like commit > 3a7c733165a4799fa1 would happen. > > > So, I think we should merge this > https://lore.kernel.org/patchwork/patch/1214332/ > then, fix-up CONFIG_AS_ADX on top of it. > > (Or, if we do not need to rush, > we can delete CONFIG_AS_ADX entirely after > we bump the binutils version to 2.23) Oh, gotcha. The easiest thing to do for that case would actually be passing 32-bit registers to adox, which are valid. I'll fix that up in my tree. And then indeed it looks like the binutils bump is coming anyway for 5.7. Your flags patch looks fine and potentially useful for other things down the line though. I suppose you had in mind something like: def_bool $(as-instr,...,-m64) if 64BIT def_bool $(as-instr,...,-m32) if !64BIT Anyway, I'll fix up the ADX code to be biarch like the AVX test code. Jason
On Mon, Mar 23, 2020 at 8:50 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > I would have expected your arch/x86/Kconfig.assembler file as > > arch/x86/crypto/Kconfig (source include needs to be adapted in > > arch/x86/Kconfig). > > CONFIG_AS_* is required for more than just the crypto. > OK. I was not aware of this. > > What about a commit subject like "x86: crypto: Probe assembler options > > via Kconfig instead of makefile"? > > Thanks. Will fix silly verbiage and update branch. > Just looked to what I see new in [0]. Would you mind to add the patch "Documentation/changes: Raise minimum supported binutils version to 2.23" from [1] to your series, please? For the meantime and clarification - you can drop it later (with adding Link-tag to [1]) when it landed in tip Git [2] where I am not seeing it. Thanks for taking care to you an Masah*e*ro :-). - Sedat - [0] https://git.zx2c4.com/linux-dev/log/?h=jd/kconfig-assembler-support [1] https://lore.kernel.org/lkml/20200316160259.GN26126@zn.tnic/ [2] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/
arch/x86/Makefile tests instruction code by $(call as-instr, ...) Some of them are very old. For example, the check for CONFIG_AS_CFI dates back to 2006. We raise GCC versions from time to time, and we clean old code away. The same policy applied to binutils. The current minimal supported version of binutils is 2.21 This is new enough to recognize the instruction in most of as-instr calls. Masahiro Yamada (7): x86: remove unneeded defined(__ASSEMBLY__) check from asm/dwarf2.h x86: remove always-defined CONFIG_AS_CFI x86: remove always-defined CONFIG_AS_CFI_SIGNAL_FRAME x86: remove always-defined CONFIG_AS_CFI_SECTIONS x86: remove always-defined CONFIG_AS_SSSE3 x86: remove always-defined CONFIG_AS_AVX x86: add comments about the binutils version to support code in as-instr arch/x86/Makefile | 21 +++------ arch/x86/crypto/Makefile | 32 ++++++-------- arch/x86/crypto/aesni-intel_avx-x86_64.S | 3 -- arch/x86/crypto/aesni-intel_glue.c | 14 +----- arch/x86/crypto/blake2s-core.S | 2 - arch/x86/crypto/poly1305-x86_64-cryptogams.pl | 8 ---- arch/x86/crypto/poly1305_glue.c | 6 +-- arch/x86/crypto/sha1_ssse3_asm.S | 4 -- arch/x86/crypto/sha1_ssse3_glue.c | 9 +--- arch/x86/crypto/sha256-avx-asm.S | 3 -- arch/x86/crypto/sha256_ssse3_glue.c | 8 +--- arch/x86/crypto/sha512-avx-asm.S | 2 - arch/x86/crypto/sha512_ssse3_glue.c | 7 +-- arch/x86/include/asm/dwarf2.h | 43 ------------------- arch/x86/include/asm/xor_avx.h | 9 ---- lib/raid6/algos.c | 2 - lib/raid6/recov_ssse3.c | 6 --- lib/raid6/test/Makefile | 3 -- 18 files changed, 26 insertions(+), 156 deletions(-)