OpenSBI Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] firmware: Fix fw_rw_offset computation in fw_base.S
@ 2023-01-26 15:52 Anup Patel
  2023-01-26 15:53 ` Anup Patel
  2023-01-26 22:23 ` Lad, Prabhakar
  0 siblings, 2 replies; 4+ messages in thread
From: Anup Patel @ 2023-01-26 15:52 UTC (permalink / raw)
  To: opensbi

From: Jessica Clarke <jrtc27@jrtc27.com>

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 <jrtc27@jrtc27.com>
Tested-by: Anup Patel <apatel@ventanamicro.com>
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 firmware/fw_base.S   | 2 +-
 firmware/fw_base.ldS | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/firmware/fw_base.S b/firmware/fw_base.S
index e04b683..fb6ac92 100644
--- a/firmware/fw_base.S
+++ b/firmware/fw_base.S
@@ -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
diff --git a/firmware/fw_base.ldS b/firmware/fw_base.ldS
index 9a1304e..3d68484 100644
--- a/firmware/fw_base.ldS
+++ b/firmware/fw_base.ldS
@@ -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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] firmware: Fix fw_rw_offset computation in fw_base.S
  2023-01-26 15:52 [PATCH] firmware: Fix fw_rw_offset computation in fw_base.S Anup Patel
@ 2023-01-26 15:53 ` Anup Patel
  2023-01-26 22:23 ` Lad, Prabhakar
  1 sibling, 0 replies; 4+ messages in thread
From: Anup Patel @ 2023-01-26 15:53 UTC (permalink / raw)
  To: opensbi

Hi Jessica,

On Thu, Jan 26, 2023 at 9:22 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> From: Jessica Clarke <jrtc27@jrtc27.com>
>
> 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 <jrtc27@jrtc27.com>
> Tested-by: Anup Patel <apatel@ventanamicro.com>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>

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

Regards,
Anup

> ---
>  firmware/fw_base.S   | 2 +-
>  firmware/fw_base.ldS | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index e04b683..fb6ac92 100644
> --- a/firmware/fw_base.S
> +++ b/firmware/fw_base.S
> @@ -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
> diff --git a/firmware/fw_base.ldS b/firmware/fw_base.ldS
> index 9a1304e..3d68484 100644
> --- a/firmware/fw_base.ldS
> +++ b/firmware/fw_base.ldS
> @@ -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
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] firmware: Fix fw_rw_offset computation in fw_base.S
  2023-01-26 15:52 [PATCH] firmware: Fix fw_rw_offset computation in fw_base.S Anup Patel
  2023-01-26 15:53 ` Anup Patel
@ 2023-01-26 22:23 ` Lad, Prabhakar
  2023-01-27  3:15   ` Anup Patel
  1 sibling, 1 reply; 4+ messages in thread
From: Lad, Prabhakar @ 2023-01-26 22:23 UTC (permalink / raw)
  To: opensbi

On Thu, Jan 26, 2023 at 3:53 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> From: Jessica Clarke <jrtc27@jrtc27.com>
>
> 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.
>
s/whne/when

> Fixes: c10e3fe5f9a1 ("firmware: Add RW section offset in scratch")
> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
> Tested-by: Anup Patel <apatel@ventanamicro.com>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  firmware/fw_base.S   | 2 +-
>  firmware/fw_base.ldS | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
Reported-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index e04b683..fb6ac92 100644
> --- a/firmware/fw_base.S
> +++ b/firmware/fw_base.S
> @@ -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
> diff --git a/firmware/fw_base.ldS b/firmware/fw_base.ldS
> index 9a1304e..3d68484 100644
> --- a/firmware/fw_base.ldS
> +++ b/firmware/fw_base.ldS
> @@ -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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] firmware: Fix fw_rw_offset computation in fw_base.S
  2023-01-26 22:23 ` Lad, Prabhakar
@ 2023-01-27  3:15   ` Anup Patel
  0 siblings, 0 replies; 4+ messages in thread
From: Anup Patel @ 2023-01-27  3:15 UTC (permalink / raw)
  To: opensbi

On Fri, Jan 27, 2023 at 3:53 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> On Thu, Jan 26, 2023 at 3:53 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > From: Jessica Clarke <jrtc27@jrtc27.com>
> >
> > 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.
> >
> s/whne/when

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

>
> > Fixes: c10e3fe5f9a1 ("firmware: Add RW section offset in scratch")
> > Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
> > Tested-by: Anup Patel <apatel@ventanamicro.com>
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  firmware/fw_base.S   | 2 +-
> >  firmware/fw_base.ldS | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> Reported-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for testing it.

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

Regards,
Anup

>
> Cheers,
> Prabhakar
>
> > diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> > index e04b683..fb6ac92 100644
> > --- a/firmware/fw_base.S
> > +++ b/firmware/fw_base.S
> > @@ -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
> > diff --git a/firmware/fw_base.ldS b/firmware/fw_base.ldS
> > index 9a1304e..3d68484 100644
> > --- a/firmware/fw_base.ldS
> > +++ b/firmware/fw_base.ldS
> > @@ -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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-01-27  3:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-26 15:52 [PATCH] firmware: Fix fw_rw_offset computation in fw_base.S Anup Patel
2023-01-26 15:53 ` Anup Patel
2023-01-26 22:23 ` Lad, Prabhakar
2023-01-27  3:15   ` Anup Patel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox