Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x86_64 firmware support: Use fixed size struct elements to support cross arch firmwares #59

Open
PatrickRudolph opened this issue Dec 9, 2020 · 1 comment

Comments

@PatrickRudolph
Copy link

As FSP binaries are released with a fixed architecture (x86_32), the header must not contain variable size fields like SIZE_T, UINTN, VOID* or UINTPTR_T. If compiled on x86_64 this would cause those structs to not match the ones used internally by FSP as the element size differs.

Please remove such elements and only use variables that have a fixed size, even on when compiled on x86_64.

@nate-desimone
Copy link
Contributor

@PatrickRudolph, that is good feedback. We will keep that in mind while developing new FSP's for future Intel platforms. Rewriting the UPD headers for pre-existing FSP binaries going back to the beginning of time isn't a maintainable activity, for the same reason that keeping coreboot local copies of UPD headers makes updates tricky.

coreboot-org-bot pushed a commit to coreboot/coreboot that referenced this issue Feb 4, 2021
Add new Kconfig symbols to mark FSP binary as x86_32.
Fix the FSP headers and replace void pointers by fixed sized integers
depending on the used mode to compile the FSP.
This issue has been reported here:
intel/FSP#59

This is necessary to run on x86_64, as pointers have different size.

Add preprocessor error to warn that x86_64 FSP isn't supported by the
current code.

Tested on Intel Skylake. FSP-M no longer returns the error "Invalid
Parameter".

Change-Id: I6015005c4ee3fc2f361985cf8cff896bcefd04fb
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48174
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
coreboot-org-bot pushed a commit to coreboot/coreboot that referenced this issue Mar 18, 2022
Fix the FSP headers and replace void pointers by fixed sized integers
depending on the used mode to compile the FSP.
Change request here:intel/FSP#59

This is necessary to run on x86_64, as pointers have different size.
Add preprocessor error to warn that x86_64 FSP isn't supported by the
current code.

BUG=b:200113959
TEST=Verified on Meteor Lake platform, without any compilation error

Signed-off-by: Subrata Banik <subratabanik@google.com>
Change-Id: I1f33db43f7932cf6d165d0c70a0e2922dad00a09
Reviewed-on: https://review.coreboot.org/c/coreboot/+/62847
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anil Kumar K <anil.kumar.k@intel.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants