Change in coreboot[master]: WIP: add transforms

[ron minnich (Code Review)]

ron minnich has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37391 )

Change subject: WIP: add transforms
......................................................................

WIP: add transforms

Transforms transform coreboot ROM images to add or remove capabilities.

So, for example, should we want an image with a rampayload, we can
run the normal build process and then transform the resulting rom image
as needed.

We can implement a postcar stage which loads a ram payload directly,
and do so in a way which has no impact on the coreboot source.

This not only lets us change the output of the build process, it can be implemented
without changing the coreboot source. Further, since the transform Makefile depends
on the presence of a coreboot.rom in the directory, we can avoid the build step
if we're just trying to transform a ROM we read from a FLASH part. Just flashrom
-r the image, drop it into coreboot.rom in this directory, and no build
will be attempted.

This example represents the first steps of building a rampayload transform.

Transforms should have zero impact on the $(top)/src tree, since they
are intended to be able to be used on prebuilt images.

Change-Id: I27c5686000f79e287adce3f0fa7b608683a9bfce
Signed-off-by: Ronald G. Minnich 
---
M 3rdparty/fsp
A transforms/rampayload/Makefile
2 files changed, 7 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37391/1

diff --git a/3rdparty/fsp b/3rdparty/fsp
index 5996417..1d2b7e1 160000
--- a/3rdparty/fsp
+++ b/3rdparty/fsp
@@ -1 +1 @@
-Subproject commit 59964173e18950debcc6b8856c5c928935ce0b4f
+Subproject commit 1d2b7e1a94c6a7c25a6fed1ac37caebf500f5f1a
diff --git a/transforms/rampayload/Makefile b/transforms/rampayload/Makefile
new file mode 100644
index 0000000..fb73b8e
--- /dev/null
+++ b/transforms/rampayload/Makefile
@@ -0,0 +1,6 @@
+image: coreboot.rom
+	cbfstool coreboot.rom print
+
+coreboot.rom:
+	cd ../../ && make
+	cp ../../build/coreboot.rom .

[ron minnich (Code Review)]

Hello Subrata Banik, Lean Sheng Tan, Jeremy Soller, build bot (Jenkins),

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

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

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

Change subject: WIP: add transforms
......................................................................

WIP: add transforms

Transforms transform coreboot ROM images to add or remove capabilities.

So, for example, should we want an image with a rampayload, we can
run the normal build process and then transform the resulting rom image
as needed.

We can implement a postcar stage which loads a ram payload directly,
and do so in a way which has no impact on the coreboot source.

This not only lets us change the output of the build process, it can be implemented
without changing the coreboot source. Further, since the transform Makefile depends
on the presence of a coreboot.rom in the directory, we can avoid the build step
if we're just trying to transform a ROM we read from a FLASH part. Just flashrom
-r the image, drop it into coreboot.rom in this directory, and no build
will be attempted.

This example represents the first steps of building a rampayload transform.

Transforms should have zero impact on the $(top)/src tree, since they
are intended to be able to be used on prebuilt images.

Change-Id: I27c5686000f79e287adce3f0fa7b608683a9bfce
Signed-off-by: Ronald G. Minnich 
---
A transforms/rampayload/Makefile
1 file changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37391/2

[ron minnich (Code Review)]

Hello Subrata Banik, Lean Sheng Tan, Jeremy Soller, build bot (Jenkins),

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

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

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

Change subject: WIP: add transforms
......................................................................

WIP: add transforms

Transforms transform coreboot ROM images to add or remove capabilities.
They let us test out new ideas for coreboot while not impacting
the coreboot source in any way.

So, for example, should we want an image with a rampayload, we can
run the normal build process and then transform the resulting rom image
as needed.

We can implement a postcar stage which loads a ram payload directly,
and do so in a way which has no impact on the coreboot source.

This not only lets us change the output of the build process, it can be implemented
without changing the coreboot source. Further, since the transform Makefile depends
on the presence of a coreboot.rom in the directory, we can avoid the build step
if we're just trying to transform a ROM we read from a FLASH part. Just flashrom
-r the image, drop it into coreboot.rom in this directory, and no build
will be attempted.

This example represents the first steps of building a rampayload transform.

Transforms should have zero impact on the $(top)/src tree, since they
are intended to be able to be used on prebuilt images.

