flashrom cannot write AP SPI contents when HW WP=0, SW WP=1 on some recent platforms |
||||||||
Issue descriptionChrome Version: ToT OS: Chrome What steps will reproduce the problem? (1) On Nocturne or Octopus platforms (there are what have been reported so far) (2) Enable HW & SW WP (flashrom -p host --wp-region WP_RO; flashrom -p host --wp-enable) (3) Disable HW WP (4) Try to flash into RO regions (flashrom -p -w image.bin) What is the expected result? flashrom should write new contents successfully What happens instead? flashrom failed writing contents Related issues: issue 915967 b/120958732 Background: For all old platforms, flashrom used to have the behavior that "if either HWWP or SWWP is disabled, then flashrom can write". This is expected by firmware updater, factory flow (initial imaging & RMA) etc. It allows us to easily "disable HW WP, update RO firmware and enable HW WP again". Without this, people will need to run extra commands like --wp-disable and "--wp-range 0 0", and would easily forget enabling right WP settings again. See issue 190921 for how we explicitly tried to make things transparent. However, on recent platforms, at least Nocturne and Octopus, we've seen flashrom behavior changes - now it will fail in writing when HW WP=0, SW WP=1. Some partners have enabled special workarounds in factory RMA flow due to this, but I think the right solution is to fix flashrom, or we'll introduce the risk of having more devices leaving RMA in WP=off.
,
Jan 4
furquan/dlaurie/reinauer, do you know who implemented SPI flash driver for Nocturne/Octopus? Or did someone just refactored/changed flashrom recently?
,
Jan 7
> do you know who implemented SPI flash driver for Nocturne/Octopus? It is the same generic driver for all Intel platforms. > did someone just refactored/changed flashrom recently? I will have to go back and look at the history. But I am not aware of any recent changes in the WP behavior. BTW, while looking at the issue b/122182142, I identified some problems. Copying comment from that bug: "From a quick scan of flashrom sources, it looks like the hwseq flashchip (which is used by host flashrom) does not have the right callback set. Also, spi_read_status_register seems to be handling the read status call incorrectly for flash->chip->read_status. I have uploaded the following changes (untested, so there might be other issues): http://crrev.com/c/1397904 http://crrev.com/c/1397905 Can you please run the following test on it: 1. Enable HW and SW WP 2. Ensure write to GBB region fails 3. Disable HW WP 4. Ensure wp-range is not set to 0 5. Ensure write to GBB is successful "
,
Jan 7
@yhong, chuntsen, stimim, marcochen, Can you test furquan's changes in c#3 and see if that would solve the flashing related issues on nocturne/octopos?
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/2652d26fcda5ead1d8469b2425ed62db5af94565 commit 2652d26fcda5ead1d8469b2425ed62db5af94565 Author: Furquan Shaikh <furquan@google.com> Date: Wed Jan 16 09:46:47 2019 spi25: Fix spi_read_status_register when using flash chip read_status flash->chip->read_status returns status register value and not an error code. This change updates spi_read_status_register to handle this correctly. BUG=b:122182142 BUG= chromium:919092 Change-Id: I11e3cb5715aa839759ff461c30a8adc162a46e5a Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://chromium-review.googlesource.com/1397904 Commit-Ready: Furquan Shaikh <furquan@chromium.org> Tested-by: Furquan Shaikh <furquan@chromium.org> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Aaron Durbin <adurbin@google.com> [modify] https://crrev.com/2652d26fcda5ead1d8469b2425ed62db5af94565/spi25.c
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/c5bc5cdb8dd9abc5147a7d05dd0918b375b1e47e commit c5bc5cdb8dd9abc5147a7d05dd0918b375b1e47e Author: Furquan Shaikh <furquan@google.com> Date: Wed Jan 16 09:46:48 2019 flashchips: Add unlock operation for hwseq flashchip This change adds spi_disable_blockprotect as the callback for unlock operation which is required to ensure that software write protect is disabled before attempting to write to WP regions when using hwseq flash chip. BUG=b:122182142 BUG= chromium:919092 Change-Id: Ib24f59f86d50e2c7315a607e8229e5c46b49b963 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://chromium-review.googlesource.com/1397905 Commit-Ready: Furquan Shaikh <furquan@chromium.org> Tested-by: Furquan Shaikh <furquan@chromium.org> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Aaron Durbin <adurbin@google.com> [modify] https://crrev.com/c5bc5cdb8dd9abc5147a7d05dd0918b375b1e47e/flashchips.c
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/f556fec1ba8fb4da78eeddf7933dfe45bfb29986 commit f556fec1ba8fb4da78eeddf7933dfe45bfb29986 Author: Furquan Shaikh <furquan@google.com> Date: Wed Jan 16 13:54:55 2019 spi25: Fix spi_read_status_register when using flash chip read_status flash->chip->read_status returns status register value and not an error code. This change updates spi_read_status_register to handle this correctly. BUG=b:122182142 BUG= chromium:919092 Change-Id: I11e3cb5715aa839759ff461c30a8adc162a46e5a Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://chromium-review.googlesource.com/1397904 Commit-Ready: Furquan Shaikh <furquan@chromium.org> Tested-by: Furquan Shaikh <furquan@chromium.org> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Aaron Durbin <adurbin@google.com> (cherry picked from commit 2652d26fcda5ead1d8469b2425ed62db5af94565) Reviewed-on: https://chromium-review.googlesource.com/c/1412203 Reviewed-by: Furquan Shaikh <furquan@chromium.org> Commit-Queue: Hung-Te Lin <hungte@chromium.org> Tested-by: Hung-Te Lin <hungte@chromium.org> Trybot-Ready: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/f556fec1ba8fb4da78eeddf7933dfe45bfb29986/spi25.c
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/89f9b3e0ec27f9c1ae20d41d21aa192f5c9cb1a7 commit 89f9b3e0ec27f9c1ae20d41d21aa192f5c9cb1a7 Author: Furquan Shaikh <furquan@google.com> Date: Wed Jan 16 13:54:58 2019 spi25: Fix spi_read_status_register when using flash chip read_status flash->chip->read_status returns status register value and not an error code. This change updates spi_read_status_register to handle this correctly. BUG=b:122182142 BUG= chromium:919092 Change-Id: I11e3cb5715aa839759ff461c30a8adc162a46e5a Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://chromium-review.googlesource.com/1397904 Commit-Ready: Furquan Shaikh <furquan@chromium.org> Tested-by: Furquan Shaikh <furquan@chromium.org> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Aaron Durbin <adurbin@google.com> (cherry picked from commit 2652d26fcda5ead1d8469b2425ed62db5af94565) Reviewed-on: https://chromium-review.googlesource.com/c/1412205 Reviewed-by: Furquan Shaikh <furquan@chromium.org> Commit-Queue: Hung-Te Lin <hungte@chromium.org> Tested-by: Hung-Te Lin <hungte@chromium.org> Trybot-Ready: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/89f9b3e0ec27f9c1ae20d41d21aa192f5c9cb1a7/spi25.c
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/d2ad5d670027d8be6047638cbb87070539dfd14b commit d2ad5d670027d8be6047638cbb87070539dfd14b Author: Furquan Shaikh <furquan@google.com> Date: Wed Jan 16 13:54:59 2019 spi25: Fix spi_read_status_register when using flash chip read_status flash->chip->read_status returns status register value and not an error code. This change updates spi_read_status_register to handle this correctly. BUG=b:122182142 BUG= chromium:919092 Change-Id: I11e3cb5715aa839759ff461c30a8adc162a46e5a Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://chromium-review.googlesource.com/1397904 Commit-Ready: Furquan Shaikh <furquan@chromium.org> Tested-by: Furquan Shaikh <furquan@chromium.org> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Aaron Durbin <adurbin@google.com> (cherry picked from commit 2652d26fcda5ead1d8469b2425ed62db5af94565) Reviewed-on: https://chromium-review.googlesource.com/c/1412204 Reviewed-by: Furquan Shaikh <furquan@chromium.org> Commit-Queue: Hung-Te Lin <hungte@chromium.org> Tested-by: Hung-Te Lin <hungte@chromium.org> Trybot-Ready: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/d2ad5d670027d8be6047638cbb87070539dfd14b/spi25.c
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/c6d21bdb4899ed7686cc352be53ea5d914f3f5a7 commit c6d21bdb4899ed7686cc352be53ea5d914f3f5a7 Author: Furquan Shaikh <furquan@google.com> Date: Wed Jan 16 13:55:00 2019 spi25: Fix spi_read_status_register when using flash chip read_status flash->chip->read_status returns status register value and not an error code. This change updates spi_read_status_register to handle this correctly. BUG=b:122182142 BUG= chromium:919092 Change-Id: I11e3cb5715aa839759ff461c30a8adc162a46e5a Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://chromium-review.googlesource.com/1397904 Commit-Ready: Furquan Shaikh <furquan@chromium.org> Tested-by: Furquan Shaikh <furquan@chromium.org> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Aaron Durbin <adurbin@google.com> (cherry picked from commit 2652d26fcda5ead1d8469b2425ed62db5af94565) Reviewed-on: https://chromium-review.googlesource.com/c/1412206 Reviewed-by: Furquan Shaikh <furquan@chromium.org> Commit-Queue: Hung-Te Lin <hungte@chromium.org> Tested-by: Hung-Te Lin <hungte@chromium.org> Trybot-Ready: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/c6d21bdb4899ed7686cc352be53ea5d914f3f5a7/spi25.c
,
Jan 16
(6 days ago)
@furquan, what will wp-range be after we do write with the new flashrom? Will it reset wp-range to the value before flashrom was invoked?
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/64eab6f0d42774825cd52a63b6a7d32b1273986c commit 64eab6f0d42774825cd52a63b6a7d32b1273986c Author: Furquan Shaikh <furquan@google.com> Date: Wed Jan 16 14:12:13 2019 flashchips: Add unlock operation for hwseq flashchip This change adds spi_disable_blockprotect as the callback for unlock operation which is required to ensure that software write protect is disabled before attempting to write to WP regions when using hwseq flash chip. BUG=b:122182142 BUG= chromium:919092 Change-Id: Ib24f59f86d50e2c7315a607e8229e5c46b49b963 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://chromium-review.googlesource.com/1397905 Commit-Ready: Furquan Shaikh <furquan@chromium.org> Tested-by: Furquan Shaikh <furquan@chromium.org> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Aaron Durbin <adurbin@google.com> (cherry picked from commit c5bc5cdb8dd9abc5147a7d05dd0918b375b1e47e) Reviewed-on: https://chromium-review.googlesource.com/c/1412210 Reviewed-by: Hung-Te Lin <hungte@chromium.org> Commit-Queue: Hung-Te Lin <hungte@chromium.org> Tested-by: Hung-Te Lin <hungte@chromium.org> Trybot-Ready: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/64eab6f0d42774825cd52a63b6a7d32b1273986c/flashchips.c
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/28d1ea2fddde9f81b25e0a51c8d99f7ae6f2bb50 commit 28d1ea2fddde9f81b25e0a51c8d99f7ae6f2bb50 Author: Furquan Shaikh <furquan@google.com> Date: Wed Jan 16 14:12:16 2019 flashchips: Add unlock operation for hwseq flashchip This change adds spi_disable_blockprotect as the callback for unlock operation which is required to ensure that software write protect is disabled before attempting to write to WP regions when using hwseq flash chip. BUG=b:122182142 BUG= chromium:919092 Change-Id: Ib24f59f86d50e2c7315a607e8229e5c46b49b963 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://chromium-review.googlesource.com/1397905 Commit-Ready: Furquan Shaikh <furquan@chromium.org> Tested-by: Furquan Shaikh <furquan@chromium.org> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Aaron Durbin <adurbin@google.com> (cherry picked from commit c5bc5cdb8dd9abc5147a7d05dd0918b375b1e47e) Reviewed-on: https://chromium-review.googlesource.com/c/1412208 Reviewed-by: Hung-Te Lin <hungte@chromium.org> Commit-Queue: Hung-Te Lin <hungte@chromium.org> Tested-by: Hung-Te Lin <hungte@chromium.org> Trybot-Ready: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/28d1ea2fddde9f81b25e0a51c8d99f7ae6f2bb50/flashchips.c
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/97934fcda31a50cfa333d940af44a9a7d59dc7d8 commit 97934fcda31a50cfa333d940af44a9a7d59dc7d8 Author: Furquan Shaikh <furquan@google.com> Date: Wed Jan 16 14:12:17 2019 flashchips: Add unlock operation for hwseq flashchip This change adds spi_disable_blockprotect as the callback for unlock operation which is required to ensure that software write protect is disabled before attempting to write to WP regions when using hwseq flash chip. BUG=b:122182142 BUG= chromium:919092 Change-Id: Ib24f59f86d50e2c7315a607e8229e5c46b49b963 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://chromium-review.googlesource.com/1397905 Commit-Ready: Furquan Shaikh <furquan@chromium.org> Tested-by: Furquan Shaikh <furquan@chromium.org> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Aaron Durbin <adurbin@google.com> (cherry picked from commit c5bc5cdb8dd9abc5147a7d05dd0918b375b1e47e) Reviewed-on: https://chromium-review.googlesource.com/c/1412209 Reviewed-by: Hung-Te Lin <hungte@chromium.org> Commit-Queue: Hung-Te Lin <hungte@chromium.org> Tested-by: Hung-Te Lin <hungte@chromium.org> Trybot-Ready: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/97934fcda31a50cfa333d940af44a9a7d59dc7d8/flashchips.c
,
Jan 16
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/cb5579dae30101ef632eea0059112bf3066c2f5a commit cb5579dae30101ef632eea0059112bf3066c2f5a Author: Furquan Shaikh <furquan@google.com> Date: Wed Jan 16 14:12:18 2019 flashchips: Add unlock operation for hwseq flashchip This change adds spi_disable_blockprotect as the callback for unlock operation which is required to ensure that software write protect is disabled before attempting to write to WP regions when using hwseq flash chip. BUG=b:122182142 BUG= chromium:919092 Change-Id: Ib24f59f86d50e2c7315a607e8229e5c46b49b963 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://chromium-review.googlesource.com/1397905 Commit-Ready: Furquan Shaikh <furquan@chromium.org> Tested-by: Furquan Shaikh <furquan@chromium.org> Tested-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Karthikeyan Ramasubramanian <kramasub@chromium.org> Reviewed-by: Aaron Durbin <adurbin@google.com> (cherry picked from commit c5bc5cdb8dd9abc5147a7d05dd0918b375b1e47e) Reviewed-on: https://chromium-review.googlesource.com/c/1412207 Reviewed-by: Hung-Te Lin <hungte@chromium.org> Commit-Queue: Hung-Te Lin <hungte@chromium.org> Tested-by: Hung-Te Lin <hungte@chromium.org> Trybot-Ready: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/cb5579dae30101ef632eea0059112bf3066c2f5a/flashchips.c
,
Today
(4 hours ago)
According to reports in few issues, including tomhughes@ offline discussion, those CL really solved the problem. Thanks!
,
Today
(4 hours ago)
Re#11: Sorry I was OOO for a long time and hence the delay in response. > @furquan, what will wp-range be after we do write with the new flashrom? > Will it reset wp-range to the value before flashrom was invoked? Yes, flashrom restores the wp-range after doing the write/erase operation. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by hungte@chromium.org
, Jan 4