[L] Change in coreboot[main]: [UNTESTED] soc/amd/common/psp: add support for early PSP v2 access vi...
Attention is currently required from: Felix Held.
Hello Felix Held,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/86106?usp=email
to review the following change.
Change subject: [UNTESTED] soc/amd/common/psp: add support for early PSP v2 access via SMN ......................................................................
[UNTESTED] soc/amd/common/psp: add support for early PSP v2 access via SMN
copy of https://review.coreboot.org/c/coreboot/+/85242
The MMIO mapping of the PSP mailbox registers can only be accessed once the FSP or in the future openSIL have initialized the PSP MMIO region. Before that, the PSP mailbox registers can only be accessed via their SMN mapping. When using the AMD A/B recovery scheme, which is not to be confused with the VBOOT RO/A/B recovery scheme, we need to detect in bootblock if we're running the A or the B part of the image to then load the correct romstage. To retrieve this info from the PSP, we'll need to use the SMN mapping, since the MMIO mapping hasn't been set up at that point in the boot process. Once the PSP MMIO is set up, we should however still use that one. In order to accomplish this, factor out the actual PSP register access and have separate register access functions that either use SMN or MMIO to access the registers. To add the functions for the early access via SMN in bootblock and romstage to the build, the SOC_AMD_COMMON_BLOCK_PSP_GEN2_EARLY_ACCESS Kconfig option is introduced. Since the base address of the SMN and the MMIO access is different, an access method specific 'get_psp_mmio_base' is implemented; the register offsets from those SMN and MMIO bases is identical.
TEST=TODO
Change-Id: I863cc94e882b4558dcdbb18d8d7ad42e3580c878 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/soc/amd/common/block/psp/Kconfig M src/soc/amd/common/block/psp/Makefile.mk M src/soc/amd/common/block/psp/psb.c M src/soc/amd/common/block/psp/psp_def.h M src/soc/amd/common/block/psp/psp_gen2.c A src/soc/amd/common/block/psp/psp_gen2_mmio.c A src/soc/amd/common/block/psp/psp_gen2_smn.c 7 files changed, 177 insertions(+), 100 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/86106/1
diff --git a/src/soc/amd/common/block/psp/Kconfig b/src/soc/amd/common/block/psp/Kconfig index 17853fa..018e87a 100644 --- a/src/soc/amd/common/block/psp/Kconfig +++ b/src/soc/amd/common/block/psp/Kconfig @@ -20,6 +20,14 @@ help Used by the PSP in AMD family 17h, 19h and possibly newer CPUs.
+config SOC_AMD_COMMON_BLOCK_PSP_GEN2_EARLY_ACCESS + bool + depends on SOC_AMD_COMMON_BLOCK_PSP_GEN2 + select SOC_AMD_COMMON_BLOCK_SMN + help + Allow PSP mailbox access in bootblock and romstage using the SMN + access method to the mailbox registers. + config SOC_AMD_PSP_SELECTABLE_SMU_FW bool help diff --git a/src/soc/amd/common/block/psp/Makefile.mk b/src/soc/amd/common/block/psp/Makefile.mk index dc811f90..91d9766 100644 --- a/src/soc/amd/common/block/psp/Makefile.mk +++ b/src/soc/amd/common/block/psp/Makefile.mk @@ -1,7 +1,6 @@ ## SPDX-License-Identifier: GPL-2.0-only ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_PSP),y)
-romstage-y += psp.c ramstage-y += psp.c smm-y += psp.c smm-$(CONFIG_SOC_AMD_COMMON_BLOCK_PSP_SMI) += psp_smi.c @@ -15,6 +14,7 @@
ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_PSP_GEN1),y)
+romstage-y += psp.c romstage-y += psp_gen1.c ramstage-y += psp_gen1.c
@@ -25,12 +25,22 @@
ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_PSP_GEN2),y)
+ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_PSP_GEN2_EARLY_ACCESS),y) +bootblock-y += psp.c +bootblock-y += psp_gen2.c +bootblock-y += psp_gen2_smn.c +romstage-y += psp.c romstage-y += psp_gen2.c +romstage-y += psp_gen2_smn.c +endif # CONFIG_SOC_AMD_COMMON_BLOCK_PSP_GEN2_EARLY_ACCESS + ramstage-y += psp_gen2.c +ramstage-y += psp_gen2_mmio.c ramstage-$(CONFIG_PSP_PLATFORM_SECURE_BOOT) += psb.c ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_I2C3_TPM_SHARED_WITH_PSP) += tpm.c
smm-y += psp_gen2.c +smm-y += psp_gen2_mmio.c smm-$(CONFIG_SOC_AMD_COMMON_BLOCK_PSP_SMI) += psp_smi_flash_gen2.c smm-y += psp_smm_gen2.c
diff --git a/src/soc/amd/common/block/psp/psb.c b/src/soc/amd/common/block/psp/psb.c index 5e497a2..71b766b 100644 --- a/src/soc/amd/common/block/psp/psb.c +++ b/src/soc/amd/common/block/psp/psb.c @@ -84,13 +84,13 @@
static enum cb_err get_psb_status(uint32_t *psb_status_value) { - const uintptr_t psp_mmio = get_psp_mmio_base(); + const uint64_t base = get_psp_base();
- if (!psp_mmio) { + if (!base) { printk(BIOS_WARNING, "PSP: PSP_ADDR_MSR uninitialized\n"); return CB_ERR; } - *psb_status_value = read32p(psp_mmio | PSB_STATUS_OFFSET); + *psb_status_value = psp_read32(base, PSB_STATUS_OFFSET); return CB_SUCCESS; }
diff --git a/src/soc/amd/common/block/psp/psp_def.h b/src/soc/amd/common/block/psp/psp_def.h index 369ef08..ff58dab 100644 --- a/src/soc/amd/common/block/psp/psp_def.h +++ b/src/soc/amd/common/block/psp/psp_def.h @@ -146,7 +146,11 @@ MBOX_PSP_SPI_BUSY = 0x0b, };
-uintptr_t get_psp_mmio_base(void); +/* TODO */ +uint64_t psp_get_base(void); +uint32_t psp_read32(uint64_t base, uint32_t offset); +void psp_write32(uint64_t base, uint32_t offset, uint32_t data); +void psp_write64(uint64_t base, uint32_t offset, uint64_t data);
void psp_print_cmd_status(int cmd_status, struct mbox_buffer_header *header);
diff --git a/src/soc/amd/common/block/psp/psp_gen2.c b/src/soc/amd/common/block/psp/psp_gen2.c index 026d32f..04e7ba1 100644 --- a/src/soc/amd/common/block/psp/psp_gen2.c +++ b/src/soc/amd/common/block/psp/psp_gen2.c @@ -1,86 +1,15 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <amdblocks/psp.h> +#include <console/console.h> +#include <device/mmio.h> #include <timer.h> #include <types.h> -#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 CONFIG_PSPV2_MBOX_CMD_OFFSET /* 4 bytes */ #define PSP_MAILBOX_BUFFER_OFFSET (CONFIG_PSPV2_MBOX_CMD_OFFSET + 4) /* 8 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 { uint32_t val; struct pspv2_mbox_cmd_fields { @@ -92,37 +21,37 @@ } __packed fields; };
-static uint16_t rd_mbox_sts(uintptr_t psp_mmio) +static uint16_t rd_mbox_sts(uint64_t base) { union pspv2_mbox_command tmp;
- tmp.val = read32p(psp_mmio | PSP_MAILBOX_COMMAND_OFFSET); + tmp.val = psp_read32(base, PSP_MAILBOX_COMMAND_OFFSET); return tmp.fields.mbox_status; }
-static void wr_mbox_cmd(uintptr_t psp_mmio, uint8_t cmd) +static void wr_mbox_cmd(uint64_t base, uint8_t cmd) { union pspv2_mbox_command tmp = { .val = 0 };
/* Write entire 32-bit area to begin command execution */ tmp.fields.mbox_command = cmd; - write32p(psp_mmio | PSP_MAILBOX_COMMAND_OFFSET, tmp.val); + psp_write32(base, PSP_MAILBOX_COMMAND_OFFSET, tmp.val); }
-static uint8_t rd_mbox_recovery(uintptr_t psp_mmio) +static uint8_t rd_mbox_recovery(uint64_t base) { union pspv2_mbox_command tmp;
- tmp.val = read32p(psp_mmio | PSP_MAILBOX_COMMAND_OFFSET); + tmp.val = psp_read32(base, PSP_MAILBOX_COMMAND_OFFSET); return !!tmp.fields.recovery; }
-static void wr_mbox_buffer_ptr(uintptr_t psp_mmio, void *buffer) +static void wr_mbox_buffer_ptr(uint64_t base, void *buffer) { - write64p(psp_mmio | PSP_MAILBOX_BUFFER_OFFSET, (uintptr_t)buffer); + psp_write64(base, PSP_MAILBOX_BUFFER_OFFSET, (uintptr_t)buffer); }
-static int wait_command(uintptr_t psp_mmio, bool wait_for_ready) +static int wait_command(uint64_t base, bool wait_for_ready) { union pspv2_mbox_command and_mask = { .val = ~0 }; union pspv2_mbox_command expected = { .val = 0 }; @@ -140,7 +69,7 @@ stopwatch_init_msecs_expire(&sw, PSP_CMD_TIMEOUT);
do { - tmp = read32p(psp_mmio | PSP_MAILBOX_COMMAND_OFFSET); + tmp = psp_read32(base, PSP_MAILBOX_COMMAND_OFFSET); tmp &= ~and_mask.val; if (tmp == expected.val) return 0; @@ -151,27 +80,27 @@
int send_psp_command(uint32_t command, void *buffer) { - const uintptr_t psp_mmio = get_psp_mmio_base(); - if (!psp_mmio) + const uint64_t base = psp_get_base(); + if (!base) return -PSPSTS_NOBASE;
- if (rd_mbox_recovery(psp_mmio)) + if (rd_mbox_recovery(base)) return -PSPSTS_RECOVERY;
- if (wait_command(psp_mmio, true)) + if (wait_command(base, true)) return -PSPSTS_CMD_TIMEOUT;
/* set address of command-response buffer and write command register */ - wr_mbox_buffer_ptr(psp_mmio, buffer); - wr_mbox_cmd(psp_mmio, command); + wr_mbox_buffer_ptr(base, buffer); + wr_mbox_cmd(base, command);
/* PSP clears command register when complete. All commands except * SxInfo set the Ready bit. */ - if (wait_command(psp_mmio, command != MBOX_BIOS_CMD_SX_INFO)) + if (wait_command(base, command != MBOX_BIOS_CMD_SX_INFO)) return -PSPSTS_CMD_TIMEOUT;
/* check delivery status */ - if (rd_mbox_sts(psp_mmio)) + if (rd_mbox_sts(base)) return -PSPSTS_SEND_ERROR;
return 0; @@ -202,12 +131,12 @@
enum cb_err soc_read_c2p38(uint32_t *msg_38_value) { - const uintptr_t psp_mmio = get_psp_mmio_base(); + const uint64_t base = psp_get_base();
- if (!psp_mmio) { + if (!base) { printk(BIOS_WARNING, "PSP: PSP_ADDR_MSR uninitialized\n"); return CB_ERR; } - *msg_38_value = read32p(psp_mmio | CORE_2_PSP_MSG_38_OFFSET); + *msg_38_value = psp_read32(base, CORE_2_PSP_MSG_38_OFFSET); return CB_SUCCESS; } diff --git a/src/soc/amd/common/block/psp/psp_gen2_mmio.c b/src/soc/amd/common/block/psp/psp_gen2_mmio.c new file mode 100644 index 0000000..14f0327 --- /dev/null +++ b/src/soc/amd/common/block/psp/psp_gen2_mmio.c @@ -0,0 +1,99 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <amdblocks/psp.h> +#include <amdblocks/root_complex.h> +#include <amdblocks/smn.h> +#include <console/console.h> +#include <device/mmio.h> +#include <types.h> +#include "psp_def.h" + +#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 */ +static 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; +} + +uint64_t psp_get_base(void) +{ + return get_psp_mmio_base(); +} + +uint32_t psp_read32(uint64_t base, uint32_t offset) +{ + return read32p((uintptr_t)(base | offset)); +} + +void psp_write32(uint64_t base, uint32_t offset, uint32_t data) +{ + write32p((uintptr_t)(base | offset), data); +} + +void psp_write64(uint64_t base, uint32_t offset, uint64_t data) +{ + write64p((uintptr_t)(base | offset), data); +} diff --git a/src/soc/amd/common/block/psp/psp_gen2_smn.c b/src/soc/amd/common/block/psp/psp_gen2_smn.c new file mode 100644 index 0000000..45b7487 --- /dev/null +++ b/src/soc/amd/common/block/psp/psp_gen2_smn.c @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <amdblocks/psp.h> +#include <amdblocks/smn.h> +#include <types.h> +#include "psp_def.h" + +uint64_t psp_get_base(void) +{ + return SMN_PSP_PUBLIC_BASE; +} + +uint32_t psp_read32(uint64_t base, uint32_t offset) +{ + return smn_read32(base | offset); +} + +void psp_write32(uint64_t base, uint32_t offset, uint32_t data) +{ + smn_write32(base | offset, data); +} + +void psp_write64(uint64_t base, uint32_t offset, uint64_t data) +{ + smn_write32(base | (offset + sizeof(uint32_t)), data >> 32); + smn_write32(base | offset, data & 0xffffffff); +}
-
Bao Zheng (Code Review)