I haven't investigated fully yet.
But I did take a quick look at the paes_s390.ko.debug file.
A few quick comments as early feedback.

Note that it doesn't contain .debug_frame but only a .eh_frame section.
Other architectures force .debug_frame for kernel modules to store the CFI.

The .eh_frame is a loaded section, which means it won't go into the .debug
file. So you would need to load the CFI from the main ELF .ko.

One tricky issue here is the fact that kernel modules are ET_REL files
that still need relocations applied (you can see there is also a .rela.eh_frame section). libdwfl can do simple relocations, but might not know about all relocation types used in an .eh_frame section (I haven't checked yet).

(In reply to Mark Wielaard from comment #1)
> I haven't investigated fully yet.
> But I did take a quick look at the paes_s390.ko.debug file.
> A few quick comments as early feedback.
> 
> Note that it doesn't contain .debug_frame but only a .eh_frame section.
> Other architectures force .debug_frame for kernel modules to store the CFI.
> 
> The .eh_frame is a loaded section, which means it won't go into the .debug
> file. So you would need to load the CFI from the main ELF .ko.

Initially, I have tried on my own kernel build (including debug_info) and ran the test program against the .ko file.  (Because I am not permitted to share binary code and, please, do not ask why, it is difficult to provide you an alternative.  If you have access to a s390 or use a cross-compiler it would be fine but that typically requires some effort on your side.

In any case, if you require some output in text form, please let me know.

> One tricky issue here is the fact that kernel modules are ET_REL files
> that still need relocations applied (you can see there is also a
> .rela.eh_frame section). libdwfl can do simple relocations, but might not
> know about all relocation types used in an .eh_frame section (I haven't
> checked yet).

Ok.

(In reply to Mark Wielaard from comment #1)

> One tricky issue here is the fact that kernel modules are ET_REL files
> that still need relocations applied (you can see there is also a
> .rela.eh_frame section). libdwfl can do simple relocations, but might not
> know about all relocation types used in an .eh_frame section (I haven't
> checked yet).

At least, I checked the reloc types for s390 (backends/s390_reloc.def) and the list is complete wrt kernel modules.  Not sure if there is further work to be done for non-simple relocations.

(In reply to H. Brueckner from comment #2)
> (In reply to Mark Wielaard from comment #1)
> > The .eh_frame is a loaded section, which means it won't go into the .debug
> > file. So you would need to load the CFI from the main ELF .ko.
> 
> Initially, I have tried on my own kernel build (including debug_info) and
> ran the test program against the .ko file.  (Because I am not permitted to
> share binary code and, please, do not ask why, it is difficult to provide
> you an alternative.  If you have access to a s390 or use a cross-compiler it
> would be fine but that typically requires some effort on your side.

Don't worry, we have the split .ko/.debug files, it should in theory also work (or show the same bug) with that.

> In any case, if you require some output in text form, please let me know.
> 
> > One tricky issue here is the fact that kernel modules are ET_REL files
> > that still need relocations applied (you can see there is also a
> > .rela.eh_frame section). libdwfl can do simple relocations, but might not
> > know about all relocation types used in an .eh_frame section (I haven't
> > checked yet).
> 
> Ok.

So, if you could provide the output of eu-readelf --relocs paes_s390.ko (on your combined paes_s390.ko containing both code and debug) that would be helpful.

If there are relocations in the .rela.eh_frame section that aren't "simple" as defined in s390_reloc_simple_type (backends/backends/s390_symbol.c) that might be a hint.

Also if you could trick you kernel build to generate CFI in .debug_frame instead of .eh_frame like other arches seem to do, that would make a good testcase.

(In reply to Mark Wielaard from comment #4)
> So, if you could provide the output of eu-readelf --relocs paes_s390.ko (on
> your combined paes_s390.ko containing both code and debug) that would be
> helpful.

eu-readelf --debug-dump=frames --relocs paes_s390.ko

would be even better.

Created attachment 10613 [details]
paes_s390_ko-relocs-eu-readelf.txt

using eu-readelf

Created attachment 10614 [details]
paes_s390_ko-relocs-readelf.txt

and what binutils readelfs reports for comparision

(In reply to Mark Wielaard from comment #5)
> (In reply to Mark Wielaard from comment #4)
> > So, if you could provide the output of eu-readelf --relocs paes_s390.ko (on
> > your combined paes_s390.ko containing both code and debug) that would be
> > helpful.
> 
> eu-readelf --debug-dump=frames --relocs paes_s390.ko
> 
> would be even better.

Just attached the output; also the output of the binutils readelf command for comparision. I guess that there are some more issues because lots of invalid relocations are listed.

(In reply to H. Brueckner from comment #8)
> (In reply to Mark Wielaard from comment #5)
> > (In reply to Mark Wielaard from comment #4)
> > > So, if you could provide the output of eu-readelf --relocs paes_s390.ko (on
> > > your combined paes_s390.ko containing both code and debug) that would be
> > > helpful.
> > 
> > eu-readelf --debug-dump=frames --relocs paes_s390.ko
> > 
> > would be even better.
> 
> Just attached the output; also the output of the binutils readelf command
> for comparision. I guess that there are some more issues because lots of
> invalid relocations are listed.

Is that eu-readelf from a distro install? Then there is something wrong with that install. If it is a build from source then make sure LD_LIBRARY_PATH contains the backends directory that has libebl_s390.so. Otherwise eu-readelf won't know about machine specific ELF/DWARF mappings (like the relocation types/names).

But the output does confirm what I feared. The R_390_PC32 .rela.eh_frame relocations are not "simple". They are load based.

It might not be too hard to add support for those. But if there is any possibility to generate .debug_frame based CFI for the s390x kernel modules that would IMHO a much better/simpler solution because it makes things more in line with other arches. It should also make the kernel modules smaller. Does anything actually use the loaded .eh_frame CFI on s390x?

(In reply to Mark Wielaard from comment #9)
> (In reply to H. Brueckner from comment #8)
> > (In reply to Mark Wielaard from comment #5)
> > > (In reply to Mark Wielaard from comment #4)
> > > > So, if you could provide the output of eu-readelf --relocs paes_s390.ko (on
> > > > your combined paes_s390.ko containing both code and debug) that would be
> > > > helpful.
> > > 
> > > eu-readelf --debug-dump=frames --relocs paes_s390.ko
> > > 
> > 
> > Just attached the output; also the output of the binutils readelf command
> > for comparision. I guess that there are some more issues because lots of
> > invalid relocations are listed.
> 
> Is that eu-readelf from a distro install? Then there is something wrong with
> that install. If it is a build from source then make sure LD_LIBRARY_PATH
> contains the backends directory that has libebl_s390.so. Otherwise
> eu-readelf won't know about machine specific ELF/DWARF mappings (like the
> relocation types/names).

It was a local built and install, the LD_LIBRARY_PATH was not set properly.
However, setting is correctly, the relocation are displayed like with readelf.
Will update the attachment soon.

> 
> But the output does confirm what I feared. The R_390_PC32 .rela.eh_frame
> relocations are not "simple". They are load based.

So now it comes to that point I have feared, as it looks like the libdw just
performs "simple" relocations.  Your comment and the ebl_reloc_simple_type()
call in  libdwfl/relocate.c proofs that. Anyhow, do you plan to extend the
relocation code to support more complex relocations tool?

> It might not be too hard to add support for those.

What needs to be done / is necessary for that?


> But if there is any
> possibility to generate .debug_frame based CFI for the s390x kernel modules
> that would IMHO a much better/simpler solution because it makes things more
> in line with other arches. It should also make the kernel modules smaller.
> Does anything actually use the loaded .eh_frame CFI on s390x?

I will check the module usage on .eh_frame and will have a try on including the
CFI in the .debug_frame.

Thanks.

x86 enables generation of .debug_frame by using:

KBUILD_CFLAGS += -fno-asynchronous-unwind-tables

in the kernel Makefile.  This makes GCC to add ".cfi_sections .debug_frame" to the assembler output which in turn triggers gas to generate the section.

I don't see a reason why we couldn't do the same for s390. The .eh_frame section aren't used in the kernel.

(In reply to H. Brueckner from comment #10)
> (In reply to Mark Wielaard from comment #9)
> > But the output does confirm what I feared. The R_390_PC32 .rela.eh_frame
> > relocations are not "simple". They are load based.
> 
> So now it comes to that point I have feared, as it looks like the libdw just
> performs "simple" relocations.  Your comment and the ebl_reloc_simple_type()
> call in  libdwfl/relocate.c proofs that. Anyhow, do you plan to extend the
> relocation code to support more complex relocations tool?
> 
> > It might not be too hard to add support for those.
> 
> What needs to be done / is necessary for that?

We would need to extend ebl_reloc_simple_type, or add a new ebl backend hook, that signals a relocation uses the load address. Then in libdwfl/relocate.c use that to adjust the value to include the load address that libdwfl assigns to the section (given by struct dwfl_relocation start). This is similar to what arch/s390/kernel/module.c in the kernel does, but slightly more complicated because we want it to work "offline" cross architecture.

This would be helpful in general for any ET_REL file for which we want to inspect the .eh_frame section CFI data. But since this is just for the kernel modules at the moment I would wait with implementing it till you have experimented with adjusting the kernel build like suggested in comment #11.

The .eh_frame section in an .o file uses PC-relative relocations in order to get its offset to the code determined during final link step. It probably does this to avoid runtime relocations which would be necessary for an absolute relocation. .eh_frame is read-only in the mapping, so runtime relocs are not supposed to occur after final link step.
Since in the kernel module scenario libdw has to deal with the .eh_frame section from the .o file it encounters these PC-relative relocs which it cannot handle. I agree with Mark that libdw does not really need to support these relocs since they only occur in that weird scenario which can be avoided using .debug_frame instead.

.debug_frame does not have a location in the address space so a PC-relative relocation would be pointless. Instead it is supposed to be read by tools where applying absolute relocs is no problem.

Hi Mark, Andreas,

(In reply to Andreas Krebbel from comment #11)
> x86 enables generation of .debug_frame by using:
> 
> KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> 
> in the kernel Makefile.  This makes GCC to add ".cfi_sections .debug_frame"
> to the assembler output which in turn triggers gas to generate the section.
> 
> I don't see a reason why we couldn't do the same for s390. The .eh_frame
> section aren't used in the kernel.

Thanks for this direction: Emitting the CFI data in the .debug_frame work nicely. With that change, perf probe and my test program works.  For reference:

./ko-func-call-frame-cfa xts_paes_crypt ~/git/linux/arch/s390/crypto/paes_s390.ko 
I: Looking up xts_paes_crypt@/root/git/linux/arch/s390/crypto/paes_s390.ko
DIE: name=xts_paes_crypt tag=subprogram offset=53957 (0xd2c5)
        low   pc: 10ca8
        high  pc: 10e78
        entry pc: 10ca8
I: Frame Base: Number of locations: 1 (len=1)
I: Frame Base: Location operation 0: call_frame_cfa
E: Could not obtain CFI (eh): no DWARF information
I: CFA operation: 0: bregx

clearly shows a successful call to dwarf_frame_cfa() obtaining the CFI from the .debug_frame (and not longe from the .eh_frame).

Having explicit ebl hooks would still be nice but I guess will not be the top-priority item in the TODO list.  I pursue to integrate my change for s390 in the upstream kernel.

Thanks!

(In reply to H. Brueckner from comment #14)
> Having explicit ebl hooks would still be nice but I guess will not be the
> top-priority item in the TODO list.  I pursue to integrate my change for
> s390 in the upstream kernel.

I have retitled this bug and will keep it open in case someone has time for implementing the ebl hooks for relative load address relocations.

Did you manage to get a kernel build change upstream?
It would be nice to point people at it in this bug in case they hit the same issue. So they know how to rebuild their kernel.

(In reply to Mark Wielaard from comment #15)
> Did you manage to get a kernel build change upstream?
> It would be nice to point people at it in this bug in case they hit the same
> issue. So they know how to rebuild their kernel.

commit bc3703f21cec8a2ac6a64f6fb3686fbcb1ba1513
Author: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Date:   Mon Nov 20 11:41:07 2017 +0100

    s390/kernel: emit CFI data in .debug_frame and discard .eh_frame sections
    
    Using perf probe and libdw on kernel modules failed to find CFI
    data for symbols.  The CFI data is stored in the .eh_frame section.
    The elfutils libdw is not able to extract the CFI data correctly,
    because the .eh_frame section requires "non-simple" relocations
    for kernel modules.
    
    The suggestion is to avoid these "non-simple" relocations by emitting
    the CFI data in the .debug_frame section.  Let gcc emit respective
    directives by specifying the -fno-asynchronous-unwind-tables option.
    
    Using the .debug_frame section for CFI data, the .eh_frame section
    becomes unused and, thus, discard it for kernel and modules builds
    
    The vDSO requires the .eh_frame section and, hence, emit the CFI data
    in both, the .eh_frame and .debug_frame sections.
    
    See also discussion on elfutils/libdw bugzilla:
    https://sourceware.org/bugzilla/show_bug.cgi?id=22452
    
    Suggested-by: Mark Wielaard <mark@klomp.org>
    Signed-off-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
    Reviewed-by: Heiko Carstens <heiko.carstens@de.ibm.com>
    Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

> I have retitled this bug and will keep it open in case someone has
> time for implementing the ebl hooks for relative load address relocations.

I think the above kernel fix is a better. And there really is not much benefit to have relative load address relocations for .eh_frame except for these ET_REL .ko files. Also, nobody seems to have time. So lets close this for now.