Change-Id: I27c5686000f79e287adce3f0fa7b608683a9bfce
Signed-off-by: Ronald G. Minnich 
---
M payloads/Kconfig
M payloads/Makefile.inc
A payloads/payloader/Makefile
A payloads/payloader/i386.c
A payloads/payloader/payloader.c
A payloads/payloader/payloader.h
A transforms/README
A transforms/rampayload/Makefile
8 files changed, 172 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37391/3

[Jeremy Soller (Code Review)]

Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37391 )

Change subject: WIP: add transforms
......................................................................

Patch Set 3: Code-Review+1

This looks good to me

[Jeremy Soller (Code Review)]

Jeremy Soller has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/37391 )

Change subject: WIP: add transforms
......................................................................

Removed Code-Review+1 by Jeremy Soller

[Jeremy Soller (Code Review)]

Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37391 )

Change subject: WIP: add transforms
......................................................................

Patch Set 3: Code-Review+2

[Lean Sheng Tan (Code Review)]

Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37391 )

Change subject: WIP: add transforms
......................................................................

Patch Set 3: Code-Review+1

Change in coreboot[master]: add transforms

[ron minnich (Code Review)]

Hello Subrata Banik, Lean Sheng Tan, Jeremy Soller, build bot (Jenkins), Patrick Georgi, Martin Roth,

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

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

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

Change subject: add transforms
......................................................................

add transforms

Transforms transform coreboot ROM images to add or remove capabilities.
They let us test out new ideas for coreboot while not impacting
the coreboot source in any way.

So, for example, should we want an image with a rampayload, we can
run the normal build process and then transform the resulting rom image
as needed.

We can implement a postcar stage which loads a ram payload directly,
and do so in a way which has no impact on the coreboot source.

This not only lets us change the output of the build process, it can be implemented
without changing the coreboot source. Further, since the transform Makefile depends
on the presence of a coreboot.rom in the directory, we can avoid the build step
if we're just trying to transform a ROM we read from a FLASH part. Just flashrom
-r the image, drop it into coreboot.rom in this directory, and no build
will be attempted.

This example represents the first steps of building a rampayload transform.

Transforms should have zero impact on the $(top)/src tree, since they
are intended to be able to be used on prebuilt images.

Currently the payloader works as a payload but fails as a stage with
the cryptic message:
CBFS: Locating 'fallback/postcar'
CBFS: Found @ offset 14bc0 size 1008c
Decompressing stage fallback/postcar @ 0x07f85fc0 (349968 bytes)
Failed to load after CAR program.

Change-Id: I27c5686000f79e287adce3f0fa7b608683a9bfce
Signed-off-by: Ronald G. Minnich 
---
M payloads/Kconfig
M payloads/Makefile.inc
A payloads/payloader/Makefile
A payloads/payloader/i386.c
A payloads/payloader/payloader.c
A payloads/payloader/payloader.h
A transforms/README
A transforms/rampayload/Makefile
8 files changed, 188 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37391/4

Change in coreboot[master]: add transforms

[build bot (Jenkins) (Code Review)]

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

Change subject: add transforms
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37391/4/payloads/payloader/payloade... 
File payloads/payloader/payloader.c:

