- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: ichspi: Extract initialisation of swseq and hwseq into a function
by Nico Huber (Code Review) 02 Feb '22

02 Feb '22

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58737 ) Change subject: ichspi: Extract initialisation of swseq and hwseq into a function ...................................................................... ichspi: Extract initialisation of swseq and hwseq into a function Initialisation of swseq_data and hwseq_data gets its own function, which is called from init_ich_default. This makes init_ich_default more readable. This patch also gives a name to (previously anonymous) struct swseq_data. Its sibling struct hwseq_data already has a name. Structs need names to be able to declare function parameters. BUG=b:204488958 TEST=Check that the following scenarios still behave properly: 1) probe-read-verify-erase section-write-reboot on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E 2) probe and read on Panther Point (7 series PCH) Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3 Tested-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Nico Huber <nico.h(a)gmx.de> Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/58737 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M ichspi.c 1 file changed, 57 insertions(+), 50 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/ichspi.c b/ichspi.c index 5d18b37..c4df20c 100644 --- a/ichspi.c +++ b/ichspi.c @@ -507,7 +507,7 @@ pprint_reg(DLOCK, SSEQ_LOCKDN, reg_val, "\n"); } -static struct { +static struct swseq_data { size_t reg_ssfsc; size_t reg_preop; size_t reg_optype; @@ -1784,6 +1784,61 @@ return 0; } +static void init_chipset_properties(struct swseq_data *swseq, struct hwseq_data *hwseq, + size_t *num_freg, size_t *num_pr, size_t *reg_pr0, + enum ich_chipset ich_gen) +{ + /* Moving registers / bits */ + switch (ich_gen) { + case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: + case CHIPSET_400_SERIES_COMET_POINT: + case CHIPSET_500_SERIES_TIGER_POINT: + case CHIPSET_APOLLO_LAKE: + case CHIPSET_GEMINI_LAKE: + *num_pr = 6; /* Includes GPR0 */ + *reg_pr0 = PCH100_REG_FPR0; + swseq->reg_ssfsc = PCH100_REG_SSFSC; + swseq->reg_preop = PCH100_REG_PREOP; + swseq->reg_optype = PCH100_REG_OPTYPE; + swseq->reg_opmenu = PCH100_REG_OPMENU; + hwseq->addr_mask = PCH100_FADDR_FLA; + hwseq->only_4k = true; + hwseq->hsfc_fcycle = PCH100_HSFC_FCYCLE; + break; + default: + *num_pr = 5; + *reg_pr0 = ICH9_REG_PR0; + swseq->reg_ssfsc = ICH9_REG_SSFS; + swseq->reg_preop = ICH9_REG_PREOP; + swseq->reg_optype = ICH9_REG_OPTYPE; + swseq->reg_opmenu = ICH9_REG_OPMENU; + hwseq->addr_mask = ICH9_FADDR_FLA; + hwseq->only_4k = false; + hwseq->hsfc_fcycle = HSFC_FCYCLE; + break; + } + + switch (ich_gen) { + case CHIPSET_100_SERIES_SUNRISE_POINT: + *num_freg = 10; + break; + case CHIPSET_C620_SERIES_LEWISBURG: + *num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */ + break; + case CHIPSET_300_SERIES_CANNON_POINT: + case CHIPSET_400_SERIES_COMET_POINT: + case CHIPSET_500_SERIES_TIGER_POINT: + case CHIPSET_APOLLO_LAKE: + case CHIPSET_GEMINI_LAKE: + *num_freg = 16; + break; + default: + *num_freg = 5; + break; + } +} static int init_ich_default(void *spibar, enum ich_chipset ich_gen) { @@ -1796,55 +1851,7 @@ enum ich_spi_mode ich_spi_mode = ich_auto; size_t num_freg, num_pr, reg_pr0; - /* Moving registers / bits */ - switch (ich_gen) { - case CHIPSET_100_SERIES_SUNRISE_POINT: - case CHIPSET_C620_SERIES_LEWISBURG: - case CHIPSET_300_SERIES_CANNON_POINT: - case CHIPSET_400_SERIES_COMET_POINT: - case CHIPSET_500_SERIES_TIGER_POINT: - case CHIPSET_APOLLO_LAKE: - case CHIPSET_GEMINI_LAKE: - num_pr = 6; /* Includes GPR0 */ - reg_pr0 = PCH100_REG_FPR0; - swseq_data.reg_ssfsc = PCH100_REG_SSFSC; - swseq_data.reg_preop = PCH100_REG_PREOP; - swseq_data.reg_optype = PCH100_REG_OPTYPE; - swseq_data.reg_opmenu = PCH100_REG_OPMENU; - hwseq_data.addr_mask = PCH100_FADDR_FLA; - hwseq_data.only_4k = true; - hwseq_data.hsfc_fcycle = PCH100_HSFC_FCYCLE; - break; - default: - num_pr = 5; - reg_pr0 = ICH9_REG_PR0; - swseq_data.reg_ssfsc = ICH9_REG_SSFS; - swseq_data.reg_preop = ICH9_REG_PREOP; - swseq_data.reg_optype = ICH9_REG_OPTYPE; - swseq_data.reg_opmenu = ICH9_REG_OPMENU; - hwseq_data.addr_mask = ICH9_FADDR_FLA; - hwseq_data.only_4k = false; - hwseq_data.hsfc_fcycle = HSFC_FCYCLE; - break; - } - switch (ich_gen) { - case CHIPSET_100_SERIES_SUNRISE_POINT: - num_freg = 10; - break; - case CHIPSET_C620_SERIES_LEWISBURG: - num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */ - break; - case CHIPSET_300_SERIES_CANNON_POINT: - case CHIPSET_400_SERIES_COMET_POINT: - case CHIPSET_500_SERIES_TIGER_POINT: - case CHIPSET_APOLLO_LAKE: - case CHIPSET_GEMINI_LAKE: - num_freg = 16; - break; - default: - num_freg = 5; - break; - } + init_chipset_properties(&swseq_data, &hwseq_data, &num_freg, &num_pr, &reg_pr0, ich_gen); int ret = get_ich_spi_mode_param(&ich_spi_mode); if (ret) -- To view, visit https://review.coreboot.org/c/flashrom/+/58737 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3 Gerrit-Change-Number: 58737 Gerrit-PatchSet: 11 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ichspi: Extract handling programmer param into a function
by Nico Huber (Code Review) 02 Feb '22

02 Feb '22

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58736 ) Change subject: ichspi: Extract handling programmer param into a function ...................................................................... ichspi: Extract handling programmer param into a function Extract processing of ich_spi_mode into a separate function which is called from init_ich_default. This makes init_ich_default more readable and avoids one local variable. BUG=b:204488958 TEST=Check that the following scenarios still behave properly: 1) probe-read-verify-erase section-write-reboot on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E 2) probe and read on Panther Point (7 series PCH) Change-Id: I20e2379a6fd58c9346f0a2d6daf2b8decf1f6976 Tested-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Nico Huber <nico.h(a)gmx.de> Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/58736 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M ichspi.c 1 file changed, 37 insertions(+), 27 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/ichspi.c b/ichspi.c index 09a0755..5d18b37 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1752,20 +1752,48 @@ return 0; } +enum ich_spi_mode { + ich_auto, + ich_hwseq, + ich_swseq +}; + +static int get_ich_spi_mode_param(enum ich_spi_mode *ich_spi_mode) +{ + char *const arg = extract_programmer_param("ich_spi_mode"); + if (arg && !strcmp(arg, "hwseq")) { + *ich_spi_mode = ich_hwseq; + msg_pspew("user selected hwseq\n"); + } else if (arg && !strcmp(arg, "swseq")) { + *ich_spi_mode = ich_swseq; + msg_pspew("user selected swseq\n"); + } else if (arg && !strcmp(arg, "auto")) { + msg_pspew("user selected auto\n"); + *ich_spi_mode = ich_auto; + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for ich_spi_mode.\n"); + free(arg); + return ERROR_FATAL; + } else if (arg) { + msg_perr("Unknown argument for ich_spi_mode: %s\n", arg); + free(arg); + return ERROR_FATAL; + } + free(arg); + + return 0; +} + + static int init_ich_default(void *spibar, enum ich_chipset ich_gen) { unsigned int i; uint16_t tmp2; uint32_t tmp; - char *arg; int ich_spi_rw_restricted = 0; int desc_valid = 0; struct ich_descriptors desc = { 0 }; - enum ich_spi_mode { - ich_auto, - ich_hwseq, - ich_swseq - } ich_spi_mode = ich_auto; + enum ich_spi_mode ich_spi_mode = ich_auto; size_t num_freg, num_pr, reg_pr0; /* Moving registers / bits */ @@ -1818,27 +1846,9 @@ break; } - arg = extract_programmer_param("ich_spi_mode"); - if (arg && !strcmp(arg, "hwseq")) { - ich_spi_mode = ich_hwseq; - msg_pspew("user selected hwseq\n"); - } else if (arg && !strcmp(arg, "swseq")) { - ich_spi_mode = ich_swseq; - msg_pspew("user selected swseq\n"); - } else if (arg && !strcmp(arg, "auto")) { - msg_pspew("user selected auto\n"); - ich_spi_mode = ich_auto; - } else if (arg && !strlen(arg)) { - msg_perr("Missing argument for ich_spi_mode.\n"); - free(arg); - return ERROR_FATAL; - } else if (arg) { - msg_perr("Unknown argument for ich_spi_mode: %s\n", - arg); - free(arg); - return ERROR_FATAL; - } - free(arg); + int ret = get_ich_spi_mode_param(&ich_spi_mode); + if (ret) + return ret; tmp2 = mmio_readw(spibar + ICH9_REG_HSFS); msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2); 7 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/58736 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I20e2379a6fd58c9346f0a2d6daf2b8decf1f6976 Gerrit-Change-Number: 58736 Gerrit-PatchSet: 11 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Nico Huber (Code Review) 02 Feb '22

02 Feb '22

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... ichspi: Split very long init function into two ich_init_spi is very long, but logically it can be split. Init function detects the chipset and then the rest of operations depends on the chipset. Init function is more readable now, it consists of only a switch. Initialisation of hwseq and swseq that used to happen in the beginning of init function now moved to init_ich_default, because hwseq and swseq are only used for chipsets served by init_ich_default. BUG=b:204488958 TEST=Check that the following scenarios still behave properly: 1) probe-read-verify-erase section-write-reboot on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E 2) probe and read on Panther Point (7 series PCH) 3) on machine with ich7 chipset, output from probe and read is the same between master and this patch Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Nico Huber <nico.h(a)gmx.de> Tested-by: Nicholas Chin <nic.c3.14(a)gmail.com> Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/58735 Reviewed-by: Nico Huber <nico.h(a)gmx.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M ichspi.c 1 file changed, 271 insertions(+), 258 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/ichspi.c b/ichspi.c index 47b23eb..09a0755 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1713,7 +1713,46 @@ .erase = ich_hwseq_block_erase, }; -int ich_init_spi(void *spibar, enum ich_chipset ich_gen) +static int init_ich7_spi(void *spibar, enum ich_chipset ich_gen) +{ + unsigned int i; + + msg_pdbg("0x00: 0x%04x (SPIS)\n", + mmio_readw(spibar + 0)); + msg_pdbg("0x02: 0x%04x (SPIC)\n", + mmio_readw(spibar + 2)); + msg_pdbg("0x04: 0x%08x (SPIA)\n", + mmio_readl(spibar + 4)); + ichspi_bbar = mmio_readl(spibar + 0x50); + msg_pdbg("0x50: 0x%08x (BBAR)\n", + ichspi_bbar); + msg_pdbg("0x54: 0x%04x (PREOP)\n", + mmio_readw(spibar + 0x54)); + msg_pdbg("0x56: 0x%04x (OPTYPE)\n", + mmio_readw(spibar + 0x56)); + msg_pdbg("0x58: 0x%08x (OPMENU)\n", + mmio_readl(spibar + 0x58)); + msg_pdbg("0x5c: 0x%08x (OPMENU+4)\n", + mmio_readl(spibar + 0x5c)); + + for (i = 0; i < 3; i++) { + int offs; + offs = 0x60 + (i * 4); + msg_pdbg("0x%02x: 0x%08x (PBR%u)\n", offs, + mmio_readl(spibar + offs), i); + } + if (mmio_readw(spibar) & (1 << 15)) { + msg_pwarn("WARNING: SPI Configuration Lockdown activated.\n"); + ichspi_lock = 1; + } + ich_init_opcodes(ich_gen); + ich_set_bbar(0, ich_gen); + register_spi_master(&spi_master_ich7, NULL); + + return 0; +} + +static int init_ich_default(void *spibar, enum ich_chipset ich_gen) { unsigned int i; uint16_t tmp2; @@ -1729,9 +1768,6 @@ } ich_spi_mode = ich_auto; size_t num_freg, num_pr, reg_pr0; - ich_generation = ich_gen; - ich_spibar = spibar; - /* Moving registers / bits */ switch (ich_gen) { case CHIPSET_100_SERIES_SUNRISE_POINT: @@ -1782,90 +1818,130 @@ break; } - switch (ich_gen) { - case CHIPSET_ICH7: - case CHIPSET_TUNNEL_CREEK: - case CHIPSET_CENTERTON: - msg_pdbg("0x00: 0x%04x (SPIS)\n", - mmio_readw(spibar + 0)); - msg_pdbg("0x02: 0x%04x (SPIC)\n", - mmio_readw(spibar + 2)); - msg_pdbg("0x04: 0x%08x (SPIA)\n", - mmio_readl(spibar + 4)); - ichspi_bbar = mmio_readl(spibar + 0x50); - msg_pdbg("0x50: 0x%08x (BBAR)\n", - ichspi_bbar); - msg_pdbg("0x54: 0x%04x (PREOP)\n", - mmio_readw(spibar + 0x54)); - msg_pdbg("0x56: 0x%04x (OPTYPE)\n", - mmio_readw(spibar + 0x56)); - msg_pdbg("0x58: 0x%08x (OPMENU)\n", - mmio_readl(spibar + 0x58)); - msg_pdbg("0x5c: 0x%08x (OPMENU+4)\n", - mmio_readl(spibar + 0x5c)); - for (i = 0; i < 3; i++) { - int offs; - offs = 0x60 + (i * 4); - msg_pdbg("0x%02x: 0x%08x (PBR%u)\n", offs, - mmio_readl(spibar + offs), i); - } - if (mmio_readw(spibar) & (1 << 15)) { - msg_pwarn("WARNING: SPI Configuration Lockdown activated.\n"); - ichspi_lock = 1; - } - ich_init_opcodes(ich_gen); - ich_set_bbar(0, ich_gen); - register_spi_master(&spi_master_ich7, NULL); - break; - case CHIPSET_ICH8: - default: /* Future version might behave the same */ - arg = extract_programmer_param("ich_spi_mode"); - if (arg && !strcmp(arg, "hwseq")) { - ich_spi_mode = ich_hwseq; - msg_pspew("user selected hwseq\n"); - } else if (arg && !strcmp(arg, "swseq")) { - ich_spi_mode = ich_swseq; - msg_pspew("user selected swseq\n"); - } else if (arg && !strcmp(arg, "auto")) { - msg_pspew("user selected auto\n"); - ich_spi_mode = ich_auto; - } else if (arg && !strlen(arg)) { - msg_perr("Missing argument for ich_spi_mode.\n"); - free(arg); - return ERROR_FATAL; - } else if (arg) { - msg_perr("Unknown argument for ich_spi_mode: %s\n", - arg); - free(arg); - return ERROR_FATAL; - } + arg = extract_programmer_param("ich_spi_mode"); + if (arg && !strcmp(arg, "hwseq")) { + ich_spi_mode = ich_hwseq; + msg_pspew("user selected hwseq\n"); + } else if (arg && !strcmp(arg, "swseq")) { + ich_spi_mode = ich_swseq; + msg_pspew("user selected swseq\n"); + } else if (arg && !strcmp(arg, "auto")) { + msg_pspew("user selected auto\n"); + ich_spi_mode = ich_auto; + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for ich_spi_mode.\n"); free(arg); + return ERROR_FATAL; + } else if (arg) { + msg_perr("Unknown argument for ich_spi_mode: %s\n", + arg); + free(arg); + return ERROR_FATAL; + } + free(arg); - tmp2 = mmio_readw(spibar + ICH9_REG_HSFS); - msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2); - prettyprint_ich9_reg_hsfs(tmp2, ich_gen); - if (tmp2 & HSFS_FLOCKDN) { - msg_pinfo("SPI Configuration is locked down.\n"); - ichspi_lock = 1; - } - if (tmp2 & HSFS_FDV) - desc_valid = 1; - if (!(tmp2 & HSFS_FDOPSS) && desc_valid) - msg_pinfo("The Flash Descriptor Override Strap-Pin is set. Restrictions implied by\n" - "the Master Section of the flash descriptor are NOT in effect. Please note\n" - "that Protected Range (PR) restrictions still apply.\n"); - ich_init_opcodes(ich_gen); + tmp2 = mmio_readw(spibar + ICH9_REG_HSFS); + msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2); + prettyprint_ich9_reg_hsfs(tmp2, ich_gen); + if (tmp2 & HSFS_FLOCKDN) { + msg_pinfo("SPI Configuration is locked down.\n"); + ichspi_lock = 1; + } + if (tmp2 & HSFS_FDV) + desc_valid = 1; + if (!(tmp2 & HSFS_FDOPSS) && desc_valid) + msg_pinfo("The Flash Descriptor Override Strap-Pin is set. Restrictions implied by\n" + "the Master Section of the flash descriptor are NOT in effect. Please note\n" + "that Protected Range (PR) restrictions still apply.\n"); + ich_init_opcodes(ich_gen); - if (desc_valid) { - tmp2 = mmio_readw(spibar + ICH9_REG_HSFC); - msg_pdbg("0x06: 0x%04x (HSFC)\n", tmp2); - prettyprint_ich9_reg_hsfc(tmp2, ich_gen); - } + if (desc_valid) { + tmp2 = mmio_readw(spibar + ICH9_REG_HSFC); + msg_pdbg("0x06: 0x%04x (HSFC)\n", tmp2); + prettyprint_ich9_reg_hsfc(tmp2, ich_gen); + } - tmp = mmio_readl(spibar + ICH9_REG_FADDR); - msg_pdbg2("0x08: 0x%08x (FADDR)\n", tmp); + tmp = mmio_readl(spibar + ICH9_REG_FADDR); + msg_pdbg2("0x08: 0x%08x (FADDR)\n", tmp); + switch (ich_gen) { + case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: + case CHIPSET_400_SERIES_COMET_POINT: + case CHIPSET_500_SERIES_TIGER_POINT: + case CHIPSET_APOLLO_LAKE: + case CHIPSET_GEMINI_LAKE: + tmp = mmio_readl(spibar + PCH100_REG_DLOCK); + msg_pdbg("0x0c: 0x%08x (DLOCK)\n", tmp); + prettyprint_pch100_reg_dlock(tmp); + break; + default: + break; + } + + if (desc_valid) { + tmp = mmio_readl(spibar + ICH9_REG_FRAP); + msg_pdbg("0x50: 0x%08x (FRAP)\n", tmp); + msg_pdbg("BMWAG 0x%02x, ", ICH_BMWAG(tmp)); + msg_pdbg("BMRAG 0x%02x, ", ICH_BMRAG(tmp)); + msg_pdbg("BRWA 0x%02x, ", ICH_BRWA(tmp)); + msg_pdbg("BRRA 0x%02x\n", ICH_BRRA(tmp)); + + /* Handle FREGx and FRAP registers */ + for (i = 0; i < num_freg; i++) + ich_spi_rw_restricted |= ich9_handle_frap(tmp, i); + if (ich_spi_rw_restricted) + msg_pinfo("Not all flash regions are freely accessible by flashrom. This is " + "most likely\ndue to an active ME. Please see " + "https://flashrom.org/ME for details.\n"); + } + + /* Handle PR registers */ + for (i = 0; i < num_pr; i++) { + /* if not locked down try to disable PR locks first */ + if (!ichspi_lock) + ich9_set_pr(reg_pr0, i, 0, 0); + ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i); + } + + switch (ich_spi_rw_restricted) { + case WRITE_PROT: + msg_pwarn("At least some flash regions are write protected. For write operations,\n" + "you should use a flash layout and include only writable regions. See\n" + "manpage for more details.\n"); + break; + case READ_PROT: + case LOCKED: + msg_pwarn("At least some flash regions are read protected. You have to use a flash\n" + "layout and include only accessible regions. For write operations, you'll\n" + "additionally need the --noverify-all switch. See manpage for more details.\n" + ); + break; + } + + tmp = mmio_readl(spibar + swseq_data.reg_ssfsc); + msg_pdbg("0x%zx: 0x%02x (SSFS)\n", swseq_data.reg_ssfsc, tmp & 0xff); + prettyprint_ich9_reg_ssfs(tmp); + if (tmp & SSFS_FCERR) { + msg_pdbg("Clearing SSFS.FCERR\n"); + mmio_writeb(SSFS_FCERR, spibar + swseq_data.reg_ssfsc); + } + msg_pdbg("0x%zx: 0x%06x (SSFC)\n", swseq_data.reg_ssfsc + 1, tmp >> 8); + prettyprint_ich9_reg_ssfc(tmp); + + msg_pdbg("0x%zx: 0x%04x (PREOP)\n", + swseq_data.reg_preop, mmio_readw(spibar + swseq_data.reg_preop)); + msg_pdbg("0x%zx: 0x%04x (OPTYPE)\n", + swseq_data.reg_optype, mmio_readw(spibar + swseq_data.reg_optype)); + msg_pdbg("0x%zx: 0x%08x (OPMENU)\n", + swseq_data.reg_opmenu, mmio_readl(spibar + swseq_data.reg_opmenu)); + msg_pdbg("0x%zx: 0x%08x (OPMENU+4)\n", + swseq_data.reg_opmenu + 4, mmio_readl(spibar + swseq_data.reg_opmenu + 4)); + + if (desc_valid) { switch (ich_gen) { + case CHIPSET_ICH8: case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: case CHIPSET_300_SERIES_CANNON_POINT: @@ -1873,194 +1949,131 @@ case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: - tmp = mmio_readl(spibar + PCH100_REG_DLOCK); - msg_pdbg("0x0c: 0x%08x (DLOCK)\n", tmp); - prettyprint_pch100_reg_dlock(tmp); + case CHIPSET_BAYTRAIL: break; default: + ichspi_bbar = mmio_readl(spibar + ICH9_REG_BBAR); + msg_pdbg("0x%x: 0x%08x (BBAR)\n", ICH9_REG_BBAR, ichspi_bbar); + ich_set_bbar(0, ich_gen); break; } - if (desc_valid) { - tmp = mmio_readl(spibar + ICH9_REG_FRAP); - msg_pdbg("0x50: 0x%08x (FRAP)\n", tmp); - msg_pdbg("BMWAG 0x%02x, ", ICH_BMWAG(tmp)); - msg_pdbg("BMRAG 0x%02x, ", ICH_BMRAG(tmp)); - msg_pdbg("BRWA 0x%02x, ", ICH_BRWA(tmp)); - msg_pdbg("BRRA 0x%02x\n", ICH_BRRA(tmp)); - - /* Handle FREGx and FRAP registers */ - for (i = 0; i < num_freg; i++) - ich_spi_rw_restricted |= ich9_handle_frap(tmp, i); - if (ich_spi_rw_restricted) - msg_pinfo("Not all flash regions are freely accessible by flashrom. This is " - "most likely\ndue to an active ME. Please see " - "https://flashrom.org/ME for details.\n"); - } - - /* Handle PR registers */ - for (i = 0; i < num_pr; i++) { - /* if not locked down try to disable PR locks first */ - if (!ichspi_lock) - ich9_set_pr(reg_pr0, i, 0, 0); - ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i); - } - - switch (ich_spi_rw_restricted) { - case WRITE_PROT: - msg_pwarn("At least some flash regions are write protected. For write operations,\n" - "you should use a flash layout and include only writable regions. See\n" - "manpage for more details.\n"); - break; - case READ_PROT: - case LOCKED: - msg_pwarn("At least some flash regions are read protected. You have to use a flash\n" - "layout and include only accessible regions. For write operations, you'll\n" - "additionally need the --noverify-all switch. See manpage for more details.\n" - ); - break; - } - - tmp = mmio_readl(spibar + swseq_data.reg_ssfsc); - msg_pdbg("0x%zx: 0x%02x (SSFS)\n", swseq_data.reg_ssfsc, tmp & 0xff); - prettyprint_ich9_reg_ssfs(tmp); - if (tmp & SSFS_FCERR) { - msg_pdbg("Clearing SSFS.FCERR\n"); - mmio_writeb(SSFS_FCERR, spibar + swseq_data.reg_ssfsc); - } - msg_pdbg("0x%zx: 0x%06x (SSFC)\n", swseq_data.reg_ssfsc + 1, tmp >> 8); - prettyprint_ich9_reg_ssfc(tmp); - - msg_pdbg("0x%zx: 0x%04x (PREOP)\n", - swseq_data.reg_preop, mmio_readw(spibar + swseq_data.reg_preop)); - msg_pdbg("0x%zx: 0x%04x (OPTYPE)\n", - swseq_data.reg_optype, mmio_readw(spibar + swseq_data.reg_optype)); - msg_pdbg("0x%zx: 0x%08x (OPMENU)\n", - swseq_data.reg_opmenu, mmio_readl(spibar + swseq_data.reg_opmenu)); - msg_pdbg("0x%zx: 0x%08x (OPMENU+4)\n", - swseq_data.reg_opmenu + 4, mmio_readl(spibar + swseq_data.reg_opmenu + 4)); - - if (desc_valid) { - switch (ich_gen) { - case CHIPSET_ICH8: - case CHIPSET_100_SERIES_SUNRISE_POINT: - case CHIPSET_C620_SERIES_LEWISBURG: - case CHIPSET_300_SERIES_CANNON_POINT: - case CHIPSET_400_SERIES_COMET_POINT: - case CHIPSET_500_SERIES_TIGER_POINT: - case CHIPSET_APOLLO_LAKE: - case CHIPSET_GEMINI_LAKE: - case CHIPSET_BAYTRAIL: - break; - default: - ichspi_bbar = mmio_readl(spibar + ICH9_REG_BBAR); - msg_pdbg("0x%x: 0x%08x (BBAR)\n", ICH9_REG_BBAR, ichspi_bbar); - ich_set_bbar(0, ich_gen); - break; - } - - if (ich_gen == CHIPSET_ICH8) { - tmp = mmio_readl(spibar + ICH8_REG_VSCC); - msg_pdbg("0x%x: 0x%08x (VSCC)\n", ICH8_REG_VSCC, tmp); - msg_pdbg("VSCC: "); - prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); - } else { - tmp = mmio_readl(spibar + ICH9_REG_LVSCC); - msg_pdbg("0x%x: 0x%08x (LVSCC)\n", ICH9_REG_LVSCC, tmp); - msg_pdbg("LVSCC: "); - prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); - - tmp = mmio_readl(spibar + ICH9_REG_UVSCC); - msg_pdbg("0x%x: 0x%08x (UVSCC)\n", ICH9_REG_UVSCC, tmp); - msg_pdbg("UVSCC: "); - prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, false); - } - - switch (ich_gen) { - case CHIPSET_ICH8: - case CHIPSET_100_SERIES_SUNRISE_POINT: - case CHIPSET_C620_SERIES_LEWISBURG: - case CHIPSET_300_SERIES_CANNON_POINT: - case CHIPSET_400_SERIES_COMET_POINT: - case CHIPSET_500_SERIES_TIGER_POINT: - case CHIPSET_APOLLO_LAKE: - case CHIPSET_GEMINI_LAKE: - break; - default: - tmp = mmio_readl(spibar + ICH9_REG_FPB); - msg_pdbg("0x%x: 0x%08x (FPB)\n", ICH9_REG_FPB, tmp); - break; - } - - if (read_ich_descriptors_via_fdo(ich_gen, spibar, &desc) == ICH_RET_OK) - prettyprint_ich_descriptors(ich_gen, &desc); - - /* If the descriptor is valid and indicates multiple - * flash devices we need to use hwseq to be able to - * access the second flash device. - */ - if (ich_spi_mode == ich_auto && desc.content.NC != 0) { - msg_pinfo("Enabling hardware sequencing due to " - "multiple flash chips detected.\n"); - ich_spi_mode = ich_hwseq; - } - } - - if (ich_spi_mode == ich_auto && ichspi_lock && - ich_missing_opcodes()) { - msg_pinfo("Enabling hardware sequencing because " - "some important opcode is locked.\n"); - ich_spi_mode = ich_hwseq; - } - - if (ich_spi_mode == ich_auto && - (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT || - ich_gen == CHIPSET_300_SERIES_CANNON_POINT || - ich_gen == CHIPSET_400_SERIES_COMET_POINT || - ich_gen == CHIPSET_500_SERIES_TIGER_POINT)) { - msg_pdbg("Enabling hardware sequencing by default for 100+ series PCH.\n"); - ich_spi_mode = ich_hwseq; - } - - if (ich_spi_mode == ich_auto && - (ich_gen == CHIPSET_APOLLO_LAKE || - ich_gen == CHIPSET_GEMINI_LAKE)) { - msg_pdbg("Enabling hardware sequencing by default for Apollo/Gemini Lake.\n"); - ich_spi_mode = ich_hwseq; - } - - if (ich_spi_mode == ich_hwseq) { - if (!desc_valid) { - msg_perr("Hardware sequencing was requested " - "but the flash descriptor is not " - "valid. Aborting.\n"); - return ERROR_FATAL; - } - - int tmpi = getFCBA_component_density(ich_gen, &desc, 0); - if (tmpi < 0) { - msg_perr("Could not determine density of flash component %d.\n", 0); - return ERROR_FATAL; - } - hwseq_data.size_comp0 = tmpi; - - tmpi = getFCBA_component_density(ich_gen, &desc, 1); - if (tmpi < 0) { - msg_perr("Could not determine density of flash component %d.\n", 1); - return ERROR_FATAL; - } - hwseq_data.size_comp1 = tmpi; - - register_opaque_master(&opaque_master_ich_hwseq, NULL); + if (ich_gen == CHIPSET_ICH8) { + tmp = mmio_readl(spibar + ICH8_REG_VSCC); + msg_pdbg("0x%x: 0x%08x (VSCC)\n", ICH8_REG_VSCC, tmp); + msg_pdbg("VSCC: "); + prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); } else { - register_spi_master(&spi_master_ich9, NULL); + tmp = mmio_readl(spibar + ICH9_REG_LVSCC); + msg_pdbg("0x%x: 0x%08x (LVSCC)\n", ICH9_REG_LVSCC, tmp); + msg_pdbg("LVSCC: "); + prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); + + tmp = mmio_readl(spibar + ICH9_REG_UVSCC); + msg_pdbg("0x%x: 0x%08x (UVSCC)\n", ICH9_REG_UVSCC, tmp); + msg_pdbg("UVSCC: "); + prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, false); } - break; + + switch (ich_gen) { + case CHIPSET_ICH8: + case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: + case CHIPSET_400_SERIES_COMET_POINT: + case CHIPSET_500_SERIES_TIGER_POINT: + case CHIPSET_APOLLO_LAKE: + case CHIPSET_GEMINI_LAKE: + break; + default: + tmp = mmio_readl(spibar + ICH9_REG_FPB); + msg_pdbg("0x%x: 0x%08x (FPB)\n", ICH9_REG_FPB, tmp); + break; + } + + if (read_ich_descriptors_via_fdo(ich_gen, spibar, &desc) == ICH_RET_OK) + prettyprint_ich_descriptors(ich_gen, &desc); + + /* If the descriptor is valid and indicates multiple + * flash devices we need to use hwseq to be able to + * access the second flash device. + */ + if (ich_spi_mode == ich_auto && desc.content.NC != 0) { + msg_pinfo("Enabling hardware sequencing due to " + "multiple flash chips detected.\n"); + ich_spi_mode = ich_hwseq; + } + } + + if (ich_spi_mode == ich_auto && ichspi_lock && + ich_missing_opcodes()) { + msg_pinfo("Enabling hardware sequencing because " + "some important opcode is locked.\n"); + ich_spi_mode = ich_hwseq; + } + + if (ich_spi_mode == ich_auto && + (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT || + ich_gen == CHIPSET_300_SERIES_CANNON_POINT || + ich_gen == CHIPSET_400_SERIES_COMET_POINT || + ich_gen == CHIPSET_500_SERIES_TIGER_POINT)) { + msg_pdbg("Enabling hardware sequencing by default for 100+ series PCH.\n"); + ich_spi_mode = ich_hwseq; + } + + if (ich_spi_mode == ich_auto && + (ich_gen == CHIPSET_APOLLO_LAKE || + ich_gen == CHIPSET_GEMINI_LAKE)) { + msg_pdbg("Enabling hardware sequencing by default for Apollo/Gemini Lake.\n"); + ich_spi_mode = ich_hwseq; + } + + if (ich_spi_mode == ich_hwseq) { + if (!desc_valid) { + msg_perr("Hardware sequencing was requested " + "but the flash descriptor is not " + "valid. Aborting.\n"); + return ERROR_FATAL; + } + + int tmpi = getFCBA_component_density(ich_gen, &desc, 0); + if (tmpi < 0) { + msg_perr("Could not determine density of flash component %d.\n", 0); + return ERROR_FATAL; + } + hwseq_data.size_comp0 = tmpi; + + tmpi = getFCBA_component_density(ich_gen, &desc, 1); + if (tmpi < 0) { + msg_perr("Could not determine density of flash component %d.\n", 1); + return ERROR_FATAL; + } + hwseq_data.size_comp1 = tmpi; + + register_opaque_master(&opaque_master_ich_hwseq, NULL); + } else { + register_spi_master(&spi_master_ich9, NULL); } return 0; } +int ich_init_spi(void *spibar, enum ich_chipset ich_gen) +{ + ich_generation = ich_gen; + ich_spibar = spibar; + + switch (ich_gen) { + case CHIPSET_ICH7: + case CHIPSET_TUNNEL_CREEK: + case CHIPSET_CENTERTON: + return init_ich7_spi(spibar, ich_gen); + case CHIPSET_ICH8: + default: /* Future version might behave the same */ + return init_ich_default(spibar, ich_gen); + } +} + static const struct spi_master spi_master_via = { .max_data_read = 16, .max_data_write = 16, -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 10 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: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Nico Huber (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, Nicholas Chin. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 9: (1 comment) Patchset: PS9: Thanks for patience, testing and everything folks! -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 9 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: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 02 Feb 2022 21:51:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Extract initialisation of swseq and hwseq into a function
by Nico Huber (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Felix Singer, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58737 ) Change subject: ichspi: Extract initialisation of swseq and hwseq into a function ...................................................................... Patch Set 10: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58737 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3 Gerrit-Change-Number: 58737 Gerrit-PatchSet: 10 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 02 Feb 2022 21:44:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Nico Huber (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, Nicholas Chin. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 9: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 9 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: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 02 Feb 2022 21:42:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: fmap.c: Avoid undefined behaviour with fmap_lsearch([len:=0])
by Nico Huber (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Julius Werner. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61545 ) Change subject: fmap.c: Avoid undefined behaviour with fmap_lsearch([len:=0]) ...................................................................... Patch Set 1: (2 comments) File fmap.c: https://review.coreboot.org/c/flashrom/+/61545/comment/d921bd87_42d3b6cd PS1, Line 99: if (len == 0) > How does a length of zero cause problems? What if len is non-zero but smaller than `sizeof(struct fm […] +1 we should simply check that the calculation below doesn't overflow. https://review.coreboot.org/c/flashrom/+/61545/comment/1ebc9fd9_bb0f456a PS1, Line 102: (off_t)(len - sizeof(struct fmap) Hrmm, I added this cast at some point, but it doesn't seem to fix the issue fully. In case `size_t` is smaller than `off_t`, the overflown `size_t` (a positive number) would fit into `off_t` and not be converted to a negative number, right? -- To view, visit https://review.coreboot.org/c/flashrom/+/61545 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifb408c55c3b69ddff453dcc704b7389298050473 Gerrit-Change-Number: 61545 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Wed, 02 Feb 2022 16:48:37 +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]: [RFC][OTP] cli_classic: add options for managing OTP regions
by Sergii Dmytruk (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59407 to look at the new patch set (#23). Change subject: [RFC][OTP] cli_classic: add options for managing OTP regions ...................................................................... [RFC][OTP] cli_classic: add options for managing OTP regions New actions operate on OTP region specified by `--otp-region`, which is a required option if there is more than one OTP region, otherwise it can be omitted. Region numbering starts at 1. All actions can be combined with `--otp-status`, which is processed last and thus can print new state of the OTP. Change-Id: If77694e3b1c453b3fc0561da7a9eefdbce4fbb2b Signed-off-by: Hatim Kanchwala <hatim at hatimak.me> Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M cli_classic.c M flashrom.8.tmpl 2 files changed, 292 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/07/59407/23 -- To view, visit https://review.coreboot.org/c/flashrom/+/59407 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If77694e3b1c453b3fc0561da7a9eefdbce4fbb2b Gerrit-Change-Number: 59407 Gerrit-PatchSet: 23 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: [RFC][OTP] spi25_statusreg: support reading/writing security register
by Sergii Dmytruk (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59709 to look at the new patch set (#19). Change subject: [RFC][OTP] spi25_statusreg: support reading/writing security register ...................................................................... [RFC][OTP] spi25_statusreg: support reading/writing security register Not to be confused with "secure registers" of OTP. Security register is a dedicated status register for security-related bits. You don't write its value directly, issuing corresponding write command with no data just sets OTP bit to 1 automatically. No WREN is necessary, but at least some datasheets indicate BUSY state after write command. Unlike cases where OTP bit is part of SR and can only be written while in OTP mode, security register can only be written outside of the mode. Change-Id: Iae1753ca4cb051127a5bcbeba7f064053adb8dae Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M flash.h M spi.h M spi25_statusreg.c 3 files changed, 75 insertions(+), 29 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/09/59709/19 -- To view, visit https://review.coreboot.org/c/flashrom/+/59709 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iae1753ca4cb051127a5bcbeba7f064053adb8dae Gerrit-Change-Number: 59709 Gerrit-PatchSet: 19 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: test write protection
by Sergii Dmytruk (Code Review) 02 Feb '22

02 Feb '22

Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk. Hello build bot (Jenkins), Nico Huber, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59075 to look at the new patch set (#28). Change subject: tests: test write protection ...................................................................... tests: test write protection Tests both WP implementation and its emulation in dummy programmer. Change-Id: I49af7f6d173eb4c56c22d80b01a473b8c499c0f8 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- A tests/chip_wp.c M tests/meson.build M tests/tests.c M tests/tests.h 4 files changed, 336 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/59075/28 -- To view, visit https://review.coreboot.org/c/flashrom/+/59075 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I49af7f6d173eb4c56c22d80b01a473b8c499c0f8 Gerrit-Change-Number: 59075 Gerrit-PatchSet: 28 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit