coreboot-gerrit October 2020

coreboot-gerrit@coreboot.org

1 participants
3474 discussions

Change in coreboot[master]: Revert "mb/google/dedede: Add mainboard acpi support for GPIO PM conf...
by Justin TerAvest (Code Review) 22 Oct '20

22 Oct '20

Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46652 ) Change subject: Revert "mb/google/dedede: Add mainboard acpi support for GPIO PM configuration" ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/46652 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e3be42cd82fd3ae919d23d6f19c84a90b9c737a Gerrit-Change-Number: 46652 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin TerAvest <teravest(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 22 Oct 2020 20:21:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Revert "soc/intel/jasperlake: Allow mainboard to override chip config...
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46653 ) Change subject: Revert "soc/intel/jasperlake: Allow mainboard to override chip configuration" ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/46653 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I166ba7e5ee50a6329032eae8e17b9a554b094e2e Gerrit-Change-Number: 46653 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 22 Oct 2020 20:08:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Revert "mb/google/dedede: Add mainboard acpi support for GPIO PM conf...
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46652 ) Change subject: Revert "mb/google/dedede: Add mainboard acpi support for GPIO PM configuration" ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/46652 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e3be42cd82fd3ae919d23d6f19c84a90b9c737a Gerrit-Change-Number: 46652 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin TerAvest <teravest(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 22 Oct 2020 20:08:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: sec/intel/txt: Split MTRR setup ASM code into a macro
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46603 ) Change subject: sec/intel/txt: Split MTRR setup ASM code into a macro ...................................................................... sec/intel/txt: Split MTRR setup ASM code into a macro If necessary, SCLEAN needs to run in early romstage, where DRAM is not working yet. In fact, that the DRAM isn't working is the reason to run SCLEAN in the first place. Before running GETSEC, CAR needs to be torn down, as MTRRs have to be reprogrammed to cache the BIOS ACM. Further, running SCLEAN leaves the system in an undefined state, where the only sane thing to do is reset the platform. Thus, invoking SCLEAN requires specific assembly prologue and epilogue sections before and after MTRR setup, and neither DRAM nor CAR may be relied upon for the MTRR setup. In order to handle this without duplicating the MTRR setup code, place it in a macro on a separate file. This needs to be a macro because the call and return instructions rely on the stack being usable, and it is not the case for SCLEAN. The MTRR code clobbers many registers, but no other choice remains when the registers cannot be saved anywhere else. Tested on Asrock B85M Pro4, BIOS ACM can still be launched. Change-Id: I2f5e82f57b458ca1637790ddc1ddc14bba68ac49 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/getsec_enteraccs.S A src/security/intel/txt/getsec_mtrr_setup.S 2 files changed, 84 insertions(+), 64 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/46603/1 diff --git a/src/security/intel/txt/getsec_enteraccs.S b/src/security/intel/txt/getsec_enteraccs.S index be038b0..f3c6804 100644 --- a/src/security/intel/txt/getsec_enteraccs.S +++ b/src/security/intel/txt/getsec_enteraccs.S @@ -4,7 +4,7 @@ #include <cpu/x86/cr.h> #include <cpu/x86/msr.h> -#define MTRR_HIGH_MASK $((1 << (CONFIG_CPU_ADDR_BITS - 32)) - 1) +#include "getsec_mtrr_setup.S" .macro PUSH_MSR x movl $(\x), %ecx @@ -166,15 +166,6 @@ * Intel TXT Software Development Guide (Document: 315168-015) */ - /* - * Important note: The MTRRs must cache less than a page (4 KiB) - * of unused memory after the BIOS ACM. Failure to do so will - * result in a TXT reset with Class Code 5, Major Error Code 2. - * - * The caller must have checked that there are enough variable - * MTRRs to cache the ACM size prior to invoking this routine. - */ - /* Determine size of AC module */ movl 12(%ebp), %eax /* %eax = acmbase */ movl $1, %ebx @@ -191,60 +182,15 @@ movd %eax, %xmm0 /* XMM0: Base address of next MTRR */ movd %ebx, %xmm1 /* XMM1: Remaining size to cache */ - /* Get the number of variable MTRRs */ - movl $(MTRR_CAP_MSR), %ecx - rdmsr - andl $(0xff), %eax - - /* Initialize ECX */ - movl $(MTRR_PHYS_BASE(0)), %ecx - - jmp cond_allocate_var_mtrrs - -body_allocate_var_mtrrs: - - /* Program MTRR base */ - xorl %edx, %edx - movd %xmm0, %eax - orl $(MTRR_TYPE_WRBACK), %eax - wrmsr - incl %ecx /* Move index to MTRR_PHYS_MASK */ - - /* Temporarily transfer MSR index to EDX so that CL can be used */ - movl %ecx, %edx - - /* Determine next size to cache */ - bsr %ebx, %ecx - movl $(1), %ebx - shl %cl, %ebx /* Can only use CL here */ - - /* Restore ECX */ - movl %edx, %ecx - - /* Update saved base address */ - addl %ebx, %eax - movd %eax, %xmm0 - - /* Update saved remaining size */ - movd %xmm1, %eax - subl %ebx, %eax - movd %eax, %xmm1 - - /* Program MTRR mask */ - movl MTRR_HIGH_MASK, %edx - xorl %eax, %eax - subl %ebx, %eax /* %eax = 4GIB - size to cache */ - orl $(MTRR_PHYS_MASK_VALID), %eax - wrmsr - incl %ecx /* Move index to next MTRR_PHYS_BASE */ - -cond_allocate_var_mtrrs: - - /* Check if we still need to cache something */ - movd %xmm1, %ebx - andl %ebx, %ebx - - jnz body_allocate_var_mtrrs + /* + * Important note: The MTRRs must cache less than a page (4 KiB) + * of unused memory after the BIOS ACM. Failure to do so will + * result in a TXT reset with Class Code 5, Major Error Code 2. + * + * The caller must have checked that there are enough variable + * MTRRs to cache the ACM size prior to invoking this routine. + */ + SET_UP_MTRRS_FOR_BIOS_ACM /* * Now that the variable MTRRs have been set up, enable them. diff --git a/src/security/intel/txt/getsec_mtrr_setup.S b/src/security/intel/txt/getsec_mtrr_setup.S new file mode 100644 index 0000000..15e8cc1 --- /dev/null +++ b/src/security/intel/txt/getsec_mtrr_setup.S @@ -0,0 +1,74 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <cpu/x86/mtrr.h> +#include <cpu/x86/msr.h> + +#define MTRR_HIGH_MASK $((1 << (CONFIG_CPU_ADDR_BITS - 32)) - 1) + +/* + * Configure the MTRRs to cache the BIOS ACM. No general-purpose + * registers are preserved. Inputs are taken from SSE registers: + * + * %xmm0: BIOS ACM base + * %xmm1: BIOS ACM size + * + * These two SSE registers are not preserved, but the others are. + */ +.macro SET_UP_MTRRS_FOR_BIOS_ACM + + /* Get the number of variable MTRRs */ + movl $(MTRR_CAP_MSR), %ecx + rdmsr + andl $(0xff), %eax + + /* Initialize ECX */ + movl $(MTRR_PHYS_BASE(0)), %ecx + + jmp cond_allocate_var_mtrrs + +body_allocate_var_mtrrs: + + /* Program MTRR base */ + xorl %edx, %edx + movd %xmm0, %eax + orl $(MTRR_TYPE_WRBACK), %eax + wrmsr + incl %ecx /* Move index to MTRR_PHYS_MASK */ + + /* Temporarily transfer MSR index to EDX so that CL can be used */ + movl %ecx, %edx + + /* Determine next size to cache */ + bsr %ebx, %ecx + movl $(1), %ebx + shl %cl, %ebx /* Can only use CL here */ + + /* Restore ECX */ + movl %edx, %ecx + + /* Update saved base address */ + addl %ebx, %eax + movd %eax, %xmm0 + + /* Update saved remaining size */ + movd %xmm1, %eax + subl %ebx, %eax + movd %eax, %xmm1 + + /* Program MTRR mask */ + movl MTRR_HIGH_MASK, %edx + xorl %eax, %eax + subl %ebx, %eax /* %eax = 4GIB - size to cache */ + orl $(MTRR_PHYS_MASK_VALID), %eax + wrmsr + incl %ecx /* Move index to next MTRR_PHYS_BASE */ + +cond_allocate_var_mtrrs: + + /* Check if we still need to cache something */ + movd %xmm1, %ebx + andl %ebx, %ebx + + jnz body_allocate_var_mtrrs + +.endm -- To view, visit https://review.coreboot.org/c/coreboot/+/46603 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2f5e82f57b458ca1637790ddc1ddc14bba68ac49 Gerrit-Change-Number: 46603 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 7

Change in coreboot[master]: sec/intel/txt: Add `enable_getsec_or_reset` function
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46606 ) Change subject: sec/intel/txt: Add `enable_getsec_or_reset` function ...................................................................... sec/intel/txt: Add `enable_getsec_or_reset` function This can be used to enable GETSEC/SMX in the IA32_FEATURE_CONTROL MSR, and will be put to use on Haswell in subsequent commits. Change-Id: I5a82e515c6352b6ebbc361c6a53ff528c4b6cdba Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/getsec.c M src/security/intel/txt/txt_getsec.h 2 files changed, 33 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/46606/1 diff --git a/src/security/intel/txt/getsec.c b/src/security/intel/txt/getsec.c index 422f10d..af9b7bb 100644 --- a/src/security/intel/txt/getsec.c +++ b/src/security/intel/txt/getsec.c @@ -1,9 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <cf9_reset.h> +#include <console/console.h> +#include <cpu/intel/common/common.h> #include <cpu/x86/lapic.h> #include <cpu/x86/cr.h> #include <cpu/x86/cache.h> #include <cpu/x86/mp.h> +#include <cpu/x86/msr.h> #include <types.h> #include "txt_register.h" @@ -40,6 +44,33 @@ return true; } +void enable_getsec_or_reset(void) +{ + msr_t msr = rdmsr(IA32_FEATURE_CONTROL); + + if (!(msr.lo & FEATURE_CONTROL_LOCK_BIT)) { + /* + * MSR not locked, enable necessary GETSEC and VMX settings. + * We do not lock this MSR here, though. + */ + msr.lo |= 0xff06; + wrmsr(IA32_FEATURE_CONTROL, msr); + + } else if ((msr.lo & 0xff06) != 0xff06) { + /* + * MSR is locked without necessary GETSEC and VMX settings. + * This can happen after internally reflashing a coreboot + * image with different settings, and then doing a warm + * reboot. Perform a full reset in order to unlock the MSR. + */ + printk(BIOS_NOTICE, + "IA32_FEATURE_CONTROL MSR locked with GETSEC and/or VMX disabled.\n" + "Will perform a full reset to unlock this MSR.\n"); + + full_reset(); + } +} + /** * Get information as returned by getsec[PARAMETER]. * Arguments can be set to NULL if not needed. diff --git a/src/security/intel/txt/txt_getsec.h b/src/security/intel/txt/txt_getsec.h index 8e663d5..78171a7 100644 --- a/src/security/intel/txt/txt_getsec.h +++ b/src/security/intel/txt/txt_getsec.h @@ -5,6 +5,8 @@ #include <types.h> +void enable_getsec_or_reset(void); + bool getsec_parameter(uint32_t *version_mask, uint32_t *version_numbers_supported, uint32_t *max_size_acm_area, -- To view, visit https://review.coreboot.org/c/coreboot/+/46606 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5a82e515c6352b6ebbc361c6a53ff528c4b6cdba Gerrit-Change-Number: 46606 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

3 4

Change in coreboot[master]: sec/intel/txt: Extract BIOS ACM loading into a function
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46593 ) Change subject: sec/intel/txt: Extract BIOS ACM loading into a function ...................................................................... sec/intel/txt: Extract BIOS ACM loading into a function Tested on Asrock B85M Pro4, still boots with TXT enabled. Change-Id: I0b04955b341848ea8627a9c3ffd6a68cd49c3858 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/common.c 1 file changed, 45 insertions(+), 28 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46593/1 diff --git a/src/security/intel/txt/common.c b/src/security/intel/txt/common.c index 737ab0a..88e2b5d 100644 --- a/src/security/intel/txt/common.c +++ b/src/security/intel/txt/common.c @@ -210,29 +210,28 @@ } /* - * Test all bits for TXT execution. - * - * @return 0 on success + * Prepare to run the BIOS ACM: mmap it from the CBFS and verify that it + * can be launched. Returns pointer to ACM on success, NULL on failure. */ -int intel_txt_run_bios_acm(const u8 input_params) +static void *intel_txt_prepare_bios_acm(struct region_device *acm, size_t *acm_len) { struct cbfsf file; - void *acm_data; - struct region_device acm; - size_t acm_len; - int ret; + void *acm_data = NULL; + + if (!acm || !acm_len) + return NULL; if (cbfs_boot_locate(&file, CONFIG_INTEL_TXT_CBFS_BIOS_ACM, NULL)) { printk(BIOS_ERR, "TEE-TXT: Couldn't locate BIOS ACM in CBFS.\n"); - return -1; + return NULL; } - cbfs_file_data(&acm, &file); - acm_data = rdev_mmap_full(&acm); - acm_len = region_device_sz(&acm); - if (!acm_data || acm_len == 0) { + cbfs_file_data(acm, &file); + acm_data = rdev_mmap_full(acm); + *acm_len = region_device_sz(acm); + if (!acm_data || *acm_len == 0) { printk(BIOS_ERR, "TEE-TXT: Couldn't map BIOS ACM from CBFS.\n"); - return -1; + return NULL; } /* @@ -242,8 +241,8 @@ */ if (!IS_ALIGNED((uintptr_t)acm_data, 4096)) { printk(BIOS_ERR, "TEE-TXT: BIOS ACM isn't mapped at page boundary.\n"); - rdev_munmap(&acm, acm_data); - return -1; + rdev_munmap(acm, acm_data); + return NULL; } /* @@ -251,20 +250,20 @@ * SAFER MODE EXTENSIONS REFERENCE. * Intel 64 and IA-32 Architectures Software Developer Manuals Vol 2D */ - if (!IS_ALIGNED(acm_len, 64)) { + if (!IS_ALIGNED(*acm_len, 64)) { printk(BIOS_ERR, "TEE-TXT: BIOS ACM size isn't multiple of 64.\n"); - rdev_munmap(&acm, acm_data); - return -1; + rdev_munmap(acm, acm_data); + return NULL; } /* * The ACM should be aligned to it's size, but that's not possible, as * some ACMs are not power of two. Use the next power of two for verification. */ - if (!IS_ALIGNED((uintptr_t)acm_data, (1UL << log2_ceil(acm_len)))) { + if (!IS_ALIGNED((uintptr_t)acm_data, (1UL << log2_ceil(*acm_len)))) { printk(BIOS_ERR, "TEE-TXT: BIOS ACM isn't aligned to its size.\n"); - rdev_munmap(&acm, acm_data); - return -1; + rdev_munmap(acm, acm_data); + return NULL; } /* @@ -272,22 +271,40 @@ * a page (4 KiB) of unused memory after the BIOS ACM. On Haswell, failure * to do so will cause a TXT reset with Class Code 5, Major Error Code 2. */ - if (popcnt(ALIGN_UP(acm_len, 4096)) > get_var_mtrr_count()) { + if (popcnt(ALIGN_UP(*acm_len, 4096)) > get_var_mtrr_count()) { printk(BIOS_ERR, "TEE-TXT: Not enough MTRRs to cache this BIOS ACM's size.\n"); - rdev_munmap(&acm, acm_data); - return -1; + rdev_munmap(acm, acm_data); + return NULL; } if (CONFIG(INTEL_TXT_LOGGING)) txt_dump_acm_info(acm_data); - ret = validate_acm(acm_data); + const int ret = validate_acm(acm_data); if (ret < 0) { printk(BIOS_ERR, "TEE-TXT: Validation of ACM failed with: %d\n", ret); - rdev_munmap(&acm, acm_data); - return ret; + rdev_munmap(acm, acm_data); + return NULL; } + return acm_data; +} + +/* + * Test all bits for TXT execution. + * + * @return 0 on success + */ +int intel_txt_run_bios_acm(const u8 input_params) +{ + struct region_device acm; + size_t acm_len; + + void *acm_data = intel_txt_prepare_bios_acm(&acm, &acm_len); + + if (!acm_data) + return -1; + /* Call into assembly which invokes the referenced ACM */ getsec_enteraccs(input_params, (uintptr_t)acm_data, acm_len); -- To view, visit https://review.coreboot.org/c/coreboot/+/46593 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0b04955b341848ea8627a9c3ffd6a68cd49c3858 Gerrit-Change-Number: 46593 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: nb/intel/haswell: Only hide PEG devices if ONBOARD_VGA_IS_PRIMARY
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45534 ) Change subject: nb/intel/haswell: Only hide PEG devices if ONBOARD_VGA_IS_PRIMARY ...................................................................... nb/intel/haswell: Only hide PEG devices if ONBOARD_VGA_IS_PRIMARY The MRC will perform PCI enumeration, and if it detects a VGA device in a PEG slot, it will disable the IGD and not reserve any memory for it. Since the memory map is locked by the time MRC finishes, the IGD can't be enabled afterwards. Wonderful. If we are supposed to enable the onboard VGA as primary, hide all PEG devices during MRC execution. This will trick the MRC into thinking there aren't any, and will enable the IGD. Note that PEG AFE settings will not be programmed, which may cause stability problems at higher PCIe link speeds. The most ideal way to fix this problem for good is to implement native init. Change-Id: I4d825b1c41d8705bfafe28d8ecb0a511788901f0 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/haswell/early_init.c 1 file changed, 17 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/45534/1 diff --git a/src/northbridge/intel/haswell/early_init.c b/src/northbridge/intel/haswell/early_init.c index 9db6a9d..d3d511e 100644 --- a/src/northbridge/intel/haswell/early_init.c +++ b/src/northbridge/intel/haswell/early_init.c @@ -84,13 +84,24 @@ printk(BIOS_DEBUG, "Started PEG1%d link training.\n", PCI_FUNC(PCI_DEV2DEVFN(dev))); /* - * Hide the PEG device while the MRC runs. This is because the MRC makes - * configurations that are not ideal if it sees a VGA device in a PEG slot, - * and it locks registers preventing changes to these configurations. + * The MRC will perform PCI enumeration, and if it detects a VGA + * device in a PEG slot, it will disable the IGD and not reserve + * any memory for it. Since the memory map is locked by the time + * MRC finishes, the IGD can't be enabled afterwards. Wonderful. + * + * If we are supposed to enable the onboard VGA as primary, hide + * all PEG devices during MRC execution. This will trick the MRC + * into thinking there aren't any, and will enable the IGD. Note + * that PEG AFE settings will not be programmed, which may cause + * stability problems at higher PCIe link speeds. The most ideal + * way to fix this problem for good is to implement native init. */ - pci_update_config32(HOST_BRIDGE, DEVEN, ~mask, 0); - peg_hidden[PCI_FUNC(PCI_DEV2DEVFN(dev))] = true; - printk(BIOS_DEBUG, "Temporarily hiding PEG1%d.\n", PCI_FUNC(PCI_DEV2DEVFN(dev))); + if (CONFIG(ONBOARD_VGA_IS_PRIMARY)) { + pci_update_config32(HOST_BRIDGE, DEVEN, ~mask, 0); + peg_hidden[PCI_FUNC(PCI_DEV2DEVFN(dev))] = true; + printk(BIOS_DEBUG, "Temporarily hiding PEG1%d.\n", + PCI_FUNC(PCI_DEV2DEVFN(dev))); + } } void haswell_unhide_peg(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/45534 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d825b1c41d8705bfafe28d8ecb0a511788901f0 Gerrit-Change-Number: 45534 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

4 14

Change in coreboot[master]: sec/intel/txt: Only run LockConfig for LT-SX
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46498 ) Change subject: sec/intel/txt: Only run LockConfig for LT-SX ...................................................................... sec/intel/txt: Only run LockConfig for LT-SX LockConfig only exists on Intel TXT for Servers. Check whether this is supported using GETSEC[PARAMETERS]. This eliminates a spurious error for Client TXT platforms such as Haswell, and is a no-op on TXT for Servers. Change-Id: Ibb7b0eeba1489dc522d06ab27eafcaa0248b7083 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/ramstage.c M src/security/intel/txt/txt_register.h 2 files changed, 18 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/46498/1 diff --git a/src/security/intel/txt/ramstage.c b/src/security/intel/txt/ramstage.c index 86bf7aa..76eeaaf 100644 --- a/src/security/intel/txt/ramstage.c +++ b/src/security/intel/txt/ramstage.c @@ -316,6 +316,7 @@ { const uint64_t status = read64((void *)TXT_SPAD); + uint32_t txt_feature_flags = 0; uintptr_t tseg_base; size_t tseg_size; @@ -324,13 +325,24 @@ if (status & ACMSTS_TXT_DISABLED) return; - printk(BIOS_INFO, "TEE-TXT: Locking TEE...\n"); + /* + * Document Number: 558294 + * Chapter 5.4.3 Detection of Intel TXT Capability + */ - /* Lock TXT config, unlocks TXT_HEAP_BASE */ - if (intel_txt_run_bios_acm(ACMINPUT_LOCK_CONFIG) < 0) { - printk(BIOS_ERR, "TEE-TXT: Failed to lock registers.\n"); - printk(BIOS_ERR, "TEE-TXT: SINIT won't be supported.\n"); + if (!getsec_parameter(NULL, NULL, NULL, NULL, NULL, &txt_feature_flags)) return; + + /* LockConfig only exists on Intel TXT for Servers */ + if (txt_feature_flags & GETSEC_PARAMS_TXT_EXT_CRTM_SUPPORT) { + printk(BIOS_INFO, "TEE-TXT: Locking TEE...\n"); + + /* Lock TXT config, unlocks TXT_HEAP_BASE */ + if (intel_txt_run_bios_acm(ACMINPUT_LOCK_CONFIG) < 0) { + printk(BIOS_ERR, "TEE-TXT: Failed to lock registers.\n"); + printk(BIOS_ERR, "TEE-TXT: SINIT won't be supported.\n"); + return; + } } /* diff --git a/src/security/intel/txt/txt_register.h b/src/security/intel/txt/txt_register.h index bbf0a7e..c19ec13 100644 --- a/src/security/intel/txt/txt_register.h +++ b/src/security/intel/txt/txt_register.h @@ -132,8 +132,7 @@ #define IA32_GETSEC_SMCTRL 7 #define IA32_GETSEC_WAKEUP 8 -#define GETSEC_PARAMS_TXT_EXT (1ul << 5) -#define GETSEC_PARAMS_TXT_EXT_CRTM_SUPPORT (1ul << 1) +#define GETSEC_PARAMS_TXT_EXT_CRTM_SUPPORT (1ul << 5) #define GETSEC_PARAMS_TXT_EXT_MACHINE_CHECK (1ul << 6) /* ACM defines */ -- To view, visit https://review.coreboot.org/c/coreboot/+/46498 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibb7b0eeba1489dc522d06ab27eafcaa0248b7083 Gerrit-Change-Number: 46498 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: sec/intel/txt: Always run SCHECK on regular boots
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46497 ) Change subject: sec/intel/txt: Always run SCHECK on regular boots ...................................................................... sec/intel/txt: Always run SCHECK on regular boots When Boot Guard is disabled or not available, the IBB might not even exist. This is the case on traditional (non-ULT) Haswell, for example. Leave the S3 resume check as-is for now. Skylake and newer may need to run SCHECK on resume as well, but I lack the hardware to test this on. Change-Id: I70231f60d4d4c5bc8ee0fcbb0651896256fdd391 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/ramstage.c 1 file changed, 8 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/46497/1 diff --git a/src/security/intel/txt/ramstage.c b/src/security/intel/txt/ramstage.c index c39194b..86bf7aa 100644 --- a/src/security/intel/txt/ramstage.c +++ b/src/security/intel/txt/ramstage.c @@ -168,15 +168,16 @@ } if (status & (ACMSTS_BIOS_TRUSTED | ACMSTS_IBB_MEASURED)) { + printk(BIOS_INFO, "TEE-TXT: Logging IBB measurements...\n"); log_ibb_measurements(); + } - int s3resume = acpi_is_wakeup_s3(); - if (!s3resume) { - printk(BIOS_INFO, "TEE-TXT: Scheck...\n"); - if (intel_txt_run_bios_acm(ACMINPUT_SCHECK) < 0) { - printk(BIOS_ERR, "TEE-TXT: Error calling BIOS ACM.\n"); - return; - } + int s3resume = acpi_is_wakeup_s3(); + if (!s3resume) { + printk(BIOS_INFO, "TEE-TXT: Scheck...\n"); + if (intel_txt_run_bios_acm(ACMINPUT_SCHECK) < 0) { + printk(BIOS_ERR, "TEE-TXT: Error calling BIOS ACM.\n"); + return; } } } -- To view, visit https://review.coreboot.org/c/coreboot/+/46497 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I70231f60d4d4c5bc8ee0fcbb0651896256fdd391 Gerrit-Change-Number: 46497 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: sec/intel/txt: Allow skipping ACM NOP function
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46496 ) Change subject: sec/intel/txt: Allow skipping ACM NOP function ...................................................................... sec/intel/txt: Allow skipping ACM NOP function This is merely used to test whether the BIOS ACM calling code is working properly. There's no need to do this on production platforms. Testing on Haswell showed that running this NOP function breaks S3 resume with TXT. Add a Kconfig bool to control whether the NOP function is to be invoked. Change-Id: Ibf461c18a96f1add7867e1320726fadec65b7184 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/Kconfig M src/security/intel/txt/ramstage.c 2 files changed, 19 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/46496/1 diff --git a/src/security/intel/txt/Kconfig b/src/security/intel/txt/Kconfig index c69a217..7538da8 100644 --- a/src/security/intel/txt/Kconfig +++ b/src/security/intel/txt/Kconfig @@ -40,6 +40,12 @@ the MRC does not have an input to specify the size of DPR, so this field is only used to check if the programmed size is large enough. +config INTEL_TXT_TEST_BIOS_ACM_CALLING_CODE + bool "Test BIOS ACM calling code with NOP function" + help + Run a NOP function of the BIOS ACM to check that the ACM calling code + is functioning properly. Use in pre-production environments only! + config INTEL_TXT_LOGGING bool "Enable verbose logging" help diff --git a/src/security/intel/txt/ramstage.c b/src/security/intel/txt/ramstage.c index 00e9ce7..c39194b 100644 --- a/src/security/intel/txt/ramstage.c +++ b/src/security/intel/txt/ramstage.c @@ -151,17 +151,20 @@ return; } - printk(BIOS_INFO, "TEE-TXT: Testing BIOS ACM calling code...\n"); + if (CONFIG(INTEL_TXT_TEST_BIOS_ACM_CALLING_CODE)) { + printk(BIOS_INFO, "TEE-TXT: Testing BIOS ACM calling code...\n"); - /* - * Test BIOS ACM code. - * ACM should do nothing on reserved functions, and return an error code - * in TXT_BIOSACM_ERRORCODE. Tests showed that this is not true. - * Use special function "NOP" that does 'nothing'. - */ - if (intel_txt_run_bios_acm(ACMINPUT_NOP) < 0) { - printk(BIOS_ERR, "TEE-TXT: Error calling BIOS ACM with NOP function.\n"); - return; + /* + * Test BIOS ACM code. + * ACM should do nothing on reserved functions, and return an error code + * in TXT_BIOSACM_ERRORCODE. Tests showed that this is not true. + * Use special function "NOP" that does 'nothing'. + */ + if (intel_txt_run_bios_acm(ACMINPUT_NOP) < 0) { + printk(BIOS_ERR, + "TEE-TXT: Error calling BIOS ACM with NOP function.\n"); + return; + } } if (status & (ACMSTS_BIOS_TRUSTED | ACMSTS_IBB_MEASURED)) { -- To view, visit https://review.coreboot.org/c/coreboot/+/46496 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf461c18a96f1add7867e1320726fadec65b7184 Gerrit-Change-Number: 46496 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 3

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2020