Attention is currently required from: Krishna P Bhat D.
Anil Kumar K has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78053?usp=email )
Change subject: soc/intel/cse: Add function to get cse_bp_info early
......................................................................
Patch Set 7:
(1 comment)
Patchset:
PS5:
> I will start the validation as this CL now looks good. […]
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/78053?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: Ib1e72c950ba0f4911924805f501ec1bd54b6ba3c
Gerrit-Change-Number: 78053
Gerrit-PatchSet: 7
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Comment-Date: Fri, 29 Sep 2023 21:25:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Fred Reitberger, Jason Glenesk, Jérémy Compostella, Marshall Dawson, Matt DeVillier.
Hello Fred Reitberger, Jason Glenesk, Marshall Dawson, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/78074?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/amd/common: use common physical address bit reservation code
......................................................................
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 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 <felix-coreboot(a)felixheld.de>
Change-Id: Iad65144006f1116cd82efc3c94e1d6d1ccb31b6e
---
M src/soc/amd/common/block/cpu/Kconfig
M src/soc/amd/common/block/cpu/noncar/Makefile.inc
M src/soc/amd/common/block/cpu/noncar/cpu.c
M src/soc/amd/common/block/cpu/smm/smm_relocate.c
M src/soc/amd/common/block/data_fabric/domain.c
M src/soc/amd/common/block/include/amdblocks/cpu.h
6 files changed, 12 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/78074/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/78074?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: Iad65144006f1116cd82efc3c94e1d6d1ccb31b6e
Gerrit-Change-Number: 78074
Gerrit-PatchSet: 4
Gerrit-Owner: 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: Marshall Dawson <marshalldawson3rd(a)gmail.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: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-MessageType: newpatchset
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78073?usp=email )
Change subject: arch/x86/Kconfig: introduce RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT
......................................................................
arch/x86/Kconfig: introduce RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT
Since also some AMD CPUs have reserved physical address bits that can't
be used as normal address bits, introduce the
RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT Kconfig option which gets
selected by CPU_INTEL_COMMON, and use the new common option to configure
if the specific SoC/CPU code implements get_reserved_phys_addr_bits or
if the default of this returning 0 is used instead.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I0059e63a160e60ddee280635bba72d363deca7f7
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78073
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Reviewed-by: Jérémy Compostella <jeremy.compostella(a)intel.com>
Reviewed-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
---
M src/arch/x86/Kconfig
M src/arch/x86/include/arch/cpu.h
M src/cpu/intel/common/Kconfig
3 files changed, 12 insertions(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Jérémy Compostella: Looks good to me, but someone else must approve
Marshall Dawson: Looks good to me, but someone else must approve
Matt DeVillier: Looks good to me, approved
diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig
index 8676fad..c97fecb 100644
--- a/src/arch/x86/Kconfig
+++ b/src/arch/x86/Kconfig
@@ -92,6 +92,16 @@
The position where to place pagetables. Needs to be known at
compile time. Must not overlap other files in CBFS.
+config RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT
+ bool
+ help
+ On some systems, the upper physical address bits are reserved and
+ used as a tag which is typically related to a memory encryption
+ feature. When selecting this option, the SoC code needs to implement
+ get_reserved_phys_addr_bits so that the common code knows how many of
+ the most significant physical address bits are reserved and can't be
+ used as address bits.
+
# This is an SMP option. It relates to starting up APs.
# It is usually set in mainboard/*/Kconfig.
# TODO: Improve description.
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index b24cd23..2c98d1e 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
@@ -316,7 +316,7 @@
*/
bool fill_cpu_cache_info(uint8_t level, struct cpu_cache_info *info);
-#if CONFIG(CPU_INTEL_COMMON)
+#if CONFIG(RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT)
unsigned int get_reserved_phys_addr_bits(void);
#else
/* Default implementation */
diff --git a/src/cpu/intel/common/Kconfig b/src/cpu/intel/common/Kconfig
index 7f9033c..51b8ccb 100644
--- a/src/cpu/intel/common/Kconfig
+++ b/src/cpu/intel/common/Kconfig
@@ -1,5 +1,6 @@
config CPU_INTEL_COMMON
bool
+ select RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT
if CPU_INTEL_COMMON
--
To view, visit https://review.coreboot.org/c/coreboot/+/78073?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: I0059e63a160e60ddee280635bba72d363deca7f7
Gerrit-Change-Number: 78073
Gerrit-PatchSet: 3
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: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Felix Held, Marshall Dawson.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78073?usp=email )
Change subject: arch/x86/Kconfig: introduce RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/78073?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: I0059e63a160e60ddee280635bba72d363deca7f7
Gerrit-Change-Number: 78073
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.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: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 29 Sep 2023 20:22:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78072?usp=email )
Change subject: */include/cpu: use unsigned int for number of address bits
......................................................................
*/include/cpu: use unsigned int for number of address bits
The number of physical address bits and reserved address bits shouldn't
ever be negative, so change the return type of cpu_phys_address_size,
get_reserved_phys_addr_bits, and get_tme_keyid_bits from int to unsigned
int.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I9e67db6bf0c38f743b50e7273449cc028de13a8c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78072
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Reviewed-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Reviewed-by: Varshit Pandya <pandyavarshit(a)gmail.com>
---
M src/arch/x86/cpu_common.c
M src/arch/x86/include/arch/cpu.h
M src/cpu/intel/common/common_init.c
M src/include/cpu/cpu.h
4 files changed, 6 insertions(+), 6 deletions(-)
Approvals:
build bot (Jenkins): Verified
Matt DeVillier: Looks good to me, approved
Marshall Dawson: Looks good to me, but someone else must approve
Varshit Pandya: Looks good to me, but someone else must approve
diff --git a/src/arch/x86/cpu_common.c b/src/arch/x86/cpu_common.c
index 60ffd36..9387f3b 100644
--- a/src/arch/x86/cpu_common.c
+++ b/src/arch/x86/cpu_common.c
@@ -44,7 +44,7 @@
return cpuid_eax(0x80000000);
}
-int cpu_phys_address_size(void)
+unsigned int cpu_phys_address_size(void)
{
if (!(cpu_have_cpuid()))
return 32;
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index 96cf23b..b24cd23 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
@@ -317,10 +317,10 @@
bool fill_cpu_cache_info(uint8_t level, struct cpu_cache_info *info);
#if CONFIG(CPU_INTEL_COMMON)
-int get_reserved_phys_addr_bits(void);
+unsigned int get_reserved_phys_addr_bits(void);
#else
/* Default implementation */
-static inline int get_reserved_phys_addr_bits(void)
+static inline unsigned int get_reserved_phys_addr_bits(void)
{
/* Default implementation */
return 0;
diff --git a/src/cpu/intel/common/common_init.c b/src/cpu/intel/common/common_init.c
index ff00f02..55bc59e 100644
--- a/src/cpu/intel/common/common_init.c
+++ b/src/cpu/intel/common/common_init.c
@@ -248,7 +248,7 @@
* configured in the MSRs according to the capabilities and platform
* configuration. For instance, after FSP-M.
*/
-static int get_tme_keyid_bits(void)
+static unsigned int get_tme_keyid_bits(void)
{
msr_t msr;
@@ -256,7 +256,7 @@
return msr.hi & TME_ACTIVATE_HI_KEYID_BITS_MASK;
}
-int get_reserved_phys_addr_bits(void)
+unsigned int get_reserved_phys_addr_bits(void)
{
if (!is_tme_supported())
return 0;
diff --git a/src/include/cpu/cpu.h b/src/include/cpu/cpu.h
index fc662ee..9783976 100644
--- a/src/include/cpu/cpu.h
+++ b/src/include/cpu/cpu.h
@@ -9,7 +9,7 @@
void cpu_initialize(void);
uintptr_t cpu_get_lapic_addr(void);
struct bus;
-int cpu_phys_address_size(void);
+unsigned int cpu_phys_address_size(void);
#if ENV_RAMSTAGE
#define __cpu_driver __attribute__((used, __section__(".rodata.cpu_driver")))
--
To view, visit https://review.coreboot.org/c/coreboot/+/78072?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: I9e67db6bf0c38f743b50e7273449cc028de13a8c
Gerrit-Change-Number: 78072
Gerrit-PatchSet: 3
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: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Felix Held, Jérémy Compostella, Marshall Dawson.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78072?usp=email )
Change subject: */include/cpu: use unsigned int for number of address bits
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/78072?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: I9e67db6bf0c38f743b50e7273449cc028de13a8c
Gerrit-Change-Number: 78072
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 29 Sep 2023 20:19:25 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Karthik Ramasubramanian, Martin Roth.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78191?usp=email )
Change subject: util/amdfwtool: Check for pkg-config presence
......................................................................
Patch Set 1:
(1 comment)
File util/amdfwtool/Makefile:
https://review.coreboot.org/c/coreboot/+/78191/comment/575882da_f96f9833 :
PS1, Line 26: shell
We just need to check if pkg-config returned a valid code.
https://www.gnu.org/software/make/manual/html_node/Shell-Function.html says that we can check `.SHELLSTATUS`.
This should cover the cases where pkg-config doesn't exist, and if the library isn't found.
--
To view, visit https://review.coreboot.org/c/coreboot/+/78191?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: I5d604145c919e7f71680d1e095dc68cb21868319
Gerrit-Change-Number: 78191
Gerrit-PatchSet: 1
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 29 Sep 2023 20:15:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78136?usp=email )
Change subject: payloads/edk2: Update default branch for MrChromebox repo to 2023-09
......................................................................
payloads/edk2: Update default branch for MrChromebox repo to 2023-09
Update the default branch used for MrChromebox's edk2 fork from 2023-06
to 2023-09. This updated branch has been rebased on the latest upstream
stable tag (edk2-stable202308), and fixes some USB detection issues, as
well the coreboot Kconfig for prefering internal or external boot
devices.
TEST=build/boot google boards link, panther, lulu,reef, ampton, akemi,
banshee, zork, frostflow with edk2 payload selected.
Change-Id: I7c5f9ae1ca4edd8211f55f4ecf2b3b495f473a43
Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78136
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Sean Rhodes <sean(a)starlabs.systems>
---
M payloads/external/edk2/Kconfig
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Sean Rhodes: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/payloads/external/edk2/Kconfig b/payloads/external/edk2/Kconfig
index 76c7a76..6c7d67f 100644
--- a/payloads/external/edk2/Kconfig
+++ b/payloads/external/edk2/Kconfig
@@ -81,7 +81,7 @@
config EDK2_TAG_OR_REV
string "Insert a commit's SHA-1 or a branch name"
- default "origin/uefipayload_202306" if EDK2_REPO_MRCHROMEBOX
+ default "origin/uefipayload_202309" if EDK2_REPO_MRCHROMEBOX
default "origin/universalpayload" if EDK2_UNIVERSAL_PAYLOAD
default "origin/master" if EDK2_REPO_OFFICIAL
default "" if EDK2_REPO_CUSTOM
--
To view, visit https://review.coreboot.org/c/coreboot/+/78136?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: I7c5f9ae1ca4edd8211f55f4ecf2b3b495f473a43
Gerrit-Change-Number: 78136
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
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/+/78031?usp=email )
Change subject: payloads/edk2: Move TPM disable to separate Kconfig
......................................................................
payloads/edk2: Move TPM disable to separate Kconfig
Disabling TPM support in edk2 can actually cause problems booting from
USB on some Intel-based boards with a CR50 TPM when using the edk2
GOP driver option, so rather than disable the TPM for all CR50 boards,
restrict the default to only AMD boards, where the boot hang with
TPM enabled was originally observed.
TEST=build/boot Win11, Linux from usb on google/fizz when built
with edk2 payload and edk2 GOP driver option selected.
Change-Id: I01509fea2dd42b741c00abcf9fb8b936e895b932
Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78031
Reviewed-by: Sean Rhodes <sean(a)starlabs.systems>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de>
---
M payloads/external/Makefile.inc
M payloads/external/edk2/Kconfig
M payloads/external/edk2/Makefile
3 files changed, 9 insertions(+), 2 deletions(-)
Approvals:
build bot (Jenkins): Verified
Felix Held: Looks good to me, approved
Sean Rhodes: Looks good to me, approved
diff --git a/payloads/external/Makefile.inc b/payloads/external/Makefile.inc
index bd1269b..22b1bd3 100644
--- a/payloads/external/Makefile.inc
+++ b/payloads/external/Makefile.inc
@@ -192,7 +192,7 @@
CONFIG_EDK2_GOP_DRIVER=$(CONFIG_EDK2_GOP_DRIVER) \
CONFIG_EDK2_GOP_FILE=$(CONFIG_EDK2_GOP_FILE) \
CONFIG_INTEL_GMA_VBT_FILE=$(CONFIG_INTEL_GMA_VBT_FILE) \
- CONFIG_TPM_GOOGLE_CR50=$(CONFIG_TPM_GOOGLE_CR50) \
+ CONFIG_EDK2_DISABLE_TPM=$(CONFIG_EDK2_DISABLE_TPM) \
GCC_CC_x86_32=$(GCC_CC_x86_32) \
GCC_CC_x86_64=$(GCC_CC_x86_64) \
GCC_CC_arm=$(GCC_CC_arm) \
diff --git a/payloads/external/edk2/Kconfig b/payloads/external/edk2/Kconfig
index cd5b2f6..76c7a76 100644
--- a/payloads/external/edk2/Kconfig
+++ b/payloads/external/edk2/Kconfig
@@ -278,6 +278,13 @@
help
The name of the GOP driver file passed to edk2.
+config EDK2_DISABLE_TPM
+ bool "Disable TPM support in edk2"
+ default y if EDK2_REPO_MRCHROMEBOX && TPM_GOOGLE_CR50 && SOC_AMD_COMMON
+ help
+ Select this option to disable TPM support in edk2. This is necessary to avoid boot
+ hangs on some boards with a CR50 TPM, particularly those with an AMD Zen SoC.
+
config EDK2_CUSTOM_BUILD_PARAMS
string "edk2 additional custom build parameters"
default "-D VARIABLE_SUPPORT=SMMSTORE" if EDK2_REPO_MRCHROMEBOX && SMMSTORE_V2
diff --git a/payloads/external/edk2/Makefile b/payloads/external/edk2/Makefile
index f30d92f..b03f1d5 100644
--- a/payloads/external/edk2/Makefile
+++ b/payloads/external/edk2/Makefile
@@ -134,7 +134,7 @@
BUILD_STR += -D PRIORITIZE_INTERNAL=TRUE
endif
# TPM_ENABLE = TRUE
-ifeq ($(CONFIG_TPM_GOOGLE_CR50),y)
+ifeq ($(CONFIG_EDK2_DISABLE_TPM),y)
BUILD_STR += -D TPM_ENABLE=FALSE
endif
--
To view, visit https://review.coreboot.org/c/coreboot/+/78031?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: I01509fea2dd42b741c00abcf9fb8b936e895b932
Gerrit-Change-Number: 78031
Gerrit-PatchSet: 3
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Benjamin Doron, Lean Sheng Tan, Martin L Roth, Matt DeVillier, Sean Rhodes, Stefan Reinauer.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78031?usp=email )
Change subject: payloads/edk2: Move TPM disable to separate Kconfig
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/78031?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: I01509fea2dd42b741c00abcf9fb8b936e895b932
Gerrit-Change-Number: 78031
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Comment-Date: Fri, 29 Sep 2023 20:03:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment