diff mbox series

riscv: relocate R_RISCV_CALL_PLT in kexec_file

Message ID 20230310182726.GA25154@lst.de (mailing list archive)
State Changes Requested
Headers show
Series riscv: relocate R_RISCV_CALL_PLT in kexec_file | expand

Checks

Context Check Description
conchuod/cover_letter success Single patches do not need cover letters
conchuod/tree_selection success Guessed tree name to be for-next
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 1 and now 1
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/build_rv64_clang_allmodconfig success Errors and warnings before: 21 this patch: 21
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig success Errors and warnings before: 37 this patch: 37
conchuod/alphanumeric_selects success Out of order selects before the patch: 728 and now 728
conchuod/build_rv32_defconfig success Build OK
conchuod/dtb_warn_rv64 success Errors and warnings before: 3 this patch: 3
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch warning WARNING: From:/Signed-off-by: email address mismatch: 'From: Torsten Duwe <duwe@lst.de>' != 'Signed-off-by: Torsten Duwe <duwe@suse.de>' WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: 838b3e28488f ("RISC-V: Load purgatory in kexec_file")'
conchuod/source_inline success Was 0 now: 0
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes fail Problems with Fixes tag: 1
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Torsten Duwe March 10, 2023, 6:27 p.m. UTC 
Depending on the toolchain (here: gcc-12, binutils-2.40) the
relocation entries for function calls are no longer R_RISCV_CALL, but
R_RISCV_CALL_PLT. When trying kexec_load_file on such kernels, it will
fail with

 kexec_image: Unknown rela relocation: 19
 kexec_image: Error loading purgatory ret=-8

The binary code at the call site remains the same, so tell
arch_kexec_apply_relocations_add() to handle _PLT alike.

fixes: 838b3e28488f702 ("Load purgatory in kexec_file")
Signed-off-by: Torsten Duwe <duwe@suse.de>
Cc: stable@vger.kernel.org

---

Comments

Li Zhengyu March 13, 2023, 3:13 a.m. UTC | #1 
On Fri, 10 Mar 2023 19:27:03 +0100, Torsten Duwe <duwe@lst.de> wrote:
> Depending on the toolchain (here: gcc-12, binutils-2.40) the
> relocation entries for function calls are no longer R_RISCV_CALL, but
> R_RISCV_CALL_PLT. When trying kexec_load_file on such kernels, it will
> fail with
>
>   kexec_image: Unknown rela relocation: 19
>   kexec_image: Error loading purgatory ret=-8
>
> The binary code at the call site remains the same, so tell
> arch_kexec_apply_relocations_add() to handle _PLT alike.

R_RISCV_CALL has already been deprecated, and replaced by R_RISCV_CALL_PLT.

See Enum 18-19 in Table 3. Relocation types from 
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc 
.

It was deprecated in ("Deprecated R_RISCV_CALL, prefer 
R_RISCV_CALL_PLT") 
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/a0dced85018d7a0ec17023c9389cbd70b1dbc1b0

>
> fixes: 838b3e28488f702 ("Load purgatory in kexec_file")
> Signed-off-by: Torsten Duwe <duwe@suse.de>
> Cc: stable@vger.kernel.org
>
> ---
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -425,6 +425,7 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
>   		 * sym, instead of searching the whole relsec.
>   		 */
>   		case R_RISCV_PCREL_HI20:
> +		case R_RISCV_CALL_PLT:
>   		case R_RISCV_CALL:
>   			*(u64 *)loc = CLEAN_IMM(UITYPE, *(u64 *)loc) |
>   				 ENCODE_UJTYPE_IMM(val - addr);
>
> .

Palmer, please apply these references to the commit message.

Reviewed-by: Li Zhengyu <lizhengyu3@huawei.com>
Torsten Duwe March 21, 2023, 3:03 p.m. UTC | #2 
On Mon, 13 Mar 2023 11:13:17 +0800
Li Zhengyu <lizhengyu3@huawei.com> wrote:

> On Fri, 10 Mar 2023 19:27:03 +0100, Torsten Duwe <duwe@lst.de> wrote:
> > Depending on the toolchain (here: gcc-12, binutils-2.40) the
> > relocation entries for function calls are no longer R_RISCV_CALL, but
> > R_RISCV_CALL_PLT. When trying kexec_load_file on such kernels, it will
> > fail with
> >
> >   kexec_image: Unknown rela relocation: 19
> >   kexec_image: Error loading purgatory ret=-8
> >
> > The binary code at the call site remains the same, so tell
> > arch_kexec_apply_relocations_add() to handle _PLT alike.
> 
> R_RISCV_CALL has already been deprecated, and replaced by R_RISCV_CALL_PLT.
> 
> See Enum 18-19 in Table 3. Relocation types from 
> https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc 
> .
> 
> It was deprecated in ("Deprecated R_RISCV_CALL, prefer 
> R_RISCV_CALL_PLT") 
> https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/a0dced85018d7a0ec17023c9389cbd70b1dbc1b0
> 
> >
> > fixes: 838b3e28488f702 ("Load purgatory in kexec_file")
> > Signed-off-by: Torsten Duwe <duwe@suse.de>
> > Cc: stable@vger.kernel.org
> >
> > ---
> > --- a/arch/riscv/kernel/elf_kexec.c
> > +++ b/arch/riscv/kernel/elf_kexec.c
> > @@ -425,6 +425,7 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> >   		 * sym, instead of searching the whole relsec.
> >   		 */
> >   		case R_RISCV_PCREL_HI20:
> > +		case R_RISCV_CALL_PLT:
> >   		case R_RISCV_CALL:
> >   			*(u64 *)loc = CLEAN_IMM(UITYPE, *(u64 *)loc) |
> >   				 ENCODE_UJTYPE_IMM(val - addr);
> >
> > .
> 
> Palmer, please apply these references to the commit message.
> 
> Reviewed-by: Li Zhengyu <lizhengyu3@huawei.com>
> 

Ping?
Conor Dooley March 21, 2023, 3:35 p.m. UTC | #3 
On Tue, Mar 21, 2023 at 04:03:49PM +0100, Torsten Duwe wrote:
> On Mon, 13 Mar 2023 11:13:17 +0800
> Li Zhengyu <lizhengyu3@huawei.com> wrote:
> 
> > On Fri, 10 Mar 2023 19:27:03 +0100, Torsten Duwe <duwe@lst.de> wrote:
> > > Depending on the toolchain (here: gcc-12, binutils-2.40) the
> > > relocation entries for function calls are no longer R_RISCV_CALL, but
> > > R_RISCV_CALL_PLT. When trying kexec_load_file on such kernels, it will
> > > fail with
> > >
> > >   kexec_image: Unknown rela relocation: 19
> > >   kexec_image: Error loading purgatory ret=-8
> > >
> > > The binary code at the call site remains the same, so tell
> > > arch_kexec_apply_relocations_add() to handle _PLT alike.
> > 
> > R_RISCV_CALL has already been deprecated, and replaced by R_RISCV_CALL_PLT.
> > 
> > See Enum 18-19 in Table 3. Relocation types from 
> > https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc 
> > .
> > 
> > It was deprecated in ("Deprecated R_RISCV_CALL, prefer 
> > R_RISCV_CALL_PLT") 
> > https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/a0dced85018d7a0ec17023c9389cbd70b1dbc1b0
> > 
> > >
> > > fixes: 838b3e28488f702 ("Load purgatory in kexec_file")
> > > Signed-off-by: Torsten Duwe <duwe@suse.de>
> > > Cc: stable@vger.kernel.org
> > >
> > > ---
> > > --- a/arch/riscv/kernel/elf_kexec.c
> > > +++ b/arch/riscv/kernel/elf_kexec.c
> > > @@ -425,6 +425,7 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> > >   		 * sym, instead of searching the whole relsec.
> > >   		 */
> > >   		case R_RISCV_PCREL_HI20:
> > > +		case R_RISCV_CALL_PLT:
> > >   		case R_RISCV_CALL:
> > >   			*(u64 *)loc = CLEAN_IMM(UITYPE, *(u64 *)loc) |
> > >   				 ENCODE_UJTYPE_IMM(val - addr);
> > >
> > > .
> > 
> > Palmer, please apply these references to the commit message.
> > 
> > Reviewed-by: Li Zhengyu <lizhengyu3@huawei.com>
> > 
> 
> Ping?

It's not been all that longer than a week & you're in patchwork so you
won't be forgotten, but I noticed a complaint when I went looking on
patchwork about your fixes tag:

Commit: f28b81e30b4b ("riscv: relocate R_RISCV_CALL_PLT in kexec_file")
	Fixes tag: fixes: 838b3e28488f702 ("Load purgatory in kexec_file")
	Has these problem(s):
		- Subject does not match target commit subject
		  Just use
			git log -1 --format='Fixes: %h ("%s")'

The fixes tag should be:
Fixes: 838b3e28488f ("RISC-V: Load purgatory in kexec_file")

Note the capital F & the missed RISC-V: prefix, checkpatch should have
complained about this.
Checkpatch also complains that your suse email in the Signoff doesn't
match the lst.de email that you used to send the patch (IOW you're
missing a From: header that send-email would add).

Could you fix those things up please & I suppose you can take the
opportunity to make the changes that Li Zhengyu suggested to the commit
message itself at the same time.

Thanks,
Conor.
Conor Dooley July 23, 2023, 10:27 a.m. UTC | #4 
Hey Torsten,

On Tue, Mar 21, 2023 at 03:35:38PM +0000, Conor Dooley wrote:
> On Tue, Mar 21, 2023 at 04:03:49PM +0100, Torsten Duwe wrote:
> > On Mon, 13 Mar 2023 11:13:17 +0800
> > Li Zhengyu <lizhengyu3@huawei.com> wrote:
> > 
> > > On Fri, 10 Mar 2023 19:27:03 +0100, Torsten Duwe <duwe@lst.de> wrote:
> > > > Depending on the toolchain (here: gcc-12, binutils-2.40) the
> > > > relocation entries for function calls are no longer R_RISCV_CALL, but
> > > > R_RISCV_CALL_PLT. When trying kexec_load_file on such kernels, it will
> > > > fail with
> > > >
> > > >   kexec_image: Unknown rela relocation: 19
> > > >   kexec_image: Error loading purgatory ret=-8
> > > >
> > > > The binary code at the call site remains the same, so tell
> > > > arch_kexec_apply_relocations_add() to handle _PLT alike.
> > > 
> > > R_RISCV_CALL has already been deprecated, and replaced by R_RISCV_CALL_PLT.
> > > 
> > > See Enum 18-19 in Table 3. Relocation types from 
> > > https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc 
> > > .
> > > 
> > > It was deprecated in ("Deprecated R_RISCV_CALL, prefer 
> > > R_RISCV_CALL_PLT") 
> > > https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/a0dced85018d7a0ec17023c9389cbd70b1dbc1b0
> > > 
> > > >
> > > > fixes: 838b3e28488f702 ("Load purgatory in kexec_file")
> > > > Signed-off-by: Torsten Duwe <duwe@suse.de>
> > > > Cc: stable@vger.kernel.org
> > > >
> > > > ---
> > > > --- a/arch/riscv/kernel/elf_kexec.c
> > > > +++ b/arch/riscv/kernel/elf_kexec.c
> > > > @@ -425,6 +425,7 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> > > >   		 * sym, instead of searching the whole relsec.
> > > >   		 */
> > > >   		case R_RISCV_PCREL_HI20:
> > > > +		case R_RISCV_CALL_PLT:
> > > >   		case R_RISCV_CALL:
> > > >   			*(u64 *)loc = CLEAN_IMM(UITYPE, *(u64 *)loc) |
> > > >   				 ENCODE_UJTYPE_IMM(val - addr);
> > > >
> > > > .
> > > 
> > > Palmer, please apply these references to the commit message.
> > > 
> > > Reviewed-by: Li Zhengyu <lizhengyu3@huawei.com>
> > > 
> > 
> > Ping?
> 
> It's not been all that longer than a week & you're in patchwork so you
> won't be forgotten,

Turns out it did get forgotten, or more accurately, marked as "Changes
Requested".

> but I noticed a complaint when I went looking on
> patchwork about your fixes tag:
> 
> Commit: f28b81e30b4b ("riscv: relocate R_RISCV_CALL_PLT in kexec_file")
> 	Fixes tag: fixes: 838b3e28488f702 ("Load purgatory in kexec_file")
> 	Has these problem(s):
> 		- Subject does not match target commit subject
> 		  Just use
> 			git log -1 --format='Fixes: %h ("%s")'
> 
> The fixes tag should be:
> Fixes: 838b3e28488f ("RISC-V: Load purgatory in kexec_file")
> 
> Note the capital F & the missed RISC-V: prefix, checkpatch should have
> complained about this.
> Checkpatch also complains that your suse email in the Signoff doesn't
> match the lst.de email that you used to send the patch (IOW you're
> missing a From: header that send-email would add).
> 
> Could you fix those things up please & I suppose you can take the
> opportunity to make the changes that Li Zhengyu suggested to the commit
> message itself at the same time.

Would you mind resending this with the Fixes & Signoff corrections?

Thanks,
Conor.
diff mbox series

Patch

--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -425,6 +425,7 @@  int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
 		 * sym, instead of searching the whole relsec.
 		 */
 		case R_RISCV_PCREL_HI20:
+		case R_RISCV_CALL_PLT:
 		case R_RISCV_CALL:
 			*(u64 *)loc = CLEAN_IMM(UITYPE, *(u64 *)loc) |
 				 ENCODE_UJTYPE_IMM(val - addr);