Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46227 )
Change subject: lib/cbfs: deserialize cbfs_stage objects correctly ......................................................................
lib/cbfs: deserialize cbfs_stage objects correctly
cbfstool emits cbfs_stage objects in little endian encoding. However, big endian targets then read the wrong values from these objects. To maintain backwards compatibility with existing cbfs objects add in the little endian deserialization.
Change-Id: Ia113f7ddfa93f0ba5a76e0397f06f9b84c833727 Signed-off-by: Aaron Durbin adurbin@chromium.org --- M src/lib/cbfs.c 1 file changed, 9 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/46227/1
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index cb66f81..35193d0 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -4,8 +4,8 @@ #include <boot_device.h> #include <cbfs.h> #include <commonlib/bsd/compression.h> +#include <commonlib/endian.h> #include <console/console.h> -#include <endian.h> #include <fmap.h> #include <lib.h> #include <security/tpm/tspi/crtm.h> @@ -296,10 +296,16 @@ foffset = 0; foffset += sizeof(stage);
+ /* cbfs_stage fields are written in little endian despite the other + cbfs data types being encoded in big endian. */ + stage.compression = read_le32(&stage.compression); + stage.entry = read_le64(&stage.entry); + stage.load = read_le64(&stage.load); + stage.len = read_le32(&stage.len); + stage.memlen = read_le32(&stage.memlen); + assert(fsize == stage.len);
- /* Note: cbfs_stage fields are currently in the endianness of the - * running processor. */ load = (void *)(uintptr_t)stage.load; entry = (void *)(uintptr_t)stage.entry;
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46227 )
Change subject: lib/cbfs: deserialize cbfs_stage objects correctly ......................................................................
Patch Set 1: Code-Review+2
(FWIW I'm planning to move the stage header from the file contents into a CBFS attribute, and will change the fields to big-endian for consistency while doing that. Will hopefully have that ready for upload in a week or two.)
Marty E. Plummer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46227 )
Change subject: lib/cbfs: deserialize cbfs_stage objects correctly ......................................................................
Patch Set 1: Code-Review+1
Looks good to me. Lets me get into romstage and try to execute ramstage (I think I'm missing some bits there).
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46227 )
Change subject: lib/cbfs: deserialize cbfs_stage objects correctly ......................................................................
lib/cbfs: deserialize cbfs_stage objects correctly
cbfstool emits cbfs_stage objects in little endian encoding. However, big endian targets then read the wrong values from these objects. To maintain backwards compatibility with existing cbfs objects add in the little endian deserialization.
Change-Id: Ia113f7ddfa93f0ba5a76e0397f06f9b84c833727 Signed-off-by: Aaron Durbin adurbin@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/46227 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: Marty E. Plummer hanetzer@startmail.com --- M src/lib/cbfs.c 1 file changed, 9 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Marty E. Plummer: Looks good to me, but someone else must approve
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index cb66f81..35193d0 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -4,8 +4,8 @@ #include <boot_device.h> #include <cbfs.h> #include <commonlib/bsd/compression.h> +#include <commonlib/endian.h> #include <console/console.h> -#include <endian.h> #include <fmap.h> #include <lib.h> #include <security/tpm/tspi/crtm.h> @@ -296,10 +296,16 @@ foffset = 0; foffset += sizeof(stage);
+ /* cbfs_stage fields are written in little endian despite the other + cbfs data types being encoded in big endian. */ + stage.compression = read_le32(&stage.compression); + stage.entry = read_le64(&stage.entry); + stage.load = read_le64(&stage.load); + stage.len = read_le32(&stage.len); + stage.memlen = read_le32(&stage.memlen); + assert(fsize == stage.len);
- /* Note: cbfs_stage fields are currently in the endianness of the - * running processor. */ load = (void *)(uintptr_t)stage.load; entry = (void *)(uintptr_t)stage.entry;
-
Aaron Durbin (Code Review) -
Julius Werner (Code Review)
-
Marty E. Plummer (Code Review)
-
Nico Huber (Code Review)