Change in coreboot[master]: mb/emulation/qemu-q35: Control SMI related FADT entries

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41907 )

Change subject: mb/emulation/qemu-q35: Control SMI related FADT entries
......................................................................

mb/emulation/qemu-q35: Control SMI related FADT entries

Never advertise SMI in FADT for the emulation.

Change-Id: I170058793798648c6713de1530d89ec2ac53e39a
Signed-off-by: Kyösti Mälkki 
---
M src/mainboard/emulation/qemu-q35/acpi_tables.c
1 file changed, 7 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/41907/1

diff --git a/src/mainboard/emulation/qemu-q35/acpi_tables.c b/src/mainboard/emulation/qemu-q35/acpi_tables.c
index b9e76a9..4236257 100644
--- a/src/mainboard/emulation/qemu-q35/acpi_tables.c
+++ b/src/mainboard/emulation/qemu-q35/acpi_tables.c
@@ -42,11 +42,13 @@
    fadt->reserved = 0x00;
    fadt->preferred_pm_profile = PM_MOBILE;
    fadt->sci_int = 0x9;
-	fadt->smi_cmd = 0;
-	fadt->acpi_enable = 0;
-	fadt->acpi_disable = 0;
-	fadt->s4bios_req = 0x0;
-	fadt->pstate_cnt = 0;
+
+	if (CONFIG(HAVE_SMI_HANDLER)) {
+		/* Do not advertise SMI even if installed. */
+		fadt->smi_cmd = 0;
+		fadt->acpi_enable = 0;
+		fadt->acpi_disable = 0;
+	}

fadt->pm1a_evt_blk = pmbase;
    fadt->pm1b_evt_blk = 0x0;
@@ -65,7 +67,6 @@
    fadt->gpe0_blk_len = 16;
    fadt->gpe1_blk_len = 0;
    fadt->gpe1_base = 0;
-	fadt->cst_cnt = 0;
    fadt->p_lvl2_lat = 1;
    fadt->p_lvl3_lat = 0x39;
    fadt->flush_size = 0;

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41907 )

Change subject: mb/emulation/qemu-q35: Control SMI related FADT entries
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41907/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/41907/1//COMMIT_MSG@9 
PS1, Line 9: Never advertise SMI in FADT for the emulation.
Reason?

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41907 )

Change subject: mb/emulation/qemu-q35: Control SMI related FADT entries
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41907/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/41907/1//COMMIT_MSG@9 
PS1, Line 9: Never advertise SMI in FADT for the emulation.
> Reason?

That's the way it was done. But it may be from the days before SMI support in QEMU, need to dig into this.

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41907 )

Change subject: mb/emulation/qemu-q35: Control SMI related FADT entries
......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/41907/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/41907/1//COMMIT_MSG@9 
PS1, Line 9: Never advertise SMI in FADT for the emulation.
> That's the way it was done. […]

Right, would be nice to just mention that it's the status quo and could
change.

[Kyösti Mälkki (Code Review)]

Hello build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/41907

to look at the new patch set (#2).

Change subject: mb/emulation/qemu-q35: Control SMI related FADT entries
......................................................................

mb/emulation/qemu-q35: Control SMI related FADT entries

If running on older (like before 2.4.0) version of QEMU there is
no SMI support, so never advertise SMI in FADT for the emulation.
Behaviour if ACPI daemon tries to raise SMI under these conditions
is unknown.

Change-Id: I170058793798648c6713de1530d89ec2ac53e39a
Signed-off-by: Kyösti Mälkki 
---
M src/mainboard/emulation/qemu-q35/acpi_tables.c
1 file changed, 7 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/41907/2

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41907 )

Change subject: mb/emulation/qemu-q35: Control SMI related FADT entries
......................................................................

Patch Set 2: Code-Review+2

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41907 )

Change subject: mb/emulation/qemu-q35: Control SMI related FADT entries
......................................................................

Patch Set 2: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41907 )

Change subject: mb/emulation/qemu-q35: Control SMI related FADT entries
......................................................................

mb/emulation/qemu-q35: Control SMI related FADT entries

If running on older (like before 2.4.0) version of QEMU there is
no SMI support, so never advertise SMI in FADT for the emulation.
Behaviour if ACPI daemon tries to raise SMI under these conditions
is unknown.

Change-Id: I170058793798648c6713de1530d89ec2ac53e39a
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41907
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
Reviewed-by: Angel Pons 
---
M src/mainboard/emulation/qemu-q35/acpi_tables.c
1 file changed, 7 insertions(+), 6 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/mainboard/emulation/qemu-q35/acpi_tables.c b/src/mainboard/emulation/qemu-q35/acpi_tables.c
index b9e76a9..4236257 100644
--- a/src/mainboard/emulation/qemu-q35/acpi_tables.c
+++ b/src/mainboard/emulation/qemu-q35/acpi_tables.c
@@ -42,11 +42,13 @@
    fadt->reserved = 0x00;
    fadt->preferred_pm_profile = PM_MOBILE;
    fadt->sci_int = 0x9;
-	fadt->smi_cmd = 0;
-	fadt->acpi_enable = 0;
-	fadt->acpi_disable = 0;
-	fadt->s4bios_req = 0x0;
-	fadt->pstate_cnt = 0;
+
+	if (CONFIG(HAVE_SMI_HANDLER)) {
+		/* Do not advertise SMI even if installed. */
+		fadt->smi_cmd = 0;
+		fadt->acpi_enable = 0;
+		fadt->acpi_disable = 0;
+	}

fadt->pm1a_evt_blk = pmbase;
    fadt->pm1b_evt_blk = 0x0;
@@ -65,7 +67,6 @@
    fadt->gpe0_blk_len = 16;
    fadt->gpe1_blk_len = 0;
    fadt->gpe1_base = 0;
-	fadt->cst_cnt = 0;
    fadt->p_lvl2_lat = 1;
    fadt->p_lvl3_lat = 0x39;
    fadt->flush_size = 0;

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41907 )

Change subject: mb/emulation/qemu-q35: Control SMI related FADT entries
......................................................................

Patch Set 3:

Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4
Emulation targets:
"QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/4962
"QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4961
"QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4960
"QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/4959

Please note: This test is under development and might not be accurate at all!