coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: soc/intel/quark: Use common postcar MTRR setup
by Arthur Heymans (Code Review) 14 May '21

14 May '21

Attention is currently required from: Patrick Rudolph. Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/54300 ) Change subject: soc/intel/quark: Use common postcar MTRR setup ...................................................................... soc/intel/quark: Use common postcar MTRR setup The common MTRR setup in postcar stage should now work for intel/quark. The only difference is in the implementation of rdmsr and wrmsr. Change-Id: I9fe9dc458383930a75d9459f77e347241d8b6f33 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/exit_car.S M src/include/cpu/x86/msr.h M src/soc/intel/quark/romstage/Makefile.inc D src/soc/intel/quark/romstage/mtrr.c 4 files changed, 0 insertions(+), 109 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/54300/1 diff --git a/src/arch/x86/exit_car.S b/src/arch/x86/exit_car.S index 85d6e12..2bf1ed9 100644 --- a/src/arch/x86/exit_car.S +++ b/src/arch/x86/exit_car.S @@ -60,28 +60,6 @@ /* Ensure cache is clean. */ invd -#if CONFIG(SOC_SETS_MSRS) - - mov %esp, %ebp - /* Need to align stack to 16 bytes at the call instruction. Therefore - account for the 1 push. */ - andl $0xfffffff0, %esp -#if defined(__x86_64__) - mov %rbp, %rdi -#else - sub $12, %esp - push %ebp -#endif - - call soc_set_mtrrs - /* Ignore fixing up %esp since we're setting it a new value. */ - - /* eax: new top_of_stack with setup_stack_and_mtrrs data removed */ - movl %eax, %esp - /* Align stack to 16 bytes at call instruction. */ - andl $0xfffffff0, %esp - call soc_enable_mtrrs -#endif /* CONFIG_SOC_SETS_MSRS */ movl $_estack, %esp /* Align stack to 16 bytes at call instruction. */ andl $0xfffffff0, %esp diff --git a/src/include/cpu/x86/msr.h b/src/include/cpu/x86/msr.h index bc367d7..ce0f7ad 100644 --- a/src/include/cpu/x86/msr.h +++ b/src/include/cpu/x86/msr.h @@ -108,9 +108,6 @@ } msrinit_t; #if CONFIG(SOC_SETS_MSRS) -msr_t soc_msr_read(unsigned int index); -void soc_msr_write(unsigned int index, msr_t msr); - /* Handle MSR references in the other source code */ static __always_inline msr_t rdmsr(unsigned int index) { diff --git a/src/soc/intel/quark/romstage/Makefile.inc b/src/soc/intel/quark/romstage/Makefile.inc index 8630acb..ff9b2b6 100644 --- a/src/soc/intel/quark/romstage/Makefile.inc +++ b/src/soc/intel/quark/romstage/Makefile.inc @@ -3,10 +3,7 @@ romstage-y += car.c romstage-$(CONFIG_DISPLAY_UPD_DATA) += debug.c romstage-y += fsp_params.c -romstage-y += mtrr.c romstage-y += pcie.c romstage-y += report_platform.c romstage-y += romstage.c romstage-y += ../../../../cpu/intel/car/romstage.c - -postcar-y += mtrr.c diff --git a/src/soc/intel/quark/romstage/mtrr.c b/src/soc/intel/quark/romstage/mtrr.c deleted file mode 100644 index e72ce33..0000000 --- a/src/soc/intel/quark/romstage/mtrr.c +++ /dev/null @@ -1,81 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <cpu/x86/msr.h> -#include <cpu/x86/mtrr.h> -#include <soc/pci_devs.h> -#include <soc/reg_access.h> - -asmlinkage void *soc_set_mtrrs(void *top_of_stack) -{ - union { - uint32_t u32[2]; - uint64_t u64; - msr_t msr; - } data; - uint32_t mtrr_count; - uint32_t *mtrr_data; - uint32_t mtrr_reg; - - /* - * The stack contents are initialized in src/soc/intel/common/stack.c - * to be the following: - * - * * - * * - * * - * +36: MTRR mask 1 63:32 - * +32: MTRR mask 1 31:0 - * +28: MTRR base 1 63:32 - * +24: MTRR base 1 31:0 - * +20: MTRR mask 0 63:32 - * +16: MTRR mask 0 31:0 - * +12: MTRR base 0 63:32 - * +8: MTRR base 0 31:0 - * +4: Number of MTRRs to setup (described above) - * top_of_stack --> +0: Number of variable MTRRs to clear - * - * This routine: - * * Clears all of the variable MTRRs - * * Initializes the variable MTRRs with the data passed in - * * Returns the new top of stack after removing all of the - * data passed in. - */ - - /* Clear all of the variable MTRRs (base and mask). */ - mtrr_reg = MTRR_PHYS_BASE(0); - mtrr_data = top_of_stack; - mtrr_count = (*mtrr_data++) * 2; - data.u64 = 0; - while (mtrr_count-- > 0) - soc_msr_write(mtrr_reg++, data.msr); - - /* Setup the specified variable MTRRs */ - mtrr_reg = MTRR_PHYS_BASE(0); - mtrr_count = *mtrr_data++; - while (mtrr_count-- > 0) { - data.u32[0] = *mtrr_data++; - data.u32[1] = *mtrr_data++; - soc_msr_write(mtrr_reg++, data.msr); /* Base */ - data.u32[0] = *mtrr_data++; - data.u32[1] = *mtrr_data++; - soc_msr_write(mtrr_reg++, data.msr); /* Mask */ - } - - /* Remove setup_stack_and_mtrrs data and return the new top_of_stack */ - top_of_stack = mtrr_data; - return top_of_stack; -} - -asmlinkage void soc_enable_mtrrs(void) -{ - union { - uint32_t u32[2]; - uint64_t u64; - msr_t msr; - } data; - - /* Enable MTRR. */ - data.msr = soc_msr_read(MTRR_DEF_TYPE_MSR); - data.u32[0] |= MTRR_DEF_TYPE_EN; - soc_msr_write(MTRR_DEF_TYPE_MSR, data.msr); -} -- To view, visit https://review.coreboot.org/c/coreboot/+/54300 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9fe9dc458383930a75d9459f77e347241d8b6f33 Gerrit-Change-Number: 54300 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: [WIP]arch/x86/postcar: Set up postcar in C code
by Arthur Heymans (Code Review) 14 May '21

14 May '21

Attention is currently required from: Arthur Heymans. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/54299 to look at the new patch set (#2). Change subject: [WIP]arch/x86/postcar: Set up postcar in C code ...................................................................... [WIP]arch/x86/postcar: Set up postcar in C code Change-Id: I5ec10e84118197a04de0a5194336ef8bb049bba4 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/exit_car.S M src/arch/x86/include/arch/romstage.h M src/arch/x86/postcar.c M src/arch/x86/postcar_loader.c M src/cpu/x86/mtrr/Makefile.inc M src/cpu/x86/mtrr/earlymtrr.c M src/include/cpu/x86/mtrr.h 7 files changed, 59 insertions(+), 211 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/54299/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/54299 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5ec10e84118197a04de0a5194336ef8bb049bba4 Gerrit-Change-Number: 54299 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/amd/cezanne: adding support for the changed AMD FSP API for USB PHY
by Julian Schroeder (Code Review) 14 May '21

14 May '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Matt Papageorge, Felix Held. Julian Schroeder has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54065 ) Change subject: soc/amd/cezanne: adding support for the changed AMD FSP API for USB PHY ...................................................................... Patch Set 7: (2 comments) Patchset: PS7: I ran the checkpatch tool. Should be ok now File src/vendorcode/amd/fsp/cezanne/FspmUpd.h: https://review.coreboot.org/c/coreboot/+/54065/comment/ecffbc12_6a0a9f20 PS6, Line 95: /** Offset 0x04D8**/ struct usb_phy_config *usb_phy; > should we just leave this as array of bytes and then cast to it later? it's ugly either way. this all needs to be converted into a clean C structure -- To view, visit https://review.coreboot.org/c/coreboot/+/54065 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I011ca40a334e4fd26778ca7f18b653298b14019b Gerrit-Change-Number: 54065 Gerrit-PatchSet: 7 Gerrit-Owner: Julian Schroeder <julianmarcusschroeder(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshall.dawson(a)amd.corp-partner.google.com> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 14 May 2021 12:43:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/cezanne: adding support for the changed AMD FSP API for USB PHY
by build bot (Jenkins) (Code Review) 14 May '21

14 May '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Matt Papageorge, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54065 ) Change subject: soc/amd/cezanne: adding support for the changed AMD FSP API for USB PHY ...................................................................... Patch Set 7: (2 comments) File src/soc/amd/cezanne/fsp_m_params.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-119327): https://review.coreboot.org/c/coreboot/+/54065/comment/b2e4c198_e1c74749 PS7, Line 131: if (config->usb_phy_custom) suspect code indent for conditional statements (8, 12) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-119327): https://review.coreboot.org/c/coreboot/+/54065/comment/81e3fa66_c59f0223 PS7, Line 133: else suspect code indent for conditional statements (8, 12) -- To view, visit https://review.coreboot.org/c/coreboot/+/54065 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I011ca40a334e4fd26778ca7f18b653298b14019b Gerrit-Change-Number: 54065 Gerrit-PatchSet: 7 Gerrit-Owner: Julian Schroeder <julianmarcusschroeder(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshall.dawson(a)amd.corp-partner.google.com> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 14 May 2021 12:34:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/cezanne: adding support for the changed AMD FSP API for USB PHY
by Julian Schroeder (Code Review) 14 May '21

14 May '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Matt Papageorge, Julian Schroeder, Felix Held. Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Matt Papageorge, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/54065 to look at the new patch set (#7). Change subject: soc/amd/cezanne: adding support for the changed AMD FSP API for USB PHY ...................................................................... soc/amd/cezanne: adding support for the changed AMD FSP API for USB PHY The AMD FSP is using a new structure for USB and USB C phy settings. This patch removes old, unused structures, adds the new one and enables the devicetree interface for it. Signed-off-by: Julian Schroeder <julianmarcusschroeder(a)gmail.com> Change-Id: I011ca40a334e4fd26778ca7f18b653298b14019b --- M src/mainboard/google/guybrush/variants/baseboard/devicetree.cb M src/soc/amd/cezanne/chip.h M src/soc/amd/cezanne/fsp_m_params.c A src/vendorcode/amd/fsp/cezanne/FspUsb.h M src/vendorcode/amd/fsp/cezanne/FspmUpd.h 5 files changed, 182 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/54065/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/54065 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I011ca40a334e4fd26778ca7f18b653298b14019b Gerrit-Change-Number: 54065 Gerrit-PatchSet: 7 Gerrit-Owner: Julian Schroeder <julianmarcusschroeder(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshall.dawson(a)amd.corp-partner.google.com> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-Attention: Julian Schroeder <julianmarcusschroeder(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: [WIP]arch/x86/postcar: Set up postcar in C code
by build bot (Jenkins) (Code Review) 14 May '21

14 May '21

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54299 ) Change subject: [WIP]arch/x86/postcar: Set up postcar in C code ...................................................................... Patch Set 1: (1 comment) File src/arch/x86/postcar.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-119326): https://review.coreboot.org/c/coreboot/+/54299/comment/8fd82316_51056802 PS1, Line 26: for (int i = 0; i < ctx->max_var_mtrrs; i ++) { space prohibited before that '++' (ctx:WxB) -- To view, visit https://review.coreboot.org/c/coreboot/+/54299 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5ec10e84118197a04de0a5194336ef8bb049bba4 Gerrit-Change-Number: 54299 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 14 May 2021 11:20:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP]arch/x86/postcar: Set up postcar in C code
by Arthur Heymans (Code Review) 14 May '21

