Hi Joe, Josh

On 4/12/21 6:30 PM, Joe Lawrence wrote:
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.

Ok. I will create a pull request soon. Currently checking the integration tests results.


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.

Thank you for the detailed analysis of this usecase.

Best Regards

Sumanth


Hope this helps,

-- Joe

_______________________________________________
kpatch mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/kpatch

--
Sumanth Korikkar


_______________________________________________
kpatch mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/kpatch

Reply via email to