November 2022 - flashrom-gerrit

[S] Change in flashrom[master]: flashrom_tester: lock_test: Make the logic clear and explicit
by Evan Benn (Code Review) 09 Nov '22

09 Nov '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69400 to look at the new patch set (#2). Change subject: flashrom_tester: lock_test: Make the logic clear and explicit ...................................................................... flashrom_tester: lock_test: Make the logic clear and explicit Document clearly what the test is doing and expects. Do not use the push() function as it confuses the logs in the case of error. BUG=b:258357944 BRANCH=None TEST=flashrom_tester --libflashrom host Lock Change-Id: Ibad559a8ff9696fd91f45bca9d9ceb6e90c41393 --- M util/flashrom_tester/src/tests.rs 1 file changed, 20 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/00/69400/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69400 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibad559a8ff9696fd91f45bca9d9ceb6e90c41393 Gerrit-Change-Number: 69400 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tree/: Rename ERROR_NONFATAL to ERROR_FLASHROM_NONFATAL
by Felix Singer (Code Review) 09 Nov '22

09 Nov '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68777 ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tree/: Rename ERROR_NONFATAL to ERROR_FLASHROM_NONFATAL ...................................................................... tree/: Rename ERROR_NONFATAL to ERROR_FLASHROM_NONFATAL Change-Id: I5c30fec0cebab2b7d10e2789761889abc3a14dd3 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68777 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M chipset_enable.c M include/flash.h M sb600spi.c 3 files changed, 28 insertions(+), 13 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/chipset_enable.c b/chipset_enable.c index fec4f95..b9144d1 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -822,7 +822,7 @@ return ret_spi; if (((boot_buses & BUS_FWH) && ret_fwh) || ((boot_buses & BUS_SPI) && ret_spi)) - return ERROR_NONFATAL; + return ERROR_FLASHROM_NONFATAL; /* Suppress unknown laptop warning if we booted from SPI. */ if (boot_buses & BUS_SPI) @@ -964,7 +964,7 @@ const int ret_spi = ich_init_spi(cfg, spibar, pch_generation); if (ret_spi != ERROR_FLASHROM_FATAL) { if (ret_bc || ret_spi) - ret = ERROR_NONFATAL; + ret = ERROR_FLASHROM_NONFATAL; else ret = 0; } @@ -1083,7 +1083,7 @@ return ret_spi; if (((boot_buses & BUS_FWH) && ret_fwh) || ((boot_buses & BUS_SPI) && ret_spi)) - return ERROR_NONFATAL; + return ERROR_FLASHROM_NONFATAL; /* Suppress unknown laptop warning if we booted from SPI. */ if (boot_buses & BUS_SPI) @@ -1346,7 +1346,7 @@ if (pci_read_byte(dev, AMD_ENREG) != new) { msg_pwarn("Setting register 0x%x to 0x%02x on %s failed (WARNING ONLY).\n", AMD_ENREG, new, name); - return ERROR_NONFATAL; + return ERROR_FLASHROM_NONFATAL; } msg_pdbg2("Set ROM enable bit successfully.\n"); @@ -1470,7 +1470,7 @@ { rpci_write_byte(dev, 0x92, 0); if (enable_flash_nvidia_common(cfg, dev, name)) - return ERROR_NONFATAL; + return ERROR_FLASHROM_NONFATAL; else return 0; } @@ -1545,7 +1545,7 @@ err++; if (err > 0) - return ERROR_NONFATAL; + return ERROR_FLASHROM_NONFATAL; else return 0; } @@ -1620,7 +1620,7 @@ rpci_write_word(dev, 0x90, wordval); if (enable_flash_nvidia_common(cfg, dev, name)) - return ERROR_NONFATAL; + return ERROR_FLASHROM_NONFATAL; else return 0; } @@ -1661,7 +1661,7 @@ msg_pinfo("Please send the log files created by \"flashrom -p internal -o logfile\" to\n" "flashrom(a)flashrom.org with \"your board name: flashrom -V\" as the subject to\n" "help us finish support for your chipset. Thanks.\n"); - return ERROR_NONFATAL; + return ERROR_FLASHROM_NONFATAL; } /* Force enable SPI and disable LPC? Not a good idea. */ @@ -2240,7 +2240,7 @@ msg_pinfo("FAILED!\n"); else if (ret == 0) msg_pinfo("OK.\n"); - else if (ret == ERROR_NONFATAL) + else if (ret == ERROR_FLASHROM_NONFATAL) msg_pinfo("PROBLEMS, continuing anyway\n"); if (ret == ERROR_FLASHROM_FATAL) { msg_perr("FATAL ERROR!\n"); diff --git a/include/flash.h b/include/flash.h index e6d5679..1456134 100644 --- a/include/flash.h +++ b/include/flash.h @@ -495,7 +495,7 @@ int register_chip_restore(chip_restore_fn_cb_t func, struct flashctx *flash, uint8_t status); /* Something happened that shouldn't happen, but we can go on. */ -#define ERROR_NONFATAL 0x100 +#define ERROR_FLASHROM_NONFATAL 0x100 /* Something happened that shouldn't happen, we'll abort. */ #define ERROR_FLASHROM_FATAL -0xee diff --git a/sb600spi.c b/sb600spi.c index 68a5690..8c018a3 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -665,7 +665,7 @@ enum amd_chipset amd_gen = determine_generation(dev); if (amd_gen == CHIPSET_AMD_UNKNOWN) - return ERROR_NONFATAL; + return ERROR_FLASHROM_NONFATAL; /* How to read the following table and similar ones in this file: * "?" means we have no datasheet for this chipset generation or it doesn't have any relevant info. @@ -748,7 +748,7 @@ if (((tmp >> 22) & 0x1) == 0 || ((tmp >> 23) & 0x1) == 0) { msg_perr("ERROR: State of SpiAccessMacRomEn or SpiHostAccessRomEn prohibits full access.\n"); - return ERROR_NONFATAL; + return ERROR_FLASHROM_NONFATAL; } if (amd_gen >= CHIPSET_SB89XX) { @@ -765,7 +765,7 @@ smbus_dev = pcidev_find(0x1022, 0x790b); /* AMD FP4 */ if (!smbus_dev) { msg_perr("ERROR: SMBus device not found. Not enabling SPI.\n"); - return ERROR_NONFATAL; + return ERROR_FLASHROM_NONFATAL; } /* Note about the bit tests below: If a bit is zero, the GPIO is SPI. */ -- To view, visit https://review.coreboot.org/c/flashrom/+/68777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5c30fec0cebab2b7d10e2789761889abc3a14dd3 Gerrit-Change-Number: 68777 Gerrit-PatchSet: 4 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-MessageType: merged

