Edward O'Callaghan submitted this change.

View Change



2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved Jonathon Hall: Looks good to me, but someone else must approve
programmer.h: Guard against sending spi commands on non-spi mst

Validate (flash->chip->bustype == BUS_SPI) as ich copies the
chip flags in the opaque master and tries incorrectly
to issue 4BA commands which results in failure.

The issue was detected only in the case of chips >16MB, in
this case 'W25Q256FV' that has the feature bits:
```
.feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER_WREN |
FEATURE_4BA_EAR_C5C8 | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ |
FEATURE_WRSR2,
```
The regression was noticed from,
commit 0741727925b841c2479b993204ce58c5eb75185a ichspi.c: Read chip ID and use it to populate `flash->chip`

TEST=In the case of 'W25Q256FV' on TigerLake.

Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/71269
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jonathon Hall <jonathon.hall@puri.sm>
Reviewed-by: Sam McNally <sammc@google.com>
---
M flashrom.c
M include/programmer.h
2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/flashrom.c b/flashrom.c
index 62f38f8..822594b 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1944,7 +1944,7 @@
}

/* Enable/disable 4-byte addressing mode if flash chip supports it */
- if (flash->chip->feature_bits & (FEATURE_4BA_ENTER | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_ENTER_EAR7)) {
+ if (spi_chip_4ba(flash)) {
int ret;
if (spi_master_4ba(flash))
ret = spi_enter_4ba(flash);
diff --git a/include/programmer.h b/include/programmer.h
index 281a4f6..ad3c502 100644
--- a/include/programmer.h
+++ b/include/programmer.h
@@ -513,6 +513,12 @@
return flash->mst->buses_supported & BUS_SPI &&
flash->mst->spi.features & SPI_MASTER_NO_4BA_MODES;
}
+/* spi_chip feature checks */
+static inline bool spi_chip_4ba(const struct flashctx *const flash)
+{
+ return flash->chip->bustype == BUS_SPI &&
+ (flash->chip->feature_bits & (FEATURE_4BA_ENTER | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_ENTER_EAR7));
+}

/* usbdev.c */
struct libusb_device_handle;

To view, visit change 71269. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b
Gerrit-Change-Number: 71269
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall@puri.sm>
Gerrit-Reviewer: Sam McNally <sammc@google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Nikolai Artemiev <nartemiev@google.com>
Gerrit-CC: Subrata Banik <subratabanik@google.com>
Gerrit-MessageType: merged