Re: [PATCH v1] s390x/tcg: Fix RISBHG
On Fri, Jan 8, 2021 at 1:45 AM David Hildenbrand wrote:
>
> On 08.01.21 03:20, Nick Desaulniers wrote:
> > On Thu, Jan 7, 2021 at 3:27 PM David Hildenbrand
> > wrote:
> >>
> >>
> >>> Am 08.01.2021 um 00:21 schrieb Nick Desaulniers :
> >>>
> >>> On Thu, Jan 7, 2021 at 3:13 PM David Hildenbrand
> >>> wrote:
>
> RISBHG is broken and currently hinders clang builds of upstream kernels
> from booting: the kernel crashes early, while decompressing the image.
>
> [...]
> Kernel fault: interruption code 0005 ilc:2
> Kernel random base:
> PSW : 20018000 00017a1e
> R:0 T:0 IO:0 EX:0 Key:0 M:0 W:0 P:0 AS:0 CC:2 PM:0 RI:0 EA:3
> GPRS: 0001 000c 0003fff4
> fff0
> fff4 000c
> fff0
> fffc fff8
> 008e25a8
> 0009 0002 0008
> bce0
>
> One example of a buggy instruction is:
>
> 17dde: ec 1e 00 9f 20 5d risbhg %r1,%r14,0,159,32
>
> With %r14 = 0x9 and %r1 = 0x7 should result in %r1 = 0x90007,
> however,
> results in %r1 = 0.
>
> Let's interpret values of i3/i4 as documented in the PoP and make
> computation of "mask" only based on i3 and i4 and use "pmask" only at the
> very end to make sure wrapping is only applied to the high/low
> doubleword.
>
> With this patch, I can successfully boot a v5.10 kernel built with
> clang, and gcc builds keep on working.
>
> Fixes: 2d6a869833d9 ("target-s390: Implement RISBG")
> Reported-by: Nick Desaulniers
> Cc: Guenter Roeck
> Cc: Christian Borntraeger
> Signed-off-by: David Hildenbrand
> ---
>
> This BUG was a nightmare to debug and the code a nightmare to understand.
>
> To make clang/gcc builds boot, the following fix is required as well on
> top of current master: "[PATCH] target/s390x: Fix ALGSI"
> https://lkml.kernel.org/r/20210107202135.52379-1-da...@redhat.com
> >>>
> >>> In that case, a huge thank you!!! for this work! ++beers_owed.
> >>>
> >>
> >> :) a kernel build for z13 should work with the (default) „-cpu qemu“ cpu
> >> type.
> >
> > Hmm...so I don't think clang can build a Linux kernel image with
> > CONFIG_MARCH_Z13=y just yet; just defconfig. Otherwise looks like
> > clang barfs on some of the inline asm constraints.
> >
>
> Ah, right. I overwrote my manual config by a temporary defconfig :)
>
>
> So, I'm on x86-64 F33.
>
> clang version 11.0.0 (Fedora 11.0.0-2.fc33)
> LLVM version 11.0.0
>
> I cannot directly use "LLVM=1" for cross-compilation, as I keep getting
> "error: unknown emulation: elf64_s390" from ld.lld and "error: invalid
> output format: 'elf64-s390'" from llvm-objcopy. I assume that's fixed in
> llvm12?
Right, I suspect that even if ld.lld understood that emulation mode
target, it would still fail due to lack of big endian support. We've
been building with simply `CC=clang` for s390 linux kernels.
Via: https://www.kernel.org/doc/html/latest/kbuild/llvm.html#llvm-utilities
we usually start with `make CC=clang` then work our way up to `make
LLVM=1`. So you shouldn't need the below patching, just use
`CC=clang`.
>
> 1. I patch around it (strange, I remember CC= .. used to work, but it no
> longer does)
>
> ---
>
> index e30cf02da8b8..89c57062ed5d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -427,13 +427,13 @@ KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
> CPP= $(CC) -E
> ifneq ($(LLVM),)
> CC = clang
> -LD = ld.lld
> -AR = llvm-ar
> -NM = llvm-nm
> -OBJCOPY= llvm-objcopy
> -OBJDUMP= llvm-objdump
> -READELF= llvm-readelf
> -STRIP = llvm-strip
> +LD = $(CROSS_COMPILE)ld
> +AR = $(CROSS_COMPILE)ar
> +NM = $(CROSS_COMPILE)nm
> +OBJCOPY= $(CROSS_COMPILE)objcopy
> +OBJDUMP= $(CROSS_COMPILE)objdump
> +READELF= $(CROSS_COMPILE)readelf
> +STRIP = $(CROSS_COMPILE)strip
> else
> CC = $(CROSS_COMPILE)gcc
> LD = $(CROSS_COMPILE)ld
>
> ---
Pulling from your github branch, everything looks good; buildroot
support looks good. I'll wire this up to our CI so that we can help
report regressions!
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2 0/4] s390x/tcg: fix booting Linux kernels compiled with clang-11 and clang-12
On Fri, Jan 8, 2021 at 5:21 AM David Hildenbrand wrote: > > This series fixes booting current upstream Linux kernel compiled by > clang-11 and clang-12 under TCG. > > Decided to pull in already separatly sent patches. The last patch is > not required to fix the boot issues, but related to patch #3. > > Latest version of the patches available at: > g...@github.com:davidhildenbrand/qemu.git clang Hey looks like we're off to the races! $ qemu/build/qemu-system-s390x -M s390-ccw-virtio -display none -initrd /android1/boot-utils/images/s390/rootfs.cpio -kernel /android0/linux-next/arch/s390/boot/bzImage -m 512m -nodefaults -serial mon:stdio ... [0.365077] Linux version 5.11.0-rc2-01914-g16586f130181-dirty (ndesaulni...@ndesaulniers1.mtv.corp.google.com) (Nick Desaulniers clang version 12.0.0 (g...@github.com:llvm/llvm-project.git e75fec2b238f0e26cfb7645f2208baebe3440d41), GNU ld (GNU Binutils for Debian) 2.35.1) #76 SMP Thu Jan 7 17:51:34 PST 2021 ... / # cat /proc/version Linux version 5.11.0-rc2-01914-g16586f130181-dirty (ndesaulni...@ndesaulniers1.mtv.corp.google.com) (Nick Desaulniers clang version 12.0.0 (g...@github.com:llvm/llvm-project.git e75fec2b238f0e26cfb7645f2208baebe3440d41), GNU ld (GNU Binutils for Debian) 2.35.1) #76 SMP Thu Jan 7 17:51:34 PST 2021 / # uname -a Linux (none) 5.11.0-rc2-01914-g16586f130181-dirty #76 SMP Thu Jan 7 17:51:34 PST 2021 s390x GNU/Linux For the series: Tested-by: Nick Desaulniers > > v1 -> v2: > - Add 's390x/tcg: Don't ignore content in r0 when not specified via "b" or > "x"' > - Add 's390x/tcg: Ignore register content if b1/b2 is zero when handling > EXEUTE' > - "s390x/tcg: Fix ALGSI" > -- Fixup subject > - "s390x/tcg: Fix RISBHG" > -- Rephrase description, stating that it fixes clang-11 > > David Hildenbrand (4): > s390x/tcg: Fix ALGSI > s390x/tcg: Fix RISBHG > s390x/tcg: Only ignore content in r0 when specified via "b" or "x" > s390x/tcg: Ignore register content if b1/b2 is zero when handling > EXECUTE > > target/s390x/insn-data.def | 10 +- > target/s390x/mem_helper.c | 4 ++-- > target/s390x/translate.c | 33 + > 3 files changed, 24 insertions(+), 23 deletions(-) > > -- > 2.29.2 > -- Thanks, ~Nick Desaulniers
Re: [PATCH v1] s390x/tcg: Fix RISBHG
On Thu, Jan 7, 2021 at 3:27 PM David Hildenbrand wrote:
>
>
> > Am 08.01.2021 um 00:21 schrieb Nick Desaulniers :
> >
> > On Thu, Jan 7, 2021 at 3:13 PM David Hildenbrand wrote:
> >>
> >> RISBHG is broken and currently hinders clang builds of upstream kernels
> >> from booting: the kernel crashes early, while decompressing the image.
> >>
> >> [...]
> >> Kernel fault: interruption code 0005 ilc:2
> >> Kernel random base:
> >> PSW : 20018000 00017a1e
> >> R:0 T:0 IO:0 EX:0 Key:0 M:0 W:0 P:0 AS:0 CC:2 PM:0 RI:0 EA:3
> >> GPRS: 0001 000c 0003fff4 fff0
> >> fff4 000c fff0
> >> fffc fff8 008e25a8
> >> 0009 0002 0008 bce0
> >>
> >> One example of a buggy instruction is:
> >>
> >>17dde: ec 1e 00 9f 20 5d risbhg %r1,%r14,0,159,32
> >>
> >> With %r14 = 0x9 and %r1 = 0x7 should result in %r1 = 0x90007, however,
> >> results in %r1 = 0.
> >>
> >> Let's interpret values of i3/i4 as documented in the PoP and make
> >> computation of "mask" only based on i3 and i4 and use "pmask" only at the
> >> very end to make sure wrapping is only applied to the high/low doubleword.
> >>
> >> With this patch, I can successfully boot a v5.10 kernel built with
> >> clang, and gcc builds keep on working.
> >>
> >> Fixes: 2d6a869833d9 ("target-s390: Implement RISBG")
> >> Reported-by: Nick Desaulniers
> >> Cc: Guenter Roeck
> >> Cc: Christian Borntraeger
> >> Signed-off-by: David Hildenbrand
> >> ---
> >>
> >> This BUG was a nightmare to debug and the code a nightmare to understand.
> >>
> >> To make clang/gcc builds boot, the following fix is required as well on
> >> top of current master: "[PATCH] target/s390x: Fix ALGSI"
> >> https://lkml.kernel.org/r/20210107202135.52379-1-da...@redhat.com
> >
> > In that case, a huge thank you!!! for this work! ++beers_owed.
> >
>
> :) a kernel build for z13 should work with the (default) „-cpu qemu“ cpu type.
Hmm...so I don't think clang can build a Linux kernel image with
CONFIG_MARCH_Z13=y just yet; just defconfig. Otherwise looks like
clang barfs on some of the inline asm constraints.
It looks like with your patch applied we get further into the boot!
I'm not seeing any output with:
$ /android0/qemu/build/qemu-system-s390x -cpu qemu -append
'conmode=sclp console=ttyS0' -display none -initrd
//boot-utils/images/s390/rootfs.cpio -kernel
arch/s390/boot/bzImage -m 512m -nodefaults -serial mon:stdio
(Based on a quick skim through
https://www.ibm.com/support/knowledgecenter/en/linuxonibm/com.ibm.linux.z.ludd/ludd_r_lmtkernelparameter.html).
Do I have all of those right?
If I attach GDB to QEMU running that kernel image, I was able to view
the print banner once via `lx-dmesg` gdb macro in the kernel, but it
seems on subsequent runs control flow gets diverted unexpected post
entry to start_kernel() always to `s390_base_pgm_handler` ...errr..at
least when I try to single step in GDB. Tried with linux-5.10.y,
mainline, and linux-next.
qemu: 470dd6bd360782f5137f7e3376af6a44658eb1d3 + your patch
llvm: 106e66f3f555c8f887e82c5f04c3e77bdaf345e8
linux-5.10.y: d1988041d19dc8b532579bdbb7c4a978391c0011
linux: 71c061d2443814de15e177489d5cc00a4a253ef3
linux-next: f87684f6470f5f02bd47d4afb900366e5d2f31b6
(gdb) hbreak setup_arch
Hardware assisted breakpoint 1 at 0x142229e: file
arch/s390/kernel/setup.c, line 1091.
(gdb) c
Continuing.
Program received signal SIGTRAP, Trace/breakpoint trap.
0x014222a0 in setup_arch (cmdline_p=0x11d7ed8) at
arch/s390/kernel/setup.c:1091
1091if (MACHINE_IS_VM)
(gdb) lx-dmesg
[0.376351] Linux version 5.11.0-rc2-00157-ga2885c701c30
(ndesaulni...@ndesaulniers1.mtv.corp.google.com) (Nick Desaulniers
clang version 12.0.0 (g...@github.com:llvm/llvm-project.git
e75fec2b238f0e26cfb7645f2208baebe3440d41), GNU ld (GNU Binutils for
Debian) 2.35.1) #81 SMP Thu Jan 7 17:57:34 PST 2021
>
> >>
> >> ---
> >> target/s390x/translate.c | 18 --
> >> 1 file changed, 8 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> >> index 3d5c0d6106..39e33eeb67 100644
> >> --- a/target/s390x/translate.c
> >> +++ b/target/s390x/translate.c
> >> @@ -3815,22 +3815,23 @@ static DisasJumpType op_risbg(DisasContext *s,
> >> DisasOps *o)
> >> pmask = 0xull;
> >> break;
> >> case 0x51: /* risblg */
> >> -i3 &= 31;
> >> -i4 &= 31;
> >> +i3 = (i3 & 31) + 32;
> >> +i4 = (i4 & 31) + 32;
> >> pmask = 0xull;
> >> break;
> >> default:
> >> g_assert_not_reached();
> >> }
> >>
> >> -/* MASK is the set of bits to be inserted from R2.
> >> - Take care for I3/I4 wraparound. */
> >> -mask = pmask >> i3;
> >> +/
Re: [PATCH v1] s390x/tcg: Fix RISBHG
On Thu, Jan 7, 2021 at 3:13 PM David Hildenbrand wrote:
>
> RISBHG is broken and currently hinders clang builds of upstream kernels
> from booting: the kernel crashes early, while decompressing the image.
>
> [...]
>Kernel fault: interruption code 0005 ilc:2
>Kernel random base:
>PSW : 20018000 00017a1e
> R:0 T:0 IO:0 EX:0 Key:0 M:0 W:0 P:0 AS:0 CC:2 PM:0 RI:0 EA:3
>GPRS: 0001 000c 0003fff4 fff0
> fff4 000c fff0
> fffc fff8 008e25a8
> 0009 0002 0008 bce0
>
> One example of a buggy instruction is:
>
> 17dde: ec 1e 00 9f 20 5d risbhg %r1,%r14,0,159,32
>
> With %r14 = 0x9 and %r1 = 0x7 should result in %r1 = 0x90007, however,
> results in %r1 = 0.
>
> Let's interpret values of i3/i4 as documented in the PoP and make
> computation of "mask" only based on i3 and i4 and use "pmask" only at the
> very end to make sure wrapping is only applied to the high/low doubleword.
>
> With this patch, I can successfully boot a v5.10 kernel built with
> clang, and gcc builds keep on working.
>
> Fixes: 2d6a869833d9 ("target-s390: Implement RISBG")
> Reported-by: Nick Desaulniers
> Cc: Guenter Roeck
> Cc: Christian Borntraeger
> Signed-off-by: David Hildenbrand
> ---
>
> This BUG was a nightmare to debug and the code a nightmare to understand.
>
> To make clang/gcc builds boot, the following fix is required as well on
> top of current master: "[PATCH] target/s390x: Fix ALGSI"
> https://lkml.kernel.org/r/20210107202135.52379-1-da...@redhat.com
In that case, a huge thank you!!! for this work! ++beers_owed.
>
> ---
> target/s390x/translate.c | 18 --
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 3d5c0d6106..39e33eeb67 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3815,22 +3815,23 @@ static DisasJumpType op_risbg(DisasContext *s,
> DisasOps *o)
> pmask = 0xull;
> break;
> case 0x51: /* risblg */
> -i3 &= 31;
> -i4 &= 31;
> +i3 = (i3 & 31) + 32;
> +i4 = (i4 & 31) + 32;
> pmask = 0xull;
> break;
> default:
> g_assert_not_reached();
> }
>
> -/* MASK is the set of bits to be inserted from R2.
> - Take care for I3/I4 wraparound. */
> -mask = pmask >> i3;
> +/* MASK is the set of bits to be inserted from R2. */
> if (i3 <= i4) {
> -mask ^= pmask >> i4 >> 1;
> +/* [0...i3---i4...63] */
> +mask = (-1ull >> i3) & (-1ull << (63 - i4));
> } else {
> -mask |= ~(pmask >> i4 >> 1);
> +/* [0---i4...i3---63] */
> +mask = (-1ull >> i3) | (-1ull << (63 - i4));
> }
The expression evaluated looks the same to me for both sides of the
conditional, but the comments differ. Intentional?
> +/* For RISBLG/RISBHG, the wrapping is limited to the high/low
> doubleword. */
> mask &= pmask;
>
> /* IMASK is the set of bits to be kept from R1. In the case of the
> high/low
> @@ -3843,9 +3844,6 @@ static DisasJumpType op_risbg(DisasContext *s, DisasOps
> *o)
> len = i4 - i3 + 1;
> pos = 63 - i4;
> rot = i5 & 63;
> -if (s->fields.op2 == 0x5d) {
> -pos += 32;
> -}
>
> /* In some cases we can implement this with extract. */
> if (imask == 0 && pos == 0 && len > 0 && len <= rot) {
> --
> 2.29.2
>
--
Thanks,
~Nick Desaulniers
