coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: util/spd_tools: Add LP5 support for ADL
by Tim Wawrzynczak (Code Review) 03 Nov '21

03 Nov '21

Attention is currently required from: Reka Norman, Sridhar Siricilla, Nick Vaccaro, Balaji Manigandan, Krishna P Bhat D, Karthik Ramasubramanian. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58679 ) Change subject: util/spd_tools: Add LP5 support for ADL ...................................................................... Patch Set 8: (1 comment) Patchset: PS8: > That's a good point. […] The lp4x SPD tools support Cezanne (AMD) as well, and the ddr4 tools support Picasso (AMD) too, so the goal is to try and make a tool that can produce SPDs for different chipsets. It would be instructive I'm sure to see AMD's MRC requirements for LP5 😊 -- To view, visit https://review.coreboot.org/c/coreboot/+/58679 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic1e68d44f7c0ad64aa9904b7e1297d24bd5db56e Gerrit-Change-Number: 58679 Gerrit-PatchSet: 8 Gerrit-Owner: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 03 Nov 2021 18:49:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Reka Norman <rekanorman(a)chromium.org> Comment-In-Reply-To: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/spd_tools: Add LP5 support for ADL
by Nick Vaccaro (Code Review) 03 Nov '21

03 Nov '21

Attention is currently required from: Tim Wawrzynczak, Reka Norman, Sridhar Siricilla, Balaji Manigandan, Krishna P Bhat D, Karthik Ramasubramanian. Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58679 ) Change subject: util/spd_tools: Add LP5 support for ADL ...................................................................... Patch Set 8: (1 comment) Patchset: PS8: > It’s been bothering me how ADL-specific this implementation is. E.g. […] That's a good point. Given the spd tools are currently (AFAIK) customized around intel requirements, we probably should make that differentiation, perhaps move the current contents of util/spd_tools/src/ to util/spd_tools/src/intel/? Has the spd tool been used with AMD platforms, or just Intel? I'm not sure how different AMD requirements are. re: "wait and see what other platforms do, and generalise the code as needed then" I would agree that we should wait to add features until they are needed. -- To view, visit https://review.coreboot.org/c/coreboot/+/58679 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic1e68d44f7c0ad64aa9904b7e1297d24bd5db56e Gerrit-Change-Number: 58679 Gerrit-PatchSet: 8 Gerrit-Owner: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 03 Nov 2021 18:46:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Reka Norman <rekanorman(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/spd_tools: Add LP5 support for ADL
by Tim Wawrzynczak (Code Review) 03 Nov '21

03 Nov '21

Attention is currently required from: Reka Norman, Sridhar Siricilla, Nick Vaccaro, Balaji Manigandan, Krishna P Bhat D, Karthik Ramasubramanian. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58679 ) Change subject: util/spd_tools: Add LP5 support for ADL ...................................................................... Patch Set 8: Code-Review+1 (4 comments) Patchset: PS8: > It’s been bothering me how ADL-specific this implementation is. E.g. […] I completely agree with your assessment Reka. This is the first platform we're supporting LP5 on, so we may have to make some tweaks in the future depending on other vendors or chipsets. This is the approach to make things the most generally applicable for all chipsets. File util/spd_tools/src/spd_gen/lp5.go: https://review.coreboot.org/c/coreboot/+/58679/comment/d8481997_3f24b8b1 PS5, Line 456: ok == false nit: I think the convention is `; !ok {` https://review.coreboot.org/c/coreboot/+/58679/comment/dce75c7b_9ab51db6 PS5, Line 484: ; ok == false `; !ok {` File util/spd_tools/src/spd_gen/lp5.go: https://review.coreboot.org/c/coreboot/+/58679/comment/61050676_83672385 PS8, Line 603: k == false nit: `!ok` -- To view, visit https://review.coreboot.org/c/coreboot/+/58679 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic1e68d44f7c0ad64aa9904b7e1297d24bd5db56e Gerrit-Change-Number: 58679 Gerrit-PatchSet: 8 Gerrit-Owner: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 03 Nov 2021 18:42:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Reka Norman <rekanorman(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/cezanne/include/aoac_defs: drop leading newline
by Felix Held (Code Review) 03 Nov '21

03 Nov '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58863 ) Change subject: soc/amd/cezanne/include/aoac_defs: drop leading newline ...................................................................... soc/amd/cezanne/include/aoac_defs: drop leading newline Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I8458fbee7edd19117a207f39ac8f9575b1374fbc Reviewed-on: https://review.coreboot.org/c/coreboot/+/58863 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/cezanne/include/soc/aoac_defs.h 1 file changed, 0 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/cezanne/include/soc/aoac_defs.h b/src/soc/amd/cezanne/include/soc/aoac_defs.h index 5309cb0..25311dd 100644 --- a/src/soc/amd/cezanne/include/soc/aoac_defs.h +++ b/src/soc/amd/cezanne/include/soc/aoac_defs.h @@ -1,4 +1,3 @@ - /* SPDX-License-Identifier: GPL-2.0-only */ #ifndef AMD_CEZANNE_AOAC_DEFS_H 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/coreboot/+/58863 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8458fbee7edd19117a207f39ac8f9575b1374fbc Gerrit-Change-Number: 58863 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/cezanne/include: replace PICASSO with CEZANNE in include guards
by Felix Held (Code Review) 03 Nov '21

03 Nov '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58862 ) Change subject: soc/amd/cezanne/include: replace PICASSO with CEZANNE in include guards ...................................................................... soc/amd/cezanne/include: replace PICASSO with CEZANNE in include guards Somehow missed renaming those when creating the coreboot support for Cezanne. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I13c28f67d32ba987987cfc2b45e248d535ccdca9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58862 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/cezanne/include/soc/platform_descriptors.h M src/soc/amd/cezanne/include/soc/uart.h 2 files changed, 6 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/cezanne/include/soc/platform_descriptors.h b/src/soc/amd/cezanne/include/soc/platform_descriptors.h index 3a4bc62..d6d65f0 100644 --- a/src/soc/amd/cezanne/include/soc/platform_descriptors.h +++ b/src/soc/amd/cezanne/include/soc/platform_descriptors.h @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#ifndef AMD_PICASSO_PLATFORM_DESCRIPTORS_H -#define AMD_PICASSO_PLATFORM_DESCRIPTORS_H +#ifndef AMD_CEZANNE_PLATFORM_DESCRIPTORS_H +#define AMD_CEZANNE_PLATFORM_DESCRIPTORS_H #include <types.h> #include <platform_descriptors.h> @@ -14,4 +14,4 @@ void mb_pre_fspm(void); -#endif /* AMD_PICASSO_PLATFORM_DESCRIPTORS_H */ +#endif /* AMD_CEZANNE_PLATFORM_DESCRIPTORS_H */ diff --git a/src/soc/amd/cezanne/include/soc/uart.h b/src/soc/amd/cezanne/include/soc/uart.h index 9b99cd4..8affe10 100644 --- a/src/soc/amd/cezanne/include/soc/uart.h +++ b/src/soc/amd/cezanne/include/soc/uart.h @@ -1,11 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#ifndef AMD_PICASSO_UART_H -#define AMD_PICASSO_UART_H +#ifndef AMD_CEZANNE_UART_H +#define AMD_CEZANNE_UART_H #include <types.h> void set_uart_config(unsigned int idx); /* configure hardware of FCH UART selected by idx */ void clear_uart_legacy_config(void); /* disable legacy I/O decode for FCH UART */ -#endif /* AMD_PICASSO_UART_H */ +#endif /* AMD_CEZANNE_UART_H */ 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/coreboot/+/58862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I13c28f67d32ba987987cfc2b45e248d535ccdca9 Gerrit-Change-Number: 58862 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/*/cpu: handle mp_init_with_smm failure
by Felix Held (Code Review) 03 Nov '21

03 Nov '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58859 ) Change subject: soc/amd/*/cpu: handle mp_init_with_smm failure ...................................................................... soc/amd/*/cpu: handle mp_init_with_smm failure When the mp_init_with_smm call returns a failure, coreboot can't just continue with the initialization and boot process due to the system being in a bad state. Ignoring the failure here would just cause the boot process failing elsewhere where it may not be obvious that the failed multi-processor initialization step was the root cause of that. I'm not 100% sure if calling do_cold_reset or calling die_with_post_code is the better option here. Calling do_cold_reset likely here would likely result in a boot-failure loop, so I call die_with_post_code here. BUG=b:193809448 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ifeadffb3bae749c4bbd7ad2f3f395201e67d9e28 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58859 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/cezanne/cpu.c M src/soc/amd/picasso/cpu.c M src/soc/amd/stoneyridge/cpu.c 3 files changed, 9 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/cezanne/cpu.c b/src/soc/amd/cezanne/cpu.c index c3d89bf..440b5ba 100644 --- a/src/soc/amd/cezanne/cpu.c +++ b/src/soc/amd/cezanne/cpu.c @@ -51,9 +51,9 @@ void mp_init_cpus(struct bus *cpu_bus) { - /* Clear for take-off */ - /* TODO: Handle mp_init_with_smm failure? */ - mp_init_with_smm(cpu_bus, &mp_ops); + if (mp_init_with_smm(cpu_bus, &mp_ops) != CB_SUCCESS) + die_with_post_code(POST_HW_INIT_FAILURE, + "mp_init_with_smm failed. Halting.\n"); /* pre_mp_init made the flash not cacheable. Reset to WP for performance. */ mtrr_use_temp_range(FLASH_BASE_ADDR, CONFIG_ROM_SIZE, MTRR_TYPE_WRPROT); diff --git a/src/soc/amd/picasso/cpu.c b/src/soc/amd/picasso/cpu.c index 9822326..b80b0f7 100644 --- a/src/soc/amd/picasso/cpu.c +++ b/src/soc/amd/picasso/cpu.c @@ -55,9 +55,9 @@ void mp_init_cpus(struct bus *cpu_bus) { - /* Clear for take-off */ - /* TODO: Handle mp_init_with_smm failure? */ - mp_init_with_smm(cpu_bus, &mp_ops); + if (mp_init_with_smm(cpu_bus, &mp_ops) != CB_SUCCESS) + die_with_post_code(POST_HW_INIT_FAILURE, + "mp_init_with_smm failed. Halting.\n"); /* pre_mp_init made the flash not cacheable. Reset to WP for performance. */ mtrr_use_temp_range(FLASH_BASE_ADDR, CONFIG_ROM_SIZE, MTRR_TYPE_WRPROT); diff --git a/src/soc/amd/stoneyridge/cpu.c b/src/soc/amd/stoneyridge/cpu.c index 0655032..6be76bf 100644 --- a/src/soc/amd/stoneyridge/cpu.c +++ b/src/soc/amd/stoneyridge/cpu.c @@ -52,9 +52,9 @@ void mp_init_cpus(struct bus *cpu_bus) { - /* Clear for take-off */ - /* TODO: Handle mp_init_with_smm failure? */ - mp_init_with_smm(cpu_bus, &mp_ops); + if (mp_init_with_smm(cpu_bus, &mp_ops) != CB_SUCCESS) + die_with_post_code(POST_HW_INIT_FAILURE, + "mp_init_with_smm failed. Halting.\n"); /* The flash is now no longer cacheable. Reset to WP for performance. */ mtrr_use_temp_range(FLASH_BASE_ADDR, CONFIG_ROM_SIZE, MTRR_TYPE_WRPROT); 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/coreboot/+/58859 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifeadffb3bae749c4bbd7ad2f3f395201e67d9e28 Gerrit-Change-Number: 58859 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: [RFC] ChromeOS: Add DECLARE_x_CROS_GPIOS()
by build bot (Jenkins) (Code Review) 03 Nov '21

03 Nov '21

Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58899 ) Change subject: [RFC] ChromeOS: Add DECLARE_x_CROS_GPIOS() ...................................................................... Patch Set 4: (4 comments) File src/vendorcode/google/chromeos/chromeos.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132203): https://review.coreboot.org/c/coreboot/+/58899/comment/59ebac5e_c372d984 PS4, Line 108: #define DECLARE_CROS_GPIOS(x) \ Macros with flow control statements should be avoided Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132203): https://review.coreboot.org/c/coreboot/+/58899/comment/e3c5ebf5_0a21d10b PS4, Line 109: const struct cros_gpio *variant_cros_gpios(size_t *num) \ please, no space before tabs Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132203): https://review.coreboot.org/c/coreboot/+/58899/comment/bbf8e039_e1f2c3f9 PS4, Line 115: #define DECLARE_WEAK_CROS_GPIOS(x) \ Macros with flow control statements should be avoided Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132203): https://review.coreboot.org/c/coreboot/+/58899/comment/3525fae5_9a371e7a PS4, Line 116: const struct cros_gpio *__weak variant_cros_gpios(size_t *num) \ please, no space before tabs -- To view, visit https://review.coreboot.org/c/coreboot/+/58899 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I88406fa1b54312616e6717af3d924436dc4ff1a6 Gerrit-Change-Number: 58899 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Wed, 03 Nov 2021 18:36:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/amd/mtrr: Remove topmem global variables
by Felix Held (Code Review) 03 Nov '21

