Skip to contentSkip to conversation

firmware: Fix fw_rw_offset computation in fw_base.S

PATCH New Anup Patel 3 participants 4 messages Jan 26, 2023 – Jan 27, 2023

lore
Share & tools
lore

Anup Patel

author
#0 lore raw

PATCH 1/1 firmware: Fix fw_rw_offset computation in fw_base.S

From: Jessica Clarke <[email protected]>

It seems BFD just does totally nonsensical things for SHN_ABS symbols when producing position-independent outputs (both -pie and -shared) for various historical reasons, and so SHN_ABS symbols are still subject to relocation as far as BFD is concerned (except AArch64, which fixes it in limited cases that don?t apply here...).

The above affects the _fw_rw_offset provided through fw_base.ldS linker script which results in OpenSBI firmware failing to boot whne loaded at an address different from FW_TEXT_START.

Fixes c10e3fe5f9a1 ("firmware: Add RW section offset in scratch")
Signed-off-by Jessica Clarke <[email protected]>
Tested-by Anup Patel <[email protected]>
Signed-off-by Anup Patel <[email protected]>
---
 firmware/fw_base.S   | 2 +-
 firmware/fw_base.ldS | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
firmware/fw_base.S +1 -1
index e04b683..fb6ac92 100644
@@ -526,7 +526,7 @@ _link_start:
 _link_end:
 	RISCV_PTR	_fw_reloc_end
 __fw_rw_offset:
-	RISCV_PTR	_fw_rw_offset
+	RISCV_PTR	_fw_rw_start - _fw_start
 
 	.section .entry, "ax", %progbits
 	.align 3
firmware/fw_base.ldS +1 -1
index 9a1304e..3d68484 100644
@@ -64,7 +64,7 @@
 	. = ALIGN(1 << LOG2CEIL((SIZEOF(.rodata) + SIZEOF(.text)
 				+ SIZEOF(.dynsym) + SIZEOF(.rela.dyn))));
 
-	PROVIDE(_fw_rw_offset = (.  - _fw_start));
+	PROVIDE(_fw_rw_start = .);
 
 	/* Beginning of the read-write data sections */
Signature · 1 line
2.34.1
author
#1 lore raw

PATCH 1/1 firmware: Fix fw_rw_offset computation in fw_base.S

Hi Jessica,

Quoted 9 lines · Anup Patel

From: Jessica Clarke <[email protected]>

It seems BFD just does totally nonsensical things for SHN_ABS symbols when producing position-independent outputs (both -pie and -shared) for various historical reasons, and so SHN_ABS symbols are still subject to relocation as far as BFD is concerned (except AArch64, which fixes it in limited cases that don?t apply here...).

Quoted 8 lines

The above affects the _fw_rw_offset provided through fw_base.ldS linker script which results in OpenSBI firmware failing to boot whne loaded at an address different from FW_TEXT_START.

Fixes c10e3fe5f9a1 ("firmware: Add RW section offset in scratch")
Signed-off-by Jessica Clarke <[email protected]>
Tested-by Anup Patel <[email protected]>
Signed-off-by Anup Patel <[email protected]>

Let me know if you are okay with the above commit description ?

Regards,
Anup

Quoted 34 lines
---
 firmware/fw_base.S   | 2 +-
 firmware/fw_base.ldS | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
firmware/fw_base.S +1 -1
index e04b683..fb6ac92 100644
@@ -526,7 +526,7 @@ _link_start:
 _link_end:
        RISCV_PTR       _fw_reloc_end
 __fw_rw_offset:
-       RISCV_PTR       _fw_rw_offset
+       RISCV_PTR       _fw_rw_start - _fw_start
 
        .section .entry, "ax", %progbits
        .align 3
firmware/fw_base.ldS +1 -2
index 9a1304e..3d68484 100644
@@ -64,7 +64,7 @@
        . = ALIGN(1 << LOG2CEIL((SIZEOF(.rodata) + SIZEOF(.text)
                                + SIZEOF(.dynsym) + SIZEOF(.rela.dyn))));
 
-       PROVIDE(_fw_rw_offset = (.  - _fw_start));
+       PROVIDE(_fw_rw_start = .);
 
        /* Beginning of the read-write data sections */
 
--

2.34.1

#2 lore raw

PATCH 1/1 firmware: Fix fw_rw_offset computation in fw_base.S

In reply to Anup Patel
Quoted 9 lines · Anup Patel

From: Jessica Clarke <[email protected]>

It seems BFD just does totally nonsensical things for SHN_ABS symbols when producing position-independent outputs (both -pie and -shared) for various historical reasons, and so SHN_ABS symbols are still subject to relocation as far as BFD is concerned (except AArch64, which fixes it in limited cases that don?t apply here...).

Quoted 4 lines

The above affects the _fw_rw_offset provided through fw_base.ldS linker script which results in OpenSBI firmware failing to boot whne loaded at an address different from FW_TEXT_START.

s/whne/when

Quoted 9 lines
Fixes c10e3fe5f9a1 ("firmware: Add RW section offset in scratch")
Signed-off-by Jessica Clarke <[email protected]>
Tested-by Anup Patel <[email protected]>
Signed-off-by Anup Patel <[email protected]>
---
 firmware/fw_base.S   | 2 +-
 firmware/fw_base.ldS | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
Reported-by Lad Prabhakar <[email protected]>
Tested-by Lad Prabhakar <[email protected]>

Cheers,
Prabhakar

Quoted 34 lines
firmware/fw_base.S +1 -1
index e04b683..fb6ac92 100644
@@ -526,7 +526,7 @@ _link_start:
 _link_end:
        RISCV_PTR       _fw_reloc_end
 __fw_rw_offset:
-       RISCV_PTR       _fw_rw_offset
+       RISCV_PTR       _fw_rw_start - _fw_start
 
        .section .entry, "ax", %progbits
        .align 3
firmware/fw_base.ldS +1 -2
index 9a1304e..3d68484 100644
@@ -64,7 +64,7 @@
        . = ALIGN(1 << LOG2CEIL((SIZEOF(.rodata) + SIZEOF(.text)
                                + SIZEOF(.dynsym) + SIZEOF(.rela.dyn))));
 
-       PROVIDE(_fw_rw_offset = (.  - _fw_start));
+       PROVIDE(_fw_rw_start = .);
 
        /* Beginning of the read-write data sections */
 
--

2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Applied
#3 lore raw

PATCH 1/1 firmware: Fix fw_rw_offset computation in fw_base.S

Quoted 11 lines · Prabhakar
Quoted 9 lines · Anup Patel

From: Jessica Clarke <[email protected]>

It seems BFD just does totally nonsensical things for SHN_ABS symbols when producing position-independent outputs (both -pie and -shared) for various historical reasons, and so SHN_ABS symbols are still subject to relocation as far as BFD is concerned (except AArch64, which fixes it in limited cases that don?t apply here...).

Quoted 5 lines
Quoted 4 lines

The above affects the _fw_rw_offset provided through fw_base.ldS linker script which results in OpenSBI firmware failing to boot whne loaded at an address different from FW_TEXT_START.

s/whne/when

Okay, I will fix the typo at the time of merging this patch.

Quoted 10 lines
Quoted 10 lines
Fixes c10e3fe5f9a1 ("firmware: Add RW section offset in scratch")
Signed-off-by Jessica Clarke <[email protected]>
Tested-by Anup Patel <[email protected]>
Signed-off-by Anup Patel <[email protected]>
---
 firmware/fw_base.S   | 2 +-
 firmware/fw_base.ldS | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
Quoted 2 lines
Reported-by Lad Prabhakar <[email protected]>
Tested-by Lad Prabhakar <[email protected]>

Thanks for testing it.

I have applied this patch to the riscv/opensbi repo.

Regards,
Anup

Quoted 38 lines

Cheers,
Prabhakar

Quoted 34 lines
firmware/fw_base.S +1 -1
index e04b683..fb6ac92 100644
@@ -526,7 +526,7 @@ _link_start:
 _link_end:
        RISCV_PTR       _fw_reloc_end
 __fw_rw_offset:
-       RISCV_PTR       _fw_rw_offset
+       RISCV_PTR       _fw_rw_start - _fw_start
 
        .section .entry, "ax", %progbits
        .align 3
firmware/fw_base.ldS +1 -2
index 9a1304e..3d68484 100644
@@ -64,7 +64,7 @@
        . = ALIGN(1 << LOG2CEIL((SIZEOF(.rodata) + SIZEOF(.text)
                                + SIZEOF(.dynsym) + SIZEOF(.rela.dyn))));
 
-       PROVIDE(_fw_rw_offset = (.  - _fw_start));
+       PROVIDE(_fw_rw_start = .);
 
        /* Beginning of the read-write data sections */
 
--

2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi