March 2023 - flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: MAINTAINERS: Add Nikolai Artemiev for raiden_debug_spi
by Thomas Heijligen (Code Review) 14 Mar '23

14 Mar '23

Attention is currently required from: Felix Singer, Nikolai Artemiev. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73697 ) Change subject: MAINTAINERS: Add Nikolai Artemiev for raiden_debug_spi ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73697 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie70eafb301b3363caa8e670b67a16cd25c7e15a8 Gerrit-Change-Number: 73697 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 14 Mar 2023 11:58:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: MAINTAINERS: Add Aarya Chaumal for erasure_layout
by Thomas Heijligen (Code Review) 14 Mar '23

14 Mar '23

Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73642 ) Change subject: MAINTAINERS: Add Aarya Chaumal for erasure_layout ...................................................................... MAINTAINERS: Add Aarya Chaumal for erasure_layout Change-Id: Id0188add883336f2fb494e4169a668e8b502682f Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/73642 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> --- M MAINTAINERS 1 file changed, 20 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Thomas Heijligen: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved diff --git a/MAINTAINERS b/MAINTAINERS index 141c5be..cf10e5a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -91,6 +91,12 @@ F: util/ich_descriptors_tool/meson* F: util/ich_descriptors_tool/Makefile* +ERASE/WRITE ALGORITHM +M: Aarya Chaumal <aarya.chaumal(a)gmail.com> +S: Maintained +F: erasure_layout* +F: include/erasure_layout* + I2C HELPERS M: Peter Marheine <pmarheine(a)chromium.org> S: Supported -- To view, visit https://review.coreboot.org/c/flashrom/+/73642 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id0188add883336f2fb494e4169a668e8b502682f Gerrit-Change-Number: 73642 Gerrit-PatchSet: 3 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: MAINTAINERS: Add Aarya Chaumal for erasure_layout
by Thomas Heijligen (Code Review) 14 Mar '23

14 Mar '23

Attention is currently required from: Felix Singer, Aarya. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73642 ) Change subject: MAINTAINERS: Add Aarya Chaumal for erasure_layout ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/73642 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id0188add883336f2fb494e4169a668e8b502682f Gerrit-Change-Number: 73642 Gerrit-PatchSet: 2 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Tue, 14 Mar 2023 10:40:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: fmap: ignore areas with zero size
by Thomas Heijligen (Code Review) 14 Mar '23

14 Mar '23

Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73571 ) Change subject: fmap: ignore areas with zero size ...................................................................... fmap: ignore areas with zero size It's impossible for flashrom_layout to represent zero-sized flash regions but it is possible for a fmap to contain a zero-sized region which causes the resulting layout to fail layout_sanity_checks(), preventing use of that fmap. Because it would very rarely make sense to be able to operate on zero-sized regions anyway and changing layouts to be able to support zero-size regions would entail large changes, instead ignore zero-size regions when present in fmap. TEST=Warning is now printed when using fmap that contains a zero-sized area, and operations on other regions are allowed to proceed. BUG=b:271933192 Change-Id: Ie20971f779acece7a0b3b8f38796fff128ce689a Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/73571 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M libflashrom.c 1 file changed, 38 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Thomas Heijligen: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/libflashrom.c b/libflashrom.c index 2e89fe5..4a59e2a 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -362,6 +362,16 @@ return 1; for (i = 0, area = fmap->areas; i < fmap->nareas; i++, area++) { + if (area->size == 0) { + /* Layout regions use inclusive upper and lower bounds, + * so it's impossible to represent a region with zero + * size although it's allowed in fmap. */ + msg_gwarn("Ignoring zero-size fmap region \"%s\";" + " empty regions are unsupported.\n", + area->name); + continue; + } + snprintf(name, sizeof(name), "%s", area->name); if (flashrom_layout_add_region(l, area->offset, area->offset + area->size - 1, name)) { flashrom_layout_release(l); -- To view, visit https://review.coreboot.org/c/flashrom/+/73571 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie20971f779acece7a0b3b8f38796fff128ce689a Gerrit-Change-Number: 73571 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: tests: Add wrap for __fstat50 to fix tests for NetBSD
by Thomas Heijligen (Code Review) 14 Mar '23

14 Mar '23

Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Alexander Goncharov. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73649 ) Change subject: tests: Add wrap for __fstat50 to fix tests for NetBSD ...................................................................... Patch Set 1: Verified+1 Code-Review+2 (1 comment) Patchset: PS1: Tested on NetBSD 9.2 amd64 -- To view, visit https://review.coreboot.org/c/flashrom/+/73649 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icb8e453328cb40ab9d628f01ecdc3886a233dad5 Gerrit-Change-Number: 73649 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Tue, 14 Mar 2023 10:31:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: pcidev: remove pcidev_getdevfn() function
by Thomas Heijligen (Code Review) 14 Mar '23

14 Mar '23

Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73570 ) Change subject: pcidev: remove pcidev_getdevfn() function ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > PCI access in flashrom is a big mess. […] Thanks Edward! I didn't cached this. The revert is at CB:73730. I think the OOP stuff is not necessary. `pacc` is overwritten to make the `rpci_` functions working. When removing the `rpci_` functions and using proper shutdown functions there is no need to have `pacc` globally accessible any longer. -- To view, visit https://review.coreboot.org/c/flashrom/+/73570 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id2983420080f75ae6992edfb032bf5c83b29c803 Gerrit-Change-Number: 73570 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 14 Mar 2023 10:22:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: Revert "pcidev: remove pcidev_getdevfn() function"
by Thomas Heijligen (Code Review) 14 Mar '23

