Attention is currently required from: Felix Held, Jérémy Compostella.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79835?usp=email )
Change subject: cpu/x86/smi_trigger: use enum cb_err as apm_control return type
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/79835?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I07ced74cae915df52a9d439835b84237d51fdd11
Gerrit-Change-Number: 79835
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 08 Jan 2024 14:58:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Jérémy Compostella.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79834?usp=email )
Change subject: arch/x86/include/smm: improve documentation of call_smm
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
File src/arch/x86/include/smm_call.h:
https://review.coreboot.org/c/coreboot/+/79834/comment/4f5ec4cb_d560f813 :
PS1, Line 9: can then read
how about 'will then read' or 'then reads'? 'can' makes it sound optional
--
To view, visit https://review.coreboot.org/c/coreboot/+/79834?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I3566af191492ce00a3033335ff80e01c33e98e63
Gerrit-Change-Number: 79834
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 08 Jan 2024 14:57:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Jérémy Compostella.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79833?usp=email )
Change subject: arch/x86/include: rename smm.h to smm_call.h
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/79833?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ia907ad92459e835feeddf7eb4743a38f99549179
Gerrit-Change-Number: 79833
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 08 Jan 2024 14:56:23 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Felix Held, Jérémy Compostella.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79568?usp=email )
Change subject: arch/x86/include/smm: use pm_acpi_smi_cmd_port
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/79568?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Icb79c91cfcd75db760bd80cff7f3d0400d1f16cd
Gerrit-Change-Number: 79568
Gerrit-PatchSet: 5
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 08 Jan 2024 14:56:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth, Varshit Pandya.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79564?usp=email )
Change subject: [UNTESTED] vc/amd/genoa_poc: make openSIL set up correct APMC IO port
......................................................................
Patch Set 7:
(1 comment)
Patchset:
PS7:
this patch might need a rework after the apmc smi io rework patches have landed
--
To view, visit https://review.coreboot.org/c/coreboot/+/79564?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I026b31dbf5b9151d6ad792ceacbab58c36c449b3
Gerrit-Change-Number: 79564
Gerrit-PatchSet: 7
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Comment-Date: Mon, 08 Jan 2024 14:27:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Máté Kukri, nat ✨.
Ben Westover has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55232?usp=email )
Change subject: mb/dell: Add OptiPlex 7020/9020 port
......................................................................
Patch Set 31:
(1 comment)
Patchset:
PS31:
> You have to short the service jumper near PCIe and then boot with iomem=relaxed
Yes, this is what I get with a jumper on the SERVICE_MODE header. It's the same result with or without the jumper (always booted with iomem=relaxed). Both in the original BIOS and in Coreboot. Service mode is definitely being enabled when I have that jumper set because the original BIOS gives me a warning about it on boot.
--
To view, visit https://review.coreboot.org/c/coreboot/+/55232?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie7c7089f443aef9890711c4412209bceb1f1e96a
Gerrit-Change-Number: 55232
Gerrit-PatchSet: 31
Gerrit-Owner: Máté Kukri <kukri.mate(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Michael Büchler <michael.buechler(a)posteo.net>
Gerrit-Reviewer: Patrick Rudolph <rudolphpatrick05(a)gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ben Westover <me(a)benthetechguy.net>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: nat ✨ <nat(a)nekopon.pl>
Gerrit-Attention: Máté Kukri <kukri.mate(a)gmail.com>
Gerrit-Attention: nat ✨ <nat(a)nekopon.pl>
Gerrit-Comment-Date: Mon, 08 Jan 2024 13:54:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: nat ✨ <nat(a)nekopon.pl>
Comment-In-Reply-To: Ben Westover <me(a)benthetechguy.net>
Gerrit-MessageType: comment
Attention is currently required from: Elyes Haouas, Martin L Roth.
Jan Samek has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77147?usp=email )
Change subject: Makefile: Identify deprecated one-element or zero-length arrays
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/77147?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib704f7659d3b431ce7eebb4432c5b1a4272de3d2
Gerrit-Change-Number: 77147
Gerrit-PatchSet: 2
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Mon, 08 Jan 2024 13:47:32 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/79567?usp=email )
Change subject: cpu/x86/smi_trigger: call pm_acpi_smi_cmd_port to get APMC SMI IO port
......................................................................
cpu/x86/smi_trigger: call pm_acpi_smi_cmd_port to get APMC SMI IO port
Instead of hard-coding the APMC SMI command IO port in the FADT, call
pm_acpi_smi_cmd_port() to get the APMC SMI command IO port. Also update
the comment in apm_get_apmc to match what it's doing.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I0f36b8a0e93a82b8c6d23c5c5d8fbebb1bc6b0bc
Reviewed-on: https://review.coreboot.org/c/coreboot/+/79567
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
---
M src/cpu/x86/smi_trigger.c
1 file changed, 3 insertions(+), 3 deletions(-)
Approvals:
build bot (Jenkins): Verified
Patrick Rudolph: Looks good to me, approved
diff --git a/src/cpu/x86/smi_trigger.c b/src/cpu/x86/smi_trigger.c
index 470ecde..9781642 100644
--- a/src/cpu/x86/smi_trigger.c
+++ b/src/cpu/x86/smi_trigger.c
@@ -38,7 +38,7 @@
apmc_log(__func__, cmd);
/* Now raise the SMI. */
- outb(cmd, APM_CNT);
+ outb(cmd, pm_acpi_smi_cmd_port());
printk(BIOS_DEBUG, "APMC done.\n");
return 0;
@@ -46,8 +46,8 @@
u8 apm_get_apmc(void)
{
- /* Emulate B2 register as the FADT / Linux expects it */
- u8 cmd = inb(APM_CNT);
+ /* Read command byte from APMC SMI IO port */
+ u8 cmd = inb(pm_acpi_smi_cmd_port());
apmc_log("SMI#", cmd);
return cmd;
--
To view, visit https://review.coreboot.org/c/coreboot/+/79567?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I0f36b8a0e93a82b8c6d23c5c5d8fbebb1bc6b0bc
Gerrit-Change-Number: 79567
Gerrit-PatchSet: 6
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/79565?usp=email )
(
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: arch/x86/acpi: call pm_acpi_smi_cmd_port to get APMC SMI IO port
......................................................................
arch/x86/acpi: call pm_acpi_smi_cmd_port to get APMC SMI IO port
Instead of hard-coding the APMC SMI command IO port in the FADT, call
pm_acpi_smi_cmd_port() to get the APMC SMI command IO port.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I731c780bc6db7e7fd59688340bab1da86fc93c11
Reviewed-on: https://review.coreboot.org/c/coreboot/+/79565
Reviewed-by: Jérémy Compostella <jeremy.compostella(a)intel.com>
Reviewed-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/arch/x86/acpi.c
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Patrick Rudolph: Looks good to me, approved
build bot (Jenkins): Verified
Jérémy Compostella: Looks good to me, but someone else must approve
diff --git a/src/arch/x86/acpi.c b/src/arch/x86/acpi.c
index 018cdf5..2497143 100644
--- a/src/arch/x86/acpi.c
+++ b/src/arch/x86/acpi.c
@@ -39,7 +39,7 @@
}
if (permanent_smi_handler()) {
- fadt->smi_cmd = APM_CNT;
+ fadt->smi_cmd = pm_acpi_smi_cmd_port();
fadt->acpi_enable = APM_CNT_ACPI_ENABLE;
fadt->acpi_disable = APM_CNT_ACPI_DISABLE;
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/79565?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I731c780bc6db7e7fd59688340bab1da86fc93c11
Gerrit-Change-Number: 79565
Gerrit-PatchSet: 7
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/79566?usp=email )
Change subject: arch/x86: introduce HAVE_CONFIGURABLE_APMC_SMI_PORT
......................................................................
arch/x86: introduce HAVE_CONFIGURABLE_APMC_SMI_PORT
Introduce the HAVE_CONFIGURABLE_APMC_SMI_PORT Kconfig option that when
not selected will result in a default implementation of
pm_acpi_smi_cmd_port to be included in the build that returns APM_CNT.
SoCs that provide their own pm_acpi_smi_cmd_port implementation, need to
select this Kconfig option.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Iaceb61b0f2a630d7afe2e0780b6a2a9806ea62f9
Reviewed-on: https://review.coreboot.org/c/coreboot/+/79566
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
---
M src/arch/x86/Kconfig
M src/arch/x86/Makefile.inc
A src/arch/x86/apmc_smi_port.c
M src/mainboard/emulation/qemu-q35/Makefile.inc
D src/mainboard/emulation/qemu-q35/smi.c
M src/soc/amd/common/block/smi/Kconfig
6 files changed, 22 insertions(+), 10 deletions(-)
Approvals:
build bot (Jenkins): Verified
Patrick Rudolph: Looks good to me, approved
diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig
index e149f08..0c11653 100644
--- a/src/arch/x86/Kconfig
+++ b/src/arch/x86/Kconfig
@@ -316,6 +316,13 @@
bool
depends on HAVE_CF9_RESET
+config HAVE_CONFIGURABLE_APMC_SMI_PORT
+ bool
+ help
+ SoCs that have a configurable APMC SMI command port, should select
+ this option and implement pm_acpi_smi_cmd_port() that returns the IO
+ port.
+
config PIRQ_ROUTE
bool
default n
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc
index 62294a6..04a0e58 100644
--- a/src/arch/x86/Makefile.inc
+++ b/src/arch/x86/Makefile.inc
@@ -336,3 +336,8 @@
smm-$(CONFIG_DEBUG_NULL_DEREF_BREAKPOINTS_IN_ALL_STAGES) += null_breakpoint.c
smm-srcs += $(wildcard src/mainboard/$(MAINBOARDDIR)/smihandler.c)
+
+ifneq ($(CONFIG_HAVE_CONFIGURABLE_APMC_SMI_PORT),y)
+ramstage-y += apmc_smi_port.c
+smm-y += apmc_smi_port.c
+endif
diff --git a/src/arch/x86/apmc_smi_port.c b/src/arch/x86/apmc_smi_port.c
new file mode 100644
index 0000000..ac8f336
--- /dev/null
+++ b/src/arch/x86/apmc_smi_port.c
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <cpu/x86/smm.h>
+
+/* default implementation of the !HAVE_CONFIGURABLE_APMC_SMI_PORT case */
+uint16_t pm_acpi_smi_cmd_port(void)
+{
+ return APM_CNT;
+}
diff --git a/src/mainboard/emulation/qemu-q35/Makefile.inc b/src/mainboard/emulation/qemu-q35/Makefile.inc
index f4a89be..bc73edc 100644
--- a/src/mainboard/emulation/qemu-q35/Makefile.inc
+++ b/src/mainboard/emulation/qemu-q35/Makefile.inc
@@ -20,5 +20,4 @@
ramstage-$(CONFIG_CHROMEOS) += chromeos.c
-smm-y += smi.c
smm-y += memmap.c
diff --git a/src/mainboard/emulation/qemu-q35/smi.c b/src/mainboard/emulation/qemu-q35/smi.c
deleted file mode 100644
index 5d8d482..0000000
--- a/src/mainboard/emulation/qemu-q35/smi.c
+++ /dev/null
@@ -1,9 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-
-#include <cpu/x86/smm.h>
-
-/* The X86 qemu target uses AMD64 save states but the APM port is not configurable. */
-uint16_t pm_acpi_smi_cmd_port(void)
-{
- return APM_CNT;
-}
diff --git a/src/soc/amd/common/block/smi/Kconfig b/src/soc/amd/common/block/smi/Kconfig
index b205437..5a3f42d 100644
--- a/src/soc/amd/common/block/smi/Kconfig
+++ b/src/soc/amd/common/block/smi/Kconfig
@@ -1,5 +1,6 @@
config SOC_AMD_COMMON_BLOCK_SMI
bool
+ select HAVE_CONFIGURABLE_APMC_SMI_PORT
help
Select this option to add the common functions for setting up the SMI
configuration to the build.
--
To view, visit https://review.coreboot.org/c/coreboot/+/79566?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iaceb61b0f2a630d7afe2e0780b6a2a9806ea62f9
Gerrit-Change-Number: 79566
Gerrit-PatchSet: 7
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged