Attention is currently required from: David Wu, Karthik Ramasubramanian, Ren Kuo, Shelley Chen, Subrata Banik, Tyler Wang.
Hello David Wu, Karthik Ramasubramanian, Shelley Chen, Subrata Banik, Tyler Wang, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83212?usp=email
to look at the new patch set (#3).
Change subject: mb/google/brox: Create jubilant variant
......................................................................
mb/google/brox: Create jubilant variant
Create the jubilant variant of the brox reference board by copying
the template files to a new directory named for the variant.
BUG=b:348543712
TEST=util/abuild/abuild -p none -t google/brox -x -a
make sure the build includes GOOGLE_JUBILANT.
Change-Id: Ic54437697058f8bce2167093bd88c0880d1b7cac
Signed-off-by: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
---
M src/mainboard/google/brox/Kconfig
M src/mainboard/google/brox/Kconfig.name
A src/mainboard/google/brox/variants/jubilant/Makefile.mk
A src/mainboard/google/brox/variants/jubilant/data.vbt
A src/mainboard/google/brox/variants/jubilant/fw_config.c
A src/mainboard/google/brox/variants/jubilant/gpio.c
A src/mainboard/google/brox/variants/jubilant/include/variant/ec.h
A src/mainboard/google/brox/variants/jubilant/include/variant/gpio.h
A src/mainboard/google/brox/variants/jubilant/include/variant/hda_verb.h
A src/mainboard/google/brox/variants/jubilant/memory.c
A src/mainboard/google/brox/variants/jubilant/memory/Makefile.mk
A src/mainboard/google/brox/variants/jubilant/memory/dram_id.generated.txt
A src/mainboard/google/brox/variants/jubilant/memory/mem_parts_used.txt
A src/mainboard/google/brox/variants/jubilant/overridetree.cb
A src/mainboard/google/brox/variants/jubilant/ramstage.c
A src/mainboard/google/brox/variants/jubilant/variant.c
16 files changed, 1,045 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/83212/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/83212?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic54437697058f8bce2167093bd88c0880d1b7cac
Gerrit-Change-Number: 83212
Gerrit-PatchSet: 3
Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Attention is currently required from: Eran Mitrani, Jakub Czapiga, Kapil Porwal, Karthikeyan Ramasubramanian, Nico Huber, Paul Menzel, Rishika Raj, Subrata Banik, Tarun.
Julius Werner has posted comments on this change by Rishika Raj. ( https://review.coreboot.org/c/coreboot/+/83540?usp=email )
Change subject: soc/intel/mtl: Increase CAR_STACK_SIZE by 31KB for coreboot compatibility
......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS8:
> > FWIW, Karthik just mentioned that the numbers in alderlake/Kconfig are actually wrong, and the FSP […]
Gotcha, then please proceed with this patch.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83540?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iba3620b3b7c470176330f5e07989cd3f6238713e
Gerrit-Change-Number: 83540
Gerrit-PatchSet: 8
Gerrit-Owner: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Fri, 26 Jul 2024 00:35:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Paul Menzel, Yu-Ping Wu.
Julius Werner has posted comments on this change by Yu-Ping Wu. ( https://review.coreboot.org/c/coreboot/+/83652?usp=email )
Change subject: arch/arm64/armv8/mmu: Improve log format
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83652/comment/07d13698_86b02702?us… :
PS2, Line 9: When using format string with "%p", "(nil)" will be printed for address
I'm curious where you see this? coreboot's `vtxprintf()` doesn't do this as far as I can tell.
File src/arch/arm64/armv8/mmu.c:
https://review.coreboot.org/c/coreboot/+/83652/comment/68287cb9_d95e392e?us… :
PS2, Line 60: /* Func : table_level_name
: * Desc : Get the descriptions table level name from the given size.
: */
> Unrelated: Not one of the recommended multi-line comment styles.
The code in this file precedes those style decisions, and until someone has time to update the whole file we should probably keep things consistent inside it.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83652?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib29c201e1b096b9c7cd750d2541923616bc858ac
Gerrit-Change-Number: 83652
Gerrit-PatchSet: 2
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Fri, 26 Jul 2024 00:26:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Elyes Haouas, Martin L Roth, Matt DeVillier.
Julius Werner has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/81026?usp=email )
Change subject: Makefiles: Download 3rdparty/chromeec when needed
......................................................................
Patch Set 4:
(4 comments)
File .gitignore:
https://review.coreboot.org/c/coreboot/+/81026/comment/720c837f_190f6691?us… :
PS4, Line 15: 3rdparty
I think we should probably separate this into its own directory (`3rdparty-local`?) because otherwise it may be confusing which things in 3rdparty are tracked through the submodule system and which are ignored.
File 3rdparty/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/81026/comment/3f2a9a35_a5a1c286?us… :
PS4, Line 4: $(CONFIG_EC_GOOGLE_CHROMEEC_FIRMWARE_BUILTIN),
Is this the right check? Isn't `$(CONFIG_EC_GOOGLE_CHROMEEC_FIRMWARE_BUILTIN)` always either `y` or `n` (i.e. not empty)?
https://review.coreboot.org/c/coreboot/+/81026/comment/6c941dfe_916d528f?us… :
PS4, Line 9: CONFIG_CHROMEEC_COMMIT := e486b388a7
Shouldn't this hash just be the Kconfig default value? (Also, you should be adding those new options to Kconfig with this same patch.)
https://review.coreboot.org/c/coreboot/+/81026/comment/fa3b55dd_cfe68913?us… :
PS4, Line 15: CHROMEEC_GIT_TASK += $(shell cd $(CHROMEEC_DIR) && git checkout $(CONFIG_CHROMEEC_COMMIT) 2>&1)
Shouldn't we do the checkout in both cases (even if the directory exists, in case the current hash changed)? Otherwise, I'm not quite sure why you fetch if you don't checkout.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81026?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I1f23b9f98a7ab31ae3ea1cb5aac136ead50769d6
Gerrit-Change-Number: 81026
Gerrit-PatchSet: 4
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Fri, 26 Jul 2024 00:17:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Elyes Haouas, Martin L Roth, Matt DeVillier.
Julius Werner has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/81024?usp=email )
Change subject: 3rdparty: Remove chromeec submodule
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/81024?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I357c4c9b506dd3817a308232446144ae889bc220
Gerrit-Change-Number: 81024
Gerrit-PatchSet: 7
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Fri, 26 Jul 2024 00:06:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83448?usp=email )
Change subject: soc/amd/common/psp_gen2: use MMIO access again
......................................................................
soc/amd/common/psp_gen2: use MMIO access again
Now that we have a get_psp_mmio_base function that will work on all SoCs
that use the psp_gen2 code, we can move back to accessing the PSP
registers via their MMIO mapping. This sort-of reverts
commit 198cc26e4951 ("soc/amd/common/block/psp/psp_gen2: use SMN access
to PSP").
When doing SMN accesses from the SMI handler after the OS has taken over
ownership of the platform, there's the possibility to cause trouble by
clobbering the SMN access index register from SMM. So that should be
either avoided completely or the SMI code needs to save and restore the
original contents of the SMN index register.
The PSP MMIO base will be set up by the FSP before the resource
allocation in coreboot and be treated like a fixed resource by the
allocator. The first SMI where corresponding handler calls
'get_psp_mmio_base' happens when ramstage triggers the APM_CNT_SMMINFO
SMI right after mpinit which happens after the resource allocation. So
the PSP MMIO base address is expected to be configured and so the
'get_psp_mmio_base' function will cache the base address and won't need
to do any SMN access in subsequent calls that might happen after the OS
has take over control.
This isn't currently an issue, since the only PSP mailbox command from
the SMI handler after coreboot is done and the OS has taken over will
be during the S3/S4/S5 entry, and this will be triggered by the OS as
the last step after it is done with all its preparations for suspend/
shutdown. There will however be future patches that add SMI-handlers
which can send PSP mailbox commands during OS runtime, and so we have
to make sure we don't clobber the SMN index register.
TEST=PSP mailbox commands are still sent correctly on Mandolin.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I25f16d575991021d65b7b578956d9f90bfd15f6c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83448
Reviewed-by: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Reviewed-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/soc/amd/common/block/psp/psb.c
M src/soc/amd/common/block/psp/psp_gen2.c
2 files changed, 43 insertions(+), 26 deletions(-)
Approvals:
Matt DeVillier: Looks good to me, approved
Martin Roth: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/src/soc/amd/common/block/psp/psb.c b/src/soc/amd/common/block/psp/psb.c
index a537bff..4ac7aaf 100644
--- a/src/soc/amd/common/block/psp/psb.c
+++ b/src/soc/amd/common/block/psp/psb.c
@@ -1,7 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
#include <amdblocks/reset.h>
-#include <amdblocks/smn.h>
#include <bootstate.h>
#include <console/console.h>
#include <device/mmio.h>
@@ -83,9 +82,16 @@
}
}
-static uint32_t get_psb_status(void)
+static enum cb_err get_psb_status(uint32_t *psb_status_value)
{
- return smn_read32(SMN_PSP_PUBLIC_BASE + PSB_STATUS_OFFSET);
+ const uintptr_t psp_mmio = get_psp_mmio_base();
+
+ if (!psp_mmio) {
+ printk(BIOS_WARNING, "PSP: PSP_ADDR_MSR uninitialized\n");
+ return CB_ERR;
+ }
+ *psb_status_value = read32p(psp_mmio | PSB_STATUS_OFFSET);
+ return CB_SUCCESS;
}
/*
@@ -102,7 +108,11 @@
}
};
- status = get_psb_status();
+ if (get_psb_status(&status) != CB_SUCCESS) {
+ printk(BIOS_ERR, "PSP: Failed to get base address.\n");
+ return CB_ERR;
+ }
+
printk(BIOS_INFO, "PSB: Status = %x\n", status);
if (status & FUSE_PLATFORM_SECURE_BOOT_EN) {
diff --git a/src/soc/amd/common/block/psp/psp_gen2.c b/src/soc/amd/common/block/psp/psp_gen2.c
index 8f0bdcf..bed31c1 100644
--- a/src/soc/amd/common/block/psp/psp_gen2.c
+++ b/src/soc/amd/common/block/psp/psp_gen2.c
@@ -5,11 +5,11 @@
#include <amdblocks/psp.h>
#include <amdblocks/root_complex.h>
#include <amdblocks/smn.h>
+#include <device/mmio.h>
#include "psp_def.h"
#define PSP_MAILBOX_COMMAND_OFFSET 0x10570 /* 4 bytes */
-#define PSP_MAILBOX_BUFFER_L_OFFSET 0x10574 /* 4 bytes */
-#define PSP_MAILBOX_BUFFER_H_OFFSET 0x10578 /* 4 bytes */
+#define PSP_MAILBOX_BUFFER_OFFSET 0x10574 /* 8 bytes */
#define IOHC_MISC_PSP_MMIO_REG 0x2e0
@@ -92,40 +92,37 @@
} __packed fields;
};
-static u16 rd_mbox_sts(void)
+static u16 rd_mbox_sts(uintptr_t psp_mmio)
{
union pspv2_mbox_command tmp;
- tmp.val = smn_read32(SMN_PSP_PUBLIC_BASE + PSP_MAILBOX_COMMAND_OFFSET);
+ tmp.val = read32p(psp_mmio | PSP_MAILBOX_COMMAND_OFFSET);
return tmp.fields.mbox_status;
}
-static void wr_mbox_cmd(u8 cmd)
+static void wr_mbox_cmd(uintptr_t psp_mmio, u8 cmd)
{
union pspv2_mbox_command tmp = { .val = 0 };
/* Write entire 32-bit area to begin command execution */
tmp.fields.mbox_command = cmd;
- smn_write32(SMN_PSP_PUBLIC_BASE + PSP_MAILBOX_COMMAND_OFFSET, tmp.val);
+ write32p(psp_mmio | PSP_MAILBOX_COMMAND_OFFSET, tmp.val);
}
-static u8 rd_mbox_recovery(void)
+static u8 rd_mbox_recovery(uintptr_t psp_mmio)
{
union pspv2_mbox_command tmp;
- tmp.val = smn_read32(SMN_PSP_PUBLIC_BASE + PSP_MAILBOX_COMMAND_OFFSET);
+ tmp.val = read32p(psp_mmio | PSP_MAILBOX_COMMAND_OFFSET);
return !!tmp.fields.recovery;
}
-static void wr_mbox_buffer_ptr(void *buffer)
+static void wr_mbox_buffer_ptr(uintptr_t psp_mmio, void *buffer)
{
- const uint32_t buf_addr_h = (uint64_t)(uintptr_t)buffer >> 32;
- const uint32_t buf_addr_l = (uint64_t)(uintptr_t)buffer & 0xffffffff;
- smn_write32(SMN_PSP_PUBLIC_BASE + PSP_MAILBOX_BUFFER_H_OFFSET, buf_addr_h);
- smn_write32(SMN_PSP_PUBLIC_BASE + PSP_MAILBOX_BUFFER_L_OFFSET, buf_addr_l);
+ write64p(psp_mmio | PSP_MAILBOX_BUFFER_OFFSET, (uintptr_t)buffer);
}
-static int wait_command(bool wait_for_ready)
+static int wait_command(uintptr_t psp_mmio, bool wait_for_ready)
{
union pspv2_mbox_command and_mask = { .val = ~0 };
union pspv2_mbox_command expected = { .val = 0 };
@@ -143,7 +140,7 @@
stopwatch_init_msecs_expire(&sw, PSP_CMD_TIMEOUT);
do {
- tmp = smn_read32(SMN_PSP_PUBLIC_BASE + PSP_MAILBOX_COMMAND_OFFSET);
+ tmp = read32p(psp_mmio | PSP_MAILBOX_COMMAND_OFFSET);
tmp &= ~and_mask.val;
if (tmp == expected.val)
return 0;
@@ -154,23 +151,27 @@
int send_psp_command(u32 command, void *buffer)
{
- if (rd_mbox_recovery())
+ const uintptr_t psp_mmio = get_psp_mmio_base();
+ if (!psp_mmio)
+ return -PSPSTS_NOBASE;
+
+ if (rd_mbox_recovery(psp_mmio))
return -PSPSTS_RECOVERY;
- if (wait_command(true))
+ if (wait_command(psp_mmio, true))
return -PSPSTS_CMD_TIMEOUT;
/* set address of command-response buffer and write command register */
- wr_mbox_buffer_ptr(buffer);
- wr_mbox_cmd(command);
+ wr_mbox_buffer_ptr(psp_mmio, buffer);
+ wr_mbox_cmd(psp_mmio, command);
/* PSP clears command register when complete. All commands except
* SxInfo set the Ready bit. */
- if (wait_command(command != MBOX_BIOS_CMD_SX_INFO))
+ if (wait_command(psp_mmio, command != MBOX_BIOS_CMD_SX_INFO))
return -PSPSTS_CMD_TIMEOUT;
/* check delivery status */
- if (rd_mbox_sts())
+ if (rd_mbox_sts(psp_mmio))
return -PSPSTS_SEND_ERROR;
return 0;
@@ -178,6 +179,12 @@
enum cb_err soc_read_c2p38(uint32_t *msg_38_value)
{
- *msg_38_value = smn_read32(SMN_PSP_PUBLIC_BASE + CORE_2_PSP_MSG_38_OFFSET);
+ const uintptr_t psp_mmio = get_psp_mmio_base();
+
+ if (!psp_mmio) {
+ printk(BIOS_WARNING, "PSP: PSP_ADDR_MSR uninitialized\n");
+ return CB_ERR;
+ }
+ *msg_38_value = read32p(psp_mmio | CORE_2_PSP_MSG_38_OFFSET);
return CB_SUCCESS;
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/83448?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I25f16d575991021d65b7b578956d9f90bfd15f6c
Gerrit-Change-Number: 83448
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: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83446?usp=email )
Change subject: soc/amd/common/block/psp_gen2: add get_psp_mmio_base
......................................................................
soc/amd/common/block/psp_gen2: add get_psp_mmio_base
Add get_psp_mmio_base which reads the PSP MMIO base address from the
hardware registers. Since this function will not only be called in
ramstage, but also in SMM, we can't just look for the specific domain
resource consumer like it is done for the IOAPICs in the northbridge,
but have to get this base address from the registers. In order to limit
the performance impact of this, the base address gets cached in a static
variable if an enabled PSP MMIO base register is found. We expect that
this register is locked when it was configured and enabled; if we run
into the unexpected case that the PSP MMIO register is enabled, but not
locked, set the lock bit of the corresponding base address register to
be sure that it won't change until the next reset and that the hardware
value can't be different than the cached value.
This is a preparation to move back to using MMIO access to the PSP
registers and will also enable cases that require the use of the MMIO
mapping of the PSP registers.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I1d51e30f186508b0fe1ab5eb79c73e6d4b9d1a4a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83446
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Reviewed-by: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
---
M src/soc/amd/common/block/psp/psp_def.h
M src/soc/amd/common/block/psp/psp_gen2.c
2 files changed, 73 insertions(+), 0 deletions(-)
Approvals:
Matt DeVillier: Looks good to me, approved
build bot (Jenkins): Verified
Martin Roth: Looks good to me, approved
diff --git a/src/soc/amd/common/block/psp/psp_def.h b/src/soc/amd/common/block/psp/psp_def.h
index 9def98b..f6efa21 100644
--- a/src/soc/amd/common/block/psp/psp_def.h
+++ b/src/soc/amd/common/block/psp/psp_def.h
@@ -108,6 +108,8 @@
#define PSP_INIT_TIMEOUT 10000 /* 10 seconds */
#define PSP_CMD_TIMEOUT 1000 /* 1 second */
+uintptr_t get_psp_mmio_base(void);
+
void psp_print_cmd_status(int cmd_status, struct mbox_buffer_header *header);
/* This command needs to be implemented by the generation specific code. */
diff --git a/src/soc/amd/common/block/psp/psp_gen2.c b/src/soc/amd/common/block/psp/psp_gen2.c
index f647dcd..7b92cc2 100644
--- a/src/soc/amd/common/block/psp/psp_gen2.c
+++ b/src/soc/amd/common/block/psp/psp_gen2.c
@@ -3,6 +3,7 @@
#include <timer.h>
#include <types.h>
#include <amdblocks/psp.h>
+#include <amdblocks/root_complex.h>
#include <amdblocks/smn.h>
#include "psp_def.h"
@@ -10,6 +11,76 @@
#define PSP_MAILBOX_BUFFER_L_OFFSET 0x10574 /* 4 bytes */
#define PSP_MAILBOX_BUFFER_H_OFFSET 0x10578 /* 4 bytes */
+#define IOHC_MISC_PSP_MMIO_REG 0x2e0
+
+static uint64_t get_psp_mmio_mask(void)
+{
+ const struct non_pci_mmio_reg *mmio_regs;
+ size_t reg_count;
+ mmio_regs = get_iohc_non_pci_mmio_regs(®_count);
+
+ for (size_t i = 0; i < reg_count; i++) {
+ if (mmio_regs[i].iohc_misc_offset == IOHC_MISC_PSP_MMIO_REG)
+ return mmio_regs[i].mask;
+ }
+
+ printk(BIOS_ERR, "No PSP MMIO register description found.\n");
+ return 0;
+}
+
+#define PSP_MMIO_LOCK BIT(8)
+
+/* Getting the PSP MMIO base from the domain resources only works in ramstage, but not in SMM,
+ so we have to read this from the hardware registers */
+uintptr_t get_psp_mmio_base(void)
+{
+ static uintptr_t psp_mmio_base;
+ const struct domain_iohc_info *iohc;
+ size_t iohc_count;
+
+ if (psp_mmio_base)
+ return psp_mmio_base;
+
+ iohc = get_iohc_info(&iohc_count);
+ const uint64_t psp_mmio_mask = get_psp_mmio_mask();
+
+ if (!psp_mmio_mask)
+ return 0;
+
+ for (size_t i = 0; i < iohc_count; i++) {
+ uint64_t reg64 = smn_read64(iohc[i].misc_smn_base | IOHC_MISC_PSP_MMIO_REG);
+
+ if (!(reg64 & IOHC_MMIO_EN))
+ continue;
+
+ const uint64_t base = reg64 & psp_mmio_mask;
+
+ if (ENV_X86_32 && base >= 4ull * GiB) {
+ printk(BIOS_WARNING, "PSP MMIO base above 4GB.\n");
+ continue;
+ }
+
+ /* If the PSP MMIO base is enabled but the register isn't locked, set the lock
+ bit. This shouldn't happen, but better be a bit too careful here */
+ if (!(reg64 & PSP_MMIO_LOCK)) {
+ printk(BIOS_WARNING, "Enabled PSP MMIO in domain %zu isn't locked. "
+ "Locking it.\n", i);
+ reg64 |= PSP_MMIO_LOCK;
+ /* Since the lock bit lives in the lower one of the two 32 bit SMN
+ registers, we only need to write that one to lock it */
+ smn_write32(iohc[i].misc_smn_base | IOHC_MISC_PSP_MMIO_REG,
+ reg64 & 0xffffffff);
+ }
+
+ psp_mmio_base = base;
+ }
+
+ if (!psp_mmio_base)
+ printk(BIOS_ERR, "No usable PSP MMIO found.\n");
+
+ return psp_mmio_base;
+}
+
union pspv2_mbox_command {
u32 val;
struct pspv2_mbox_cmd_fields {
--
To view, visit https://review.coreboot.org/c/coreboot/+/83446?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I1d51e30f186508b0fe1ab5eb79c73e6d4b9d1a4a
Gerrit-Change-Number: 83446
Gerrit-PatchSet: 4
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: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>