coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

Change in coreboot[master]: RFC: Kconfig - show boards supported on 4.11 branch (POC)
by Julius Werner (Code Review) 23 Apr '22

23 Apr '22

Attention is currently required from: Martin Roth, Felix Held. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63754 ) Change subject: RFC: Kconfig - show boards supported on 4.11 branch (POC) ...................................................................... Patch Set 2: Code-Review+1 (3 comments) Patchset: PS2: Looks like a reasonable implementation to me, if people feel this feature would be helpful. File Makefile: https://review.coreboot.org/c/coreboot/+/63754/comment/28bb9d8a_3c2a4c89 PS2, Line 183: ifeq ($(CONFIG_MAINBOARD_SUPPORTED_ON_BRANCH),y) nit: I feel like this would better fit in Makefile.inc, with the other Git stuff (e.g. UPDATED_SUBMODULES). File src/mainboard/adi/Kconfig: PS2: Can we stuff these vendors that have only branch mainboards in some separate directory (e.g. src/mainboard/on_branch/adi) so we don't clutter the main vendor list with too many "fake" entries (I mean for developers browsing the code, not for menuconfig)? -- To view, visit https://review.coreboot.org/c/coreboot/+/63754 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5fe1c508e97b52a6b70b33107c6e17cc7d7faeb2 Gerrit-Change-Number: 63754 Gerrit-PatchSet: 2 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sat, 23 Apr 2022 00:20:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: libpayload: Support x86 payload loaded via Linux Boot Protocol
by Julius Werner (Code Review) 23 Apr '22

23 Apr '22

Attention is currently required from: Nico Huber, Tim Wawrzynczak. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63748 ) Change subject: libpayload: Support x86 payload loaded via Linux Boot Protocol ...................................................................... Patch Set 1: (2 comments) File payloads/libpayload/arch/x86/head.S: https://review.coreboot.org/c/coreboot/+/63748/comment/d42c2898_e033751a PS1, Line 86: jne 1f > Good to know. I only had a quick look at it and couldn't find that CB_MAGIC […] We do, but it looks like it's not actually used on x86 (it is on Arm). I think since the existing mechanism works well enough and trying to make use of that passed-in pointer would always risk introducing compatibility issues (it has been in coreboot for a long time, but for example libpayload's selfboot() still doesn't pass it along to the next payload), it's probably left to leave those things as they are. We should probably get rid of this "Payload API" thing here, though. https://review.coreboot.org/c/coreboot/+/63748/comment/b77d8c30_cc206d0b PS1, Line 105: esi > The caller points to a configuration struct (aka. zero page) in %esi. […] Yeah, I think we should never try to dereference a potentially random memory address, even with some safety checks. If a Kconfig works for your use case I think that would be the best solution. -- To view, visit https://review.coreboot.org/c/coreboot/+/63748 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ief6bca8534d07458b9c5a315f297cb62d727917f Gerrit-Change-Number: 63748 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Sat, 23 Apr 2022 00:09:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/{zork,guybrush},soc/amd/espi: Fix vw_irq_polarity
by Raul Rangel (Code Review) 23 Apr '22

23 Apr '22

Attention is currently required from: Jason Glenesk, Marshall Dawson, Rob Barnes, Fred Reitberger, Karthik Ramasubramanian, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54924 ) Change subject: mb/google/{zork,guybrush},soc/amd/espi: Fix vw_irq_polarity ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: I'm not sure what this build failure means: > Error: The parent of this patch is from before Fri Oct 22 23:04:02 UTC 2021. > Please rebase this on top of a newer patch. ....? -- To view, visit https://review.coreboot.org/c/coreboot/+/54924 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4608a7684e34ebb389e0e55ceba7e7441939afe7 Gerrit-Change-Number: 54924 Gerrit-PatchSet: 3 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> 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: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 22 Apr 2022 23:05:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/{zork,guybrush},soc/amd/espi: Fix vw_irq_polarity
by Raul Rangel (Code Review) 23 Apr '22

23 Apr '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54924 ) Change subject: mb/google/{zork,guybrush},soc/amd/espi: Fix vw_irq_polarity ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/54924 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4608a7684e34ebb389e0e55ceba7e7441939afe7 Gerrit-Change-Number: 54924 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-CC: Rob Barnes <robbarnes(a)google.com> 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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 22 Apr 2022 23:00:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/zork: Correct PIRQ_MISC0 configuration
by Raul Rangel (Code Review) 23 Apr '22

