Change in coreboot[master]: prog_loaders: Fix ramstage loading on x86

[Nico Huber (Code Review)]

Hello Kyösti Mälkki, Werner Zeh, Aaron Durbin,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: prog_loaders: Fix ramstage loading on x86
......................................................................

prog_loaders: Fix ramstage loading on x86

A regression sneaked in with 18a8ba41cc (arch/x86: Remove RELOCATABLE_
RAMSTAGE). We want to call load_relocatable_ramstage() on x86, and
cbfs_prog_stage_load() on other architectures. But with the current
code the latter is also called on x86 if the former succeeded. Fix
that and also balance the if structure to make it more obvious.

Change-Id: I5b1db5aac772b9b3a388a1a8ae490fa627334320
Signed-off-by: Nico Huber 
---
M src/lib/prog_loaders.c
1 file changed, 7 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/43142/1

diff --git a/src/lib/prog_loaders.c b/src/lib/prog_loaders.c
index 419f4cd..93efc0a 100644
--- a/src/lib/prog_loaders.c
+++ b/src/lib/prog_loaders.c
@@ -129,10 +129,13 @@

timestamp_add_now(TS_START_COPYRAM);

-	if (ENV_X86 && load_relocatable_ramstage(&ramstage))
-		goto fail;
-	else if (cbfs_prog_stage_load(&ramstage))
-		goto fail;
+	if (ENV_X86) {
+		if (load_relocatable_ramstage(&ramstage))
+			goto fail;
+	} else {
+		if (cbfs_prog_stage_load(&ramstage))
+			goto fail;
+	}

stage_cache_add(STAGE_RAMSTAGE, &ramstage);

[Nico Huber (Code Review)]

Hello build bot (Jenkins), Kyösti Mälkki, Werner Zeh, Aaron Durbin,

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

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

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

Change subject: prog_loaders: Fix ramstage loading on x86
......................................................................

prog_loaders: Fix ramstage loading on x86

A regression sneaked in with 18a8ba41cc (arch/x86: Remove RELOCATABLE_
RAMSTAGE). We want to call load_relocatable_ramstage() on x86, and
cbfs_prog_stage_load() on other architectures. But with the current
code the latter is also called on x86 if the former succeeded. Fix
that and also balance the if structure to make it more obvious.

TEST=qemu-system-x86_64 boots to payload again.

Change-Id: I5b1db5aac772b9b3a388a1a8ae490fa627334320
Signed-off-by: Nico Huber 
---
M src/lib/prog_loaders.c
1 file changed, 7 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/43142/2

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43142 )

Change subject: prog_loaders: Fix ramstage loading on x86
......................................................................

Patch Set 3: 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/+/43142 )

Change subject: prog_loaders: Fix ramstage loading on x86
......................................................................

Patch Set 3: Code-Review+1

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43142 )

Change subject: prog_loaders: Fix ramstage loading on x86
......................................................................

Patch Set 3:

a

[Mario Scheithauer (Code Review)]

Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43142 )

Change subject: prog_loaders: Fix ramstage loading on x86
......................................................................

Patch Set 3: Code-Review+2

Hi Nico, I tried this patch at our APL boards and this fixes the assertion error from patch 18a8ba41cc. Thanks!

[Werner Zeh (Code Review)]

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43142 )

Change subject: prog_loaders: Fix ramstage loading on x86
......................................................................

Patch Set 3: Code-Review+2

Since this patch fixes a broken master let it get merged fast please.

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43142 )

Change subject: prog_loaders: Fix ramstage loading on x86
......................................................................

prog_loaders: Fix ramstage loading on x86

A regression sneaked in with 18a8ba41cc (arch/x86: Remove RELOCATABLE_
RAMSTAGE). We want to call load_relocatable_ramstage() on x86, and
cbfs_prog_stage_load() on other architectures. But with the current
code the latter is also called on x86 if the former succeeded. Fix
that and also balance the if structure to make it more obvious.

TEST=qemu-system-x86_64 boots to payload again.

Change-Id: I5b1db5aac772b9b3a388a1a8ae490fa627334320
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43142
Reviewed-by: Arthur Heymans 
Reviewed-by: Kyösti Mälkki 
Reviewed-by: Mario Scheithauer 
Reviewed-by: Werner Zeh 
Tested-by: build bot (Jenkins) 
---
M src/lib/prog_loaders.c
1 file changed, 7 insertions(+), 4 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, but someone else must approve
  Werner Zeh: Looks good to me, approved
  Mario Scheithauer: Looks good to me, approved
  Arthur Heymans: Looks good to me, approved

diff --git a/src/lib/prog_loaders.c b/src/lib/prog_loaders.c
index 419f4cd..93efc0a 100644
--- a/src/lib/prog_loaders.c
+++ b/src/lib/prog_loaders.c
@@ -129,10 +129,13 @@

timestamp_add_now(TS_START_COPYRAM);

-	if (ENV_X86 && load_relocatable_ramstage(&ramstage))
-		goto fail;
-	else if (cbfs_prog_stage_load(&ramstage))
-		goto fail;
+	if (ENV_X86) {
+		if (load_relocatable_ramstage(&ramstage))
+			goto fail;
+	} else {
+		if (cbfs_prog_stage_load(&ramstage))
+			goto fail;
+	}

stage_cache_add(STAGE_RAMSTAGE, &ramstage);