https://review.coreboot.org/c/coreboot/+/37391/4/payloads/payloader/payloade... 
PS4, Line 31: 	if (res) {
braces {} are not necessary for single statement blocks

Change in coreboot[master]: add transforms

[ron minnich (Code Review)]

Hello Subrata Banik, Lean Sheng Tan, Jeremy Soller, build bot (Jenkins), Patrick Georgi, Martin Roth,

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

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

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

Change subject: add transforms
......................................................................

add transforms

Transforms transform coreboot ROM images to add or remove capabilities.
They let us test out new ideas for coreboot while not impacting
the coreboot source in any way.

So, for example, should we want an image with a rampayload, we can
run the normal build process and then transform the resulting rom image
as needed.

We can implement a postcar stage which loads a ram payload directly,
and do so in a way which has no impact on the coreboot source.

This not only lets us change the output of the build process, it can be implemented
without changing the coreboot source. Further, since the transform Makefile depends
on the presence of a coreboot.rom in the directory, we can avoid the build step
if we're just trying to transform a ROM we read from a FLASH part. Just flashrom
-r the image, drop it into coreboot.rom in this directory, and no build
will be attempted.

This example represents the first steps of building a rampayload transform.

Transforms should have zero impact on the $(top)/src tree, since they
are intended to be able to be used on prebuilt images.

Currently the payloader works as a payload but fails as a stage with
the cryptic message:
CBFS: Locating 'fallback/postcar'
CBFS: Found @ offset 14bc0 size 1008c
Decompressing stage fallback/postcar @ 0x07f85fc0 (349968 bytes)
Failed to load after CAR program.

There is a bit more work to figuring out how to make it an rmod.

Change-Id: I27c5686000f79e287adce3f0fa7b608683a9bfce
Signed-off-by: Ronald G. Minnich 
---
M payloads/Kconfig
M payloads/Makefile.inc
A payloads/payloader/Makefile
A payloads/payloader/i386.c
A payloads/payloader/payloader.c
A payloads/payloader/payloader.h
A transforms/README
A transforms/rampayload/Makefile
8 files changed, 188 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37391/5

Change in coreboot[master]: add transforms

[build bot (Jenkins) (Code Review)]

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

Change subject: add transforms
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37391/5/payloads/payloader/payloade... 
File payloads/payloader/payloader.c:

https://review.coreboot.org/c/coreboot/+/37391/5/payloads/payloader/payloade... 
PS5, Line 31: 	if (res) {
braces {} are not necessary for single statement blocks

Change in coreboot[master]: add transforms

[ron minnich (Code Review)]

Hello Subrata Banik, Lean Sheng Tan, Jeremy Soller, build bot (Jenkins), Patrick Georgi, Martin Roth,

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

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

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

Change subject: add transforms
......................................................................

add transforms

Transforms transform coreboot ROM images to add or remove capabilities.
They let us test out new ideas for coreboot while not impacting
the coreboot source in any way.

So, for example, should we want an image with a rampayload, we can
run the normal build process and then transform the resulting rom image
as needed.

We can implement a postcar stage which loads a ram payload directly,
and do so in a way which has no impact on the coreboot source.

This not only lets us change the output of the build process, it can be implemented
without changing the coreboot source. Further, since the transform Makefile depends
on the presence of a coreboot.rom in the directory, we can avoid the build step
if we're just trying to transform a ROM we read from a FLASH part. Just flashrom
-r the image, drop it into coreboot.rom in this directory, and no build
will be attempted.

This example represents the first steps of building a rampayload transform.

Transforms should have zero impact on the $(top)/src tree, since they
are intended to be able to be used on prebuilt images. I did have to make
one thing user selectable, namely, NO_RELOCATABLE_RAMSTAGE.

If you do not select NO_RELOCATABLE_RAMSTAGE,
the payloader works as a payload but fails as a stage with
the cryptic message:
CBFS: Locating 'fallback/postcar'
CBFS: Found @ offset 14bc0 size 1008c
Decompressing stage fallback/postcar @ 0x07f85fc0 (349968 bytes)
Failed to load after CAR program.

There is a bit more work to figuring out how to make it an rmod.

To test this:
build a qemu-q35 image, select the payloader payload, select
NO_RELOCATABLE_RAMSTAGE
in whatever manner you prefer. Make. To test:
/usr/bin/qemu-system-x86_64 -M q35 -bios build/coreboot.rom -serial /dev/tty -nographic

when this  runs you will see
.
.
.
BS: BS_PAYLOAD_LOAD times (ms): entry 0 run 12 exit 0
Jumping to boot code at 00100000(07fcb000)
payload loaded at 0x111830
Greetings from the payloader, which loads payloads.
Now we will halt. Bye

now cd transforms/rampayload, and
make run

You should see the same output.

Change-Id: I27c5686000f79e287adce3f0fa7b608683a9bfce
Signed-off-by: Ronald G. Minnich 
---
M payloads/Kconfig
M payloads/Makefile.inc
A payloads/payloader/Makefile
A payloads/payloader/i386.c
A payloads/payloader/payloader.c
A payloads/payloader/payloader.h
M src/Kconfig
A transforms/README
A transforms/rampayload/Makefile
9 files changed, 210 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37391/6

Change in coreboot[master]: add transforms

[build bot (Jenkins) (Code Review)]

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

Change subject: add transforms
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37391/6/payloads/payloader/payloade... 
File payloads/payloader/payloader.c:

https://review.coreboot.org/c/coreboot/+/37391/6/payloads/payloader/payloade... 
PS6, Line 31: 	if (res) {
braces {} are not necessary for single statement blocks

Change in coreboot[master]: add transforms

[ron minnich (Code Review)]

ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37391 )

Change subject: add transforms
......................................................................

Patch Set 6:

This now works to the point that the payloader that replaces the ramstage also loads a payload. Tables are not being set up, and a few other bits need doing. Further, we need to learn how to correctly build the payloader with RELOC enabled. I did that but it did not work ... so for now, the payloader is built as a non-relocatable stage.

Change in coreboot[master]: add transforms

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37391 )

Change subject: add transforms
......................................................................

Patch Set 6:

Would you mind to split this into separate commits? There are
at least 4 things done here:

* Some random cleanup lines for bayou.
* Giving NO_RELOCATABLE_RAMSTAGE a prompt.
* Adding payloads/payloader.
* Adding transforms.

Only the last point is mentioned in the summary line. I have
some comments that would better be placed on individual commits,
e.g.

* Why not make the prompt for RELOCATABLE_RAMSTAGE?
* How does it relate to `config RAMPAYLOAD`?
* What other transforms to imagine? Do they have special dependencies?
* If it's just an additional makefile why not place it below util/?

Also, please adhere to the line length limit in commit messages.
For reference this is how Gerrit shows yours in a narrow browser
window (it breaks at 72 chars):

add transforms

Transforms transform coreboot ROM images to add or remove capabilities.
    They let us test out new ideas for coreboot while not impacting
    the coreboot source in any way.

So, for example, should we want an image with a rampayload, we can
    run the normal build process and then transform the resulting rom image
    as needed.

We can implement a postcar stage which loads a ram payload directly,
    and do so in a way which has no impact on the coreboot source.

This not only lets us change the output of the build process, it can be
    implemented
    without changing the coreboot source. Further, since the transform
    Makefile depends
    on the presence of a coreboot.rom in the directory, we can avoid the
    build step
    if we're just trying to transform a ROM we read from a FLASH part. Just
    flashrom
    -r the image, drop it into coreboot.rom in this directory, and no build
    will be attempted.

This example represents the first steps of building a rampayload
    transform.

Transforms should have zero impact on the $(top)/src tree, since they
    are intended to be able to be used on prebuilt images. I did have to
    make
    one thing user selectable, namely, NO_RELOCATABLE_RAMSTAGE.

Change in coreboot[master]: add transforms

[ron minnich (Code Review)]

ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37391 )

