Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70025 )
Change subject: flashrom.c: Factor out setup_curcontents() from flashrom_image_write()
......................................................................
flashrom.c: Factor out setup_curcontents() from flashrom_image_write()
The CrOS tree factored this functionality into its
own function, seems reasonable.
Change-Id: I4baf09700d7efcbca3a6ba0cfda3231837e0a34a
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M flashrom.c
1 file changed, 54 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/70025/1
diff --git a/flashrom.c b/flashrom.c
index 51a6b64..48ac687 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1800,6 +1800,44 @@
memcpy(newcontents + start, oldcontents + start, copy_len);
}
+static int setup_curcontents(struct flashctx *const flashctx,
+ void *curcontents, const void *const refcontents)
+{
+ const size_t flash_size = flashctx->chip->total_size * 1024;
+ const bool verify_all = flashctx->flags.verify_whole_chip;
+
+ memset(curcontents, UNERASED_VALUE(flashctx), flash_size);
+
+ /* If given, assume flash chip contains same data as `refcontents`. */
+ if (refcontents) {
+ msg_cinfo("Assuming old flash chip contents as ref-file...\n");
+ memcpy(curcontents, refcontents, flash_size);
+ } else {
+ /*
+ * Read the whole chip to be able to check whether regions need to be
+ * erased and to give better diagnostics in case write fails.
+ * The alternative is to read only the regions which are to be
+ * preserved, but in that case we might perform unneeded erase which
+ * takes time as well.
+ */
+ msg_cinfo("Reading old flash chip contents... ");
+ if (verify_all) {
+ if (read_flash(flashctx, curcontents, 0, flash_size)) {
+ msg_cinfo("FAILED.\n");
+ return -1;
+ }
+ } else {
+ if (read_by_layout(flashctx, curcontents)) {
+ msg_cinfo("FAILED.\n");
+ return -1;
+ }
+ }
+ msg_cinfo("done.\n");
+ }
+
+ return 0;
+}
+
int flashrom_image_write(struct flashctx *const flashctx, void *const buffer, const size_t buffer_len,
const void *const refbuffer)
{
@@ -1840,34 +1878,12 @@
if (prepare_flash_access(flashctx, false, true, false, verify))
goto _free_ret;
- /* If given, assume flash chip contains same data as `refcontents`. */
+ if (setup_curcontents(flashctx, curcontents, refcontents) < 0)
+ goto _finalize_ret;
+
if (refcontents) {
- msg_cinfo("Assuming old flash chip contents as ref-file...\n");
- memcpy(curcontents, refcontents, flash_size);
if (oldcontents)
memcpy(oldcontents, refcontents, flash_size);
- } else {
- /*
- * Read the whole chip to be able to check whether regions need to be
- * erased and to give better diagnostics in case write fails.
- * The alternative is to read only the regions which are to be
- * preserved, but in that case we might perform unneeded erase which
- * takes time as well.
- */
- msg_cinfo("Reading old flash chip contents... ");
- if (verify_all) {
- if (read_flash(flashctx, oldcontents, 0, flash_size)) {
- msg_cinfo("FAILED.\n");
- goto _finalize_ret;
- }
- memcpy(curcontents, oldcontents, flash_size);
- } else {
- if (read_by_layout(flashctx, curcontents)) {
- msg_cinfo("FAILED.\n");
- goto _finalize_ret;
- }
- }
- msg_cinfo("done.\n");
}
if (write_by_layout(flashctx, curcontents, newcontents)) {
--
To view, visit https://review.coreboot.org/c/flashrom/+/70025
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4baf09700d7efcbca3a6ba0cfda3231837e0a34a
Gerrit-Change-Number: 70025
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newchange
Felix Singer has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/69988 )
Change subject: libpayload: Fix compiling bugs
......................................................................
Abandoned
See CB:70040. Repushed the patches so that they are shown as cherry-picks. Sorry for the noise.
--
To view, visit https://review.coreboot.org/c/flashrom/+/69988
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.3.x
Gerrit-Change-Id: Iad4435ccf3ff4a665755b353fc2b75ffb51c9cc2
Gerrit-Change-Number: 69988
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: abandon
Felix Singer has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/69987 )
Change subject: ichspi: Fix number of bytes for HW seq operations
......................................................................
Abandoned
See CB:70039. Repushed the patches so that they are shown as cherry-picks. Sorry for the noise.
--
To view, visit https://review.coreboot.org/c/flashrom/+/69987
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.3.x
Gerrit-Change-Id: Id54cb06bc66e86f95ea652ee604715e8969bcb02
Gerrit-Change-Number: 69987
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: abandon
Felix Singer has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/69986 )
Change subject: ichspi: Clear Fast SPI HSFC register before HW seq operation
......................................................................
Abandoned
See CB:69998. Repushed the patches so that they are shown as cherry-picks. Sorry for the noise.
--
To view, visit https://review.coreboot.org/c/flashrom/+/69986
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.3.x
Gerrit-Change-Id: I1b77e563f67ad2793983b65e694be60c85f126c0
Gerrit-Change-Number: 69986
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: abandon
Felix Singer has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/69985 )
Change subject: opaque_master: Mark Opaque chip as tested for WP
......................................................................
Abandoned
See CB:69997. Repushed the patches so that they are shown as cherry-picks. Sorry for the noise.
--
To view, visit https://review.coreboot.org/c/flashrom/+/69985
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.3.x
Gerrit-Change-Id: Iaef2026a9e06a823ab481853b520022ba83b1ca1
Gerrit-Change-Number: 69985
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
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-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: abandon
Felix Singer has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/69984 )
Change subject: linux_mtd: Mark Opaque chip as tested for WP
......................................................................
Abandoned
See CB:69996. Repushed the patches so that they are shown as cherry-picks. Sorry for the noise.
--
To view, visit https://review.coreboot.org/c/flashrom/+/69984
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.3.x
Gerrit-Change-Id: I2740e75c974538ddae9f5b17dc4fba9a3101cbb3
Gerrit-Change-Number: 69984
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: abandon
Attention is currently required from: Edward O'Callaghan, Arthur Heymans, Anastasia Klimchuk.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69983 )
Change subject: cli_classic.c: Be consistent with pointer types
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
See CB:69995. Repushed the patches so that they are shown as cherry-picks. Sorry for the noise.
--
To view, visit https://review.coreboot.org/c/flashrom/+/69983
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.3.x
Gerrit-Change-Id: I05dbda3923f1cd262bdad62e58f9c0ae7d7ffe6f
Gerrit-Change-Number: 69983
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sat, 26 Nov 2022 16:39:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70039 )
Change subject: ichspi: Fix number of bytes for HW seq operations
......................................................................
ichspi: Fix number of bytes for HW seq operations
This patch fixes a potential issue where the SPI controller register
HSFC.FDBC (bits 24-29) value gets incorrectly calculated while passing
the `len` as `0` instead of `1`.
As per Intel EDS, `0b` in the FDBC represents 1 byte while `0x3f`
represents 64-bytes to be transferred. The number of bytes
transferred is the value of this field plus 1.
If we would like to transfer 1 byte then we need to set `0b` in
FDBC for operations like read, write, flash id as to account for
the `set byte count` hence, the `len` argument should be `1`.
Additionally, as per EDS, the FDBC field is ignored for any block
erase command.
BUG=b:258280679
TEST=Able to build flashrom and perform below operations on Google,
Rex and Google, Kano/Taeko.
During `--wp-disable` HW seq operation that requires 1 byte data
transfer.
HSFC.FDBC value while passing `len` as `0` = 0x3f (represents 64-byte)
HSFC.FDBC value while passing `len` as `1` = 0x0 (represents 1-byte)
Original-Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/69789
Original-Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
Original-Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
Original-Reviewed-by: Nikolai Artemiev <nartemiev(a)google.com>
Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Change-Id: I5b911655649c693e576497520687d7810bbd3c54
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
---
M ichspi.c
1 file changed, 52 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/70039/1
diff --git a/ichspi.c b/ichspi.c
index 29dcd25..dd5a2c1 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1360,6 +1360,12 @@
/* Set up transaction parameters. */
hsfc |= hsfc_cycle;
+ /*
+ * The number of bytes transferred is the value of `FDBC` plus 1, hence,
+ * subtracted 1 from the length field.
+ * As per Intel EDS, `0b` in the FDBC represents 1 byte while `0x3f`
+ * represents 64-bytes to be transferred.
+ */
hsfc |= HSFC_FDBC_VAL(len - 1);
hsfc |= HSFC_FGO; /* start */
prettyprint_ich9_reg_hsfc(hsfc, ich_generation);
@@ -1399,7 +1405,7 @@
}
msg_pdbg("Reading Status register\n");
- if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_RD_STATUS, 0, len, ich_generation,
+ if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_RD_STATUS, 1, len, ich_generation,
hwseq_data->addr_mask)) {
msg_perr("Reading Status register failed\n!!");
return -1;
@@ -1422,7 +1428,7 @@
ich_fill_data(&value, len, ICH9_REG_FDATA0);
- if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_WR_STATUS, 0, len, ich_generation,
+ if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_WR_STATUS, 1, len, ich_generation,
hwseq_data->addr_mask)) {
msg_perr("Writing Status register failed\n!!");
return -1;
@@ -1518,7 +1524,7 @@
msg_pdbg("Erasing %d bytes starting at 0x%06x.\n", len, addr);
- if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_BLOCK_ERASE, addr, 0, ich_generation,
+ if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_BLOCK_ERASE, addr, 1, ich_generation,
hwseq_data->addr_mask))
return -1;
return 0;
--
To view, visit https://review.coreboot.org/c/flashrom/+/70039
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.3.x
Gerrit-Change-Id: I5b911655649c693e576497520687d7810bbd3c54
Gerrit-Change-Number: 70039
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: newchange