1 0

[M] Change in flashrom[master]: tree/: Rename ERROR_FATAL to ERROR_FLASHROM_FATAL
by Felix Singer (Code Review) 09 Nov '22

09 Nov '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68776 ) Change subject: tree/: Rename ERROR_FATAL to ERROR_FLASHROM_FATAL ...................................................................... tree/: Rename ERROR_FATAL to ERROR_FLASHROM_FATAL Change-Id: I51ee789f9a1443bfff1e3c85c9b40b5023db6062 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68776 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M atavia.c M bitbang_spi.c M chipset_enable.c M flashrom.c M ichspi.c M include/flash.h M internal.c M sb600spi.c M tests/dummyflasher.c 9 files changed, 56 insertions(+), 43 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved diff --git a/atavia.c b/atavia.c index b392999..1eebc97 100644 --- a/atavia.c +++ b/atavia.c @@ -151,14 +151,14 @@ if (strlen(arg) == 0) { msg_perr("Missing argument for offset.\n"); free(arg); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } char *endptr; atavia_offset = (void *)strtoul(arg, &endptr, 0); if (*endptr) { msg_perr("Error: Invalid offset specified: \"%s\".\n", arg); free(arg); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } msg_pinfo("Mapping addresses to base %p.\n", atavia_offset); } diff --git a/bitbang_spi.c b/bitbang_spi.c index 5e4b566..f7ded1f 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -166,7 +166,7 @@ struct bitbang_spi_master_data *data = calloc(1, sizeof(*data)); if (!data) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; data->master = master; if (spi_data) diff --git a/chipset_enable.c b/chipset_enable.c index 60b82df..fec4f95 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -275,7 +275,7 @@ switch (ich_generation) { case CHIPSET_ICH_UNKNOWN: - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; /* Non-SPI-capable */ case CHIPSET_ICH: case CHIPSET_ICH2345: @@ -409,7 +409,7 @@ uint32_t ilb_base = pci_read_long(dev, 0x50) & 0xfffffe00; /* bits 31:9 */ if (ilb_base == 0) { msg_perr("Error: Invalid ILB_BASE_ADDRESS\n"); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } ilb = rphysmap("BYT IBASE", ilb_base, 512); fwh_sel1 = 0x18; @@ -468,7 +468,7 @@ msg_perr("Error: fwh_idsel= specified, but no value given.\n"); idsel_garbage_out: free(idsel); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } free(idsel); @@ -779,13 +779,13 @@ /* Map RCBA to virtual memory */ void *rcrb = rphysmap("ICH RCRB", rcra, 0x4000); if (rcrb == ERROR_PTR) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; const enum chipbustype boot_buses = enable_flash_ich_report_gcs(dev, ich_generation, rcrb); /* Handle FWH-related parameters and initialization */ int ret_fwh = enable_flash_ich_fwh(cfg, dev, ich_generation, bios_cntl); - if (ret_fwh == ERROR_FATAL) + if (ret_fwh == ERROR_FLASHROM_FATAL) return ret_fwh; /* @@ -801,7 +801,7 @@ switch (ich_generation) { case CHIPSET_BAYTRAIL: case CHIPSET_ICH_UNKNOWN: - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; case CHIPSET_ICH7: case CHIPSET_ICH8: case CHIPSET_TUNNEL_CREEK: @@ -818,7 +818,7 @@ /* This adds BUS_SPI */ int ret_spi = ich_init_spi(cfg, spibar, ich_generation); - if (ret_spi == ERROR_FATAL) + if (ret_spi == ERROR_FLASHROM_FATAL) return ret_spi; if (((boot_buses & BUS_FWH) && ret_fwh) || ((boot_buses & BUS_SPI) && ret_spi)) @@ -920,7 +920,7 @@ struct pci_dev *const dev, const char *const name, const int slot, const int func, const enum ich_chipset pch_generation) { - int ret = ERROR_FATAL; + int ret = ERROR_FLASHROM_FATAL; /* * The SPI PCI device is usually hidden (by hiding PCI vendor @@ -951,7 +951,7 @@ const enum chipbustype boot_buses = enable_flash_ich_report_gcs(spi_dev, pch_generation, NULL); const int ret_bc = enable_flash_ich_bios_cntl_config_space(spi_dev, pch_generation, 0xdc); - if (ret_bc == ERROR_FATAL) + if (ret_bc == ERROR_FLASHROM_FATAL) goto _freepci_ret; const uint32_t phys_spibar = pci_read_long(spi_dev, PCI_BASE_ADDRESS_0) & 0xfffff000; @@ -962,7 +962,7 @@ /* This adds BUS_SPI */ const int ret_spi = ich_init_spi(cfg, spibar, pch_generation); - if (ret_spi != ERROR_FATAL) { + if (ret_spi != ERROR_FLASHROM_FATAL) { if (ret_bc || ret_spi) ret = ERROR_NONFATAL; else @@ -1055,13 +1055,13 @@ /* Handle GCS (in RCRB) */ void *rcrb = physmap("BYT RCRB", rcba, 4); if (rcrb == ERROR_PTR) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; const enum chipbustype boot_buses = enable_flash_ich_report_gcs(dev, ich_generation, rcrb); physunmap(rcrb, 4); /* Handle fwh_idsel parameter */ int ret_fwh = enable_flash_ich_fwh_decode(cfg, dev, ich_generation); - if (ret_fwh == ERROR_FATAL) + if (ret_fwh == ERROR_FLASHROM_FATAL) return ret_fwh; internal_buses_supported &= BUS_FWH; @@ -1071,7 +1071,7 @@ msg_pdbg("SPI_BASE_ADDRESS = 0x%x\n", sbase); void *spibar = rphysmap("BYT SBASE", sbase, 512); /* Last defined address on Bay Trail is 0x100 */ if (spibar == ERROR_PTR) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; /* Enable Flash Writes. * Silvermont-based: BCR at SBASE + 0xFC (some bits of BCR are also accessible via BC at IBASE + 0x1C). @@ -1079,7 +1079,7 @@ enable_flash_ich_bios_cntl_memmapped(ich_generation, spibar + 0xFC); int ret_spi = ich_init_spi(cfg, spibar, ich_generation); - if (ret_spi == ERROR_FATAL) + if (ret_spi == ERROR_FLASHROM_FATAL) return ret_spi; if (((boot_buses & BUS_FWH) && ret_fwh) || ((boot_buses & BUS_SPI) && ret_spi)) @@ -1137,7 +1137,7 @@ struct pci_dev *south_north = pcidev_find(0x1106, 0xa353); if (south_north == NULL) { msg_perr("Could not find South-North Module Interface Control device!\n"); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } msg_pdbg("Strapped to "); @@ -1157,7 +1157,7 @@ spi0_mm_base = pci_read_long(dev, 0xbc) << 8; if (spi0_mm_base == 0x0) { msg_pdbg ("MMIO not enabled!\n"); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } break; case 0x8409: /* VX855/VX875 */ @@ -1165,18 +1165,18 @@ mmio_base = pci_read_long(dev, 0xbc) << 8; if (mmio_base == 0x0) { msg_pdbg ("MMIO not enabled!\n"); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } mmio_base_physmapped = physmap("VIA VX MMIO register", mmio_base, SPI_CNTL_LEN); if (mmio_base_physmapped == ERROR_PTR) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; /* Offset 0 - Bit 0 holds SPI Bus0 Enable Bit. */ spi_cntl = mmio_readl(mmio_base_physmapped) + 0x00; if ((spi_cntl & 0x01) == 0) { msg_pdbg ("SPI Bus0 disabled!\n"); physunmap(mmio_base_physmapped, SPI_CNTL_LEN); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } /* Offset 1-3 has SPI Bus Memory Map Base Address: */ spi0_mm_base = spi_cntl & 0xFFFFFF00; @@ -1190,7 +1190,7 @@ break; default: msg_perr("%s: Unsupported chipset %x:%x!\n", __func__, dev->vendor_id, dev->device_id); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } return via_init_spi(spi0_mm_base); @@ -1578,7 +1578,7 @@ if (!smbusdev) { msg_perr("ERROR: SMBus device not found. Aborting.\n"); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } /* Enable some SMBus stuff. */ @@ -1713,7 +1713,7 @@ /* 1. Map MMCR */ mmcr = physmap("Elan SC520 MMCR", 0xfffef000, getpagesize()); if (mmcr == ERROR_PTR) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; /* 2. Scan PAR0 (0x88) - PAR15 (0xc4) for * BOOTCS region (PARx[31:29] = 100b)e @@ -2216,7 +2216,7 @@ if (chipset_enables[i].status == BAD) { msg_perr("ERROR: This chipset is not supported yet.\n"); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } if (chipset_enables[i].status == NT) { msg_pinfo("This chipset is marked as untested. If " @@ -2242,7 +2242,7 @@ msg_pinfo("OK.\n"); else if (ret == ERROR_NONFATAL) msg_pinfo("PROBLEMS, continuing anyway\n"); - if (ret == ERROR_FATAL) { + if (ret == ERROR_FLASHROM_FATAL) { msg_perr("FATAL ERROR!\n"); return ret; } diff --git a/flashrom.c b/flashrom.c index db4dd7d..e2452c9 100644 --- a/flashrom.c +++ b/flashrom.c @@ -154,7 +154,7 @@ cfg.params = strdup(param); if (!cfg.params) { msg_perr("Out of memory!\n"); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } } else { cfg.params = NULL; @@ -175,7 +175,7 @@ */ msg_perr("Unhandled programmer parameters: %s\n", cfg.params); msg_perr("Aborting.\n"); - ret = ERROR_FATAL; + ret = ERROR_FLASHROM_FATAL; } } free(cfg.params); diff --git a/ichspi.c b/ichspi.c index b3beaea..4e982f0 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1868,11 +1868,11 @@ } else if (!strlen(arg)) { msg_perr("Missing argument for ich_spi_mode.\n"); free(arg); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } else { msg_perr("Unknown argument for ich_spi_mode: %s\n", arg); free(arg); - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; } free(arg); @@ -2167,26 +2167,26 @@ if (!desc_valid) { msg_perr("Hardware sequencing was requested " "but the flash descriptor is not valid. Aborting.\n"); - return ERROR_FATAL; + return ERROR_FLASHROM_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; + return ERROR_FLASHROM_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; + return ERROR_FLASHROM_FATAL; } hwseq_data.size_comp1 = tmpi; struct hwseq_data *opaque_hwseq_data = calloc(1, sizeof(struct hwseq_data)); if (!opaque_hwseq_data) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; memcpy(opaque_hwseq_data, &hwseq_data, sizeof(*opaque_hwseq_data)); register_opaque_master(&opaque_master_ich_hwseq, opaque_hwseq_data); } else { @@ -2231,7 +2231,7 @@ ich_spibar = rphysmap("VIA SPI MMIO registers", mmio_base, 0x70); if (ich_spibar == ERROR_PTR) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; /* Do we really need no write enable? Like the LPC one at D17F0 0x40 */ /* Not sure if it speaks all these bus protocols. */ diff --git a/include/flash.h b/include/flash.h index da200ea..e6d5679 100644 --- a/include/flash.h +++ b/include/flash.h @@ -498,7 +498,7 @@ #define ERROR_NONFATAL 0x100 /* Something happened that shouldn't happen, we'll abort. */ -#define ERROR_FATAL -0xee +#define ERROR_FLASHROM_FATAL -0xee #define ERROR_FLASHROM_BUG -200 /* We reached one of the hardcoded limits of flashrom. This can be fixed by * increasing the limit of a compile-time allocation or by switching to dynamic diff --git a/internal.c b/internal.c index 8c834e2..4f68456 100644 --- a/internal.c +++ b/internal.c @@ -289,7 +289,7 @@ if (ret == -2) { msg_perr("WARNING: No chipset found. Flash detection " "will most likely fail.\n"); - } else if (ret == ERROR_FATAL) { + } else if (ret == ERROR_FLASHROM_FATAL) { goto internal_init_exit; } diff --git a/sb600spi.c b/sb600spi.c index 6020145..68a5690 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -656,7 +656,7 @@ /* Physical memory has to be mapped at page (4k) boundaries. */ sb600_spibar = rphysmap("SB600 SPI registers", tmp & 0xfffff000, 0x1000); if (sb600_spibar == ERROR_PTR) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; /* The low bits of the SPI base address are used as offset into * the mapped page. @@ -798,10 +798,10 @@ } if (handle_speed(cfg, dev, amd_gen, sb600_spibar) != 0) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; if (handle_imc(cfg, dev, amd_gen) != 0) - return ERROR_FATAL; + return ERROR_FLASHROM_FATAL; struct sb600spi_data *data = calloc(1, sizeof(*data)); if (!data) { diff --git a/tests/dummyflasher.c b/tests/dummyflasher.c index 1148217..c41f9df 100644 --- a/tests/dummyflasher.c +++ b/tests/dummyflasher.c @@ -88,7 +88,7 @@ * is successful, due to invalid param at the end of param string. */ run_init_error_path(state, &dummy_io, &programmer_dummy, - "bus=spi,emulate=W25Q128FV,invalid=value", ERROR_FATAL); + "bus=spi,emulate=W25Q128FV,invalid=value", ERROR_FLASHROM_FATAL); } void dummy_init_success_unhandled_param_test_success(void **state) @@ -107,7 +107,7 @@ * Unhandled param `voltage` is not used for dummyflasher. */ run_init_error_path(state, &dummy_io, &programmer_dummy, - "bus=spi,emulate=W25Q128FV,voltage=3.5V", ERROR_FATAL); + "bus=spi,emulate=W25Q128FV,voltage=3.5V", ERROR_FLASHROM_FATAL); } void dummy_null_prog_param_test_success(void **state) -- To view, visit https://review.coreboot.org/c/flashrom/+/68776 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I51ee789f9a1443bfff1e3c85c9b40b5023db6062 Gerrit-Change-Number: 68776 Gerrit-PatchSet: 4 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-MessageType: merged

1 0

[M] Change in flashrom[master]: tree/: Rename ERROR_FATAL to ERROR_FLASHROM_FATAL
by Felix Singer (Code Review) 09 Nov '22

09 Nov '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68776 ) Change subject: tree/: Rename ERROR_FATAL to ERROR_FLASHROM_FATAL ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: Commit 54f864779b introduced another case of ERROR_FATAL. Fixed it to get these patches merged. -- To view, visit https://review.coreboot.org/c/flashrom/+/68776 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I51ee789f9a1443bfff1e3c85c9b40b5023db6062 Gerrit-Change-Number: 68776 Gerrit-PatchSet: 3 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 09 Nov 2022 15:02:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Rename ERROR_FATAL to ERROR_FLASHROM_FATAL
by Felix Singer (Code Review) 09 Nov '22

09 Nov '22

Attention is currently required from: Felix Singer, Edward O'Callaghan. Felix Singer has uploaded a new patch set (#3) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/68776 ) Change subject: tree/: Rename ERROR_FATAL to ERROR_FLASHROM_FATAL ...................................................................... tree/: Rename ERROR_FATAL to ERROR_FLASHROM_FATAL Change-Id: I51ee789f9a1443bfff1e3c85c9b40b5023db6062 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M atavia.c M bitbang_spi.c M chipset_enable.c M flashrom.c M ichspi.c M include/flash.h M internal.c M sb600spi.c M tests/dummyflasher.c 9 files changed, 53 insertions(+), 43 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/68776/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/68776 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I51ee789f9a1443bfff1e3c85c9b40b5023db6062 Gerrit-Change-Number: 68776 Gerrit-PatchSet: 3 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tree/: Rename ERROR_NONFATAL to ERROR_FLASHROM_NONFATAL
by Felix Singer (Code Review) 09 Nov '22

09 Nov '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68777 ) Change subject: tree/: Rename ERROR_NONFATAL to ERROR_FLASHROM_NONFATAL ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5c30fec0cebab2b7d10e2789761889abc3a14dd3 Gerrit-Change-Number: 68777 Gerrit-PatchSet: 2 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 09 Nov 2022 14:41:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Rename ERROR_FATAL to ERROR_FLASHROM_FATAL
by Felix Singer (Code Review) 09 Nov '22

09 Nov '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68776 ) Change subject: tree/: Rename ERROR_FATAL to ERROR_FLASHROM_FATAL ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68776 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I51ee789f9a1443bfff1e3c85c9b40b5023db6062 Gerrit-Change-Number: 68776 Gerrit-PatchSet: 2 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 09 Nov 2022 14:41:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: bitbang_spi.c: Fix unchecked heap allocation
by Felix Singer (Code Review) 09 Nov '22

09 Nov '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68775 ) Change subject: bitbang_spi.c: Fix unchecked heap allocation ...................................................................... bitbang_spi.c: Fix unchecked heap allocation Change-Id: Ib64b1fe67fa1874875453ab9e1700e468c579e7c Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68775 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M bitbang_spi.c 1 file changed, 17 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/bitbang_spi.c b/bitbang_spi.c index 10c1087..5e4b566 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -165,6 +165,9 @@ } struct bitbang_spi_master_data *data = calloc(1, sizeof(*data)); + if (!data) + return ERROR_FATAL; + data->master = master; if (spi_data) data->spi_data = spi_data; -- To view, visit https://review.coreboot.org/c/flashrom/+/68775 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib64b1fe67fa1874875453ab9e1700e468c579e7c Gerrit-Change-Number: 68775 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(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-MessageType: merged

