Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31593
Change subject: mb/emulation/qemu: Fix fw_cfg regression ......................................................................
mb/emulation/qemu: Fix fw_cfg regression
The change I48cc943aaa999e4323e9d7e5dd666c5316533dcc "mb/emulation/qemu-i440fx: change file handling"
introduced a regression where it loads only 4 bytes of the ACPI table, instead of the whole table.
Load the whole ACPI table.
Tested on Qemu using GNU/Linux.
Change-Id: Ibacbf7caab9be5f181c12e9dd39a2893b13cf6c9 Signed-off-by: Patrick Rudolph siro@das-labor.org --- M src/mainboard/emulation/qemu-i440fx/fw_cfg.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/31593/1
diff --git a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c index 6a9ef03..a029b38 100644 --- a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c +++ b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c @@ -251,7 +251,7 @@
printk(BIOS_DEBUG, "QEMU: loading "%s" to 0x%lx (len %d)\n", s[i].alloc.file, current, f.size); - fw_cfg_get(f.select, (void *)current, sizeof(current)); + fw_cfg_get(f.select, (void *)current, f.size); addrs[i] = current; current += f.size; break;
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31593 )
Change subject: mb/emulation/qemu: Fix fw_cfg regression ......................................................................
Patch Set 1: Code-Review+2
(2 comments)
Thanks for chasing this down!
https://review.coreboot.org/#/c/31593/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31593/1//COMMIT_MSG@7 PS1, Line 7: mb/emulation/qemu: Fix fw_cfg regression You are not fixing a regression, you are fixing ACPI (table loading). The regression is the reason to fix something.
https://review.coreboot.org/#/c/31593/1//COMMIT_MSG@9 PS1, Line 9: The change I48cc943aaa999e4323e9d7e5dd666c5316533dcc Please use a (shortened) commit hash.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31593 )
Change subject: mb/emulation/qemu: Fix fw_cfg regression ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31593/1/src/mainboard/emulation/qemu-i440fx/... File src/mainboard/emulation/qemu-i440fx/fw_cfg.c:
https://review.coreboot.org/#/c/31593/1/src/mainboard/emulation/qemu-i440fx/... PS1, Line 457: Same problem here, I guess.
Hello Kyösti Mälkki, Arthur Heymans, Thomas Heijligen, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31593
to look at the new patch set (#2).
Change subject: mb/emulation/qemu: Fix fw_cfg file loading ......................................................................
mb/emulation/qemu: Fix fw_cfg file loading
The change bcd84fe "mb/emulation/qemu-i440fx: change file handling" introduced a regression where it loads only 4 bytes of the ACPI and SMBIOS table, instead of the whole table.
Load the whole ACPI and SMBIOS table.
Tested on Qemu using GNU/Linux.
Change-Id: Ibacbf7caab9be5f181c12e9dd39a2893b13cf6c9 Signed-off-by: Patrick Rudolph siro@das-labor.org --- M src/mainboard/emulation/qemu-i440fx/fw_cfg.c 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/31593/2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31593 )
Change subject: mb/emulation/qemu: Fix fw_cfg file loading ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/#/c/31593/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31593/1//COMMIT_MSG@7 PS1, Line 7: mb/emulation/qemu: Fix fw_cfg regression
You are not fixing a regression, you are fixing ACPI […]
Done
https://review.coreboot.org/#/c/31593/1//COMMIT_MSG@9 PS1, Line 9: The change I48cc943aaa999e4323e9d7e5dd666c5316533dcc
Please use a (shortened) commit hash.
Done
https://review.coreboot.org/#/c/31593/1/src/mainboard/emulation/qemu-i440fx/... File src/mainboard/emulation/qemu-i440fx/fw_cfg.c:
https://review.coreboot.org/#/c/31593/1/src/mainboard/emulation/qemu-i440fx/... PS1, Line 457:
Same problem here, I guess.
Good catch
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31593 )
Change subject: mb/emulation/qemu: Fix fw_cfg file loading ......................................................................
Patch Set 2: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31593 )
Change subject: mb/emulation/qemu: Fix fw_cfg file loading ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31593 )
Change subject: mb/emulation/qemu: Fix fw_cfg file loading ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31593 )
Change subject: mb/emulation/qemu: Fix fw_cfg file loading ......................................................................
Patch Set 2: Code-Review+1
I also tested this with QEMU Q35. Please submit.
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31593 )
Change subject: mb/emulation/qemu: Fix fw_cfg file loading ......................................................................
mb/emulation/qemu: Fix fw_cfg file loading
The change bcd84fe "mb/emulation/qemu-i440fx: change file handling" introduced a regression where it loads only 4 bytes of the ACPI and SMBIOS table, instead of the whole table.
Load the whole ACPI and SMBIOS table.
Tested on Qemu using GNU/Linux.
Change-Id: Ibacbf7caab9be5f181c12e9dd39a2893b13cf6c9 Signed-off-by: Patrick Rudolph siro@das-labor.org Reviewed-on: https://review.coreboot.org/c/31593 Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/emulation/qemu-i440fx/fw_cfg.c 1 file changed, 2 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve Philipp Deppenwiese: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c index 6a9ef03..f43c280 100644 --- a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c +++ b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c @@ -251,7 +251,7 @@
printk(BIOS_DEBUG, "QEMU: loading "%s" to 0x%lx (len %d)\n", s[i].alloc.file, current, f.size); - fw_cfg_get(f.select, (void *)current, sizeof(current)); + fw_cfg_get(f.select, (void *)current, f.size); addrs[i] = current; current += f.size; break; @@ -454,7 +454,7 @@ * We'll exclude the end marker as coreboot will add one. */ printk(BIOS_DEBUG, "QEMU: loading smbios tables to 0x%lx\n", start); - fw_cfg_get(f.select, (void *)start, sizeof(start)); + fw_cfg_get(f.select, (void *)start, f.size); end = start; do { t0 = (struct smbios_type0*)end;
-
Angel Pons (Code Review) -
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Philipp Deppenwiese (Code Review)