coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: mb/prodrive/hermes: Rename EEPROM access functions
by Angel Pons (Code Review) 28 May '21

28 May '21

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52885 ) Change subject: mb/prodrive/hermes: Rename EEPROM access functions ...................................................................... mb/prodrive/hermes: Rename EEPROM access functions Change-Id: I84b9ef080f1ac91ea6f7273457b882677abf70d3 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/52885 Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/prodrive/hermes/eeprom.c M src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h 2 files changed, 10 insertions(+), 10 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/src/mainboard/prodrive/hermes/eeprom.c b/src/mainboard/prodrive/hermes/eeprom.c index 614f069..cdea67d 100644 --- a/src/mainboard/prodrive/hermes/eeprom.c +++ b/src/mainboard/prodrive/hermes/eeprom.c @@ -21,7 +21,7 @@ { u8 blob[8] = {0}; - if (!read_write_config(blob, offset, ARRAY_SIZE(blob))) { + if (!eeprom_read_buffer(blob, offset, ARRAY_SIZE(blob))) { /* Check signature */ if (*(uint64_t *)blob == signature) { printk(BIOS_DEBUG, "CFG EEPROM: Signature valid.\n"); @@ -41,7 +41,7 @@ { const size_t board_settings_offset = offsetof(struct eeprom_layout, BoardSettings); - if (read_write_config(board_cfg, board_settings_offset, sizeof(*board_cfg))) { + if (eeprom_read_buffer(board_cfg, board_settings_offset, sizeof(*board_cfg))) { printk(BIOS_ERR, "CFG EEPROM: Failed to read board settings\n"); return false; } @@ -79,7 +79,7 @@ static int valid = 0; if (valid == 0) { - if (read_write_config(&bmc_cfg, bmc_settings_offset, sizeof(bmc_cfg))) { + if (eeprom_read_buffer(&bmc_cfg, bmc_settings_offset, sizeof(bmc_cfg))) { printk(BIOS_ERR, "CFG EEPROM: Failed to read BMC settings\n"); return NULL; } @@ -99,8 +99,8 @@ return hsi; } -/* Read data from offset and write it to offset in UPD */ -bool read_write_config(void *blob, size_t read_offset, size_t size) +/* Read data from an EEPROM on SMBus and write it to a buffer */ +bool eeprom_read_buffer(void *blob, size_t read_offset, size_t size) { int ret = 0; @@ -141,7 +141,7 @@ * Write a single byte into the EEPROM at specified offset. * Returns true on error, false on success. */ -static bool write_byte_eeprom(const uint8_t data, const uint16_t write_offset) +static bool eeprom_write_byte(const uint8_t data, const uint16_t write_offset) { int ret = 0; @@ -188,7 +188,7 @@ bool changed = false; /* Read old settings */ - if (read_write_config(&old_layout, off, sizeof(old_layout))) { + if (eeprom_read_buffer(&old_layout, off, sizeof(old_layout))) { printk(BIOS_ERR, "CFG EEPROM: Read operation failed\n"); return true; } @@ -201,7 +201,7 @@ for (size_t i = 0; i < sizeof(old_layout); i++) { if (old[i] != new[i]) { changed = true; - if (write_byte_eeprom(new[i], off + i)) { + if (eeprom_write_byte(new[i], off + i)) { printk(BIOS_ERR, "CFG EEPROM: Write operation failed\n"); ret = true; break; diff --git a/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h b/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h index 3a28219..4e6557a 100644 --- a/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h +++ b/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h @@ -91,7 +91,7 @@ _Static_assert(sizeof(FSPS_UPD) <= 0xC00, "FSPS_UPD too big"); _Static_assert(sizeof(struct eeprom_layout) == 0x2000, "EEPROM layout size mismatch"); -bool read_write_config(void *blob, size_t read_offset, size_t size); +bool eeprom_read_buffer(void *blob, size_t read_offset, size_t size); int check_signature(const size_t offset, const uint64_t signature); struct eeprom_board_settings *get_board_settings(void); struct eeprom_bmc_settings *get_bmc_settings(void); @@ -103,7 +103,7 @@ do { \ typeof(dest->opt_name) __tmp; \ size_t __off = offsetof(struct eeprom_layout, section_name); \ - bool ret = read_write_config(&__tmp, \ + bool ret = eeprom_read_buffer(&__tmp, \ __off + offsetof(section_type, opt_name), \ sizeof(__tmp)); \ if (ret) { \ -- To view, visit https://review.coreboot.org/c/coreboot/+/52885 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84b9ef080f1ac91ea6f7273457b882677abf70d3 Gerrit-Change-Number: 52885 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> 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]: mb/prodrive/hermes: Simplify `read_write_config` signature
by Angel Pons (Code Review) 28 May '21

28 May '21

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52884 ) Change subject: mb/prodrive/hermes: Simplify `read_write_config` signature ...................................................................... mb/prodrive/hermes: Simplify `read_write_config` signature The `write_offset` parameter is always zero. Remove it. Change-Id: Ib63cb25904ad6c1c7424a9c01d8bf1e84c08453b Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/52884 Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/prodrive/hermes/eeprom.c M src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h 2 files changed, 7 insertions(+), 8 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/src/mainboard/prodrive/hermes/eeprom.c b/src/mainboard/prodrive/hermes/eeprom.c index a712156..614f069 100644 --- a/src/mainboard/prodrive/hermes/eeprom.c +++ b/src/mainboard/prodrive/hermes/eeprom.c @@ -21,7 +21,7 @@ { u8 blob[8] = {0}; - if (!read_write_config(blob, offset, 0, ARRAY_SIZE(blob))) { + if (!read_write_config(blob, offset, ARRAY_SIZE(blob))) { /* Check signature */ if (*(uint64_t *)blob == signature) { printk(BIOS_DEBUG, "CFG EEPROM: Signature valid.\n"); @@ -41,7 +41,7 @@ { const size_t board_settings_offset = offsetof(struct eeprom_layout, BoardSettings); - if (read_write_config(board_cfg, board_settings_offset, 0, sizeof(*board_cfg))) { + if (read_write_config(board_cfg, board_settings_offset, sizeof(*board_cfg))) { printk(BIOS_ERR, "CFG EEPROM: Failed to read board settings\n"); return false; } @@ -79,7 +79,7 @@ static int valid = 0; if (valid == 0) { - if (read_write_config(&bmc_cfg, bmc_settings_offset, 0, sizeof(bmc_cfg))) { + if (read_write_config(&bmc_cfg, bmc_settings_offset, sizeof(bmc_cfg))) { printk(BIOS_ERR, "CFG EEPROM: Failed to read BMC settings\n"); return NULL; } @@ -100,7 +100,7 @@ } /* Read data from offset and write it to offset in UPD */ -bool read_write_config(void *blob, size_t read_offset, size_t write_offset, size_t size) +bool read_write_config(void *blob, size_t read_offset, size_t size) { int ret = 0; @@ -120,7 +120,7 @@ break; /* Write to UPD */ - uint8_t *writePointer = (uint8_t *)blob + write_offset + i; + uint8_t *writePointer = (uint8_t *)blob + i; writePointer[0] = tmp[0]; if (size - i > 1) writePointer[1] = tmp[1]; @@ -188,7 +188,7 @@ bool changed = false; /* Read old settings */ - if (read_write_config(&old_layout, off, 0, sizeof(old_layout))) { + if (read_write_config(&old_layout, off, sizeof(old_layout))) { printk(BIOS_ERR, "CFG EEPROM: Read operation failed\n"); return true; } diff --git a/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h b/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h index 58ecb08..3a28219 100644 --- a/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h +++ b/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h @@ -91,7 +91,7 @@ _Static_assert(sizeof(FSPS_UPD) <= 0xC00, "FSPS_UPD too big"); _Static_assert(sizeof(struct eeprom_layout) == 0x2000, "EEPROM layout size mismatch"); -bool read_write_config(void *blob, size_t read_offset, size_t write_offset, size_t size); +bool read_write_config(void *blob, size_t read_offset, size_t size); int check_signature(const size_t offset, const uint64_t signature); struct eeprom_board_settings *get_board_settings(void); struct eeprom_bmc_settings *get_bmc_settings(void); @@ -105,7 +105,6 @@ size_t __off = offsetof(struct eeprom_layout, section_name); \ bool ret = read_write_config(&__tmp, \ __off + offsetof(section_type, opt_name), \ - 0, \ sizeof(__tmp)); \ if (ret) { \ report_eeprom_error(__off + offsetof(section_type, opt_name)); \ -- To view, visit https://review.coreboot.org/c/coreboot/+/52884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib63cb25904ad6c1c7424a9c01d8bf1e84c08453b Gerrit-Change-Number: 52884 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> 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]: option: Allow mainboards to implement the API
by Angel Pons (Code Review) 28 May '21

28 May '21

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/54729 ) Change subject: option: Allow mainboards to implement the API ...................................................................... option: Allow mainboards to implement the API Some mainboards need a mainboard-specific mechanism to access option values. Allow mainboards to implement the option API. Also, add some documentation about the current option API, and describe when should one reimplement the option API in mainboard code: only when the code is mainboard-specific to comply with externally-imposed constraints. Change-Id: Idccdb9a008b1ebb89821961659f27b1c0b17d29c Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/54729 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M Documentation/index.md A Documentation/lib/option.md M src/Kconfig 3 files changed, 47 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/Documentation/index.md b/Documentation/index.md index 9870cf3..c518c50 100644 --- a/Documentation/index.md +++ b/Documentation/index.md @@ -183,6 +183,7 @@ * [Mainboard](mainboard/index.md) * [Payloads](lib/payloads/index.md) * [Libraries](lib/index.md) +* [Options](lib/option.md) * [Security](security/index.md) * [SuperIO](superio/index.md) * [Vendorcode](vendorcode/index.md) diff --git a/Documentation/lib/option.md b/Documentation/lib/option.md new file mode 100644 index 0000000..eb63390 --- /dev/null +++ b/Documentation/lib/option.md @@ -0,0 +1,31 @@ +# Option API + +The option API around the `set_option(const char *name, void *val)` and +`get_option(void *dest, const char *name)` functions deprecated in favor +of a type-safe API. + +Historically, options were stored in RTC battery-backed CMOS RAM inside +the chipset on PC platforms. Nowadays, options can also be stored in the +same flash chip as the boot firmware or through some BMC interface. + +The new type-safe option framework can be used by calling +`enum cb_err set_uint_option(const char *name, unsigned int value)` and +`unsigned int get_uint_option(const char *name, const unsigned int fallback)`. + +The default setting is `OPTION_BACKEND_NONE`, which disables any runtime +configurable options. If supported by a mainboard, the `USE_OPTION_TABLE` +and `USE_MAINBOARD_SPECIFIC_OPTION_BACKEND` choices are visible, and can +be selected to enable runtime configurability. + +# Mainboard-specific option backend + +Mainboards with a mainboard-specific (vendor-defined) method to access +options can select `HAVE_MAINBOARD_SPECIFIC_OPTION_BACKEND` to provide +implementations of the option API accessors. To allow choosing between +multiple option backends, the mainboard-specific implementation should +only be built when `USE_MAINBOARD_SPECIFIC_OPTION_BACKEND` is selected. + +Where possible, using a generic, mainboard-independent mechanism should +be preferred over reinventing the wheel in mainboard-specific code. The +mainboard-specific approach should only be used when the option storage +mechanism has to satisfy externally-imposed, vendor-defined constraints. diff --git a/src/Kconfig b/src/Kconfig index b019e9d..6b60767 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -122,6 +122,7 @@ choice prompt "Option backend to use" + default USE_MAINBOARD_SPECIFIC_OPTION_BACKEND if HAVE_MAINBOARD_SPECIFIC_OPTION_BACKEND default USE_OPTION_TABLE if NVRAMCUI_SECONDARY_PAYLOAD config OPTION_BACKEND_NONE @@ -134,6 +135,13 @@ Enable this option if coreboot shall read options from the "CMOS" NVRAM instead of using hard-coded values. +config USE_MAINBOARD_SPECIFIC_OPTION_BACKEND + bool "Use mainboard-specific option backend" + depends on HAVE_MAINBOARD_SPECIFIC_OPTION_BACKEND + help + Use a mainboard-specific mechanism to access runtime-configurable + options. + endchoice config STATIC_OPTION_TABLE @@ -683,6 +691,13 @@ help How many execution threads to cooperatively multitask with. +config HAVE_MAINBOARD_SPECIFIC_OPTION_BACKEND + bool + help + Selected by mainboards which implement a mainboard-specific mechanism + to access the values for runtime-configurable options. For example, a + custom BMC interface or an EEPROM with an externally-imposed layout. + config HAVE_OPTION_TABLE bool default n -- To view, visit https://review.coreboot.org/c/coreboot/+/54729 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idccdb9a008b1ebb89821961659f27b1c0b17d29c Gerrit-Change-Number: 54729 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.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]: option: Turn CMOS option backend into choice
by Angel Pons (Code Review) 28 May '21

28 May '21

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/54728 ) Change subject: option: Turn CMOS option backend into choice ...................................................................... option: Turn CMOS option backend into choice In order to add more option backends, transform the current CMOS option backend into a Kconfig choice. Replace the `select` directives, as they cannot be used with choice options. Change-Id: Id3180e9991f0e763b4bae93a92d40668e7fc99bc Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/54728 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M payloads/Kconfig M src/Kconfig M src/include/option.h 3 files changed, 12 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/payloads/Kconfig b/payloads/Kconfig index 627bb95..386b207 100644 --- a/payloads/Kconfig +++ b/payloads/Kconfig @@ -133,7 +133,6 @@ bool "Load nvramcui as a secondary payload" default n depends on ARCH_X86 && HAVE_OPTION_TABLE - select USE_OPTION_TABLE help nvramcui can be loaded as a secondary payload under SeaBIOS, GRUB, or any other payload that can load additional payloads. diff --git a/src/Kconfig b/src/Kconfig index 6d0ba0f..b019e9d 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -120,6 +120,13 @@ Otherwise, say N to use the provided pregenerated scanner/parser. +choice + prompt "Option backend to use" + default USE_OPTION_TABLE if NVRAMCUI_SECONDARY_PAYLOAD + +config OPTION_BACKEND_NONE + bool "None" + config USE_OPTION_TABLE bool "Use CMOS for configuration values" depends on HAVE_OPTION_TABLE @@ -127,6 +134,8 @@ Enable this option if coreboot shall read options from the "CMOS" NVRAM instead of using hard-coded values. +endchoice + config STATIC_OPTION_TABLE bool "Load default configuration values into CMOS on each boot" depends on USE_OPTION_TABLE diff --git a/src/include/option.h b/src/include/option.h index 4089378..3a00570 100644 --- a/src/include/option.h +++ b/src/include/option.h @@ -7,7 +7,7 @@ void sanitize_cmos(void); -#if !CONFIG(USE_OPTION_TABLE) +#if CONFIG(OPTION_BACKEND_NONE) static inline unsigned int get_uint_option(const char *name, const unsigned int fallback) { @@ -19,11 +19,11 @@ return CB_CMOS_OTABLE_DISABLED; } -#else /* USE_OPTION_TABLE */ +#else /* !OPTION_BACKEND_NONE */ unsigned int get_uint_option(const char *name, const unsigned int fallback); enum cb_err set_uint_option(const char *name, unsigned int value); -#endif /* USE_OPTION_TABLE? */ +#endif /* OPTION_BACKEND_NONE? */ #endif /* _OPTION_H_ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/54728 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id3180e9991f0e763b4bae93a92d40668e7fc99bc Gerrit-Change-Number: 54728 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.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]: arch/x86/bootblock.ld: Align the bottom of the bootblock to 64 bytes
by Arthur Heymans (Code Review) 28 May '21

28 May '21

Attention is currently required from: Anjaneya "Reddy" Chagam, Jonathan Zhang, Johnny Lin, Morgan Jang. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55046 ) Change subject: arch/x86/bootblock.ld: Align the bottom of the bootblock to 64 bytes ...................................................................... Patch Set 1: Code-Review-1 (1 comment) Patchset: PS1: Not tested with CBnT yet. -- To view, visit https://review.coreboot.org/c/coreboot/+/55046 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69cdacdd15bfca1b91b6f271f2ff76889969fd91 Gerrit-Change-Number: 55046 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Comment-Date: Fri, 28 May 2021 11:31:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: arch/x86/bootblock.ld: Align the bottom of the bootblock to 64 bytes
by Arthur Heymans (Code Review) 28 May '21

28 May '21

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55046 ) Change subject: arch/x86/bootblock.ld: Align the bottom of the bootblock to 64 bytes ...................................................................... arch/x86/bootblock.ld: Align the bottom of the bootblock to 64 bytes Align the bootblock size to 64 bytes because: - cachelines are often 64 bytes large - Bootguard/CBnT requires a 64 byte alignment - It can reduce the bootblock footprint as the size is aligned upwards Change-Id: I69cdacdd15bfca1b91b6f271f2ff76889969fd91 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/bootblock.ld M src/mainboard/ocp/deltalake/Kconfig M src/security/intel/cbnt/Kconfig 3 files changed, 1 insertion(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/55046/1 diff --git a/src/arch/x86/bootblock.ld b/src/arch/x86/bootblock.ld index 3cd0900..4ab2275 100644 --- a/src/arch/x86/bootblock.ld +++ b/src/arch/x86/bootblock.ld @@ -17,7 +17,7 @@ . = _ebootblock - CONFIG_C_ENV_BOOTBLOCK_SIZE; #else . = BOOTBLOCK_TOP - PROGRAM_SZ; - . = ALIGN(16); + . = ALIGN(64); #endif _bootblock = .; diff --git a/src/mainboard/ocp/deltalake/Kconfig b/src/mainboard/ocp/deltalake/Kconfig index b92bd96..3161495 100644 --- a/src/mainboard/ocp/deltalake/Kconfig +++ b/src/mainboard/ocp/deltalake/Kconfig @@ -58,11 +58,4 @@ bool default y -config C_ENV_BOOTBLOCK_SIZE - hex - default 0xc000 if FIXED_BOOTBLOCK_SIZE - help - This matches the IBB size used for CBnT. Adjust this to the - used CBnT settings. - endif # BOARD_OCP_DELTALAKE diff --git a/src/security/intel/cbnt/Kconfig b/src/security/intel/cbnt/Kconfig index 9d48490..5bc82fb 100644 --- a/src/security/intel/cbnt/Kconfig +++ b/src/security/intel/cbnt/Kconfig @@ -7,7 +7,6 @@ #depends on PLATFORM_HAS_DRAM_CLEAR select INTEL_TXT # With CBnT the bootblock is set up as a CBnT IBB and needs a fixed size - select FIXED_BOOTBLOCK_SIZE select TPM_MEASURED_BOOT_INIT_BOOTBLOCK if TPM_MEASURED_BOOT help Enables Intel Converged Bootguard and Trusted Execution Technology -- To view, visit https://review.coreboot.org/c/coreboot/+/55046 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69cdacdd15bfca1b91b6f271f2ff76889969fd91 Gerrit-Change-Number: 55046 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: nb/intel/sandybridge: Drop `DMIBAR32_UPDATE` macro
by Angel Pons (Code Review) 28 May '21

28 May '21

Attention is currently required from: Nico Huber, Arthur Heymans, Patrick Rudolph. Hello Nico Huber, Arthur Heymans, Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/55045 to review the following change. Change subject: nb/intel/sandybridge: Drop `DMIBAR32_UPDATE` macro ...................................................................... nb/intel/sandybridge: Drop `DMIBAR32_UPDATE` macro Now that the temporary `DMIBAR32_UPDATE` macro has served its purpose, replace it with calls to the `dmibar_clrsetbits32` function. If either the `unset` or `set` argument value is irrelevant, drop it and use the `dmibar_clrbits32` or `dmibar_setbits32` macro instead. Tested with BUILD_TIMELESS=1, Asus P8Z77-V LX2 remains identical. Change-Id: I505a77f3675f9300641b1ff5a7351051847b20af Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/sandybridge/early_dmi.c 1 file changed, 30 insertions(+), 32 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/55045/1 diff --git a/src/northbridge/intel/sandybridge/early_dmi.c b/src/northbridge/intel/sandybridge/early_dmi.c index 30e8109..3417af6 100644 --- a/src/northbridge/intel/sandybridge/early_dmi.c +++ b/src/northbridge/intel/sandybridge/early_dmi.c @@ -6,8 +6,6 @@ #include <northbridge/intel/sandybridge/sandybridge.h> #include <southbridge/intel/bd82x6x/pch.h> -#define DMIBAR32_UPDATE(x, val, clear, set) dmibar_clrsetbits32(x, clear, set) - static void dmi_recipe(void) { const u32 cpuid = cpu_get_cpuid(); @@ -38,106 +36,106 @@ } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0904 + (i << 5), 0x7a1842ec, 0x3f << 16, 0x18 << 16); - DMIBAR32_UPDATE(0x090c + (i << 5), 0x00000208, 0x3f << 5, 0x09 << 5); + dmibar_clrsetbits32(0x0904 + (i << 5), 0x3f << 16, 0x18 << 16); + dmibar_clrsetbits32(0x090c + (i << 5), 0x3f << 5, 0x09 << 5); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46139008, 0, 1 << 15); + dmibar_setbits32(0x0700 + (i << 5), 1 << 15); } - DMIBAR32_UPDATE(0x0c04, 0x2e680008, 0xff << 21, 0x73 << 21); + dmibar_clrsetbits32(0x0c04, 0xff << 21, 0x73 << 21); for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0904 + (i << 5), 0x7a1842ec, 3 << 29, 1 << 29); + dmibar_clrsetbits32(0x0904 + (i << 5), 3 << 29, 1 << 29); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0910 + (i << 5), 0x00006300, 0xf << 10, 0 << 10); + dmibar_clrbits32(0x0910 + (i << 5), 0xf << 10); } for (i = 0; i < 4; i++) { - DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042010, 0x1f << 1, 12 << 1); + dmibar_clrsetbits32(0x0a00 + (i << 4), 0x1f << 1, 12 << 1); } - DMIBAR32_UPDATE(0x0c00, 0x29700c08, 1 << 4, 0 << 4); + dmibar_clrbits32(0x0c00, 1 << 4); for (i = 0; i < 4; i++) { - DMIBAR32_UPDATE(0x0a04 + (i << 4), 0x0c0708f0, 1 << 12, 1 << 12); + dmibar_setbits32(0x0a04 + (i << 4), 1 << 12); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0900 + (i << 5), 0x50000000, 3 << 26, 0 << 26); + dmibar_clrbits32(0x0900 + (i << 5), 3 << 26); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0908 + (i << 5), 0x51ffffff, 3 << 27, 2 << 27); + dmibar_clrsetbits32(0x0908 + (i << 5), 3 << 27, 2 << 27); } for (i = 0; i < 4; i++) { - DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042018, 3 << 17, 2 << 17); + dmibar_clrsetbits32(0x0a00 + (i << 4), 3 << 17, 2 << 17); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46139008, 1 << 3, 1 << 3); + dmibar_setbits32(0x0700 + (i << 5), 1 << 3); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0904 + (i << 5), 0x3a1842ec, 3 << 10, 1 << 10); + dmibar_clrsetbits32(0x0904 + (i << 5), 3 << 10, 1 << 10); } for (i = 0; i < 4; i++) { - DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042018, 3 << 24, 3 << 24); + dmibar_setbits32(0x0a00 + (i << 4), 3 << 24); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0908 + (i << 5), 0x51ffffff, 3 << 30, 1 << 30); + dmibar_clrsetbits32(0x0908 + (i << 5), 3 << 30, 1 << 30); } - DMIBAR32_UPDATE(0x0c00, 0x29700c08, 0x1f << 25, 0x14 << 25); + dmibar_clrsetbits32(0x0c00, 0x1f << 25, 0x14 << 25); - DMIBAR32_UPDATE(0x0c0c, 0x16063400, 0x3f << 23, 0x00 << 23); + dmibar_clrbits32(0x0c0c, 0x3f << 23); for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46139008, 1 << 21, 1 << 21); + dmibar_setbits32(0x0700 + (i << 5), 1 << 21); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46339008, 3 << 24, 1 << 24); + dmibar_clrsetbits32(0x0700 + (i << 5), 3 << 24, 1 << 24); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0700 + (i << 5), 0x45339008, 1 << 19, 1 << 19); + dmibar_setbits32(0x0700 + (i << 5), 1 << 19); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0700 + (i << 5), 0x453b9008, 1 << 23, 1 << 23); + dmibar_setbits32(0x0700 + (i << 5), 1 << 23); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0700 + (i << 5), 0x45bb9008, 1 << 22, 1 << 22); + dmibar_setbits32(0x0700 + (i << 5), 1 << 22); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0914 + (i << 5), 0x9021a080, 1 << 9, 1 << 9); + dmibar_setbits32(0x0914 + (i << 5), 1 << 9); } for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0914 + (i << 5), 0x9021a280, 1 << 27, 1 << 27); + dmibar_setbits32(0x0914 + (i << 5), 1 << 27); } for (i = 0; i < 4; i++) { - DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042018, 1 << 21, 1 << 21); + dmibar_setbits32(0x0a00 + (i << 4), 1 << 21); } - DMIBAR32_UPDATE(0x0258, 0x40000600, 1 << 29, 1 << 29); + dmibar_setbits32(0x0258, 1 << 29); for (i = 0; i < 2; i++) { - DMIBAR32_UPDATE(0x0904 + (i << 5), 0x3a1846ec, 0xf << 25, 0x5 << 25); - DMIBAR32_UPDATE(0x0914 + (i << 5), 0x9821a280, 0xf << 13, 0x0 << 13); + dmibar_clrsetbits32(0x0904 + (i << 5), 0xf << 25, 0x5 << 25); + dmibar_clrbits32(0x0914 + (i << 5), 0xf << 13); } - DMIBAR32_UPDATE(DMIL0SLAT, 0x00c26460, 0xffff << 0, 0x403c << 0); + dmibar_clrsetbits32(DMIL0SLAT, 0xffff << 0, 0x403c << 0); } void early_init_dmi(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/55045 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I505a77f3675f9300641b1ff5a7351051847b20af Gerrit-Change-Number: 55045 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: nb/intel/sandybridge: Wrap DMI recipe with macros
by Angel Pons (Code Review) 28 May '21

28 May '21

Attention is currently required from: Nico Huber, Arthur Heymans, Patrick Rudolph. Hello Nico Huber, Arthur Heymans, Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/55043 to review the following change. Change subject: nb/intel/sandybridge: Wrap DMI recipe with macros ...................................................................... nb/intel/sandybridge: Wrap DMI recipe with macros To transform the odd-looking dummy-read-then-write sequences into proper read-modify-write sequences (which is what they are supposed to be), add a temporary `DMIBAR32_UPDATE` macro and use it to wrap all the instances of the pattern to be changed. With this method, non-reproducible changes do not change any lines with magic numbers, and should thus be easier to review. Tested with BUILD_TIMELESS=1, Asus P8Z77-V LX2 remains identical. Change-Id: Ia608c8204e3c4753c0753963b939fd0fba5b7b0a Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/sandybridge/early_dmi.c 1 file changed, 36 insertions(+), 60 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/55043/1 diff --git a/src/northbridge/intel/sandybridge/early_dmi.c b/src/northbridge/intel/sandybridge/early_dmi.c index 122dea9..cfd0d0b 100644 --- a/src/northbridge/intel/sandybridge/early_dmi.c +++ b/src/northbridge/intel/sandybridge/early_dmi.c @@ -6,6 +6,12 @@ #include <northbridge/intel/sandybridge/sandybridge.h> #include <southbridge/intel/bd82x6x/pch.h> +#define DMIBAR32_UPDATE(x, val, clear, set) \ + do { \ + dmibar_read32(x); \ + dmibar_write32(x, ((val) & ~(clear)) | (set)); \ + } while (0) + static void dmi_recipe(void) { const u32 cpuid = cpu_get_cpuid(); @@ -36,136 +42,106 @@ } for (i = 0; i < 2; i++) { - dmibar_read32(0x0904 + (i << 5)); // !!! = 0x7a1842ec - dmibar_write32(0x0904 + (i << 5), (0x7a1842ec & ~(0x3f << 16)) | 0x18 << 16); - dmibar_read32(0x090c + (i << 5)); // !!! = 0x00000208 - dmibar_write32(0x090c + (i << 5), (0x00000208 & ~(0x3f << 5)) | 0x09 << 5); + DMIBAR32_UPDATE(0x0904 + (i << 5), 0x7a1842ec, 0x3f << 16, 0x18 << 16); + DMIBAR32_UPDATE(0x090c + (i << 5), 0x00000208, 0x3f << 5, 0x09 << 5); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46139008 - dmibar_write32(0x0700 + (i << 5), (0x46139008 & ~0) | 1 << 15); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46139008, 0, 1 << 15); } - dmibar_read32(0x0c04); // !!! = 0x2e680008 - dmibar_write32(0x0c04, (0x2e680008 & ~(0xff << 21)) | 0x73 << 21); + DMIBAR32_UPDATE(0x0c04, 0x2e680008, 0xff << 21, 0x73 << 21); for (i = 0; i < 2; i++) { - dmibar_read32(0x0904 + (i << 5)); // !!! = 0x7a1842ec - dmibar_write32(0x0904 + (i << 5), (0x7a1842ec & ~(3 << 29)) | 1 << 29); + DMIBAR32_UPDATE(0x0904 + (i << 5), 0x7a1842ec, 3 << 29, 1 << 29); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0910 + (i << 5)); // !!! = 0x00006300 - dmibar_write32(0x0910 + (i << 5), (0x00006300 & ~(0xf << 10)) | 0 << 10); + DMIBAR32_UPDATE(0x0910 + (i << 5), 0x00006300, 0xf << 10, 0 << 10); } for (i = 0; i < 4; i++) { - dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042010 - dmibar_write32(0x0a00 + (i << 4), (0x03042010 & ~(0x1f << 1)) | 12 << 1); + DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042010, 0x1f << 1, 12 << 1); } - dmibar_read32(0x0c00); // !!! = 0x29700c08 - dmibar_write32(0x0c00, (0x29700c08 & ~(1 << 4)) | 0 << 4); + DMIBAR32_UPDATE(0x0c00, 0x29700c08, 1 << 4, 0 << 4); for (i = 0; i < 4; i++) { - dmibar_read32(0x0a04 + (i << 4)); // !!! = 0x0c0708f0 - dmibar_write32(0x0a04 + (i << 4), (0x0c0708f0 & ~(1 << 12)) | 1 << 12); + DMIBAR32_UPDATE(0x0a04 + (i << 4), 0x0c0708f0, 1 << 12, 1 << 12); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0900 + (i << 5)); // !!! = 0x50000000 - dmibar_write32(0x0900 + (i << 5), (0x50000000 & ~(3 << 26)) | 0 << 26); + DMIBAR32_UPDATE(0x0900 + (i << 5), 0x50000000, 3 << 26, 0 << 26); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0908 + (i << 5)); // !!! = 0x51ffffff - dmibar_write32(0x0908 + (i << 5), (0x51ffffff & ~(3 << 27)) | 2 << 27); + DMIBAR32_UPDATE(0x0908 + (i << 5), 0x51ffffff, 3 << 27, 2 << 27); } for (i = 0; i < 4; i++) { - dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042018 - dmibar_write32(0x0a00 + (i << 4), (0x03042018 & ~(3 << 17)) | 2 << 17); + DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042018, 3 << 17, 2 << 17); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46139008 - dmibar_write32(0x0700 + (i << 5), (0x46139008 & ~(1 << 3)) | 1 << 3); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46139008, 1 << 3, 1 << 3); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0904 + (i << 5)); // !!! = 0x3a1842ec - dmibar_write32(0x0904 + (i << 5), (0x3a1842ec & ~(3 << 10)) | 1 << 10); + DMIBAR32_UPDATE(0x0904 + (i << 5), 0x3a1842ec, 3 << 10, 1 << 10); } for (i = 0; i < 4; i++) { - dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042018 - dmibar_write32(0x0a00 + (i << 4), (0x03042018 & ~(3 << 24)) | 3 << 24); + DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042018, 3 << 24, 3 << 24); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0908 + (i << 5)); // !!! = 0x51ffffff - dmibar_write32(0x0908 + (i << 5), (0x51ffffff & ~(3 << 30)) | 1 << 30); + DMIBAR32_UPDATE(0x0908 + (i << 5), 0x51ffffff, 3 << 30, 1 << 30); } - dmibar_read32(0x0c00); // !!! = 0x29700c08 - dmibar_write32(0x0c00, (0x29700c08 & ~(0x1f << 25)) | 0x14 << 25); + DMIBAR32_UPDATE(0x0c00, 0x29700c08, 0x1f << 25, 0x14 << 25); - dmibar_read32(0x0c0c); // !!! = 0x16063400 - dmibar_write32(0x0c0c, (0x16063400 & ~(0x3f << 23)) | 0x00 << 23); + DMIBAR32_UPDATE(0x0c0c, 0x16063400, 0x3f << 23, 0x00 << 23); for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46139008 - dmibar_write32(0x0700 + (i << 5), (0x46139008 & ~(1 << 21)) | 1 << 21); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46139008, 1 << 21, 1 << 21); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46339008 - dmibar_write32(0x0700 + (i << 5), (0x46339008 & ~(3 << 24)) | 1 << 24); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46339008, 3 << 24, 1 << 24); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x45339008 - dmibar_write32(0x0700 + (i << 5), (0x45339008 & ~(1 << 19)) | 1 << 19); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x45339008, 1 << 19, 1 << 19); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x453b9008 - dmibar_write32(0x0700 + (i << 5), (0x453b9008 & ~(1 << 23)) | 1 << 23); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x453b9008, 1 << 23, 1 << 23); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x45bb9008 - dmibar_write32(0x0700 + (i << 5), (0x45bb9008 & ~(1 << 22)) | 1 << 22); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x45bb9008, 1 << 22, 1 << 22); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0914 + (i << 5)); // !!! = 0x9021a080 - dmibar_write32(0x0914 + (i << 5), (0x9021a080 & ~(1 << 9)) | 1 << 9); + DMIBAR32_UPDATE(0x0914 + (i << 5), 0x9021a080, 1 << 9, 1 << 9); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0914 + (i << 5)); // !!! = 0x9021a280 - dmibar_write32(0x0914 + (i << 5), (0x9021a280 & ~(1 << 27)) | 1 << 27); + DMIBAR32_UPDATE(0x0914 + (i << 5), 0x9021a280, 1 << 27, 1 << 27); } for (i = 0; i < 4; i++) { - dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042018 - dmibar_write32(0x0a00 + (i << 4), (0x03042018 & ~(1 << 21)) | 1 << 21); + DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042018, 1 << 21, 1 << 21); } - dmibar_read32(0x0258); // !!! = 0x40000600 - dmibar_write32(0x0258, (0x40000600 & ~(1 << 29)) | 1 << 29); + DMIBAR32_UPDATE(0x0258, 0x40000600, 1 << 29, 1 << 29); for (i = 0; i < 2; i++) { - dmibar_read32(0x0904 + (i << 5)); // !!! = 0x3a1846ec - dmibar_write32(0x0904 + (i << 5), (0x3a1846ec & ~(0xf << 25)) | 0x5 << 25); - dmibar_read32(0x0914 + (i << 5)); // !!! = 0x9821a280 - dmibar_write32(0x0914 + (i << 5), (0x9821a280 & ~(0xf << 13)) | 0x0 << 13); + DMIBAR32_UPDATE(0x0904 + (i << 5), 0x3a1846ec, 0xf << 25, 0x5 << 25); + DMIBAR32_UPDATE(0x0914 + (i << 5), 0x9821a280, 0xf << 13, 0x0 << 13); } - dmibar_read32(DMIL0SLAT); // !!! = 0x00c26460 - dmibar_write32(DMIL0SLAT, (0x00c26460 & ~(0xffff << 0)) | 0x403c << 0); + DMIBAR32_UPDATE(DMIL0SLAT, 0x00c26460, 0xffff << 0, 0x403c << 0); } void early_init_dmi(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/55043 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia608c8204e3c4753c0753963b939fd0fba5b7b0a Gerrit-Change-Number: 55043 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: [TESTME] nb/intel/sandybridge: Redefine `DMIBAR32_UPDATE`
by Angel Pons (Code Review) 28 May '21

28 May '21

1 0

Change in coreboot[master]: nb/intel/sandybridge: Spell out DMI recipe operations
by Angel Pons (Code Review) 28 May '21

28 May '21

Attention is currently required from: Nico Huber, Arthur Heymans, Patrick Rudolph. Hello Nico Huber, Arthur Heymans, Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/55042 to review the following change. Change subject: nb/intel/sandybridge: Spell out DMI recipe operations ...................................................................... nb/intel/sandybridge: Spell out DMI recipe operations There's many register writes preceded by a dummy read, which is probably because the code was derived from trace dumps, where a read-modify-write sequence appears as a register read followed by a register write using a slightly different value. In order to replace the hardcoded register writes with read-modify-write sequences, rewrite the magic values in the code as bitwise operations on the original register values. Bitwise operation values were derived from reference code, and the original register values appear in some comments on the read statements, preceding the revised register writes. Note that one of the comments had an incorrect value, which is most likely because of a copy-paste error. Tested with BUILD_TIMELESS=1, Asus P8Z77-V LX2 remains identical. Change-Id: Ie921de004f283b560288a8c2cff277de89440f57 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/northbridge/intel/sandybridge/early_dmi.c 1 file changed, 32 insertions(+), 32 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/55042/1 diff --git a/src/northbridge/intel/sandybridge/early_dmi.c b/src/northbridge/intel/sandybridge/early_dmi.c index d9b7ed2..122dea9 100644 --- a/src/northbridge/intel/sandybridge/early_dmi.c +++ b/src/northbridge/intel/sandybridge/early_dmi.c @@ -24,7 +24,7 @@ dmibar_clrbits32(0x0a00 + (i << 4), 3 << 26); dmibar_setbits32(0x0a04 + (i << 4), 1 << 11); } - dmibar_clrsetbits32(0x0c30, 0xf << 28, 1 << 30); + dmibar_clrsetbits32(0x0c30, 0xf << 28, 4 << 28); for (i = 0; i < 2; i++) { dmibar_clrbits32(0x0904 + (i << 5), 7 << 22); @@ -37,135 +37,135 @@ for (i = 0; i < 2; i++) { dmibar_read32(0x0904 + (i << 5)); // !!! = 0x7a1842ec - dmibar_write32(0x0904 + (i << 5), 0x7a1842ec); + dmibar_write32(0x0904 + (i << 5), (0x7a1842ec & ~(0x3f << 16)) | 0x18 << 16); dmibar_read32(0x090c + (i << 5)); // !!! = 0x00000208 - dmibar_write32(0x090c + (i << 5), 0x00000128); + dmibar_write32(0x090c + (i << 5), (0x00000208 & ~(0x3f << 5)) | 0x09 << 5); } for (i = 0; i < 2; i++) { dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46139008 - dmibar_write32(0x0700 + (i << 5), 0x46139008); + dmibar_write32(0x0700 + (i << 5), (0x46139008 & ~0) | 1 << 15); } dmibar_read32(0x0c04); // !!! = 0x2e680008 - dmibar_write32(0x0c04, 0x2e680008); + dmibar_write32(0x0c04, (0x2e680008 & ~(0xff << 21)) | 0x73 << 21); for (i = 0; i < 2; i++) { dmibar_read32(0x0904 + (i << 5)); // !!! = 0x7a1842ec - dmibar_write32(0x0904 + (i << 5), 0x3a1842ec); + dmibar_write32(0x0904 + (i << 5), (0x7a1842ec & ~(3 << 29)) | 1 << 29); } for (i = 0; i < 2; i++) { dmibar_read32(0x0910 + (i << 5)); // !!! = 0x00006300 - dmibar_write32(0x0910 + (i << 5), 0x00004300); + dmibar_write32(0x0910 + (i << 5), (0x00006300 & ~(0xf << 10)) | 0 << 10); } for (i = 0; i < 4; i++) { dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042010 - dmibar_write32(0x0a00 + (i << 4), 0x03042018); + dmibar_write32(0x0a00 + (i << 4), (0x03042010 & ~(0x1f << 1)) | 12 << 1); } dmibar_read32(0x0c00); // !!! = 0x29700c08 - dmibar_write32(0x0c00, 0x29700c08); + dmibar_write32(0x0c00, (0x29700c08 & ~(1 << 4)) | 0 << 4); for (i = 0; i < 4; i++) { dmibar_read32(0x0a04 + (i << 4)); // !!! = 0x0c0708f0 - dmibar_write32(0x0a04 + (i << 4), 0x0c0718f0); + dmibar_write32(0x0a04 + (i << 4), (0x0c0708f0 & ~(1 << 12)) | 1 << 12); } for (i = 0; i < 2; i++) { dmibar_read32(0x0900 + (i << 5)); // !!! = 0x50000000 - dmibar_write32(0x0900 + (i << 5), 0x50000000); + dmibar_write32(0x0900 + (i << 5), (0x50000000 & ~(3 << 26)) | 0 << 26); } for (i = 0; i < 2; i++) { dmibar_read32(0x0908 + (i << 5)); // !!! = 0x51ffffff - dmibar_write32(0x0908 + (i << 5), 0x51ffffff); + dmibar_write32(0x0908 + (i << 5), (0x51ffffff & ~(3 << 27)) | 2 << 27); } for (i = 0; i < 4; i++) { dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042018 - dmibar_write32(0x0a00 + (i << 4), 0x03042018); + dmibar_write32(0x0a00 + (i << 4), (0x03042018 & ~(3 << 17)) | 2 << 17); } for (i = 0; i < 2; i++) { dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46139008 - dmibar_write32(0x0700 + (i << 5), 0x46139008); + dmibar_write32(0x0700 + (i << 5), (0x46139008 & ~(1 << 3)) | 1 << 3); } for (i = 0; i < 2; i++) { dmibar_read32(0x0904 + (i << 5)); // !!! = 0x3a1842ec - dmibar_write32(0x0904 + (i << 5), 0x3a1846ec); + dmibar_write32(0x0904 + (i << 5), (0x3a1842ec & ~(3 << 10)) | 1 << 10); } for (i = 0; i < 4; i++) { dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042018 - dmibar_write32(0x0a00 + (i << 4), 0x03042018); + dmibar_write32(0x0a00 + (i << 4), (0x03042018 & ~(3 << 24)) | 3 << 24); } for (i = 0; i < 2; i++) { dmibar_read32(0x0908 + (i << 5)); // !!! = 0x51ffffff - dmibar_write32(0x0908 + (i << 5), 0x51ffffff); + dmibar_write32(0x0908 + (i << 5), (0x51ffffff & ~(3 << 30)) | 1 << 30); } dmibar_read32(0x0c00); // !!! = 0x29700c08 - dmibar_write32(0x0c00, 0x29700c08); + dmibar_write32(0x0c00, (0x29700c08 & ~(0x1f << 25)) | 0x14 << 25); dmibar_read32(0x0c0c); // !!! = 0x16063400 - dmibar_write32(0x0c0c, 0x00063400); + dmibar_write32(0x0c0c, (0x16063400 & ~(0x3f << 23)) | 0x00 << 23); for (i = 0; i < 2; i++) { dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46139008 - dmibar_write32(0x0700 + (i << 5), 0x46339008); + dmibar_write32(0x0700 + (i << 5), (0x46139008 & ~(1 << 21)) | 1 << 21); } for (i = 0; i < 2; i++) { dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46339008 - dmibar_write32(0x0700 + (i << 5), 0x45339008); + dmibar_write32(0x0700 + (i << 5), (0x46339008 & ~(3 << 24)) | 1 << 24); } for (i = 0; i < 2; i++) { dmibar_read32(0x0700 + (i << 5)); // !!! = 0x45339008 - dmibar_write32(0x0700 + (i << 5), 0x453b9008); + dmibar_write32(0x0700 + (i << 5), (0x45339008 & ~(1 << 19)) | 1 << 19); } for (i = 0; i < 2; i++) { dmibar_read32(0x0700 + (i << 5)); // !!! = 0x453b9008 - dmibar_write32(0x0700 + (i << 5), 0x45bb9008); + dmibar_write32(0x0700 + (i << 5), (0x453b9008 & ~(1 << 23)) | 1 << 23); } for (i = 0; i < 2; i++) { dmibar_read32(0x0700 + (i << 5)); // !!! = 0x45bb9008 - dmibar_write32(0x0700 + (i << 5), 0x45fb9008); + dmibar_write32(0x0700 + (i << 5), (0x45bb9008 & ~(1 << 22)) | 1 << 22); } for (i = 0; i < 2; i++) { dmibar_read32(0x0914 + (i << 5)); // !!! = 0x9021a080 - dmibar_write32(0x0914 + (i << 5), 0x9021a280); + dmibar_write32(0x0914 + (i << 5), (0x9021a080 & ~(1 << 9)) | 1 << 9); } for (i = 0; i < 2; i++) { - dmibar_read32(0x0914 + (i << 5)); // !!! = 0x9021a080 - dmibar_write32(0x0914 + (i << 5), 0x9821a280); + dmibar_read32(0x0914 + (i << 5)); // !!! = 0x9021a280 + dmibar_write32(0x0914 + (i << 5), (0x9021a280 & ~(1 << 27)) | 1 << 27); } for (i = 0; i < 4; i++) { dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042018 - dmibar_write32(0x0a00 + (i << 4), 0x03242018); + dmibar_write32(0x0a00 + (i << 4), (0x03042018 & ~(1 << 21)) | 1 << 21); } dmibar_read32(0x0258); // !!! = 0x40000600 - dmibar_write32(0x0258, 0x60000600); + dmibar_write32(0x0258, (0x40000600 & ~(1 << 29)) | 1 << 29); for (i = 0; i < 2; i++) { dmibar_read32(0x0904 + (i << 5)); // !!! = 0x3a1846ec - dmibar_write32(0x0904 + (i << 5), 0x2a1846ec); + dmibar_write32(0x0904 + (i << 5), (0x3a1846ec & ~(0xf << 25)) | 0x5 << 25); dmibar_read32(0x0914 + (i << 5)); // !!! = 0x9821a280 - dmibar_write32(0x0914 + (i << 5), 0x98200280); + dmibar_write32(0x0914 + (i << 5), (0x9821a280 & ~(0xf << 13)) | 0x0 << 13); } dmibar_read32(DMIL0SLAT); // !!! = 0x00c26460 - dmibar_write32(DMIL0SLAT, 0x00c2403c); + dmibar_write32(DMIL0SLAT, (0x00c26460 & ~(0xffff << 0)) | 0x403c << 0); } void early_init_dmi(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/55042 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie921de004f283b560288a8c2cff277de89440f57 Gerrit-Change-Number: 55042 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021