Change in coreboot[master]: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld

[Furquan Shaikh (Code Review)]

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld

This change drops the inclusion of entry16.ld and reset16.ld instead
adds the content of those files directly in memlayout.ld in
picasso. This is done to allow the work for top-aligning bootblock to
happen independent of Picasso layout. Once that is complete, Picasso
layout can be re-evaluated to see if it can make use of the common
bootblock linker file includes.

TEST=Verified that coreboot.rom generated using --timeless is the same
with and without this change for trembyle.

Change-Id: Ib1218b24a06d0f69b856fb21458a6183fd21fcbc
Signed-off-by: Furquan Shaikh 
---
M src/soc/amd/picasso/memlayout.ld
1 file changed, 16 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/43281/1

diff --git a/src/soc/amd/picasso/memlayout.ld b/src/soc/amd/picasso/memlayout.ld
index 6f43ba1..b7f9167 100644
--- a/src/soc/amd/picasso/memlayout.ld
+++ b/src/soc/amd/picasso/memlayout.ld
@@ -95,7 +95,20 @@
 }

#if ENV_BOOTBLOCK
-/* Bootblock specific scripts which provide more SECTION directives. */
-#include <cpu/x86/16bit/entry16.ld>
-#include <cpu/x86/16bit/reset16.ld>
+
+gdtptr16_offset = gdtptr16 & 0xffff;
+nullidt_offset = nullidt & 0xffff;
+
+SECTIONS {
+	/* Trigger an error if I have an unuseable start address */
+	_TOO_LOW = CONFIG_X86_RESET_VECTOR - 0xfff0;
+	_bogus = ASSERT(_start16bit >= _TOO_LOW, "_start16bit too low. Please report.");
+
+	. = CONFIG_X86_RESET_VECTOR;
+	.reset . : {
+		*(.reset);
+		. = 15;
+		BYTE(0x00);
+	}
+}
 #endif  /* ENV_BOOTBLOCK */

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 1:

Done based on the comments here: https://review.coreboot.org/c/coreboot/+/42348

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43281/1/src/soc/amd/picasso/memlayo... 
File src/soc/amd/picasso/memlayout.ld:

https://review.coreboot.org/c/coreboot/+/43281/1/src/soc/amd/picasso/memlayo... 
PS1, Line 103: 	/* Trigger an error if I have an unuseable start address */
'unuseable' may be misspelled - perhaps 'unusable'?

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 1: Code-Review+2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 1:

Needs a manual rebase.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/43281/2/src/soc/amd/picasso/memlayo... 
File src/soc/amd/picasso/memlayout_x86.ld:

https://review.coreboot.org/c/coreboot/+/43281/2/src/soc/amd/picasso/memlayo... 
PS2, Line 103: 	/* Trigger an error if I have an unuseable start address */
> 'unuseable' may be misspelled - perhaps 'unusable'?

It's a misspelling

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43281/3/src/soc/amd/picasso/memlayo... 
File src/soc/amd/picasso/memlayout_x86.ld:

https://review.coreboot.org/c/coreboot/+/43281/3/src/soc/amd/picasso/memlayo... 
PS3, Line 105: 	/* Trigger an error if I have an unuseable start address */
'unuseable' may be misspelled - perhaps 'unusable'?

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43281/3/src/soc/amd/picasso/memlayo... 
File src/soc/amd/picasso/memlayout_x86.ld:

https://review.coreboot.org/c/coreboot/+/43281/3/src/soc/amd/picasso/memlayo... 
PS3, Line 105: 	/* Trigger an error if I have an unuseable start address */
> 'unuseable' may be misspelled - perhaps 'unusable'?

The bot is right

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 3:

BTW, this patch is stalled since Jul 17 just because of that one misspelling, otherwise I'd've +2'd it already.

[Furquan Shaikh (Code Review)]

Hello Jason Glenesk, build bot (Jenkins), Raul Rangel, Angel Pons, Julius Werner, Kyösti Mälkki, Aaron Durbin, Felix Held,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43281

to look at the new patch set (#4).

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld

This change drops the inclusion of entry16.ld and reset16.ld and
instead adds the content of those files directly in memlayout_x86.ld
in amd/picasso. This is done to allow the work for top-aligning
bootblock to happen independent of Picasso layout. Once that is
complete, Picasso layout can be re-evaluated to see if it can make use
of the common bootblock linker file includes.

TEST=Verified that coreboot.rom generated using --timeless is the same
with and without this change for trembyle.

Change-Id: Ib1218b24a06d0f69b856fb21458a6183fd21fcbc
Signed-off-by: Furquan Shaikh 
---
M src/soc/amd/picasso/memlayout_x86.ld
1 file changed, 16 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/43281/4

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 4:

(1 comment)
> Patch Set 3:
>
> BTW, this patch is stalled since Jul 17 just because of that one misspelling, otherwise I'd've +2'd it already.

Done.

https://review.coreboot.org/c/coreboot/+/43281/3/src/soc/amd/picasso/memlayo... 
File src/soc/amd/picasso/memlayout_x86.ld:

https://review.coreboot.org/c/coreboot/+/43281/3/src/soc/amd/picasso/memlayo... 
PS3, Line 105: 	/* Trigger an error if I have an unuseable start address */
> The bot is right

Done.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43281/2/src/soc/amd/picasso/memlayo... 
File src/soc/amd/picasso/memlayout_x86.ld:

https://review.coreboot.org/c/coreboot/+/43281/2/src/soc/amd/picasso/memlayo... 
PS2, Line 103: 	/* Trigger an error if I have an unuseable start address */
> It's a misspelling

Done

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 4:

the patch needs a rebase on current master; it is currently based on a patch where the fix for the issue causing the build to fail hadn't landed yet

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 5: Code-Review+2

[Jason Glenesk (Code Review)]

Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 5: Code-Review+1

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

Patch Set 5: Code-Review+2

timeless build for mandolin results in identical binary

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43281 )

Change subject: soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld
......................................................................

soc/amd/picasso: Drop the inclusion of entry16.ld and reset16.ld

This change drops the inclusion of entry16.ld and reset16.ld and
instead adds the content of those files directly in memlayout_x86.ld
in amd/picasso. This is done to allow the work for top-aligning
bootblock to happen independent of Picasso layout. Once that is
complete, Picasso layout can be re-evaluated to see if it can make use
of the common bootblock linker file includes.

TEST=Verified that coreboot.rom generated using --timeless is the same
with and without this change for trembyle.

Change-Id: Ib1218b24a06d0f69b856fb21458a6183fd21fcbc
Signed-off-by: Furquan Shaikh 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43281
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
Reviewed-by: Jason Glenesk 
Reviewed-by: Felix Held 
---
M src/soc/amd/picasso/memlayout_x86.ld
1 file changed, 16 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Felix Held: Looks good to me, approved
  Angel Pons: Looks good to me, approved
  Jason Glenesk: Looks good to me, but someone else must approve

diff --git a/src/soc/amd/picasso/memlayout_x86.ld b/src/soc/amd/picasso/memlayout_x86.ld
index eeb6dda..369d431 100644
--- a/src/soc/amd/picasso/memlayout_x86.ld
+++ b/src/soc/amd/picasso/memlayout_x86.ld
@@ -97,7 +97,20 @@
 }

#if ENV_BOOTBLOCK
-/* Bootblock specific scripts which provide more SECTION directives. */
-#include <cpu/x86/16bit/entry16.ld>
-#include <cpu/x86/16bit/reset16.ld>
+
+gdtptr16_offset = gdtptr16 & 0xffff;
+nullidt_offset = nullidt & 0xffff;
+
+SECTIONS {
+	/* Trigger an error if I have an unusable start address */
+	_TOO_LOW = CONFIG_X86_RESET_VECTOR - 0xfff0;
+	_bogus = ASSERT(_start16bit >= _TOO_LOW, "_start16bit too low. Please report.");
+
+	. = CONFIG_X86_RESET_VECTOR;
+	.reset . : {
+		*(.reset);
+		. = 15;
+		BYTE(0x00);
+	}
+}
 #endif  /* ENV_BOOTBLOCK */