June 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops
by Michael Niewöhner (Code Review) 23 Jun '21

23 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > > > Knowing the mainboard avoids running the code on the wrong board where it's more likely to do harm. It's not about the flashing process, flash contents or the contents to be flashed. The code starts with `ec_write_reg(0xf9, 0x20, EC_MAX_STATUS_CHECKS)`. There may be EC firmware that reacts badly to such a write (or chips that completely misunderstand what is written to their registers). Knowing the board it runs on, makes it much less likely to hit such a case. > > > > That's the exact reason why I have put DMI checks. I don't want people complaining on flashrom bricking/damaging someone's device and losing trust in this project. That's what SVID/SDID can be used for, too, since it identifies the mainboard as DMI does. Neither of them can replace the check for the correct EC being present but avoid confusing other chips, like Nico mentioned. > > To be effective, the DMI checks should happen as part of programmer initialisation. Looks like they do now, which is good. > > > If we still want some possibility to probe the EC, aware of the risks, we could use the `force` flag from the CLI to forcefully send those commands under a warning that it may brick their device. > > Sounds like a reasonable compromise. Sounds good. > > > I am in favor of not breaking things wherever possible. Flashrom is not only an utility to change flash contents for hackers and developers right now (it probably was earlier). It is also used as a plugin in fwupd for example, where reliability and safety of the update is in the first place. > > Definitely. -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 23 Jun 2021 12:27:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops
by Michael Niewöhner (Code Review) 23 Jun '21

23 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops ...................................................................... Patch Set 4: (4 comments) File tuxedo_ec.c: https://review.coreboot.org/c/flashrom/+/55715/comment/91b4046b_a4f0aaa5 PS4, Line 2: * This file is part of the flashrom project. > Why? This isn't coreboot. oops. you're right. thanks https://review.coreboot.org/c/flashrom/+/55715/comment/8c22e996_5d74a24b PS4, Line 4: * Copyright (C) 2021, TUXEDO Computers GmbH > Why? oops. you're right. thanks https://review.coreboot.org/c/flashrom/+/55715/comment/315becee_8742677d PS4, Line 6: * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : > Why? oops. you're right. thanks https://review.coreboot.org/c/flashrom/+/55715/comment/46b59a5c_adeca3a2 PS4, Line 822: if (strcmp(current_ec_project, new_ec_project)) { : msg_perr("Wrong EC project. This file can't be used on this machine\n"); : return 1; : } : can we add a parameter for forcing here? One might use flashrom to switch from system76 ec to vendor, or from vendor to <whatever> -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 23 Jun 2021 12:24:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops
by Michał Żygowski (Code Review) 23 Jun '21

23 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops ...................................................................... Patch Set 4: (2 comments) File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/55715/comment/25ecd7a4_763bb9f8 PS4, Line 1376: The default is 1 > I don't see the code for this? `tuxedo_ec_init_ctx` is setting these defaults File tuxedo_ec.c: https://review.coreboot.org/c/flashrom/+/55715/comment/d090df84_8fa3de51 PS4, Line 661: dmi_init(); : : if (!dmi_match("^TUXEDO$")) { : msg_perr("Not a TUXEDO device\n"); : return 1; : } : : for (i = 0; i < ARRAY_SIZE(tuxedo_supported_boards); i++) { : if (dmi_match(tuxedo_supported_boards[i])) { : match = true; : break; : } : } : : if (!match) { : msg_perr("TUXEDO EC programmer not yet supported on this device\n"); : return 1; : } > I'd move this into a separate function, and refactor to allow adding support for non-TUXEDO machines […] I was thinking about the same. Thanks for the piece of code. At this point I should probably consider adding your sign-off to the commit 😊 -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Wed, 23 Jun 2021 10:03:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops
by Angel Pons (Code Review) 23 Jun '21

23 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Michael Niewöhner. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops ...................................................................... Patch Set 4: (4 comments) File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/55715/comment/979a13a3_85ed865f PS4, Line 1376: The default is 1 I don't see the code for this? File tuxedo_ec.c: https://review.coreboot.org/c/flashrom/+/55715/comment/edc61093_61fc0ffb PS4, Line 661: dmi_init(); : : if (!dmi_match("^TUXEDO$")) { : msg_perr("Not a TUXEDO device\n"); : return 1; : } : : for (i = 0; i < ARRAY_SIZE(tuxedo_supported_boards); i++) { : if (dmi_match(tuxedo_supported_boards[i])) { : match = true; : break; : } : } : : if (!match) { : msg_perr("TUXEDO EC programmer not yet supported on this device\n"); : return 1; : } I'd move this into a separate function, and refactor to allow adding support for non-TUXEDO machines easily: static const char *const tuxedo_boards[] = { "^InfinityBook S 14 Gen6$", "^InfinityBook S 15 Gen6$", }; struct dmi_vendor_entry { const char *vendor; const char **boards; size_t num_boards; }; static const struct dmi_vendor_entry ite_ecfw_supported_boards[] = { {"^TUXEDO$", tuxedo_boards, ARRAY_SIZE(tuxedo_boards)}, }; static bool is_board_supported(void) { size_t v, b; dmi_init(); for (v = 0; v < ARRAY_SIZE(ite_ecfw_supported_boards); v++) { const struct dmi_vendor_entry *entry = &ite_ecfw_supported_boards[v]; if (!dmi_match(entry->vendor)) continue; for (b = 0; b < entry->num_boards; b++) { if (dmi_match(entry->boards[b])) return true; } msg_perr("Found DMI match for vendor %s, but device is not supported\n", entry->vendor); } msg_perr("ITE ECFW programmer not (yet) supported on this device\n"); return false; } https://review.coreboot.org/c/flashrom/+/55715/comment/5f8eee6c_c05d7985 PS4, Line 688: sizeof(struct tuxedo_ec_data) sizeof(*ctx_data) https://review.coreboot.org/c/flashrom/+/55715/comment/e88a0e4c_285712f0 PS4, Line 740: uint8_t *const contents, char *buf nit: I'd flip the parameter order to match the natural assignment semantics (leftmost parameter is the destination: char *buf, uint8_t *const contents This order matches what the code does: buf[i] = contents[i]; -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Wed, 23 Jun 2021 09:48:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops
by Angel Pons (Code Review) 23 Jun '21

23 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Michael Niewöhner. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > > Knowing the mainboard avoids running the code on the wrong board where it's more likely to do harm. It's not about the flashing process, flash contents or the contents to be flashed. The code starts with `ec_write_reg(0xf9, 0x20, EC_MAX_STATUS_CHECKS)`. There may be EC firmware that reacts badly to such a write (or chips that completely misunderstand what is written to their registers). Knowing the board it runs on, makes it much less likely to hit such a case. > > That's the exact reason why I have put DMI checks. I don't want people complaining on flashrom bricking/damaging someone's device and losing trust in this project. To be effective, the DMI checks should happen as part of programmer initialisation. Looks like they do now, which is good. > If we still want some possibility to probe the EC, aware of the risks, we could use the `force` flag from the CLI to forcefully send those commands under a warning that it may brick their device. Sounds like a reasonable compromise. > I am in favor of not breaking things wherever possible. Flashrom is not only an utility to change flash contents for hackers and developers right now (it probably was earlier). It is also used as a plugin in fwupd for example, where reliability and safety of the update is in the first place. Definitely. -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Wed, 23 Jun 2021 09:20:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Guard sys/io.h the same as in hwaccess_x86_io.h
by Anastasia Klimchuk (Code Review) 23 Jun '21