14 May '21

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/54299 ) Change subject: [WIP]arch/x86/postcar: Set up postcar in C code ...................................................................... [WIP]arch/x86/postcar: Set up postcar in C code Change-Id: I5ec10e84118197a04de0a5194336ef8bb049bba4 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/exit_car.S M src/arch/x86/include/arch/romstage.h M src/arch/x86/postcar.c M src/arch/x86/postcar_loader.c M src/cpu/x86/mtrr/earlymtrr.c M src/include/cpu/x86/mtrr.h 6 files changed, 61 insertions(+), 190 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/54299/1 diff --git a/src/arch/x86/exit_car.S b/src/arch/x86/exit_car.S index 925fd46..346cb5f 100644 --- a/src/arch/x86/exit_car.S +++ b/src/arch/x86/exit_car.S @@ -4,13 +4,6 @@ #include <cpu/x86/cr.h> #include <cpu/x86/cache.h> -.section ".module_parameters", "aw", @progbits -/* stack_top indicates the stack to pull MTRR information from. */ -.global post_car_stack_top -post_car_stack_top: -.long 0 -.long 0 - #if defined(__x86_64__) .code64 .macro pop_eax_edx @@ -91,23 +84,6 @@ /* Ensure cache is clean. */ invd - /* Set up new stack. */ - mov post_car_stack_top, %esp - - /* - * Honor variable MTRR information pushed on the stack with the - * following layout: - * - * Offset: Value - * ... - * 0x14: MTRR mask 0 63:32 - * 0x10: MTRR mask 0 31:0 - * 0x0c: MTRR base 0 63:32 - * 0x08: MTRR base 0 31:0 - * 0x04: Number of variable MTRRs to set - * 0x00: Number of variable MTRRs to clear - */ - #if CONFIG(SOC_SETS_MSRS) mov %esp, %ebp @@ -129,50 +105,7 @@ /* Align stack to 16 bytes at call instruction. */ andl $0xfffffff0, %esp call soc_enable_mtrrs -#else /* CONFIG_SOC_SETS_MSRS */ - /* Clear variable MTRRs. */ - pop_ebx_esi /* ebx: Number to clear, esi: Number to set */ - test %ebx, %ebx - jz 2f - xor %eax, %eax - xor %edx, %edx - mov $(MTRR_PHYS_BASE(0)), %ecx -1: - wrmsr - inc %ecx - wrmsr - inc %ecx - dec %ebx - jnz 1b -2: - - /* Set Variable MTRRs based on stack contents. */ - test %esi, %esi - jz 2f - mov $(MTRR_PHYS_BASE(0)), %ecx -1: - /* Write MTRR base. */ - pop_eax_edx - wrmsr - inc %ecx - /* Write MTRR mask. */ - pop_eax_edx - wrmsr - inc %ecx - - dec %esi - jnz 1b -2: - - /* Enable MTRR. */ - mov $(MTRR_DEF_TYPE_MSR), %ecx - rdmsr - /* Make default type uncacheable. */ - and $(~(MTRR_DEF_TYPE_MASK)), %eax - or $(MTRR_DEF_TYPE_EN), %eax - wrmsr #endif /* CONFIG_SOC_SETS_MSRS */ - movl $_estack, %esp /* Align stack to 16 bytes at call instruction. */ andl $0xfffffff0, %esp diff --git a/src/arch/x86/include/arch/romstage.h b/src/arch/x86/include/arch/romstage.h index 28d29bf..7a2a417 100644 --- a/src/arch/x86/include/arch/romstage.h +++ b/src/arch/x86/include/arch/romstage.h @@ -16,17 +16,15 @@ */ struct postcar_frame { - uintptr_t stack; int skip_common_mtrr; - struct var_mtrr_context ctx; + struct var_mtrr_context *mtrr; }; /* - * Initialize postcar_frame object allocating stack from cbmem, - * with stack_size == 0, default 4 KiB is allocated. + * Initialize postcar_frame object. * Returns 0 on success, < 0 on error. */ -int postcar_frame_init(struct postcar_frame *pcf, size_t stack_size); +int postcar_frame_init(struct postcar_frame *pcf); /* * Add variable MTRR covering the provided range with MTRR type. diff --git a/src/arch/x86/postcar.c b/src/arch/x86/postcar.c index 1df8c44..a6d5c6f 100644 --- a/src/arch/x86/postcar.c +++ b/src/arch/x86/postcar.c @@ -1,11 +1,13 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <cpu/x86/msr.h> #include <arch/romstage.h> #include <cbmem.h> #include <console/console.h> #include <cpu/x86/mtrr.h> #include <main_decl.h> #include <program_loading.h> +#include <stdint.h> #include <timestamp.h> /* @@ -14,8 +16,32 @@ */ __weak void late_car_teardown(void) { /* do nothing */ } +/* This is set by romstage */ +__attribute__((used, __section__(".module_parameters"))) const uint64_t post_car_mtrrs; + +static void mtrr_setup(void) +{ + const struct var_mtrr_context *ctx = (void *)(uintptr_t)post_car_mtrrs; + const msr_t zero = { .lo = 0, .hi = 0 }; + for (int i = 0; i < ctx->max_var_mtrrs; i ++) { + wrmsr(MTRR_PHYS_BASE(i), zero); + wrmsr(MTRR_PHYS_MASK(i), zero); + } + for (int i = 0; i < ctx->used_var_mtrrs; i++) { + wrmsr(MTRR_PHYS_BASE(i), ctx->mtrr[i].base); + wrmsr(MTRR_PHYS_MASK(i), ctx->mtrr[i].mask); + } + /* Enable MTRR */ + msr_t mtrr_def_type = rdmsr(MTRR_DEF_TYPE_MSR); + mtrr_def_type.lo &= MTRR_DEF_TYPE_MASK; + mtrr_def_type.lo |= MTRR_DEF_TYPE_EN; + wrmsr(MTRR_DEF_TYPE_MSR, mtrr_def_type); +} + void main(void) { + mtrr_setup(); + late_car_teardown(); console_init(); diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index 8a5f902..9f5c47c 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -13,65 +13,32 @@ #include <timestamp.h> #include <security/vboot/vboot_common.h> -static inline void stack_push(struct postcar_frame *pcf, uint32_t val) +static size_t var_mtrr_ctx_size(void) { - uint32_t *ptr; - - pcf->stack -= sizeof(val); - ptr = (void *)pcf->stack; - *ptr = val; + int mtrr_count = get_var_mtrr_count(); + return sizeof(struct var_mtrr_context) + mtrr_count * 2 * sizeof(msr_t); } -static void postcar_frame_prepare(struct postcar_frame *pcf) +int postcar_frame_init(struct postcar_frame *pcf) { - var_mtrr_context_init(&pcf->ctx, pcf); -} + struct var_mtrr_context *ctx; -int postcar_frame_init(struct postcar_frame *pcf, size_t stack_size) -{ - void *stack; - - /* - * Use default postcar stack size of 4 KiB. This value should - * not be decreased, because if mainboards use vboot, 1 KiB will - * not be enough anymore. - */ - - if (stack_size == 0) - stack_size = 4 * KiB; - - stack = cbmem_add(CBMEM_ID_ROMSTAGE_RAM_STACK, stack_size); - if (stack == NULL) { - printk(BIOS_ERR, "Couldn't add %zd byte stack in cbmem.\n", - stack_size); + ctx = cbmem_add(CBMEM_ID_ROMSTAGE_RAM_STACK, var_mtrr_ctx_size()); + if (ctx == NULL) { + printk(BIOS_ERR, "Couldn't add var_mtrr_ctx setup in cbmem.\n"); return -1; } - postcar_frame_prepare(pcf); - pcf->stack = (uintptr_t)stack; - pcf->stack += stack_size; + pcf->mtrr = ctx; + var_mtrr_context_init(pcf->mtrr); + return 0; } -static void postcar_var_mtrr_set(const struct var_mtrr_context *ctx, - uintptr_t addr, size_t size, - msr_t base, msr_t mask) -{ - struct postcar_frame *pcf = ctx->arg; - - printk(BIOS_DEBUG, "MTRR Range: Start=%lx End=%lx (Size %zx)\n", - addr, addr + size, size); - - stack_push(pcf, mask.hi); - stack_push(pcf, mask.lo); - stack_push(pcf, base.hi); - stack_push(pcf, base.lo); -} - void postcar_frame_add_mtrr(struct postcar_frame *pcf, uintptr_t addr, size_t size, int type) { - var_mtrr_set_with_cb(&pcf->ctx, addr, size, type, postcar_var_mtrr_set); + var_mtrr_set_with_cb(pcf->mtrr, addr, size, type); } void postcar_frame_add_romcache(struct postcar_frame *pcf, int type) @@ -94,7 +61,7 @@ * cache-as-ram is torn down as well as the MTRR settings to use. */ void prepare_and_run_postcar(struct postcar_frame *pcf) { - if (postcar_frame_init(pcf, 0)) + if (postcar_frame_init(pcf)) die("Unable to initialize postcar frame.\n"); fill_postcar_frame(pcf); @@ -105,16 +72,6 @@ /* We do not return here. */ } -static void postcar_commit_mtrrs(struct postcar_frame *pcf) -{ - /* - * Place the number of used variable MTRRs on stack then max number - * of variable MTRRs supported in the system. - */ - stack_push(pcf, pcf->ctx.used_var_mtrrs); - stack_push(pcf, pcf->ctx.max_var_mtrrs); -} - static void finalize_load(uintptr_t *stack_top_ptr, uintptr_t stack_top) { *stack_top_ptr = stack_top; @@ -122,8 +79,8 @@ * Signal to rest of system that another update was made to the * postcar program prior to running it. */ - prog_segment_loaded((uintptr_t)stack_top_ptr, sizeof(uintptr_t), - SEG_FINAL); + prog_segment_loaded((uintptr_t)stack_top_ptr, sizeof(uintptr_t), SEG_FINAL); + prog_segment_loaded((uintptr_t)stack_top, var_mtrr_ctx_size(), SEG_FINAL); } static void load_postcar_cbfs(struct prog *prog, struct postcar_frame *pcf) @@ -144,7 +101,7 @@ die_with_post_code(POST_INVALID_ROM, "No parameters found in after CAR program.\n"); - finalize_load(rsl.params, pcf->stack); + finalize_load(rsl.params, (uintptr_t)pcf->mtrr); stage_cache_add(STAGE_POSTCAR, prog); } @@ -177,14 +134,12 @@ struct prog prog = PROG_INIT(PROG_POSTCAR, CONFIG_CBFS_PREFIX "/postcar"); - postcar_commit_mtrrs(pcf); - if (resume_from_stage_cache()) { stage_cache_load_stage(STAGE_POSTCAR, &prog); /* This is here to allow platforms to pass different stack parameters between S3 resume and normal boot. On the platforms where the values are the same it's a nop. */ - finalize_load(prog.arg, pcf->stack); + finalize_load(prog.arg, (uintptr_t)pcf->mtrr); if (prog_entry(&prog) == NULL) postcar_cache_invalid(); diff --git a/src/cpu/x86/mtrr/earlymtrr.c b/src/cpu/x86/mtrr/earlymtrr.c index aa301d0..25507a5 100644 --- a/src/cpu/x86/mtrr/earlymtrr.c +++ b/src/cpu/x86/mtrr/earlymtrr.c @@ -5,6 +5,7 @@ #include <cpu/x86/msr.h> #include <console/console.h> #include <commonlib/bsd/helpers.h> +#include <stdint.h> /* Get first available variable MTRR. * Returns var# if available, else returns -1. @@ -66,19 +67,15 @@ } } -void var_mtrr_context_init(struct var_mtrr_context *ctx, void *arg) +void var_mtrr_context_init(struct var_mtrr_context *ctx) { - ctx->upper_mask = (1U << (cpu_phys_address_size() - 32)) - 1; ctx->max_var_mtrrs = get_var_mtrr_count(); ctx->used_var_mtrrs = 0; - ctx->arg = arg; } -int var_mtrr_set_with_cb(struct var_mtrr_context *ctx, uintptr_t addr, size_t size, - int type, void (*callback)(const struct var_mtrr_context *ctx, - uintptr_t base_addr, size_t size, - msr_t base, msr_t mask)) +int var_mtrr_set_with_cb(struct var_mtrr_context *ctx, uintptr_t addr, size_t size, int type) { + const uint32_t upper_mask = (1U << (cpu_phys_address_size() - 32)) - 1; /* Utilize additional MTRRs if the specified size is greater than the base address alignment. */ while (size != 0) { @@ -107,9 +104,10 @@ base.hi = (uint64_t)addr >> 32; base.lo = addr | type; - mask.hi = ctx->upper_mask; + mask.hi = upper_mask; mask.lo = ~(mtrr_size - 1) | MTRR_PHYS_MASK_VALID; - callback(ctx, addr, mtrr_size, base, mask); + ctx->mtrr[ctx->used_var_mtrrs].base = base; + ctx->mtrr[ctx->used_var_mtrrs].mask = mask; ctx->used_var_mtrrs++; size -= mtrr_size; @@ -118,17 +116,3 @@ return 0; } - -static void set_mtrr(const struct var_mtrr_context *ctx, uintptr_t base_addr, size_t size, - msr_t base, msr_t mask) -{ - int i = var_mtrr_context_current_mtrr(ctx); - - wrmsr(MTRR_PHYS_BASE(i), base); - wrmsr(MTRR_PHYS_MASK(i), mask); -} - -int var_mtrr_set(struct var_mtrr_context *ctx, uintptr_t addr, size_t size, int type) -{ - return var_mtrr_set_with_cb(ctx, addr, size, type, set_mtrr); -} diff --git a/src/include/cpu/x86/mtrr.h b/src/include/cpu/x86/mtrr.h index b8d1517..491291e 100644 --- a/src/include/cpu/x86/mtrr.h +++ b/src/include/cpu/x86/mtrr.h @@ -113,43 +113,18 @@ asmlinkage void display_mtrrs(void); -/* Variable MTRR structure to help track and set MTRRs prior to ramstage. This - and the following APIs can be used to set up more complex MTRR solutions - instead of open coding get_free_var_mtrr() and set_var_mtrr() or for determining - a future solution, such as postcar_loader. */ struct var_mtrr_context { - uint32_t upper_mask; - int max_var_mtrrs; - int used_var_mtrrs; - void *arg; /* optional callback parameter */ + uint32_t max_var_mtrrs; + uint32_t used_var_mtrrs; + struct { + msr_t base; + msr_t mask; + } mtrr[]; }; -/* Returns 0-relative MTRR from context. Use MTRR_PHYS_BASE|MASK macros for calculating - MSR address value. */ -static inline int var_mtrr_context_current_mtrr(const struct var_mtrr_context *ctx) -{ - return ctx->used_var_mtrrs; -} +void var_mtrr_context_init(struct var_mtrr_context *ctx); +int var_mtrr_set_with_cb(struct var_mtrr_context *ctx, uintptr_t addr, size_t size, int type); -/* Initialize var_mtrr_context object. Assumes all variable MTRRs are not yet used. */ -void var_mtrr_context_init(struct var_mtrr_context *ctx, void *arg); -/* Allocate a variable mtrr base and mask, calling the provided callback for each MTRR - MSR base-mask pair needed to accommodate the address and size request. - Returns < 0 on error and 0 on success. */ -int var_mtrr_set_with_cb(struct var_mtrr_context *ctx, - uintptr_t addr, size_t size, int type, - void (*callback)(const struct var_mtrr_context *ctx, - uintptr_t base_addr, size_t size, - msr_t base, msr_t mask)); -/* Same as var_mtrr_set_with_cb() but just write the MSRs directly. */ -int var_mtrr_set(struct var_mtrr_context *ctx, uintptr_t addr, size_t size, int type); - -/* - * Set the MTRRs using the data on the stack from setup_stack_and_mtrrs. - * Return a new top_of_stack value which removes the setup_stack_and_mtrrs data. - */ -asmlinkage void *soc_set_mtrrs(void *top_of_stack); -asmlinkage void soc_enable_mtrrs(void); /* fms: find most significant bit set, stolen from Linux Kernel Source. */ static inline unsigned int fms(unsigned int x) -- To view, visit https://review.coreboot.org/c/coreboot/+/54299 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5ec10e84118197a04de0a5194336ef8bb049bba4 Gerrit-Change-Number: 54299 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: arch/x86/postcar: Use a separate stack for C execution
by Arthur Heymans (Code Review) 14 May '21

14 May '21

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/54298 ) Change subject: arch/x86/postcar: Use a separate stack for C execution ...................................................................... arch/x86/postcar: Use a separate stack for C execution Add a stack in .bss for C execution. This will make it easier to move the setup of MTRRs in C code. Change-Id: I67cbc988051036b1a0519cec9ed614acede31fd7 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/exit_car.S 1 file changed, 13 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/54298/1 diff --git a/src/arch/x86/exit_car.S b/src/arch/x86/exit_car.S index fae7899..925fd46 100644 --- a/src/arch/x86/exit_car.S +++ b/src/arch/x86/exit_car.S @@ -35,6 +35,18 @@ .endm #endif +/* Place the stack in the bss section. It's not necessary to define it in the + * the linker script. */ + .section .bss, "aw", @nobits +.global _stack +.global _estack +.global _stack_size + +_stack: +.space CONFIG_STACK_SIZE +_estack: +.set _stack_size, _estack - _stack + .text .global _start _start: @@ -161,6 +173,7 @@ wrmsr #endif /* CONFIG_SOC_SETS_MSRS */ + movl $_estack, %esp /* Align stack to 16 bytes at call instruction. */ andl $0xfffffff0, %esp /* Call into main for postcar. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/54298 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I67cbc988051036b1a0519cec9ed614acede31fd7 Gerrit-Change-Number: 54298 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/google/mancomb: enable DDI0-DP port
by Ivy Jian (Code Review) 14 May '21

