Skip to content
This repository was archived by the owner on Apr 13, 2024. It is now read-only.

i386#182

Merged
nickdesaulniers merged 3 commits into
masterfrom
i386
May 6, 2020
Merged

i386#182
nickdesaulniers merged 3 commits into
masterfrom
i386

Conversation

@tpimh

@tpimh tpimh commented Jun 26, 2019

Copy link
Copy Markdown
Contributor

This is WIP. Currently, lacking buildroot image (will add it when the kernel is able to boot), probably shouldn't be merged until ClangBuiltLinux/linux#3 is fixed.

UPD: rootfs added, booting fine with GNU ld, but not LLD (UPD2: fixed). After these issues are fixed, we can proceed with this PR:

@tpimh tpimh added the WIP Work in progress label Jun 26, 2019
@tpimh tpimh changed the title Add i386 targets i386 Jun 26, 2019
@nickdesaulniers

Copy link
Copy Markdown
Member

https://travis-ci.com/ClangBuiltLinux/continuous-integration/jobs/211110274 shows that you're hitting what looks slightly similar to ClangBuiltLinux/linux#186.

ld: arch/x86/entry/vsyscall/vsyscall_gtod.o: in function `update_vsyscall':
vsyscall_gtod.c:(.text+0x262): undefined reference to `__udivdi3'

If the Makefile in arch/x86/entry/vsyscall/ does not have -Oz, then looks like we have another issue to deal with.

@nathanchance

Copy link
Copy Markdown
Member

I was able to bisect the problematic commit as well: ClangBuiltLinux/linux#186 (comment)

@nickdesaulniers

Copy link
Copy Markdown
Member

(I'll bet the sub expression tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift involves 64b numbers).

@tpimh

tpimh commented Jun 27, 2019

Copy link
Copy Markdown
Contributor Author

Should ClangBuiltLinux/linux#186 be reopenned then?

@nathanchance

Copy link
Copy Markdown
Member

A new issue should probably be opened.

@tpimh

tpimh commented Jun 27, 2019

Copy link
Copy Markdown
Contributor Author

The patch to revert vgtod_ts commits is huge and most of it is unnecessary, but it works. Now there is also a problem with LLD, I will try to fix it later (or at least create an issue for it as I can't find one).

@nathanchance

Copy link
Copy Markdown
Member

Tentatively seems good.

I assume those two patches are going to be upstreamed at some point? Given that we just got all of our LLVM 9 targets building patch free, it's a little sad to add some back.

@nathanchance

Copy link
Copy Markdown
Member

It also looks like your percpu patch needs to be rebased?

@nickdesaulniers

Copy link
Copy Markdown
Member

make sure to remove the WIP label when it's ready for code review

@tpimh

tpimh commented Jul 15, 2019

Copy link
Copy Markdown
Contributor Author

The percpu patch is easy to fix so it builds, but this is not a proper fix, I'll check how similar issue was fixed with x86_64. The other patch should probably be rewritten as well. After these patches are upstreamed, i386 can be merged to master.

@tpimh tpimh force-pushed the i386 branch 6 times, most recently from 2af6a05 to caa9f23 Compare September 22, 2019 11:52
@tpimh

tpimh commented Sep 22, 2019

Copy link
Copy Markdown
Contributor Author

Rebased with master and surprisingly it's broken. Builds fine, but doesn't boot. I will try to build 4.19 to see if it works.

@nickdesaulniers

Copy link
Copy Markdown
Member

I recommend:

  1. commit the buildroot changes separately first.
  2. use qemu+gdb to see where we hang.

@nathanchance

Copy link
Copy Markdown
Member
  1. use qemu+gdb to see where we hang.

I was going to try this today but I cannot get the GDB scripts to load, am I doing something wrong?

% curl -LSs https://raw.githubusercontent.com/ClangBuiltLinux/continuous-integration/i386/patches/llvm-all/linux/i386/i386-percpu.patch | git apply -v
Checking patch arch/x86/include/asm/percpu.h...
Applied patch arch/x86/include/asm/percpu.h cleanly.

% echo "CONFIG_DEBUG_INFO=y\nCONFIG_GDB_SCRIPTS=y" >> arch/x86/configs/i386_defconfig

% make -j$(nproc) -s \
ARCH=i386 \
CC=clang \
O=out \
distclean defconfig bzImage

% qemu-system-i386 \
-m 512m \
-drive file=${HOME}/cbl/git/ci/images/i386/rootfs.ext4,format=raw,if=ide \
-append 'console=ttyS0 root=/dev/sda' \
-display none \
-serial mon:stdio \
-kernel ${HOME}/src/linux/out/arch/i386/boot/bzImage \
-s \
-S

% echo "add-auto-load-safe-path ${PWD}/scripts/gdb/vmlinux-gdb.py" >> ~/.gdbinit

% gdb out/vmlinux
GNU gdb (GDB) 8.3
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from out/vmlinux...
(gdb) target remote :1234
Remote debugging using :1234
0x0000fff0 in ?? ()
(gdb) lx-version
Undefined command: "lx-version".  Try "help".
(gdb) apropos lx
(gdb) 

@tpimh

tpimh commented Sep 30, 2019

Copy link
Copy Markdown
Contributor Author

Exactly the same problem here. I was trying to get it working for a while, tried everything I could think of, but no luck, apropos lx always showed nothing. Created a couple forum threads about my problem, but got zero responses. Really started to think that this is just my problem, and I am doing something wrong. If it's the case, at least I'm not alone now.

The only useful piece information I got with gdb is that the kernel panics with "Attempted to kill the idle task!"

fengguang pushed a commit to 0day-ci/linux that referenced this pull request Apr 27, 2020
The top level Makefile disables this warning. When building an
i386_defconfig with Clang, this warning is triggered a whole bunch via
includes of headers from perf.

Link: ClangBuiltLinux/continuous-integration#182
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
@tpimh

tpimh commented Apr 28, 2020

Copy link
Copy Markdown
Contributor Author

@nickdesaulniers which version of fix for invalid output size for constraint '=q' do you think is better: ClangBuiltLinux/linux#194 (comment)?

@nickdesaulniers

Copy link
Copy Markdown
Member

which version of fix for invalid output size for constraint '=q' do you think is better: ClangBuiltLinux/linux#194 (comment)?

https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints says for q:

Any register accessible as rl. In 32-bit mode, a, b, c, and d; in 64-bit mode, any integer register.

so for inputs I prefer @dwmw2's approach of casting to (unsigned char) which is truncation. I don't understand why there's a cast to (unsigned long) though, which will zero extend. I need to look more at the sources (the diff doesn't have all the info I need to make a more informed decision). We already know the size, so I think we could just cast to the unsigned integral type of the equivalent byte size.

For outputs, I also prefer @dwmw2's approach of using a temporary unsigned char as output, though I again don't think the cast to (unsigned long) is necessary.

Though @arndb is cleaner, and the b suffix on the assembler mnemonics should use the rl register aliases regardless of the q vs r constraint. It would be good to check whether q changes the disassembly for gcc. I suspect it won't but I could be wrong.

Either way, we could start a thread with @arndb , @dwmw2, and Linus, since Linus emailed me about this case recently, and understands the point @dwmw2 makes in his commit message.

@tpimh

tpimh commented Apr 29, 2020

Copy link
Copy Markdown
Contributor Author

Building with no LLD: Build #1397. All should be fine.

Should I rename the ARCH from i386 to x86 to match the new images in boot-utils?

@nickdesaulniers

nickdesaulniers commented Apr 29, 2020

Copy link
Copy Markdown
Member

yes please. Originally, I had the images as i386, but the kernel image gets produced in arch/x86/boot/, so it was simpler to just use x86 everywhere.

Comment thread patches/llvm-all/linux-next Outdated
Comment thread .travis.yml Outdated
@tpimh tpimh force-pushed the i386 branch 3 times, most recently from 05796f6 to 391a214 Compare May 2, 2020 10:11
@nickdesaulniers

nickdesaulniers commented May 3, 2020

Copy link
Copy Markdown
Member

Boot failures on -next look like:

Could not access KVM kernel module: No such file or directory
qemu-system-i386: failed to initialize KVM: No such file or directory

hmmm

Also, patches aren't applying cleanly to mainline.

@nathanchance

Copy link
Copy Markdown
Member

Boot failures on -next look like:


Could not access KVM kernel module: No such file or directory

qemu-system-i386: failed to initialize KVM: No such file or directory

hmmm

Fixed by ClangBuiltLinux/boot-utils#12.

@nickdesaulniers

Copy link
Copy Markdown
Member

I've rekicked the tests.

@tpimh tpimh force-pushed the i386 branch 2 times, most recently from 46aae82 to 7dcdba7 Compare May 4, 2020 20:28
Comment thread driver.sh
Comment thread patches/llvm-all/linux-next/x86/i386-uaccess.patch Outdated
@tpimh tpimh force-pushed the i386 branch 2 times, most recently from 0a02044 to 3e01229 Compare May 5, 2020 04:32
@tpimh tpimh requested a review from nathanchance May 5, 2020 12:29
@tpimh tpimh removed the WIP Work in progress label May 5, 2020

@nathanchance nathanchance left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the one comment, LGTM.

Comment thread driver.sh
@nickdesaulniers nickdesaulniers merged commit 0aceafc into master May 6, 2020
@nickdesaulniers nickdesaulniers deleted the i386 branch May 6, 2020 21:16
@nickdesaulniers

Copy link
Copy Markdown
Member

Nice work @tpimh ! Thanks

@tpimh tpimh mentioned this pull request May 7, 2020
@nickdesaulniers

nickdesaulniers commented May 12, 2020

Copy link
Copy Markdown
Member

I plan to land https://reviews.llvm.org/D79804 which is an improvement to Clang, but will make the patches we're carrying no longer work.

krasCGQ pushed a commit to krasCGQ/linux that referenced this pull request Jun 25, 2020
The top level Makefile disables this warning. When building an
i386_defconfig with Clang, this warning is triggered a whole bunch via
includes of headers from perf.

Link: ClangBuiltLinux/continuous-integration#182
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20200426214215.139435-1-ndesaulniers@google.com
Signed-off-by: Albert I <kras@raphielgang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants