Patchwork Add .text into romstage sections.

login
register
about
Submitter Bao, Zheng
Date 2011-02-22 11:24:33
Message ID <DD1CC71B621B004FA76856E5129D6B17049C88BD@sbjgexmb1.amd.com>
Download mbox | patch
Permalink /patch/2685/
State New
Headers show

Comments

Bao, Zheng - 2011-02-22 11:24:33
The text sections in *.romstage.o are called .text instead of .rom.text.

The .text can be built in, but the _erom cannot be calculated correctly

without this patch. Nobody uses _erom currently, so nobody seems cares
it. 

 

Signed-off-by: Zheng Bao <zheng.bao@amd.com>

 

Index: src/arch/x86/init/ldscript_fallback_cbfs.lb

===================================================================

--- src/arch/x86/init/ldscript_fallback_cbfs.lb (revision 6374)

+++ src/arch/x86/init/ldscript_fallback_cbfs.lb (working copy)

@@ -35,6 +35,7 @@

    .rom . : {

          _rom = .;

          *(.rom.text);

+          *(.text);

          *(.rom.data);

          *(.rodata);

          *(.rodata.*);
The text sections in *.romstage.o are called .text instead of .rom.text.
The _erom can not be calculated correctly without this patch. Nobody uses
_erom currently, so nobody seems cares it.

Signed-off-by: Zheng Bao <zheng.bao@amd.com>
Marc Jones - 2011-02-22 21:25:02
On Tue, Feb 22, 2011 at 4:24 AM, Bao, Zheng <Zheng.Bao@amd.com> wrote:
> The text sections in *.romstage.o are called .text instead of .rom.text.
>
> The .text can be built in, but the _erom cannot be calculated correctly
>
> without this patch. Nobody uses _erom currently, so nobody seems cares it.
>
>
>
> Signed-off-by: Zheng Bao <zheng.bao@amd.com>
>
>
>
> Index: src/arch/x86/init/ldscript_fallback_cbfs.lb
>
> ===================================================================
>
> --- src/arch/x86/init/ldscript_fallback_cbfs.lb (revision 6374)
>
> +++ src/arch/x86/init/ldscript_fallback_cbfs.lb (working copy)
>
> @@ -35,6 +35,7 @@
>
>     .rom . : {
>
>           _rom = .;
>
>           *(.rom.text);
>
> +          *(.text);
>
>           *(.rom.data);
>
>           *(.rodata);
>
>           *(.rodata.*);

Zheng,

I think that .text should be like .data. and should only be in
ramstage. Is something being built into romstage that shouldn't be?

Marc
Bao, Zheng - 2011-02-23 02:31:21
The ldscript_fallback_cbfs.lb is only for the romstage. It does nothing to change the building of ramstage. And it doesn't have area like .data,
which can be read and wrote.

From the attached build output, we can see that only crt0.romstage.o changes .rodata to rom.data, changes .text to .rom.text. 
Other romstage files like console.romstage.o only have .text., without .rom.text, .rom.data, or .data.

From the 2 romstage.map(s), we can see where the _erom should be.

Compared to the patch to ldscript_fallback_cbfs.lb, we can also change other romstage's .text to .rom.text. But I don't think it is worth to do such a complicated job.

Zheng

-----Original Message-----
From: Marc Jones [mailto:marcj303@gmail.com] 
Sent: Wednesday, February 23, 2011 5:25 AM
To: Bao, Zheng
Cc: coreboot@coreboot.org
Subject: Re: [coreboot] [PATCH] Add .text into romstage sections.

On Tue, Feb 22, 2011 at 4:24 AM, Bao, Zheng <Zheng.Bao@amd.com> wrote:
> The text sections in *.romstage.o are called .text instead of .rom.text.
>
> The .text can be built in, but the _erom cannot be calculated correctly
>
> without this patch. Nobody uses _erom currently, so nobody seems cares it.
>
>
>
> Signed-off-by: Zheng Bao <zheng.bao@amd.com>
>
>
>
> Index: src/arch/x86/init/ldscript_fallback_cbfs.lb
>
> ===================================================================
>
> --- src/arch/x86/init/ldscript_fallback_cbfs.lb (revision 6374)
>
> +++ src/arch/x86/init/ldscript_fallback_cbfs.lb (working copy)
>
> @@ -35,6 +35,7 @@
>
>     .rom . : {
>
>           _rom = .;
>
>           *(.rom.text);
>
> +          *(.text);
>
>           *(.rom.data);
>
>           *(.rodata);
>
>           *(.rodata.*);

Zheng,

I think that .text should be like .data. and should only be in
ramstage. Is something being built into romstage that shouldn't be?

Marc
Stefan Reinauer - 2011-02-24 01:45:22
* Bao, Zheng <Zheng.Bao@amd.com> [110223 03:31]:
> The ldscript_fallback_cbfs.lb is only for the romstage. It does
> nothing to change the building of ramstage. And it doesn't have area
> like .data, which can be read and wrote.
> 
> From the attached build output, we can see that only crt0.romstage.o
> changes .rodata to rom.data, changes .text to .rom.text.  Other
> romstage files like console.romstage.o only have .text., without
> .rom.text, .rom.data, or .data.
 
I believe this is an odd survivor from romcc-only times. Is there a
reason on non-romcc builds to move any code into .rom.* sections at all?

If not, we should remove that behavior completely and just have it
include .text and .rodata in the ld script. (which should be called
romstage.ld and not ldscript_fallback_cbfs.lb)

> From the 2 romstage.map(s), we can see where the _erom should be.
> 
> Compared to the patch to ldscript_fallback_cbfs.lb, we can also change
> other romstage's .text to .rom.text. But I don't think it is worth to
> do such a complicated job.
> 
> Zheng

Stefan

Patch

Index: src/arch/x86/init/ldscript_fallback_cbfs.lb
===================================================================
--- src/arch/x86/init/ldscript_fallback_cbfs.lb	(revision 6374)
+++ src/arch/x86/init/ldscript_fallback_cbfs.lb	(working copy)
@@ -35,6 +35,7 @@ 
 	.rom . : {
 		_rom = .;
 		*(.rom.text);
+		*(.text);
 		*(.rom.data);
 		*(.rodata);
 		*(.rodata.*);