03 Nov '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58693 ) Change subject: cpu/amd/mtrr: Remove topmem global variables ...................................................................... cpu/amd/mtrr: Remove topmem global variables The comments are not correct anymore. With AGESA there is no need to synchronize TOM_MEMx msr's between AP's. It's also not the best place to do so anyway. Change-Id: Iecbe1553035680b7c3780338070b852606d74d15 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58693 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/cpu/amd/mtrr/amd_mtrr.c M src/include/cpu/amd/mtrr.h M src/northbridge/amd/agesa/family14/northbridge.c M src/northbridge/amd/agesa/family15tn/northbridge.c M src/northbridge/amd/agesa/family16kb/northbridge.c M src/northbridge/amd/pi/00730F01/northbridge.c M src/soc/amd/stoneyridge/chip.c M src/soc/amd/stoneyridge/northbridge.c 8 files changed, 16 insertions(+), 82 deletions(-) Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved diff --git a/src/cpu/amd/mtrr/amd_mtrr.c b/src/cpu/amd/mtrr/amd_mtrr.c index 7502ca8..d17eedd 100644 --- a/src/cpu/amd/mtrr/amd_mtrr.c +++ b/src/cpu/amd/mtrr/amd_mtrr.c @@ -3,54 +3,11 @@ #include <amdblocks/biosram.h> #include <console/console.h> #include <device/device.h> -#include <arch/cpu.h> -#include <cpu/cpu.h> -#include <cpu/x86/mtrr.h> -#include <cpu/x86/msr.h> #include <cpu/amd/mtrr.h> -#include <cpu/x86/cache.h> - -/* These will likely move to some device node or cbmem. */ -static uint64_t amd_topmem = 0; -static uint64_t amd_topmem2 = 0; - -uint64_t bsp_topmem(void) -{ - return amd_topmem; -} - -uint64_t bsp_topmem2(void) -{ - return amd_topmem2; -} - -/* Take a copy of BSP CPUs TOP_MEM and TOP_MEM2 registers, - * so they can be distributed to AP CPUs. Not strictly MTRRs, - * but this is not that bad a place to have this code. - */ -void setup_bsp_ramtop(void) -{ - msr_t msr, msr2; - - /* TOP_MEM: the top of DRAM below 4G */ - msr = rdmsr(TOP_MEM); - printk(BIOS_INFO, - "%s, TOP MEM: msr.lo = 0x%08x, msr.hi = 0x%08x\n", - __func__, msr.lo, msr.hi); - - /* TOP_MEM2: the top of DRAM above 4G */ - msr2 = rdmsr(TOP_MEM2); - printk(BIOS_INFO, - "%s, TOP MEM2: msr.lo = 0x%08x, msr.hi = 0x%08x\n", - __func__, msr2.lo, msr2.hi); - - amd_topmem = (uint64_t) msr.hi << 32 | msr.lo; - amd_topmem2 = (uint64_t) msr2.hi << 32 | msr2.lo; -} void add_uma_resource_below_tolm(struct device *nb, int idx) { - uint32_t topmem = bsp_topmem(); + uint32_t topmem = amd_topmem(); uint32_t top_of_cacheable = restore_top_of_low_cacheable(); if (top_of_cacheable == topmem) diff --git a/src/include/cpu/amd/mtrr.h b/src/include/cpu/amd/mtrr.h index 9a943ac..6fe1628 100644 --- a/src/include/cpu/amd/mtrr.h +++ b/src/include/cpu/amd/mtrr.h @@ -66,10 +66,16 @@ ); } -/* To distribute topmem MSRs to APs. */ -void setup_bsp_ramtop(void); -uint64_t bsp_topmem(void); -uint64_t bsp_topmem2(void); +static inline uint64_t amd_topmem(void) +{ + return rdmsr(TOP_MEM).lo; +} + +static inline uint64_t amd_topmem2(void) +{ + msr_t msr = rdmsr(TOP_MEM2); + return (uint64_t)msr.hi << 32 | msr.lo; +} #endif #endif /* CPU_AMD_MTRR_H */ diff --git a/src/northbridge/amd/agesa/family14/northbridge.c b/src/northbridge/amd/agesa/family14/northbridge.c index 4aed96b..4b4ab67 100644 --- a/src/northbridge/amd/agesa/family14/northbridge.c +++ b/src/northbridge/amd/agesa/family14/northbridge.c @@ -818,13 +818,6 @@ static void root_complex_enable_dev(struct device *dev) { - static int done = 0; - - if (!done) { - setup_bsp_ramtop(); - done = 1; - } - /* Set the operations if it is a special bus type */ if (dev->path.type == DEVICE_PATH_DOMAIN) { dev->ops = &pci_domain_ops; diff --git a/src/northbridge/amd/agesa/family15tn/northbridge.c b/src/northbridge/amd/agesa/family15tn/northbridge.c index fe56736..bd0a5c8 100644 --- a/src/northbridge/amd/agesa/family15tn/northbridge.c +++ b/src/northbridge/amd/agesa/family15tn/northbridge.c @@ -737,7 +737,7 @@ sizek = 0; } else { - uint64_t topmem2 = bsp_topmem2(); + uint64_t topmem2 = amd_topmem2(); basek = 4*1024*1024; sizek = topmem2/1024 - basek; } @@ -900,13 +900,6 @@ static void root_complex_enable_dev(struct device *dev) { - static int done = 0; - - if (!done) { - setup_bsp_ramtop(); - done = 1; - } - /* Set the operations if it is a special bus type */ if (dev->path.type == DEVICE_PATH_DOMAIN) { dev->ops = &pci_domain_ops; diff --git a/src/northbridge/amd/agesa/family16kb/northbridge.c b/src/northbridge/amd/agesa/family16kb/northbridge.c index 21b4d69..3d5313a 100644 --- a/src/northbridge/amd/agesa/family16kb/northbridge.c +++ b/src/northbridge/amd/agesa/family16kb/northbridge.c @@ -750,7 +750,7 @@ sizek = 0; } else { - uint64_t topmem2 = bsp_topmem2(); + uint64_t topmem2 = amd_topmem2(); basek = 4*1024*1024; sizek = topmem2/1024 - basek; } @@ -922,13 +922,6 @@ static void root_complex_enable_dev(struct device *dev) { - static int done = 0; - - if (!done) { - setup_bsp_ramtop(); - done = 1; - } - /* Set the operations if it is a special bus type */ if (dev->path.type == DEVICE_PATH_DOMAIN) { dev->ops = &pci_domain_ops; diff --git a/src/northbridge/amd/pi/00730F01/northbridge.c b/src/northbridge/amd/pi/00730F01/northbridge.c index f1c5e23..332bf1ac 100644 --- a/src/northbridge/amd/pi/00730F01/northbridge.c +++ b/src/northbridge/amd/pi/00730F01/northbridge.c @@ -770,7 +770,7 @@ pci_domain_read_resources(dev); /* TOP_MEM MSR is our boundary between DRAM and MMIO under 4G */ - mmio_basek = bsp_topmem() >> 10; + mmio_basek = amd_topmem() >> 10; #if CONFIG_HW_MEM_HOLE_SIZEK != 0 /* if the hw mem hole is already set in raminit stage, here we will compare @@ -826,7 +826,7 @@ sizek = 0; } else { - uint64_t topmem2 = bsp_topmem2(); + uint64_t topmem2 = amd_topmem2(); basek = 4*1024*1024; sizek = topmem2/1024 - basek; } @@ -995,13 +995,6 @@ static void root_complex_enable_dev(struct device *dev) { - static int done = 0; - - if (!done) { - setup_bsp_ramtop(); - done = 1; - } - /* Set the operations if it is a special bus type */ if (dev->path.type == DEVICE_PATH_DOMAIN) { dev->ops = &pci_domain_ops; diff --git a/src/soc/amd/stoneyridge/chip.c b/src/soc/amd/stoneyridge/chip.c index 5cc52b6..666deda 100644 --- a/src/soc/amd/stoneyridge/chip.c +++ b/src/soc/amd/stoneyridge/chip.c @@ -131,7 +131,6 @@ static void soc_init(void *chip_info) { fch_init(chip_info); - setup_bsp_ramtop(); } static void soc_final(void *chip_info) diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index d5231ad..5d5ed7a 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -332,7 +332,7 @@ */ void amd_initcpuio(void) { - uintptr_t topmem = bsp_topmem(); + uintptr_t topmem = amd_topmem(); uintptr_t base, limit; /* Enable legacy video routing: D18F1xF4 VGA Enable */ -- To view, visit https://review.coreboot.org/c/coreboot/+/58693 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iecbe1553035680b7c3780338070b852606d74d15 Gerrit-Change-Number: 58693 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: intel/strago: Fix some CHROMEOS guards
by Kyösti Mälkki (Code Review) 03 Nov '21