23 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55741 to look at the new patch set (#5). Change subject: tests: Guard sys/io.h the same as in hwaccess_x86_io.h ...................................................................... tests: Guard sys/io.h the same as in hwaccess_x86_io.h Inclusion of this header in hwaccess_x86_io.h is guarded by `defined(__linux__) || defined(__GLIBC__)` , in addition to that inclusion of hwaccess_x86_io.h into hwaccess.h is guarded by IS_X86. Combining these two together, this patch adds the same guards for unittest header file, so that test build don't break on non-x86. This is a follow up on commit 21e22ba8a7750f1cfe5cd3323e3137695ffef0a4 which introduced hwaccess_x86_io_unittest.h BUG=b:181803212 TEST= 1) builds and ninja test on x86 (same as before) 2) echo "#include \"hwaccess_x86_io_unittest.h\"" > foo.c arm-linux-gnueabi-gcc -E foo.c Result: guarded code not included, hwaccess_x86_io_unittest.h is empty for arm-linux-gnueabi Change-Id: I3f2f0408be7c00f954b899031b52b2b97ef19ca3 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/hwaccess_x86_io_unittest.h 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/55741/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/55741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f2f0408be7c00f954b899031b52b2b97ef19ca3 Gerrit-Change-Number: 55741 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Do not run a test if its driver is not built
by Angel Pons (Code Review) 23 Jun '21

23 Jun '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55295 ) Change subject: tests: Do not run a test if its driver is not built ...................................................................... Patch Set 5: Code-Review+2 (1 comment) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/55295/comment/eb58c6da_9cc699ee PS1, Line 35: #if CONFIG_DUMMY == 1 : void dummy_init_and_shutdown_test_success(void **state) : { : run_lifecycle(state, PROGRAMMER_DUMMY, "bus=parallel+lpc+fwh+spi"); : } : #endif /* CONFIG_DUMMY */ > This is a work of art! I evaluated it step by step on a piece of paper, but wow to someone who wrote […] If you're mocking the port I/O completely, it should be possible to run the test on non-x86, e.g. a SBC with an ARM SoC. -- To view, visit https://review.coreboot.org/c/flashrom/+/55295 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic1c48e41f658045a608f46636071f478ba646f77 Gerrit-Change-Number: 55295 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 23 Jun 2021 09:05:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops
by Michał Żygowski (Code Review) 23 Jun '21

23 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops ...................................................................... Patch Set 4: (2 comments) Patchset: PS4: > Also, does Tuxedo have a custom (EC) firmware or do they use the ODM (Clevo) one? If it’s the ODM, t […] It's hard to say for sure. Since these laptops are simply customized Clevo laptops (chassis signatures), they could use the same firmware. I have only seen the change in SMBIOS entries in the BIOS image, EC firmware is flashed as is. PS4: > Another idea to avoid DMI would be checking the SVID/SDID (subsystem id) The SVID/SDID is: 00:00.0 Host bridge [0600]: Intel Corporation Device [8086:9a14] (rev 01) Subsystem: CLEVO/KAPOK Computer Device [1558:14a1] But I am not sure how does it help in probing and ensuring the matching EC is present? > Knowing the mainboard avoids running the code on the wrong board where it's more likely to do harm. It's not about the flashing process, flash contents or the contents to be flashed. The code starts with `ec_write_reg(0xf9, 0x20, EC_MAX_STATUS_CHECKS)`. There may be EC firmware that reacts badly to such a write (or chips that completely misunderstand what is written to their registers). Knowing the board it runs on, makes it much less likely to hit such a case. That's the exact reason why I have put DMI checks. I don't want people complaining on flashrom bricking/damaging someone's device and losing trust in this project. If we still want some possibility to probe the EC, aware of the risks, we could use the `force` flag from the CLI to forcefully send those commands under a warning that it may brick their device. I am in favor of not breaking things wherever possible. Flashrom is not only an utility to change flash contents for hackers and developers right now (it probably was earlier). It is also used as a plugin in fwupd for example, where reliability and safety of the update is in the first place. -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Wed, 23 Jun 2021 08:18:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops
by Paul Menzel (Code Review) 23 Jun '21

23 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Michael Niewöhner. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: tuxedo_ec: Implement support for flashing ECs on TUXEDO laptops ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: Also, does Tuxedo have a custom (EC) firmware or do they use the ODM (Clevo) one? If it’s the ODM, the file should probably have a generic name. -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Wed, 23 Jun 2021 07:16:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 23 Jun '21

23 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Samir Ibradžić. Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... Patch Set 34: (1 comment) Patchset: PS34: Thanks for paying attention again. Feel free to do the changes yourself as I won´t get time within the next two weeks. Coming back I will see what´s left. -- To view, visit https://review.coreboot.org/c/flashrom/+/40477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a Gerrit-Change-Number: 40477 Gerrit-PatchSet: 34 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Wed, 23 Jun 2021 06:31:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021