Change subject: add transforms
......................................................................

Patch Set 6:
> Patch Set 6:
>
> Would you mind to split this into separate commits?

sure
> There are
> at least 4 things done here:
> Some random cleanup lines for bayou.

I've just +2'ed the other CL to remove bayou so once that's gone bayou cleanup will go away
> Why not make the prompt for RELOCATABLE_RAMSTAGE?

I'll try that one more time. It did not seem to work. The need for NO_RELOCATABLE_RAMSTAGE is unclear to me, but RELOCATABLE_RAMSTAGE seems to depend on that so I went with the root of this dependency chain.
> How does it relate to `config RAMPAYLOAD`?

The group of us that had interest in RAMPAYLOADS have decided this transforms idea is more practical. It may be time to remove the RAMPAYLOAD config, but not in this CL.
> What other transforms to imagine? Do they have special dependencies?

The point of this is to define the idea of transforms and present an example. As for what else might be imagined, I can't imagine that yet :-)
> If it's just an additional makefile why not place it below util/?

Because we want to break out transforms as a new thing, and putting it below util/ is less compelling. We expect there to be more transforms in the future.

Change in coreboot[master]: add transforms

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37391 )

Change subject: add transforms
......................................................................

Patch Set 6:
> > Would you mind to split this into separate commits?
>
> sure

thanks :)
> > Why not make the prompt for RELOCATABLE_RAMSTAGE?
>
> I'll try that one more time. It did not seem to work. The need for NO_RELOCATABLE_RAMSTAGE is unclear to me, but RELOCATABLE_RAMSTAGE seems to depend on that so I went with the root of this dependency chain.

Hmmm, IIRC, NO_RELOCATABLE_RAMSTAGE was there to have something
x86 boards can select to opt out. We might not need it anymore.