Attention is currently required from: Arthur Heymans.
Hello Arthur Heymans,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/69326
to review the following change.
Change subject: acpi/acpi.c: Fix einj generation pointer arithmetics ......................................................................
acpi/acpi.c: Fix einj generation pointer arithmetics
Without a cast the aritmetics of
tat = einj + sizeof(acpi_einj_smi_t)
is the same as
tat = (uintptr_t)einj + size(acpi_einj_smi_t) * size(acpi_einj_smi_t)
So it overshoots the intended offset by a lot.
This issue only came apparent because now einj is in the small IMD region which is close to TSEG. With the wrong aritmetics the tat pointer ended up inside TSEG which is not accessible from the OS causing exceptions.
TEST: observe that tat pointer is inside the small IBM below TSEG (0x78000000 on our setup). "acpi_create_einj trigger_action_table = 0x77ffe89c"
Change-Id: I3ab64b95c33eef01b2048816a21e17855bcb2f54 Signed-off-by: Arthur Heymans arthur.heymans@9elements.com Signed-off-by: Jonathan Zhang jonzhang@meta.com --- M src/acpi/acpi.c 1 file changed, 31 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/69326/1
diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c index e211558..4d0c59d 100644 --- a/src/acpi/acpi.c +++ b/src/acpi/acpi.c @@ -26,6 +26,7 @@ #include <device/mmio.h> #include <device/pci.h> #include <pc80/mc146818rtc.h> +#include <stdint.h> #include <string.h> #include <types.h> #include <version.h> @@ -868,7 +869,7 @@
printk(BIOS_DEBUG, "%s einj_smi = %p\n", __func__, einj_smi); memset(einj_smi, 0, sizeof(acpi_einj_smi_t)); - tat = (acpi_einj_trigger_table_t *)(einj_smi + sizeof(acpi_einj_smi_t)); + tat = (acpi_einj_trigger_table_t *)((uint8_t *)einj_smi + sizeof(acpi_einj_smi_t)); tat->header_size = 16; tat->revision = 0; tat->table_size = sizeof(acpi_einj_trigger_table_t) +