coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

Change in coreboot[master]: [RFC] ChromeOS: Declare CHROMEOS_NO_GPIOS
by Julius Werner (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Kyösti Mälkki, Fred Reitberger, Yu-Ping Wu, Felix Held. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58956 ) Change subject: [RFC] ChromeOS: Declare CHROMEOS_NO_GPIOS ...................................................................... Patch Set 7: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/58956/comment/049d901d_0a7ab6dc PS1, Line 9: Implementation of fill_lb_gpios() is not about VBOOT > fill_Lb_gpios() is an ugly piece of open-coding. The identifier strings should be enumerations, as the allowed choices come from the history of depthcharge so the specification is depthcharge git history? No objections there in principle, but the interface is long-entrenched and the coreboot table struct is designed to take strings, etc... overall, I think the effort and churn of replacing it with an enum isn't really worth it. I do however agree that the risk of typos is stupid, and a really easy fix for that would be to just put a bunch of #defines in a file (ideally in commonlib/bsd so both coreboot and depthcharge can share it) to lock down the available strings, e.g.: #define CHROMEOS_GPIO_LID "lid" #define CHROMEOS_GPIO_SPEAKER_ENABLE "speaker enable" ...etc.... I think that would solve the practical issue with much less churn, so if you have time to spend on a little cleanup here, how about doing that? (The allowed choices are just specified by historical practice, yes, and technically every board is allowed to make up its own but for practical reasons we have been trying to keep using the same names for the same things across all boards. It does happen from time to time that a new board needs a shared GPIO for something that we don't have an existing concept for yet, though, so the option for easily adding new kinds of pins needs to remain.) > We should be able to remove the fill_lb_gpios. We override them in depthcharge anyway: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/pla… Well, that looks like a temporary hack for early bring-up code, so I think in general my earlier suggestion still makes sense that usually every Chrome OS board needs fill_lb_gpios some point, and so there should be no option to leave it out (i.e. weak implementation). For bring-up, boards can just write an empty stub to fill out later. (Also, btw, you don't need to handcraft that fake_gpio stuff you have there, we do have new_gpio_low()/new_gpio_high() for that in depthcharge already.) -- To view, visit https://review.coreboot.org/c/coreboot/+/58956 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f1eaaeffe388a7f8a7fc2430b6593eb8298461b Gerrit-Change-Number: 58956 Gerrit-PatchSet: 7 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 13 Apr 2022 00:22:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: implement ioc driver
by Wonkyu Kim (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Tim Wawrzynczak, Angel Pons, Nick Vaccaro. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63198 ) Change subject: soc/intel/common: implement ioc driver ...................................................................... Patch Set 14: (2 comments) File src/soc/intel/common/block/ioc/Kconfig: https://review.coreboot.org/c/coreboot/+/63198/comment/3869baec_221f9024 PS5, Line 1: SOC_INTEL_COMMON_BLOCK_IOC > use SOC_INTEL_COMMON_BLOCK_SA Done File src/soc/intel/common/block/ioc/ioc.c: https://review.coreboot.org/c/coreboot/+/63198/comment/ec7f767c_c2002fe2 PS5, Line 9: MCH_BASE_ADDRESS > I think it'll be safe to add. I'll add condition check and print out error message. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/63198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I768027c2ca78310c03845f70f17df19dc8cd0982 Gerrit-Change-Number: 63198 Gerrit-PatchSet: 14 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Wed, 13 Apr 2022 00:20:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Wonkyu Kim <wonkyu.kim(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP]cpu/x86/smm: Use struct region to check overlapping sections
by Arthur Heymans (Code Review) 13 Apr '22

13 Apr '22

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63602 ) Change subject: [WIP]cpu/x86/smm: Use struct region to check overlapping sections ...................................................................... Patch Set 1: (1 comment) File src/cpu/x86/smm/smm_module_loader.c: https://review.coreboot.org/c/coreboot/+/63602/comment/9a7ef304_8cd1691a PS1, Line 399: struct region_list *entry = list; : struct region_list *next_entry; : while (1) { : if (entry == NULL) : return; : next_entry = entry->next; : free(entry); : entry = next_entry; : } won't work. Only the last allocated element is freed. -- To view, visit https://review.coreboot.org/c/coreboot/+/63602 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib7e2e3ae16c223ecfd8d5bce6ff6c17c53496925 Gerrit-Change-Number: 63602 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 13 Apr 2022 00:15:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP]cpu/x86/smm: Use struct region to check overlapping sections
by Arthur Heymans (Code Review) 13 Apr '22

