When coreboot calls payloads it passes a pointer to the coreboot tables as an argument. SeaBIOS does not make use of this functionality and instead tries to find the coreboot tables via a pointer placed in lower memory. To improve compatibility when calling payloads from SeaBIOS call payloads with the coreboot table as an argument.
This fixes Tianocore with UefiPayloadPkg not booting when loaded from SeaBIOS.
Signed-off-by: Arthur Heymans arthur@aheymans.xyz
diff --git a/src/fw/coreboot.c b/src/fw/coreboot.c index 7c0954b..25317ca 100644 --- a/src/fw/coreboot.c +++ b/src/fw/coreboot.c @@ -518,9 +518,10 @@ cbfs_run_payload(struct cbfs_file *fhdr) memset(dest, 0, dest_len); break; case PAYLOAD_SEGMENT_ENTRY: { - dprintf(1, "Calling addr %p\n", dest); - void (*func)(void) = dest; - func(); + void *arg = find_cb_table(); + dprintf(1, "Calling addr %p(%p)\n", dest, arg); + void (*func)(void *) = dest; + func(arg); return; } default:
On Fri, Oct 18, 2019 at 09:39:01PM +0200, Arthur Heymans wrote:
When coreboot calls payloads it passes a pointer to the coreboot tables as an argument. SeaBIOS does not make use of this functionality and instead tries to find the coreboot tables via a pointer placed in lower memory.
I guess this was needed at some point in the past ...
Probably makes sense to add support for pointer passing to seabios, and depending on how long this is supported by coreboot maybe even remove the old code.
To improve compatibility when calling payloads from SeaBIOS call payloads with the coreboot table as an argument.
Looks reasonable.
diff --git a/src/fw/coreboot.c b/src/fw/coreboot.c index 7c0954b..25317ca 100644 --- a/src/fw/coreboot.c +++ b/src/fw/coreboot.c @@ -518,9 +518,10 @@ cbfs_run_payload(struct cbfs_file *fhdr) memset(dest, 0, dest_len); break; case PAYLOAD_SEGMENT_ENTRY: {
dprintf(1, "Calling addr %p\n", dest);void (*func)(void) = dest;func();
void *arg = find_cb_table();dprintf(1, "Calling addr %p(%p)\n", dest, arg);
Minor issue: tab sneaked in here (seabios uses spaces for intention).
cheers, Gerd
Gerd Hoffmann kraxel@redhat.com writes:
On Fri, Oct 18, 2019 at 09:39:01PM +0200, Arthur Heymans wrote:
When coreboot calls payloads it passes a pointer to the coreboot tables as an argument. SeaBIOS does not make use of this functionality and instead tries to find the coreboot tables via a pointer placed in lower memory.
I guess this was needed at some point in the past ...
It looks like I'm wrong about this. On x86 coreboot doesn't call programs (both stages and payloads) with arguments (pushed to the stack on protected mode). Therefore all coreboot payloads need to fetch a pointer to the coreboot tables from lower memory like SeaBIOS does.
For now this patch is not really useful, but I'm have some patches changing that behavior, so I'll keep you updated.
Sorry for the noise.
diff --git a/src/fw/coreboot.c b/src/fw/coreboot.c index 7c0954b..25317ca 100644 --- a/src/fw/coreboot.c +++ b/src/fw/coreboot.c @@ -518,9 +518,10 @@ cbfs_run_payload(struct cbfs_file *fhdr) memset(dest, 0, dest_len); break; case PAYLOAD_SEGMENT_ENTRY: {
dprintf(1, "Calling addr %p\n", dest);void (*func)(void) = dest;func();
void *arg = find_cb_table();dprintf(1, "Calling addr %p(%p)\n", dest, arg);Minor issue: tab sneaked in here (seabios uses spaces for intention).
Right. My editor was configured for using TABs for indentation.
cheers, Gerd
-
Arthur Heymans -
Gerd Hoffmann