On Fri, Apr 09, 2021 at 11:14:37AM +0200, Sumanth Korikkar wrote:
> Hi All,
>
> I am currently working on enabling kpatch application on our platform s390x.
> I have made few required changes to kpatch-build and it seems to work with
> the upstream kernel.
>
> My current work on s390x:
> - Marking the profiling functions.
> - Addition of s390x specific compile flags.
> - Handling of s390x specific sections.
> - Analyzing the output of various patches and checking for correctness
>
Hi Sumanth,
This sounds great and good to hear that you've been making progress thus
far. With respect to testing, have you tried any of the integration
test patches found in the test/integration/ directory?
Also, feel free to open a pull request on github when you are ready to
share. Most of the code review and discussion happens over there.
>
> I checked out the optimization which is being performed in
> kpatch_line_macro_change_only(). To test this:
> - Added few comments in fs/namei.c, which contains various
> WARN_ON()/BUG_ON() - upstream kernel.
> - Currently shows as "no functional changes found" on s390x, without
> implementing kpatch_line_macro_change_only() function, which seems to be
> correct.
> - Then, I also performed some quick test on x86 by commenting out
> kpatch_line_macro_change_only() function and returning 0.
> - This actually provided the same result "no functional changes found"
> found. But as per my understanding, this should have shown various
> section changes due to change in their line number
>
> I was also searching for pattern which was suggested in commit 6b03bc8e
> ("fix WARN*_ONCE detection on newer kernels") on x86 upstream kernel, but
> could not get these patterns. May be I am missing something.
>
> Could you provide some feedback on these?
Yes, this is definitely one of the compiler and arch-dependent dark
corners of kpatch-build :)
I initially fired up a RHEL-8 install to try and demo the WARN
instruction patterns that kpatch_line_macro_change_only() looks for,
only to reproduce your upstream results. Good news, bad news :D
I crawled around the warn / bug / arch-specific code for a bit and I
think this is the upstream commit that explains why we didn't see the
patterns in x86 upstream / RHEL-8:
commit 9a93848fe787a53aec404e4e00d8f7f9bbdaebb4
Author: Peter Zijlstra <[email protected]>
Date: Thu Feb 2 14:43:51 2017 +0100
x86/debug: Implement __WARN() using UD0
By using "UD0" for WARN()s we remove the function call and its possible
__FILE__ and __LINE__ immediate arguments from the instruction stream.
Total image size will not change much, what we win in the instruction
stream we'll lose because of the __bug_table entries. Still, saves on
I$ footprint and the total image size does go down a bit.
text data filename
10702123 4530992 defconfig-build/vmlinux.orig
10682460 4530992 defconfig-build/vmlinux.patched
(UML didn't seem to use GENERIC_BUG at all, so remove it)
In my RHEL-8 example, I built a module with code:
static int __init mymodule_init (void)
{
WARN(1, "test");
return 0;
}
and look how the file/line is saved in the __bug_table section:
macro-change.o: file format elf64-x86-64
Disassembly of section .init.text:
0000000000000000 <init_module>:
0: e8 00 00 00 00 callq 5 <init_module+0x5>
1: R_X86_64_PLT32 __fentry__-0x4
5: 48 c7 c7 00 00 00 00 mov $0x0,%rdi
8: R_X86_64_32S .rodata.str1.1
c: e8 00 00 00 00 callq 11 <init_module+0x11>
d: R_X86_64_PLT32 __warn_printk-0x4
11: 0f 0b ud2
13: 31 c0 xor %eax,%eax
15: c3 retq
Disassembly of section __bug_table:
0000000000000000 <__bug_table>:
...
0: R_X86_64_PC32 .init.text+0x11
4: R_X86_64_PC32 .LC1+0x4
8: 06 (bad)
9: 00 01 add %al,(%rcx)
b: 09 .byte 0x9
versus what I saw when sanity checking on RHEL-7:
macro-change.o: file format elf64-x86-64
Disassembly of section .init.text:
0000000000000000 <init_module>:
0: 55 push %rbp
1: 31 c0 xor %eax,%eax
3: 48 c7 c2 00 00 00 00 mov $0x0,%rdx
6: R_X86_64_32S .rodata.str1.1
a: be 06 00 00 00 mov $0x6,%esi
f: 48 c7 c7 00 00 00 00 mov $0x0,%rdi
12: R_X86_64_32S .rodata.str1.8
16: 48 89 e5 mov %rsp,%rbp
19: e8 00 00 00 00 callq 1e <init_module+0x1e>
1a: R_X86_64_PC32 warn_slowpath_fmt-0x4
1e: 31 c0 xor %eax,%eax
20: 5d pop %rbp
21: c3 retq
For s390x purposes, I think providing a kpatch_line_macro_change_only()
implementation depends on what the compiler is going to generate for
supported kernel releases and configs.
In upstream (and rhel-7,8) arch/s390/include/asm/bug.h, it looks like
when CONFIG_DEBUG_BUGVERBOSE is set, __EMIT_BUG() will include the
file/line combo in the __bug_table, like x86 does now. So it might be
the case that s390x won't need kpatch_line_macro_change_only() as long
as the __bug_table changes are handled accordingly.
Hope this helps,
-- Joe
_______________________________________________
kpatch mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/kpatch
