From 89ca4788439ddcdbf60311cae6939211cf463a8c Mon Sep 17 00:00:00 2001 From: Felix Held Date: Tue, 12 Sep 2023 15:01:02 +0200 Subject: [PATCH] soc/amd/common: use common physical address bit reservation code Instead of having the get_usable_physical_address_bits function that only got used in the data fabric domain resource reporting code, drop this function, select RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT in the common AMD non-CAR CPU and rename get_sme_reserved_address_bits to get_reserved_phys_addr_bits so that the common cpu_phys_address_size function will return the correct number of usable physical address bits which now can be used everywhere. The common AMD CAR CPU support is only selected by Stoneyridge which doesn't support secure memory encryption, so RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT isn't selected by the SOC_AMD_COMMON_BLOCK_CAR Kconfig option. Before only the MMIO region reporting took the reserved physical address bits into account, but now also the MTRR calculation will take those reserved bits into account. See the AMD64 Programmers Manual volume 2 (document number 24593) for details. Chapter 7.10.5 from revision 3.41 of this document was used as a reference. The MTRR handling code in older Linux kernels complains when the upper reserved bits in the MTRR mask weren't set, but sets them after complaining and then continues to boot. This issue is no longer present in version 6.5 of the Linux kernel. The calculation of the TSEG mask however still needs to take all physical bits into account, including the ones reserved for the memory encryption. When not setting the reserved bits in the TSEG mask, the Mandolin board with a Picasso APU won't boot to the OS any more due to not returning from SeaBIOS calling into the VBIOS. Haven't root-caused what exactly causes this breakage, but I think previously when something else was wrong with the SMM initialization, also something went wrong when calling into the VBIOS. TEST=Ubuntu 2023.10 nightly build boots on Mandolin via SeaBIOS and EDK2 and Windows 10 boots on it via EDK2. TEST=On Ubuntu 2022.04 LTS, the kernel complained with the following warning, but it still continues the boot process as described above: mtrr: your BIOS has configured an incorrect mask, fixing it. Signed-off-by: Felix Held Change-Id: Iad65144006f1116cd82efc3c94e1d6d1ccb31b6e Reviewed-on: https://review.coreboot.org/c/coreboot/+/78074 Tested-by: build bot (Jenkins) Reviewed-by: Matt DeVillier --- src/soc/amd/common/block/cpu/Kconfig | 1 + src/soc/amd/common/block/cpu/noncar/Makefile.inc | 1 + src/soc/amd/common/block/cpu/noncar/cpu.c | 8 ++------ src/soc/amd/common/block/cpu/smm/smm_relocate.c | 7 ++++++- src/soc/amd/common/block/data_fabric/domain.c | 4 ++-- src/soc/amd/common/block/include/amdblocks/cpu.h | 1 - 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/soc/amd/common/block/cpu/Kconfig b/src/soc/amd/common/block/cpu/Kconfig index 6c5329aa9b..f926887cf5 100644 --- a/src/soc/amd/common/block/cpu/Kconfig +++ b/src/soc/amd/common/block/cpu/Kconfig @@ -13,6 +13,7 @@ config SOC_AMD_COMMON_BLOCK_CAR config SOC_AMD_COMMON_BLOCK_NONCAR bool + select RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT help From family 17h on AMD CPUs/APUs don't use cache as RAM (CAR) any more, since the RAM initialization is already done by the PSP when diff --git a/src/soc/amd/common/block/cpu/noncar/Makefile.inc b/src/soc/amd/common/block/cpu/noncar/Makefile.inc index 3204667fc1..f8ca357c40 100644 --- a/src/soc/amd/common/block/cpu/noncar/Makefile.inc +++ b/src/soc/amd/common/block/cpu/noncar/Makefile.inc @@ -2,6 +2,7 @@ ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_NONCAR),y) bootblock-y += bootblock.c +bootblock-y += cpu.c bootblock-y += early_cache.c bootblock-y += pre_c.S bootblock-y += write_resume_eip.c diff --git a/src/soc/amd/common/block/cpu/noncar/cpu.c b/src/soc/amd/common/block/cpu/noncar/cpu.c index eec593c8a9..eefd62fcf0 100644 --- a/src/soc/amd/common/block/cpu/noncar/cpu.c +++ b/src/soc/amd/common/block/cpu/noncar/cpu.c @@ -35,7 +35,8 @@ void set_cstate_io_addr(void) wrmsr(MSR_CSTATE_ADDRESS, cst_addr); } -static uint32_t get_sme_reserved_address_bits(void) +/* Number of most significant physical address bits reserved for secure memory encryption */ +unsigned int get_reserved_phys_addr_bits(void) { if (rdmsr(SYSCFG_MSR).raw & SYSCFG_MSR_SMEE) return (cpuid_ebx(CPUID_EBX_MEM_ENCRYPT) & @@ -44,8 +45,3 @@ static uint32_t get_sme_reserved_address_bits(void) else return 0; } - -uint32_t get_usable_physical_address_bits(void) -{ - return cpu_phys_address_size() - get_sme_reserved_address_bits(); -} diff --git a/src/soc/amd/common/block/cpu/smm/smm_relocate.c b/src/soc/amd/common/block/cpu/smm/smm_relocate.c index 4004726e29..ae008853bd 100644 --- a/src/soc/amd/common/block/cpu/smm/smm_relocate.c +++ b/src/soc/amd/common/block/cpu/smm/smm_relocate.c @@ -65,6 +65,11 @@ static void smm_relocation_handler(void) uintptr_t tseg_base; size_t tseg_size; + /* For the TSEG masks all physical address bits including the ones reserved for memory + encryption need to be taken into account. TODO: Find out why this is the case */ + const unsigned int total_physical_address_bits = + cpu_phys_address_size() + get_reserved_phys_addr_bits(); + smm_region(&tseg_base, &tseg_size); msr_t msr; @@ -73,7 +78,7 @@ static void smm_relocation_handler(void) msr.lo = ~(tseg_size - 1); msr.lo |= SMM_TSEG_WB; - msr.hi = (1 << (cpu_phys_address_size() - 32)) - 1; + msr.hi = (1 << (total_physical_address_bits - 32)) - 1; wrmsr(SMM_MASK_MSR, msr); uintptr_t smbase = smm_get_cpu_smbase(cpu_index()); diff --git a/src/soc/amd/common/block/data_fabric/domain.c b/src/soc/amd/common/block/data_fabric/domain.c index ca685b2fde..01fad999b4 100644 --- a/src/soc/amd/common/block/data_fabric/domain.c +++ b/src/soc/amd/common/block/data_fabric/domain.c @@ -1,13 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include -#include #include #include #include #include #include #include +#include #include #include #include @@ -90,7 +90,7 @@ static void add_data_fabric_mmio_regions(struct device *domain, unsigned int *id /* The last 12GB of the usable address space are reserved and can't be used for MMIO */ const resource_t reserved_upper_mmio_base = - (1ULL << get_usable_physical_address_bits()) - DF_RESERVED_TOP_12GB_MMIO_SIZE; + (1ULL << cpu_phys_address_size()) - DF_RESERVED_TOP_12GB_MMIO_SIZE; for (unsigned int i = 0; i < DF_MMIO_REG_SET_COUNT; i++) { ctrl.raw = data_fabric_broadcast_read32(DF_MMIO_CONTROL(i)); diff --git a/src/soc/amd/common/block/include/amdblocks/cpu.h b/src/soc/amd/common/block/include/amdblocks/cpu.h index 10dd23f11b..4aa225bda3 100644 --- a/src/soc/amd/common/block/include/amdblocks/cpu.h +++ b/src/soc/amd/common/block/include/amdblocks/cpu.h @@ -12,7 +12,6 @@ void early_cache_setup(void); int get_cpu_count(void); unsigned int get_threads_per_core(void); void set_cstate_io_addr(void); -uint32_t get_usable_physical_address_bits(void); void write_resume_eip(void); union pstate_msr; /* proper definition is in soc/msr.h */