03 Nov '21

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/58924 ) Change subject: intel/strago: Fix some CHROMEOS guards ...................................................................... intel/strago: Fix some CHROMEOS guards Change-Id: I0d5f1520a180ae6762c07dca7284894d9cf661b4 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/mainboard/intel/strago/Makefile.inc M src/mainboard/intel/strago/chromeos.c 2 files changed, 6 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/58924/1 diff --git a/src/mainboard/intel/strago/Makefile.inc b/src/mainboard/intel/strago/Makefile.inc index 21ae380..511a9b9 100644 --- a/src/mainboard/intel/strago/Makefile.inc +++ b/src/mainboard/intel/strago/Makefile.inc @@ -2,11 +2,10 @@ bootblock-$(CONFIG_ENABLE_BUILTIN_COM1) += com_init.c -romstage-$(CONFIG_MAINBOARD_HAS_CHROMEOS) += chromeos.c +all-y += chromeos.c -ramstage-$(CONFIG_MAINBOARD_HAS_CHROMEOS) += chromeos.c -ramstage-$(CONFIG_MAINBOARD_HAS_CHROMEOS) += ec.c -ramstage-$(CONFIG_MAINBOARD_HAS_CHROMEOS) += gpio.c +ramstage-y += ec.c +ramstage-y += gpio.c ramstage-y += irqroute.c ramstage-y += ramstage.c ramstage-y += w25q64.c diff --git a/src/mainboard/intel/strago/chromeos.c b/src/mainboard/intel/strago/chromeos.c index 1a945eb..0fd63c8 100644 --- a/src/mainboard/intel/strago/chromeos.c +++ b/src/mainboard/intel/strago/chromeos.c @@ -23,14 +23,13 @@ int get_write_protect_state(void) { /* - * The vboot loader queries this function in romstage. The GPIOs have + * This function might get queried early in romstage. The GPIOs have * not been set up yet as that configuration is done in ramstage. * Configuring this GPIO as input so that there isn't any ambiguity * in the reading. */ -#if ENV_ROMSTAGE - gpio_input_pullup(WP_GPIO); -#endif + if (ENV_ROMSTAGE_OR_BEFORE) + gpio_input_pullup(WP_GPIO); /* WP is enabled when the pin is reading high. */ return !!gpio_get(WP_GPIO); -- To view, visit https://review.coreboot.org/c/coreboot/+/58924 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0d5f1520a180ae6762c07dca7284894d9cf661b4 Gerrit-Change-Number: 58924 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: cpu/amd/mtrr: Remove topmem global variables
by Felix Held (Code Review) 03 Nov '21

03 Nov '21

Attention is currently required from: Marshall Dawson, Arthur Heymans. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58693 ) Change subject: cpu/amd/mtrr: Remove topmem global variables ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/58693 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iecbe1553035680b7c3780338070b852606d74d15 Gerrit-Change-Number: 58693 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 03 Nov 2021 18:36:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021