1 0

[S] Change in flashrom[master]: flashrom_tester: Log some cros lsb-release information
by Evan Benn (Code Review) 09 Nov '22

09 Nov '22

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69403 ) Change subject: flashrom_tester: Log some cros lsb-release information ...................................................................... flashrom_tester: Log some cros lsb-release information os-release does not quite have as much information so use lsb-release. BUG=None BRANCH=None TEST=None Change-Id: If3452ead9e02e0ddeaa0fdf4852d7c17a8ab7650 --- M util/flashrom_tester/src/cros_sysinfo.rs M util/flashrom_tester/src/tester.rs M util/flashrom_tester/src/tests.rs 3 files changed, 29 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/69403/1 diff --git a/util/flashrom_tester/src/cros_sysinfo.rs b/util/flashrom_tester/src/cros_sysinfo.rs index e047f8e..37e1ec6 100644 --- a/util/flashrom_tester/src/cros_sysinfo.rs +++ b/util/flashrom_tester/src/cros_sysinfo.rs @@ -34,6 +34,7 @@ // use std::ffi::OsStr; +use std::fs; use std::io::Result as IoResult; use std::process::{Command, Stdio}; @@ -58,3 +59,12 @@ pub fn bios_info() -> IoResult<String> { dmidecode_dispatch(&["-q", "-t0"]) } + +pub fn release_description() -> IoResult<String> { + for l in fs::read_to_string("/etc/lsb-release")?.lines() { + if l.starts_with("CHROMEOS_RELEASE_DESCRIPTION") { + return Ok(l.to_string()); + } + } + Err(std::io::ErrorKind::NotFound.into()) +} diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs index 96ba34e..2644052 100644 --- a/util/flashrom_tester/src/tester.rs +++ b/util/flashrom_tester/src/tester.rs @@ -335,6 +335,7 @@ pub struct ReportMetaData { pub chip_name: String, pub os_release: String, + pub cros_release: String, pub system_info: String, pub bios_info: String, } @@ -412,6 +413,7 @@ println!(); println!(" %---------------------------%"); println!(" os release: {}", meta_data.os_release); + println!(" cros release: {}", meta_data.cros_release); println!(" chip name: {}", meta_data.chip_name); println!(" system info: \n{}", meta_data.system_info); println!(" bios info: \n{}", meta_data.bios_info); diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index 6f2bf1e..558dcfc 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -152,12 +152,14 @@ } let os_release = sys_info::os_release().unwrap_or("<Unknown OS>".to_string()); + let cros_release = cros_sysinfo::release_description().unwrap_or("<Unknown CROS>".to_string()); let system_info = cros_sysinfo::system_info().unwrap_or("<Unknown System>".to_string()); let bios_info = cros_sysinfo::bios_info().unwrap_or("<Unknown BIOS>".to_string()); let meta_data = tester::ReportMetaData { chip_name, os_release, + cros_release, system_info, bios_info, }; -- To view, visit https://review.coreboot.org/c/flashrom/+/69403 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If3452ead9e02e0ddeaa0fdf4852d7c17a8ab7650 Gerrit-Change-Number: 69403 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: flashrom_tester: Check the WP state when setting
by Evan Benn (Code Review) 09 Nov '22