23 Apr '22

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63796 ) Change subject: mb/google/zork: Correct PIRQ_MISC0 configuration ...................................................................... mb/google/zork: Correct PIRQ_MISC0 configuration The current configuration is masking off IRQ 1 and IRQ 12 to the PIC. This for some reason causes problems when using level triggered interrupts. This change updates the PIRQ_MISC0 value to match what skyrim is doing. This will enable level interrupts to work correctly. BUG=b:218874489, b:160595155 TEST=Boot zork and verify keyboard still works. Boot with patch train and verify keyboard works as expected. Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I46b1fd68915c6f7aa4c34cdba57d24425752bc38 --- M src/mainboard/google/zork/mainboard.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/63796/1 diff --git a/src/mainboard/google/zork/mainboard.c b/src/mainboard/google/zork/mainboard.c index 54da830..05e948e 100644 --- a/src/mainboard/google/zork/mainboard.c +++ b/src/mainboard/google/zork/mainboard.c @@ -80,7 +80,7 @@ /* The MISC registers are not interrupt numbers */ { PIRQ_MISC, 0xfa, 0x00 }, - { PIRQ_MISC0, 0xf1, 0x00 }, + { PIRQ_MISC0, 0x91, 0x00 }, { PIRQ_MISC1, 0x00, 0x00 }, { PIRQ_MISC2, 0x00, 0x00 }, }; -- To view, visit https://review.coreboot.org/c/coreboot/+/63796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I46b1fd68915c6f7aa4c34cdba57d24425752bc38 Gerrit-Change-Number: 63796 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/common/block: Add irq_override flags
by Raul Rangel (Code Review) 23 Apr '22

23 Apr '22

Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held. Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/54923 to look at the new patch set (#2). Change subject: soc/amd/common/block: Add irq_override flags ...................................................................... soc/amd/common/block: Add irq_override flags The irq_override field requires we use the MP_IRQ_* #define values. These are currently provided in the x86/arch/smp/mpspec.h header. This header is not available when building for PSP verstage. So instead we copy the definitions into chip.h. I renamed the prefix to MPS_ to not conflict with the existing MP_ #defines. The MPS_ prefix is what is used in the ACPI spec. BUG=none TEST=Use MPS_IRQ_ enum in device tree Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: If6f44d4ef4b7bb7398e173f42269e4fa833b545c --- M src/soc/amd/common/block/include/amdblocks/chip.h 1 file changed, 8 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/54923/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/54923 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If6f44d4ef4b7bb7398e173f42269e4fa833b545c Gerrit-Change-Number: 54923 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: util/cbmem: add an option to append timestamp
by Julius Werner (Code Review) 23 Apr '22

23 Apr '22

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62639 ) Change subject: util/cbmem: add an option to append timestamp ...................................................................... util/cbmem: add an option to append timestamp Add an option to the cbmem utility that can be used to append an entry to the cbmem timestamp table from userspace. This is useful for bookkeeping of post-coreboot timing information while still being able to use cbmem-based tooling for processing the generated data. BUG=b:217638034 BRANCH=none TEST=Manual: cbmem -a 1234 to append timestamp, verify that cbmem -t shows the added timestamp. Change-Id: Ic99e5a11d8cc3f9fffae8eaf2787652105cf4842 Signed-off-by: Mattias Nissler <mnissler(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62639 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Julius Werner <jwerner(a)chromium.org> --- M util/cbmem/cbmem.c 1 file changed, 96 insertions(+), 7 deletions(-) Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved diff --git a/util/cbmem/cbmem.c b/util/cbmem/cbmem.c index 3af8a25..748bc36 100644 --- a/util/cbmem/cbmem.c +++ b/util/cbmem/cbmem.c @@ -29,6 +29,10 @@ #include <sys/sysctl.h> #endif +#if defined(__i386__) || defined(__x86_64__) +#include <x86intrin.h> +#endif + #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) typedef uint8_t u8; @@ -57,6 +61,9 @@ static int mem_fd; static struct mapping lbtable_mapping; +/* TSC frequency from the LB_TAG_TSC_INFO record. 0 if not present. */ +static uint32_t tsc_freq_khz = 0; + static void die(const char *msg) { if (msg) @@ -80,9 +87,9 @@ } /* Return mapping of physical address requested. */ -static const void *mapping_virt(const struct mapping *mapping) +static void *mapping_virt(const struct mapping *mapping) { - const char *v = mapping->virt; + char *v = mapping->virt; if (v == NULL) return NULL; @@ -91,8 +98,8 @@ } /* Returns virtual address on success, NULL on error. mapping is filled in. */ -static const void *map_memory(struct mapping *mapping, unsigned long long phys, - size_t sz) +static void *map_memory_with_prot(struct mapping *mapping, + unsigned long long phys, size_t sz, int prot) { void *v; unsigned long long page_size; @@ -114,7 +121,7 @@ phys); } - v = mmap(NULL, mapping->virt_size, PROT_READ, MAP_SHARED, mem_fd, + v = mmap(NULL, mapping->virt_size, prot, MAP_SHARED, mem_fd, phys - mapping->offset); if (v == MAP_FAILED) { @@ -132,6 +139,14 @@ return mapping_virt(mapping); } +/* Convenience helper for the common case of read-only mappings. */ +static const void *map_memory(struct mapping *mapping, unsigned long long phys, + size_t sz) +{ + return map_memory_with_prot(mapping, phys, sz, PROT_READ); +} + + /* Returns 0 on success, < 0 on error. mapping is cleared if successful. */ static int unmap_memory(struct mapping *mapping) { @@ -327,6 +342,10 @@ parse_cbmem_ref((struct lb_cbmem_ref *)lbr_p); continue; } + case LB_TAG_TSC_INFO: + debug(" Found TSC info.\n"); + tsc_freq_khz = ((struct lb_tsc_info *)lbr_p)->freq_khz; + continue; case LB_TAG_FORWARD: { int ret; /* @@ -529,6 +548,22 @@ } } +static uint64_t timestamp_get(uint64_t table_tick_freq_mhz) +{ +#if defined(__i386__) || defined(__x86_64__) + uint64_t tsc = __rdtsc(); + + /* No tick frequency specified means raw TSC values. */ + if (!table_tick_freq_mhz) + return tsc; + + if (tsc_freq_khz) + return tsc * table_tick_freq_mhz * 1000 / tsc_freq_khz; +#endif + die("Don't know how to obtain timestamps on this platform.\n"); + return 0; +} + static const char *timestamp_name(uint32_t id) { for (size_t i = 0; i < ARRAY_SIZE(timestamp_ids); i++) { @@ -538,6 +573,15 @@ return "<unknown>"; } +static uint32_t timestamp_enum_name_to_id(const char *name) +{ + for (size_t i = 0; i < ARRAY_SIZE(timestamp_ids); i++) { + if (!strcmp(timestamp_ids[i].enum_name, name)) + return timestamp_ids[i].id; + } + return 0; +} + static uint64_t timestamp_print_parseable_entry(uint32_t id, uint64_t stamp, uint64_t prev_stamp) { @@ -765,6 +809,38 @@ free(sorted_tst_p); } +/* add a timestamp entry */ +static void timestamp_add_now(uint32_t timestamp_id) +{ + struct timestamp_table *tst_p; + struct mapping timestamp_mapping; + + if (timestamps.tag != LB_TAG_TIMESTAMPS) { + die("No timestamps found in coreboot table.\n"); + } + + tst_p = map_memory_with_prot(&timestamp_mapping, timestamps.cbmem_addr, + timestamps.size, PROT_READ | PROT_WRITE); + if (!tst_p) + die("Unable to map timestamp table\n"); + + /* + * Note that coreboot sizes the cbmem entry in the table according to + * max_entries, so it's OK to just add more entries if there's room. + */ + if (tst_p->num_entries >= tst_p->max_entries) { + die("Not enough space to add timestamp.\n"); + } else { + int64_t time = + timestamp_get(tst_p->tick_freq_mhz) - tst_p->base_time; + tst_p->entries[tst_p->num_entries].entry_id = timestamp_id; + tst_p->entries[tst_p->num_entries].entry_stamp = time; + tst_p->num_entries += 1; + } + + unmap_memory(&timestamp_mapping); +} + /* dump the tcpa log table */ static void dump_tcpa_log(void) { @@ -1260,6 +1336,7 @@ " -t | --timestamps: print timestamp information\n" " -T | --parseable-timestamps: print parseable timestamps\n" " -S | --stacked-timestamps: print stacked timestamps (e.g. for flame graph tools)\n" + " -a | --add-timestamp ID: append timestamp with ID\n" " -L | --tcpa-log print TCPA log\n" " -V | --verbose: verbose (debugging) output\n" " -v | --version: print the version\n" @@ -1397,6 +1474,7 @@ unsigned int rawdump_id = 0; int max_loglevel = BIOS_NEVER; int print_unknown_logs = 1; + uint32_t timestamp_id = 0; int opt, option_index = 0; static struct option long_options[] = { @@ -1410,6 +1488,7 @@ {"timestamps", 0, 0, 't'}, {"parseable-timestamps", 0, 0, 'T'}, {"stacked-timestamps", 0, 0, 'S'}, + {"add-timestamp", required_argument, 0, 'a'}, {"hexdump", 0, 0, 'x'}, {"rawdump", required_argument, 0, 'r'}, {"verbose", 0, 0, 'V'}, @@ -1417,7 +1496,7 @@ {"help", 0, 0, 'h'}, {0, 0, 0, 0} }; - while ((opt = getopt_long(argc, argv, "c12B:CltTSLxVvh?r:", + while ((opt = getopt_long(argc, argv, "c12B:CltTSa:LxVvh?r:", long_options, &option_index)) != EOF) { switch (opt) { case 'c': @@ -1470,6 +1549,13 @@ timestamp_type = TIMESTAMPS_PRINT_STACKED; print_defaults = 0; break; + case 'a': + print_defaults = 0; + timestamp_id = timestamp_enum_name_to_id(optarg); + /* Parse numeric value if name is unknown */ + if (timestamp_id == 0) + timestamp_id = strtoul(optarg, NULL, 0); + break; case 'V': verbose = 1; break; @@ -1492,7 +1578,7 @@ print_usage(argv[0], 1); } - mem_fd = open("/dev/mem", O_RDONLY, 0); + mem_fd = open("/dev/mem", timestamp_id ? O_RDWR : O_RDONLY, 0); if (mem_fd < 0) { fprintf(stderr, "Failed to gain memory access: %s\n", strerror(errno)); @@ -1579,6 +1665,9 @@ if (print_rawdump) dump_cbmem_raw(rawdump_id); + if (timestamp_id) + timestamp_add_now(timestamp_id); + if (print_defaults) timestamp_type = TIMESTAMPS_PRINT_NORMAL; -- To view, visit https://review.coreboot.org/c/coreboot/+/62639 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic99e5a11d8cc3f9fffae8eaf2787652105cf4842 Gerrit-Change-Number: 62639 Gerrit-PatchSet: 5 Gerrit-Owner: Mattias Nissler <mnissler(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.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]: util/cbmem: add an option to append timestamp
by Julius Werner (Code Review) 23 Apr '22

23 Apr '22

Attention is currently required from: Mattias Nissler, Paul Menzel. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62639 ) Change subject: util/cbmem: add an option to append timestamp ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/62639 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic99e5a11d8cc3f9fffae8eaf2787652105cf4842 Gerrit-Change-Number: 62639 Gerrit-PatchSet: 4 Gerrit-Owner: Mattias Nissler <mnissler(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mattias Nissler <mnissler(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 22 Apr 2022 22:43:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib/spd: Change log prefix for ddr4 params
by Julius Werner (Code Review) 23 Apr '22

23 Apr '22

Attention is currently required from: Kangheui Won, Tim Wawrzynczak, Reka Norman, Ivy Jian, Eric Lai. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63774 ) Change subject: lib/spd: Change log prefix for ddr4 params ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > As I mentioned in the issue, we still lack of jedec official SPD document for the LPDDR5 and DDR5... […] I agree with Tim, I'm not very familiar with this matter but it looks like you're taking the wrong approach here. If returning true here is intentional for LPDDR5, then clearly you should just be adding another case label for that. If we don't know whether that's the right thing to do, then do we really want to ship hardware with this? The whole point of having tests that check for warnings in the log is that we make sure we resolve these kinds of questions before release, not that we just delete/downgrade a perfectly valid warning just so we can check the "no warnings on boot" box on a release checklist. Besides, in general we should not be changing messages in upstream coreboot just to make a Chrome OS test pass unless that change also makes sense from the upstream perspective. In this case, it doesn't seem like it does. If you ever have a situation where it makes sense for a log message to be counted as "error" according to upstream coreboot's understanding of log levels, but for Chrome OS we really decided that having this message in the boot log doesn't signify a problem or something that should be solved more cleanly in a different way, then you should add an allowlist for that specific log message to the Chrome OS test, not change the behavior for everyone upstream. -- To view, visit https://review.coreboot.org/c/coreboot/+/63774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I55f0968b78baaa2fc9a6bbebf6712fb8bfd349f6 Gerrit-Change-Number: 63774 Gerrit-PatchSet: 1 Gerrit-Owner: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Kangheui Won <khwon(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Fri, 22 Apr 2022 22:20:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Factor TI50/CR50 config
by Julius Werner (Code Review) 23 Apr '22

23 Apr '22

Julius Werner has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/63285 ) Change subject: Factor TI50/CR50 config ...................................................................... Abandoned Superseded by CB:63424 -- To view, visit https://review.coreboot.org/c/coreboot/+/63285 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I037b1b717a16c468e2f97a912da99f125b61e1ce Gerrit-Change-Number: 63285 Gerrit-PatchSet: 2 Gerrit-Owner: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: abandon

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2022