coreboot-gerrit August 2021

coreboot-gerrit@coreboot.org

1 participants
4518 discussions

Change in coreboot[master]: elogtool: add "clear" command
by Ricardo Quesada (Code Review) 07 Aug '21

07 Aug '21

Ricardo Quesada has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56866 ) Change subject: elogtool: add "clear" command ...................................................................... elogtool: add "clear" command Adds "clear" command to cbfsutil/elogtool tool. "clear" clears the RW_ELOG using flashrom with ELOG_TYPE_EOL. And inserts a ELOG_TYPE_LOG_CLEAR event. Usage: $ elogtool clear BUG=b:172210863 Change-Id: I7ea64ea748ee4c3647b0256be51203464a54c2dd Signed-off-by: Ricardo Quesada <ricardoq(a)google.com> --- M src/commonlib/bsd/elog.c M src/commonlib/bsd/include/commonlib/bsd/elog.h M util/cbfstool/elogtool.c M util/cbfstool/eventlog.c M util/cbfstool/eventlog.h 5 files changed, 160 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/56866/1 diff --git a/src/commonlib/bsd/elog.c b/src/commonlib/bsd/elog.c index 6c927aa..a538dd3 100644 --- a/src/commonlib/bsd/elog.c +++ b/src/commonlib/bsd/elog.c @@ -44,3 +44,56 @@ // right after the header. return (const void *)(&event[1]); } + +static inline uint8_t mybin2bcd(uint8_t val) +{ + return ((val / 10) << 4) | (val % 10); +} + +/* + * Populate timestamp in event header with given time. + */ +void elog_fill_timestamp(struct event_header *event, uint8_t sec, uint8_t min, + uint8_t hour, uint8_t mday, uint8_t mon, uint8_t year) +{ + event->second = mybin2bcd(sec); + event->minute = mybin2bcd(min); + event->hour = mybin2bcd(hour); + event->day = mybin2bcd(mday); + event->month = mybin2bcd(mon); + event->year = mybin2bcd(year % 100); + + /* Basic sanity check of expected ranges */ + if (event->month > 0x12 || event->day > 0x31 || event->hour > 0x23 || + event->minute > 0x59 || event->second > 0x59) + { + event->year = 0; + event->month = 0; + event->day = 0; + event->hour = 0; + event->minute = 0; + event->second = 0; + } +} + +/* + * Update the checksum at the last byte + */ +void elog_update_checksum(struct event_header *event, uint8_t checksum) +{ + uint8_t *event_data = (uint8_t *)event; + event_data[event->length - 1] = checksum; +} + +/* + * Simple byte checksum for events + */ +uint8_t elog_checksum_event(struct event_header *event) +{ + uint8_t index, checksum = 0; + uint8_t *data = (uint8_t *)event; + + for (index = 0; index < event->length; index++) + checksum += data[index]; + return checksum; +} \ No newline at end of file diff --git a/src/commonlib/bsd/include/commonlib/bsd/elog.h b/src/commonlib/bsd/include/commonlib/bsd/elog.h index 0d41157..54e7ec4 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/elog.h +++ b/src/commonlib/bsd/include/commonlib/bsd/elog.h @@ -313,5 +313,9 @@ enum cb_err elog_verify_header(const struct elog_header *header); const struct event_header *elog_get_next_event(const struct event_header *event); const void *event_get_data(const struct event_header *event); +void elog_fill_timestamp(struct event_header *event, uint8_t sec, uint8_t min, + uint8_t hour, uint8_t mday, uint8_t mon, uint8_t year); +void elog_update_checksum(struct event_header *event, uint8_t checksum); +uint8_t elog_checksum_event(struct event_header *event); #endif /* _COMMONLIB_BSD_ELOG_H_ */ diff --git a/util/cbfstool/elogtool.c b/util/cbfstool/elogtool.c index ba59eac..4b05f6c 100644 --- a/util/cbfstool/elogtool.c +++ b/util/cbfstool/elogtool.c @@ -33,7 +33,9 @@ "USAGE:\n" "\t%s COMMAND [-f <filename>]\n\n" "where, COMMAND is:\n" - " list = lists all the event logs in human readable format\n\n" + " list = lists all the event logs in human readable format\n" + " clear = clears the contents RW_ELOG region from flash\n" + "\n" "ARGS\n" "-f, --file <filename> Input file that holds event log partition.\n" " If empty it will try to read from the RW_ELOG ROM region\n" @@ -111,6 +113,76 @@ return ret; } +/* + * Clears the elog events from buf, which is a valid RW_ELOG region. + * A LOG_CLEAR event is appended + */ +static int elog_clear_events(struct buffer* buf) +{ + const struct event_header *event; + uint32_t used_data_size = 0; + void* data_offset; + size_t data_size; + + data_size = buf->size - sizeof(struct elog_header); + data_offset = buf->data + sizeof(struct elog_header); + + /* Must have at least size for event + data + checksum */ + if (data_size < sizeof(struct event_header) + sizeof(used_data_size) + 1) + return ELOGTOOL_EXIT_INVALID_ELOG_FORMAT; + + /* + * We calcualte the size of the "used" buffer, needed for ELOG_TYPE_LOG_CLEAR. + * Then we overwritte the entire buffer with ELOG_TYPE_EOL. + * Finally we insert a LOG_CLEAR event into the buffer. + */ + event = (const struct event_header *)(data_offset); + + while ((const char*)(event) < buf->data + buf->size) { + if (event->type == ELOG_TYPE_EOL || event->length == 0) + break; + + used_data_size += event->length; + event = elog_get_next_event(event); + } + + memset(data_offset, ELOG_TYPE_EOL, data_size); + + eventlog_init_event(data_offset, ELOG_TYPE_LOG_CLEAR, + &used_data_size, sizeof(used_data_size)); + + return ELOGTOOL_EXIT_SUCCESS; +} + +static int cmd_clear(void) +{ + int ret; + + /* Returned buffer must be freed. */ + struct buffer buf; + ret = elog_read(NULL, &buf); + if (ret != 0) + return ret; + + ret = elog_clear_events(&buf); + if (ret != 0) { + buffer_delete(&buf); + return ret; + } + + if (flashrom_write(FLASHROM_PROGRAMMER_INTERNAL_AP, "RW_ELOG", + (void *)buf.data, buf.size) != VB2_SUCCESS) { + + fprintf(stderr, + "Failed to write to RW_ELOG region using flashrom\n"); + ret = ELOGTOOL_EXIT_BAD_INPUT_PATH; + + /* Fallthrough */ + } + + buffer_delete(&buf); + return ret; +} int main(int argc, char **argv) { @@ -157,5 +229,8 @@ if (!strcmp(argv[optind], "list")) return cmd_list(input_file); + if (!strcmp(argv[optind], "clear")) + return cmd_clear(); + return ELOGTOOL_EXIT_SUCCESS; } diff --git a/util/cbfstool/eventlog.c b/util/cbfstool/eventlog.c index 2d99f92..ccec32b 100644 --- a/util/cbfstool/eventlog.c +++ b/util/cbfstool/eventlog.c @@ -634,3 +634,27 @@ eventlog_printf_ignore_separator_once = 1; eventlog_printf("\n"); } + +void eventlog_init_event(struct event_header* event, uint8_t type, const void* data, int data_size) +{ + time_t secs = time(NULL); + struct tm tm; + + event->type = type; + gmtime_r(&secs, &tm); + elog_fill_timestamp(event, tm.tm_sec, tm.tm_min, tm.tm_hour, tm.tm_mday, tm.tm_mon, tm.tm_year); + + if (data && data) { + void* ptr = (uint32_t*) &event[1]; + memcpy(ptr, data, data_size); + } + + /* Header + data + checksum */ + event->length = sizeof(*event) + data_size + 1; + + /* Zero the checksum byte and then compute checksum */ + elog_update_checksum(event, 0); + elog_update_checksum(event, -(elog_checksum_event(event))); + + +} diff --git a/util/cbfstool/eventlog.h b/util/cbfstool/eventlog.h index baecb21..e968e83 100644 --- a/util/cbfstool/eventlog.h +++ b/util/cbfstool/eventlog.h @@ -3,8 +3,11 @@ #ifndef EVENTLOG_H_ #define EVENTLOG_H_ +#include <stdint.h> + struct event_header; void eventlog_print_event(const struct event_header *event, int count); +void eventlog_init_event(struct event_header* event, uint8_t type, const void* data, int data_size); #endif // EVENTLOG_H_ -- To view, visit https://review.coreboot.org/c/coreboot/+/56866 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ea64ea748ee4c3647b0256be51203464a54c2dd Gerrit-Change-Number: 56866 Gerrit-PatchSet: 1 Gerrit-Owner: Ricardo Quesada <ricardoq(a)google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: tests/Makefile.inc: Add function wrapping mechanism
by Julius Werner (Code Review) 07 Aug '21

07 Aug '21

Attention is currently required from: Jakub Czapiga, Jan Dabros. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56719 ) Change subject: tests/Makefile.inc: Add function wrapping mechanism ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56719 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7cd0d66a17029955cbf75c8b155a7ebb7f5513aa Gerrit-Change-Number: 56719 Gerrit-PatchSet: 7 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com> 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: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com> Gerrit-Comment-Date: Sat, 07 Aug 2021 00:36:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lenovo/x230: introduce FHD variant
by Charles Moyes (Code Review) 07 Aug '21

07 Aug '21

Attention is currently required from: Kilian Neuner, Alexander Couzens. Charles Moyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/28950 ) Change subject: lenovo/x230: introduce FHD variant ...................................................................... Patch Set 12: (1 comment) Patchset: PS12: This is quite a popular hardware mod for Thinkpad x230 laptops. What would it take to get this merged in? -- To view, visit https://review.coreboot.org/c/coreboot/+/28950 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0355d39a61956792e69bccd5274cfc2749d72bf0 Gerrit-Change-Number: 28950 Gerrit-PatchSet: 12 Gerrit-Owner: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kilian Neuner <cb(a)9-r.net> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Peter Lemenkov <lemenkov(a)gmail.com> Gerrit-Reviewer: Richard Slindee Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Aleksei Kharlamov <derlafff(a)ya.ru> Gerrit-CC: Charles Moyes <thechuckster(a)gmail.com> Gerrit-CC: Christian Herzog Gerrit-CC: Holger Levsen <holger(a)layer-acht.org> Gerrit-CC: Matthias Wiedhalm Gerrit-CC: Pavel Kovalenko <su(a)nitrocaster.me> Gerrit-CC: Sergey Alirzaev <zl29ah(a)gmail.com> Gerrit-CC: Stanislaw Kaminski <stanislaw.kaminski(a)gmail.com> Gerrit-CC: Tom Hiller <thrilleratplay(a)gmail.com> Gerrit-CC: Tomasz Jan Góralczyk Gerrit-CC: clayton craft Gerrit-CC: nullmark <nullmark(a)googlemail.com> Gerrit-CC: slact Gerrit-Attention: Kilian Neuner <cb(a)9-r.net> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Comment-Date: Sat, 07 Aug 2021 00:29:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common/block/spi: Don't update spi speed if EFS is changed
by Felix Held (Code Review) 06 Aug '21

06 Aug '21

Attention is currently required from: Furquan Shaikh, Martin Roth, Karthik Ramasubramanian. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56818 ) Change subject: soc/amd/common/block/spi: Don't update spi speed if EFS is changed ...................................................................... Patch Set 2: (1 comment) File src/soc/amd/common/block/spi/fch_spi.c: https://review.coreboot.org/c/coreboot/+/56818/comment/7ae2b55a_f04b6e45 PS2, Line 84: if (CONFIG(EM100)) : fch_spi_config_em100_modes(); : else : fch_spi_config_mb_modes(); > Agreement that these separate calls for em100 need to be dropped and configure read mode, fast speed […] the mainboard's Kconfig options have separate cases for the EM100 mode -- To view, visit https://review.coreboot.org/c/coreboot/+/56818 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I278768e361499919666d07e1dd92d83c2390035e Gerrit-Change-Number: 56818 Gerrit-PatchSet: 2 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 06 Aug 2021 23:40:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Switch from 33MHz to 66MHz SPI Speed
by Felix Held (Code Review) 06 Aug '21

06 Aug '21

Attention is currently required from: Raul Rangel, Furquan Shaikh, Karthik Ramasubramanian. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56645 ) Change subject: mb/google/guybrush: Switch from 33MHz to 66MHz SPI Speed ...................................................................... Patch Set 3: (1 comment) File src/mainboard/google/guybrush/variants/baseboard/devicetree.cb: https://review.coreboot.org/c/coreboot/+/56645/comment/54dc9cae_335e118e PS3, Line 13: 33M > Are you referring to the 8 dummy cycles that are added for fast read command? What is the maximum su […] yes, i was referring to the 8 dummy cycles for the fast read command that gives the flash more time to have the data ready to output. the maximum data rate for sequential reads can only be achieved when using the fast read commands. the maximum frequencies the flash can reach for the different modes should be in the flash chip's datasheet; non-ideal signal integrity in the board design might lower the maximum frequency though -- To view, visit https://review.coreboot.org/c/coreboot/+/56645 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icc5c0580ed0d19f1fffce59df3888dd7963255a1 Gerrit-Change-Number: 56645 Gerrit-PatchSet: 3 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 06 Aug 2021 23:37:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede: allow MKBP devices
by Boris Mittelberg (Code Review) 06 Aug '21

06 Aug '21

Attention is currently required from: Paul Menzel, Karthik Ramasubramanian. Boris Mittelberg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56840 ) Change subject: mb/google/dedede: allow MKBP devices ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/56840/comment/449678fc_bbe5a034 PS2, Line 14: Cq-Depend:chromium:3069163 > Nit: Move this into the last paragraph i.e. just above Signed-off-by: line. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/56840 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9d1f43e4dd56318af4c1d5f5c1c3a2c237a05c5f Gerrit-Change-Number: 56840 Gerrit-PatchSet: 3 Gerrit-Owner: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Aseda Aboagye <aaboagye(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 06 Aug 2021 23:37:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede: allow MKBP devices
by Boris Mittelberg (Code Review) 06 Aug '21

06 Aug '21

Attention is currently required from: Paul Menzel, Boris Mittelberg. Hello build bot (Jenkins), Furquan Shaikh, Karthik Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56840 to look at the new patch set (#3). Change subject: mb/google/dedede: allow MKBP devices ...................................................................... mb/google/dedede: allow MKBP devices Enable MKBP interface for all dedede family to use for buttons and switches. BUG=b:170966461 TEST=manual test on Madoo: Volume Up/Down and Power buttons, Tablet Mode switch Cq-Depend: chromium:3069163 Signed-off-by: Boris Mittelberg <bmbm(a)google.com> Change-Id: I9d1f43e4dd56318af4c1d5f5c1c3a2c237a05c5f --- M src/mainboard/google/dedede/variants/baseboard/include/baseboard/ec.h 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/56840/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/56840 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9d1f43e4dd56318af4c1d5f5c1c3a2c237a05c5f Gerrit-Change-Number: 56840 Gerrit-PatchSet: 3 Gerrit-Owner: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Aseda Aboagye <aaboagye(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: util/elogtool: add tool to print elog events
by Julius Werner (Code Review) 06 Aug '21

06 Aug '21

Attention is currently required from: Ricardo Quesada. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56406 ) Change subject: util/elogtool: add tool to print elog events ...................................................................... Patch Set 9: (6 comments) Patchset: PS9: Apologies for being too late on the review, but please consider these for a future fixup CL. File util/cbfstool/elogtool.c: https://review.coreboot.org/c/coreboot/+/56406/comment/697aff4e_32ab3087 PS9, Line 160: ELOGTOOL_EXIT_SUCCESS Doesn't reaching this point mean BAD_ARGS (i.e. the command was not recognized)? File util/cbfstool/eventlog.c: https://review.coreboot.org/c/coreboot/+/56406/comment/5a4673e7_3254d539 PS9, Line 80: strftime(tm_string, sizeof(tm_string), "%Y-%m-%d %H:%M:%S", localtime(&time)); nit: I know it didn't use to do this, but should we maybe consider adding the time zone (%z) here? We have some logs in UTC and some in local, it's always super confusing to figure out which is which when it doesn't say so explicitly. https://review.coreboot.org/c/coreboot/+/56406/comment/61e79a7f_21630a00 PS9, Line 355: {VB2_RECOVERY_LEGACY, "Legacy Utility"}, Now that this is linking vboot it should just call vb2_get_recovery_reason_string() rather than maintaining a separate list. https://review.coreboot.org/c/coreboot/+/56406/comment/b3273c94_5ca06bdd PS9, Line 550: eventlog_printf("%s", val2str(*code, coreboot_post_codes)); There needs to be more range checking in various parts of this, you're just printing a potentially unbounded string from an untrusted source here. This should be written to always keep track of how much eventlog data was loaded in total (e.g. by using the cbfstool buffer_xxx() APIs) and then for each parsing operation make sure that it cannot run over the end of that without blindly trusting the data. https://review.coreboot.org/c/coreboot/+/56406/comment/0bc24f7c_884ff5e1 PS9, Line 626: eventlog_printf_ignore_separator_once = 1; nit: Maybe it's just me but this printf API that passes state through a global seems pretty horrible. Can't we just insert the printf(" | ") manually where it's needed? -- To view, visit https://review.coreboot.org/c/coreboot/+/56406 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia1fe1c9ed3c4c6bda846055d4b10943b54463935 Gerrit-Change-Number: 56406 Gerrit-PatchSet: 9 Gerrit-Owner: Ricardo Quesada <ricardoq(a)google.com> Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Ricardo Quesada <ricardoq(a)google.com> Gerrit-Comment-Date: Fri, 06 Aug 2021 23:30:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/system76/oryp6: Drop DIMM_SPD_SIZE
by Tim Crawford (Code Review) 06 Aug '21

06 Aug '21

Tim Crawford has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56865 ) Change subject: mb/system76/oryp6: Drop DIMM_SPD_SIZE ...................................................................... mb/system76/oryp6: Drop DIMM_SPD_SIZE The board uses the default size specified in the SoC. Change-Id: Ie71a0fea1ff9de6c4f1ce8db2db09bb3cd35d04d Signed-off-by: Tim Crawford <tcrawford(a)system76.com> --- M src/mainboard/system76/oryp6/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/56865/1 diff --git a/src/mainboard/system76/oryp6/Kconfig b/src/mainboard/system76/oryp6/Kconfig index 56ccd175..d063f40 100644 --- a/src/mainboard/system76/oryp6/Kconfig +++ b/src/mainboard/system76/oryp6/Kconfig @@ -54,10 +54,6 @@ int default 2 -config DIMM_SPD_SIZE - int - default 512 - config VGA_BIOS_ID string default "8086,9bc4" -- To view, visit https://review.coreboot.org/c/coreboot/+/56865 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie71a0fea1ff9de6c4f1ce8db2db09bb3cd35d04d Gerrit-Change-Number: 56865 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: elogtool: add to gitignore
by Furquan Shaikh (Code Review) 06 Aug '21

06 Aug '21

Attention is currently required from: Jack Rosenthal. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56864 ) Change subject: elogtool: add to gitignore ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56864 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I806338a4b33abbc3d55e4edef2736c19d56fa005 Gerrit-Change-Number: 56864 Gerrit-PatchSet: 1 Gerrit-Owner: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Ricardo Quesada <ricardoq(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Comment-Date: Fri, 06 Aug 2021 22:59:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2021