14 May '21

Ivy Jian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/54297 ) Change subject: mb/google/mancomb: enable DDI0-DP port ...................................................................... mb/google/mancomb: enable DDI0-DP port Configure DDI-0 connector type to DP. BUG=b:187856682 TEST=Build and boot into OS Signed-off-by: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com> Change-Id: Ic8af14509b0d246c5c2da6e1a48991384471e69f --- M src/mainboard/google/mancomb/port_descriptors.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/54297/1 diff --git a/src/mainboard/google/mancomb/port_descriptors.c b/src/mainboard/google/mancomb/port_descriptors.c index b894cb2..b992906 100644 --- a/src/mainboard/google/mancomb/port_descriptors.c +++ b/src/mainboard/google/mancomb/port_descriptors.c @@ -79,8 +79,8 @@ /* TODO: verify the DDI table, since this is mostly an educated guess right now */ static const fsp_ddi_descriptor mancomb_czn_ddi_descriptors[] = { - { /* DDI0 - eDP */ - .connector_type = DDI_UNUSED_TYPE, + { /* DDI0 - DP */ + .connector_type = DDI_DP, .aux_index = DDI_AUX1, .hdp_index = DDI_HDP1 }, -- To view, visit https://review.coreboot.org/c/coreboot/+/54297 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic8af14509b0d246c5c2da6e1a48991384471e69f Gerrit-Change-Number: 54297 Gerrit-PatchSet: 1 Gerrit-Owner: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: Documentation/distributions: List System76
by Patrick Georgi (Code Review) 14 May '21

14 May '21

Attention is currently required from: Tim Crawford, Jeremy Soller. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54273 ) Change subject: Documentation/distributions: List System76 ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Paul, thank you for noticing and starting the effort, System76 really is missing on that list! Jeremy, Tim, could you propose a more descriptive paragraph in line with the other vendor descriptions? -- To view, visit https://review.coreboot.org/c/coreboot/+/54273 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I805f558514eb50580b5bd79bd4f964e66a15158d Gerrit-Change-Number: 54273 Gerrit-PatchSet: 2 Gerrit-Owner: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Comment-Date: Fri, 14 May 2021 10:18:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021