09 Nov '22

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69402 ) Change subject: flashrom_tester: Check the WP state when setting ...................................................................... flashrom_tester: Check the WP state when setting Check that the hardware and software WP state are as expected in the setter methods. BUG=None BRANCH=None TEST=None Change-Id: Ie7f90ab478dca6f92eaa0908443e3cb156ea0319 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/src/tester.rs 1 file changed, 38 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/69402/1 diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs index 49a663c..96ba34e 100644 --- a/util/flashrom_tester/src/tester.rs +++ b/util/flashrom_tester/src/tester.rs @@ -220,28 +220,40 @@ /// Set the software write protect and check that the state is as expected. pub fn set_sw(&mut self, enable: bool) -> Result<&mut Self, String> { - info!("request={}, current={}", enable, self.current.sw); + info!("set_sw request={}, current={}", enable, self.current.sw); if self.current.sw != enable { self.cmd .wp_toggle(/* en= */ enable) .map_err(|e| e.to_string())?; } - Ok(self) + let actual_state = Self::get_sw(self.cmd).map_err(|e| e.to_string())?; + if actual_state != enable { + Err(format!("set_sw request={}, real={}", enable, actual_state)) + } else { + self.current.sw = enable; + Ok(self) + } } - /// Set the hardware write protect. + /// Set the hardware write protect if supported and check that the state is as expected. pub fn set_hw(&mut self, enable: bool) -> Result<&mut Self, String> { + info!("set_hw request={}, current={}", enable, self.current.hw); if self.can_control_hw_wp() { if self.current.hw != enable { super::utils::toggle_hw_wp(/* dis= */ !enable)?; - self.current.hw = enable; - } else if enable { - info!( - "Ignoring attempt to enable hardware WP with {:?} programmer", - self.fc - ); } + // toggle_hw_wp does check, but we might not have called toggle_hw_wp so check again. + let actual_state = Self::get_hw(self.cmd)?; + if actual_state != enable { + return Err(format!("set_hw request={}, real={}", enable, actual_state)); + } + } else { + info!( + "Ignoring attempt to set hardware WP with {:?} programmer", + self.fc + ); } + self.current.hw = enable; Ok(self) } -- To view, visit https://review.coreboot.org/c/flashrom/+/69402 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie7f90ab478dca6f92eaa0908443e3cb156ea0319 Gerrit-Change-Number: 69402 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022