14 Mar '23

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73730 ) Change subject: Revert "pcidev: remove pcidev_getdevfn() function" ...................................................................... Revert "pcidev: remove pcidev_getdevfn() function" As Edward pointed out correctly this function does more than abstracting the different versions of `pci_get_dev()`. It also hide the usage of the global `struct pci_access pacc` from the caller. This reverts commit 0e8902f1ff2b927bb91a5e89e4fde3d8d71f6692. Change-Id: I0cd2f54cb6a6e35dc353476e0f5e502cbbd06cba Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M board_enable.c M include/programmer.h M pcidev.c 3 files changed, 26 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/73730/1 diff --git a/board_enable.c b/board_enable.c index 3e6a5a7..d35b151 100644 --- a/board_enable.c +++ b/board_enable.c @@ -1095,12 +1095,11 @@ return -1; } - dev = pci_get_dev(pacc, dev->domain, dev->bus, dev->dev, 1); + dev = pcidev_getdevfn(dev, 1); if (!dev) { msg_perr("MCP SMBus controller could not be found\n"); return -1; } - pci_fill_info(dev, PCI_FILL_IDENT); devclass = pci_read_word(dev, PCI_CLASS_DEVICE); if (devclass != 0x0C05) { msg_perr("Unexpected device class %04x for SMBus" diff --git a/include/programmer.h b/include/programmer.h index 9728d8d..d8d8b4a 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -125,6 +125,7 @@ uintptr_t pcidev_readbar(struct pci_dev *dev, int bar); struct pci_dev *pcidev_init(const struct programmer_cfg *cfg, const struct dev_entry *devs, int bar); struct pci_dev *pcidev_scandev(struct pci_filter *filter, struct pci_dev *start); +struct pci_dev *pcidev_getdevfn(struct pci_dev *dev, const int func); struct pci_dev *pcidev_find_vendorclass(uint16_t vendor, uint16_t devclass); struct pci_dev *pcidev_card_find(uint16_t vendor, uint16_t device, uint16_t card_vendor, uint16_t card_device); struct pci_dev *pcidev_find(uint16_t vendor, uint16_t device); diff --git a/pcidev.c b/pcidev.c index 68da327..696510e 100644 --- a/pcidev.c +++ b/pcidev.c @@ -190,6 +190,14 @@ return pcidev_scandev(&filter, NULL); } +struct pci_dev *pcidev_getdevfn(struct pci_dev *dev, const int func) +{ + struct pci_dev *const new = pci_get_dev(pacc, dev->domain, dev->bus, dev->dev, func); + if (new) + pci_fill_info(new, PCI_FILL_IDENT); + return new; +} + struct pci_dev *pcidev_find_vendorclass(uint16_t vendor, uint16_t devclass) { struct pci_dev *temp = NULL; -- To view, visit https://review.coreboot.org/c/flashrom/+/73730 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0cd2f54cb6a6e35dc353476e0f5e502cbbd06cba Gerrit-Change-Number: 73730 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

[L] Change in flashrom[master]: doc: Convert build documentation to sphinx
by Thomas Heijligen (Code Review) 14 Mar '23

14 Mar '23

Attention is currently required from: Thomas Heijligen, Stefan Reinauer, Alexander Goncharov. Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, Peter Marheine, Alexander Goncharov, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/73359 to look at the new patch set (#6). Change subject: doc: Convert build documentation to sphinx ...................................................................... doc: Convert build documentation to sphinx These build instructions are mostly based on `Documentation/building.md` and the wiki. There are some `.. todo::` sections still present in the documentation. They will be completed later. For some of the todos content needs to be written, some others require custom sphinx-plugins to be implemented. The `.. todo::` sections are only visible in the source, not in the rendered html. Change-Id: I96771e98b313a6d26dd2be940ff37998d4124324 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- D Documentation/building.md M doc/conf.py A doc/dev_guide/building_from_source.rst A doc/dev_guide/index.rst M doc/index.rst 5 files changed, 291 insertions(+), 148 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/73359/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/73359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I96771e98b313a6d26dd2be940ff37998d4124324 Gerrit-Change-Number: 73359 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Convert unlock func ptr into enumerate values
by Sam McNally (Code Review) 14 Mar '23

14 Mar '23

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69933 ) Change subject: tree/: Convert unlock func ptr into enumerate values ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69933 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie5c5db1b09d07e1a549990d6f5a622fae4c83233 Gerrit-Change-Number: 69933 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 14 Mar 2023 05:30:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Convert unlock func ptr into enumerate values
by Edward O'Callaghan (Code Review) 14 Mar '23

14 Mar '23

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69933 to look at the new patch set (#6). Change subject: tree/: Convert unlock func ptr into enumerate values ...................................................................... tree/: Convert unlock func ptr into enumerate values Converting the blockprotect unlock function pointer within the flashchip struct into enum values allows for the flashchips db to be turn into pure, declarative data. A nice side-effect of this is to reduce link-time symbol space of chipdrivers and increase modularity of the spi25_statusreg.c and related implementations. BUG=none TEST=ninja test. Change-Id: Ie5c5db1b09d07e1a549990d6f5a622fae4c83233 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M 82802ab.c M flashrom.c M include/chipdrivers.h M include/flash.h M printlock.c M spi25_statusreg.c M tests/chip.c M tests/chip_wp.c 8 files changed, 150 insertions(+), 74 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/69933/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/69933 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie5c5db1b09d07e1a549990d6f5a622fae4c83233 Gerrit-Change-Number: 69933 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2023