13 Apr '22

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63602 ) Change subject: [WIP]cpu/x86/smm: Use struct region to check overlapping sections ...................................................................... [WIP]cpu/x86/smm: Use struct region to check overlapping sections This allows for some runtime checks on all SMM elements and removes the need for manually checks. Change-Id: Ib7e2e3ae16c223ecfd8d5bce6ff6c17c53496925 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/x86/smm/smm_module_loader.c 1 file changed, 109 insertions(+), 39 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/63602/1 diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 441ecf8..5744624 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -1,15 +1,18 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <acpi/acpi_gnvs.h> +#include <cbmem.h> +#include <commonlib/helpers.h> +#include <commonlib/region.h> +#include <console/console.h> +#include <cpu/x86/smm.h> +#include <rmodule.h> +#include <security/intel/stm/SmmStm.h> #include <stddef.h> #include <stdint.h> +#include <stdio.h> +#include <stdlib.h> #include <string.h> -#include <rmodule.h> -#include <cbmem.h> -#include <cpu/x86/smm.h> -#include <commonlib/helpers.h> -#include <console/console.h> -#include <security/intel/stm/SmmStm.h> #define FXSAVE_SIZE 512 #define SMM_CODE_SEGMENT_SIZE 0x10000 @@ -114,8 +117,8 @@ for (unsigned int i = 0; i < num_cpus; i++) { if (i % cpus_per_segment == 0) - printk(BIOS_DEBUG, "-------------NEW CODE SEGMENT --------------\n"); - printk(BIOS_DEBUG, "CPU 0x%x\n", i); + printk(BIOS_SPEW, "-------------NEW CODE SEGMENT --------------\n"); + printk(BIOS_SPEW, "CPU 0x%x\n", i); /* We copy the same stub for each CPU so they all need the same 'smbase'. */ const size_t segment_number = i / cpus_per_segment; cpus[i].code_start = smbase - SMM_CODE_SEGMENT_SIZE * segment_number @@ -125,9 +128,9 @@ cpus[i].ss_top = cpus[i].code_start + SMM_ENTRY_OFFSET; cpus[i].ss_start = cpus[i].ss_top - params->cpu_save_state_size; cpus[i].code_end = cpus[i].code_start + stub_size; - printk(BIOS_DEBUG, " Stub [0x%lx-0x%lx[\n", cpus[i].code_start, + printk(BIOS_SPEW, " Stub [0x%lx-0x%lx[\n", cpus[i].code_start, cpus[i].code_end); - printk(BIOS_DEBUG, " Save state [0x%lx-0x%lx[\n", cpus[i].ss_start, + printk(BIOS_SPEW, " Save state [0x%lx-0x%lx[\n", cpus[i].ss_start, cpus[i].ss_top); cpus[i].active = 1; } @@ -353,6 +356,57 @@ mod_params->save_state_top[i] = cpus[i].ss_top; } +struct region_list { + struct region region; + struct region_list *next; +}; + +static struct region_list *list; + +static int check_region(const struct region smram, + const struct region new_region, + const char *region_name) +{ + if (!region_is_subregion(&smram, &new_region)) { + printk(BIOS_ERR, "%s not in SMM\n", region_name); + return 1; + } + + struct region_list *test; + for (test = list; test != NULL; test = test->next) { + if (region_overlap(&test->region, &new_region)) { + printk(BIOS_ERR, "%s overlaps with a previous region\n", region_name); + return 1; + } + } + /* All is fine. Add the new region to the linked list */ + struct region_list *new_entry = malloc(sizeof(struct region_list)); + if (new_entry == NULL) { + printk(BIOS_ERR, "Out of memory\n"); + return 1; + } + + test->next = new_entry; + new_entry->region = new_region; + new_entry->next = NULL; + printk(BIOS_DEBUG, "%-12s [0x%lx-0x%lx]\n", region_name, region_offset(&new_region), + region_end(&new_region)); + return 0; +} + +static void free_list(void) +{ + struct region_list *entry = list; + struct region_list *next_entry; + while (1) { + if (entry == NULL) + return; + next_entry = entry->next; + free(entry); + entry = next_entry; + } +} + /* *The SMM module is placed within the provided region in the following * manner: @@ -389,62 +443,78 @@ if (rmodule_parse(&_binary_smm_start, &smi_handler)) return -1; - const uintptr_t smram_top = smram_base + smram_size; + const struct region smram = { .offset = smram_base, .size = smram_size }; + const uintptr_t smram_top = region_end(&smram); const size_t stm_size = CONFIG(STM) ? CONFIG_MSEG_SIZE - CONFIG_BIOS_RESOURCE_LIST_SIZE : 0; - const uintptr_t stm_base = CONFIG(STM) ? smram_top - stm_size : 0; - if (stm_size) { - printk(BIOS_DEBUG, "STM [0x%lx-0x%lx[\n", stm_base, - stm_base + stm_size); + struct region stm = {}; + if (CONFIG(STPM)) { + stm.offset = smram_top - stm_size; + stm.size = stm_size; + if (check_region(smram, stm, "STM")) + return 1; printk(BIOS_DEBUG, "MSEG size 0x%x\n", CONFIG_MSEG_SIZE); printk(BIOS_DEBUG, "BIOS res list 0x%x\n", CONFIG_BIOS_RESOURCE_LIST_SIZE); } + const size_t fx_save_area_size = CONFIG(SSE) ? FXSAVE_SIZE * params->num_cpus : 0; - const uintptr_t fx_save_area_base = - CONFIG(SSE) ? smram_top - stm_size - fx_save_area_size : 0; - if (fx_save_area_size) - printk(BIOS_DEBUG, "fx_save [0x%lx-0x%lx[\n", fx_save_area_base, - fx_save_area_base + fx_save_area_size); + struct region fx_save = {}; + if (CONFIG(SSE)) { + fx_save.offset = smram_top - stm_size - fx_save_area_size; + if (check_region(smram, fx_save, "STM")) + return 1; + } const size_t handler_size = rmodule_memory_size(&smi_handler); const size_t handler_alignment = rmodule_load_alignment(&smi_handler); const uintptr_t handler_base = ALIGN_DOWN(smram_top - stm_size - fx_save_area_size - handler_size, handler_alignment); - printk(BIOS_DEBUG, "smihandler [0x%lx-0x%lx[\n", handler_base, - handler_base + handler_size); - - if (handler_base <= smram_base) { - printk(BIOS_ERR, "Permanent handler won't FIT smram\n"); - return -1; - } + struct region handler = { + .offset = handler_base, + .size = handler_size + }; + if (check_region(smram, handler, "HANDLER")) + return 1; const uintptr_t stub_segment_base = handler_base - SMM_CODE_SEGMENT_SIZE; if (!smm_create_map(stub_segment_base, params->num_concurrent_save_states, params)) { printk(BIOS_ERR, "%s: Error creating CPU map\n", __func__); return -1; } - - const uintptr_t lowest_stub = cpus[params->num_concurrent_save_states - 1].code_start; - const uintptr_t smm_stack_top = - smram_base + params->num_concurrent_save_states * CONFIG_SMM_MODULE_STACK_SIZE; - printk(BIOS_DEBUG, "cpu stacks [0x%lx-0x%lx[\n", smram_base, smm_stack_top); - if (lowest_stub < smm_stack_top) { - printk(BIOS_ERR, "SMM stubs won't fit in SMRAM 0x%lx\n", - cpus[params->num_concurrent_save_states - 1].code_start); - return -1; + for (unsigned int i = 0; i < params->num_concurrent_save_states; i++) { + printk(BIOS_DEBUG, "\nCPU %d\n", i); + struct region ss = { .offset = cpus[i].ss_start, .size = cpus[i].ss_top - cpus[i].ss_start }; + char string[13]; + snprintf(string, sizeof(string), " ss%d", i); + if (check_region(smram, ss, string)) + return 1; + struct region stub = {.offset = cpus[i].code_start, + .size = cpus[i].code_end - cpus[i].code_start}; + snprintf(string, sizeof(string), " stub%d", i); + if (check_region(smram, stub, string)) + return 1; } - if (rmodule_load((void *)handler_base, &smi_handler)) - return -1; + struct region stacks = { + .offset = smram_base, + .size = params->num_concurrent_save_states * CONFIG_SMM_MODULE_STACK_SIZE + }; + printk(BIOS_DEBUG, "\n"); + if (check_region(smram, stacks, "stacks")) + return 1; + + if (rmodule_load((void *)handler_base, &smi_handler)) return -1; struct smm_runtime *smihandler_params = rmodule_parameters(&smi_handler); params->handler = rmodule_entry(&smi_handler); setup_smihandler_params(smihandler_params, smram_base, smram_size, params); + free_list(); + return smm_module_setup_stub(stub_segment_base, smram_size, params, - (void *)fx_save_area_base); + (void *)region_offset(&fx_save)); } /* -- To view, visit https://review.coreboot.org/c/coreboot/+/63602 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib7e2e3ae16c223ecfd8d5bce6ff6c17c53496925 Gerrit-Change-Number: 63602 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status
by Tim Van Patten (Code Review) 13 Apr '22

13 Apr '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63558 to look at the new patch set (#2). Change subject: drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status ...................................................................... drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status Add the member `use_gpio_for_status` to the structure `drivers_usb_acpi_config`, so the `devicetree.cb` can specify it. This field is then used to initialize the corresponding field in the structure `acpi_power_res_params` in `usb_acpi_fill_ssdt_generator()`. The member `acpi_power_res_params::use_gpio_for_status()` is already being used by `acpi_device_add_power_res()` to determine which version of the `_STA()` method to output. This field now also allows for checking if the device is already enabled and skipping re-enabling it (and the corresponding sleep) if it already is. BRANCH=None Signed-off-by: Tim Van Patten <timvp(a)google.com> BUG=b:225022810 TEST=Dump SSDT table for guybrush Change-Id: I69eb5f1ad79f3b2980f43dcf4a36585fca198ec9 --- M src/acpi/device.c M src/drivers/usb/acpi/chip.h M src/drivers/usb/acpi/usb_acpi.c 3 files changed, 39 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/63558/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63558 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69eb5f1ad79f3b2980f43dcf4a36585fca198ec9 Gerrit-Change-Number: 63558 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/qualcomm/common: Make clock_configure() check for exact matches
by Julius Werner (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Shelley Chen, Taniya Das, Paul Menzel. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63289 ) Change subject: soc/qualcomm/common: Make clock_configure() check for exact matches ...................................................................... Patch Set 10: (1 comment) Patchset: PS10: > Seems to be a problem on the build node: […] So the weird thing is, I retriggered this build and it seemed to have finished successfully the second time (https://qa.coreboot.org/job/coreboot-gerrit/201800/), but the Jenkins bot didn't update this patch. +Martin, isn't the whole point of retriggering that it would then give us the Verified+1? Does something no longer work as intended here? In the meantime, I'd suggest trying a rebase... -- To view, visit https://review.coreboot.org/c/coreboot/+/63289 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9cfad7236241f4d03ff1a56683654649658b68fc Gerrit-Change-Number: 63289 Gerrit-PatchSet: 10 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Taniya Das <quic_tdas(a)quicinc.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Ravi Kumar Bokka <rbokka(a)codeaurora.org> Gerrit-CC: Taniya Das <tdas(a)qualcomm.corp-partner.google.com> Gerrit-CC: mturney mturney <quic_mturney(a)quicinc.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Taniya Das <quic_tdas(a)quicinc.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 13 Apr 2022 00:03:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shelley Chen <shchen(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/apcb/apcb_v3_edit.py: Edit APCB based on different SPD magic
by Jon Murphy (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Raul Rangel, Rob Barnes, Karthik Ramasubramanian. Jon Murphy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63598 ) Change subject: util/apcb/apcb_v3_edit.py: Edit APCB based on different SPD magic ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/63598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e96c89e4e5ce8e0567a17bf7685b69080fa1708 Gerrit-Change-Number: 63598 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Tue, 12 Apr 2022 23:51:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/skyrim: Inject SPDs into APCB
by Robert Zieba (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Raul Rangel, Karthik Ramasubramanian. Robert Zieba has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63600 ) Change subject: mb/google/skyrim: Inject SPDs into APCB ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/63600 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3b3f6f248d54681c6f55c00660d1f2988ae906ba Gerrit-Change-Number: 63600 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Tue, 12 Apr 2022 23:49:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/skyrim: Inject SPDs into APCB
by Karthik Ramasubramanian (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Robert Zieba, Raul Rangel. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63600 ) Change subject: mb/google/skyrim: Inject SPDs into APCB ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/skyrim/Makefile.inc: https://review.coreboot.org/c/coreboot/+/63600/comment/fa87e3bc_243e929c PS1, Line 31: --spd_magic '2311130E' > If possible you should mention where this value is coming from. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/63600 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3b3f6f248d54681c6f55c00660d1f2988ae906ba Gerrit-Change-Number: 63600 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Attention: Robert Zieba <robertzieba(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Comment-Date: Tue, 12 Apr 2022 23:48:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Robert Zieba <robertzieba(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/skyrim: Inject SPDs into APCB
by Karthik Ramasubramanian (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Raul Rangel, Karthik Ramasubramanian. Hello Robert Zieba, Raul Rangel, Jon Murphy, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63600 to look at the new patch set (#2). Change subject: mb/google/skyrim: Inject SPDs into APCB ...................................................................... mb/google/skyrim: Inject SPDs into APCB Update the build scripts to inject variant specific SPDs into APCB. BUG=None TEST=Build and boot to OS in Skyrim boards with all the concerned memory parts. Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Change-Id: I3b3f6f248d54681c6f55c00660d1f2988ae906ba --- M src/mainboard/google/skyrim/Makefile.inc 1 file changed, 18 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/63600/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63600 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3b3f6f248d54681c6f55c00660d1f2988ae906ba Gerrit-Change-Number: 63600 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2022