October 2022 - flashrom-gerrit

[S] Change in flashrom[master]: manibuilder/apt-get: Drop `apt-get dist-upgrade` commands
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67187 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: manibuilder/apt-get: Drop `apt-get dist-upgrade` commands ...................................................................... manibuilder/apt-get: Drop `apt-get dist-upgrade` commands Such invasive updates shouldn't be needed. And if, we'd notice. Change-Id: I8915ad0f8b348c1bb532c261a73ea2ab6d602565 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67187 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M util/manibuilder/Dockerfile.anita M util/manibuilder/Dockerfile.debian-debootstrap M util/manibuilder/Dockerfile.ubuntu-debootstrap 3 files changed, 15 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/util/manibuilder/Dockerfile.anita b/util/manibuilder/Dockerfile.anita index 6ef41b9..52befce 100644 --- a/util/manibuilder/Dockerfile.anita +++ b/util/manibuilder/Dockerfile.anita @@ -7,7 +7,6 @@ useradd -p locked -m mani && \ apt-get -qq update && \ apt-get -qq upgrade && \ - apt-get -qq dist-upgrade && \ apt-get -qqy install git python-is-python3 python3-pexpect \ python3-distutils genisoimage qemu-system && \ apt-get clean diff --git a/util/manibuilder/Dockerfile.debian-debootstrap b/util/manibuilder/Dockerfile.debian-debootstrap index d2e0eee..c9af0bf 100644 --- a/util/manibuilder/Dockerfile.debian-debootstrap +++ b/util/manibuilder/Dockerfile.debian-debootstrap @@ -4,7 +4,6 @@ useradd -p locked -m mani && \ apt-get -qq update && \ apt-get -qq upgrade && \ - apt-get -qq dist-upgrade && \ apt-get -qqy install gcc make git doxygen ccache pkg-config \ libpci-dev libusb-dev libftdi-dev libusb-1.0-0-dev && \ { apt-get -qqy install libjaylink-dev || true; } && \ diff --git a/util/manibuilder/Dockerfile.ubuntu-debootstrap b/util/manibuilder/Dockerfile.ubuntu-debootstrap index f002d9a..62cc461 100644 --- a/util/manibuilder/Dockerfile.ubuntu-debootstrap +++ b/util/manibuilder/Dockerfile.ubuntu-debootstrap @@ -17,7 +17,6 @@ fi && \ apt-get -qq update && \ apt-get -qq upgrade && \ - apt-get -qq dist-upgrade && \ apt-get -qqy install gcc make git doxygen ccache pkg-config \ libpci-dev libusb-dev libftdi-dev libusb-1.0-0-dev && \ { apt-get -qqy install libjaylink-dev || true; } && \ -- To view, visit https://review.coreboot.org/c/flashrom/+/67187 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8915ad0f8b348c1bb532c261a73ea2ab6d602565 Gerrit-Change-Number: 67187 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: s25f.c: Fix undefined behaviour on shift
by Felix Singer (Code Review) 08 Oct '22

08 Oct '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68127 ) Change subject: s25f.c: Fix undefined behaviour on shift ...................................................................... s25f.c: Fix undefined behaviour on shift dev_id, a uint8_t, was shifted left by 24 bits. After promotion to int, this results in shifting into the sign bit, which is undefined behaviour. Cast to uint32_t to prevent the promotion to signed int. BUG=None BRANCH=None TEST=None Change-Id: I88188ef2ba2af919eeae9ba08916374d31d8b989 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68127 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 s25f.c 1 file changed, 26 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/s25f.c b/s25f.c index 67715ed..1133ef8 100644 --- a/s25f.c +++ b/s25f.c @@ -380,10 +380,10 @@ */ uint32_t model_id = - dev_id[1] << 24 | - dev_id[2] << 16 | - dev_id[4] << 8 | - dev_id[5] << 0; + (uint32_t)dev_id[1] << 24 | + (uint32_t)dev_id[2] << 16 | + (uint32_t)dev_id[4] << 8 | + (uint32_t)dev_id[5] << 0; if (dev_id[0] == flash->chip->manufacture_id && model_id == flash->chip->model_id) return 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/68127 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I88188ef2ba2af919eeae9ba08916374d31d8b989 Gerrit-Change-Number: 68127 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: flashrom.c: Remove custom mappers from opaque_master
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68092 ) Change subject: flashrom.c: Remove custom mappers from opaque_master ...................................................................... flashrom.c: Remove custom mappers from opaque_master No opaque masters have a custom mapper. The returned chipaddr is not fed back into the read/write/erase functions, so this would only be useful for side effects. Change-Id: I36f05154edda371b51f8ff416f019837ff1c243d Signed-off-by: Jonathon Hall <jonathon.hall(a)puri.sm> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68092 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M flashrom.c M include/programmer.h 2 files changed, 21 insertions(+), 11 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 90899f0..c5f712a 100644 --- a/flashrom.c +++ b/flashrom.c @@ -211,9 +211,7 @@ * fallback if it does not specialize it */ void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len) = NULL; - if (mst->buses_supported & BUS_PROG) - map_flash_region = mst->opaque.map_flash_region; - else if (mst->buses_supported & BUS_SPI) + if (mst->buses_supported & BUS_SPI) map_flash_region = mst->spi.map_flash_region; else if (mst->buses_supported & BUS_NONSPI) map_flash_region = mst->par.map_flash_region; @@ -232,9 +230,7 @@ void *virt_addr, size_t len) { void (*unmap_flash_region) (void *virt_addr, size_t len) = NULL; - if (mst->buses_supported & BUS_PROG) - unmap_flash_region = mst->opaque.unmap_flash_region; - else if (mst->buses_supported & BUS_SPI) + if (mst->buses_supported & BUS_SPI) unmap_flash_region = mst->spi.unmap_flash_region; else if (mst->buses_supported & BUS_NONSPI) unmap_flash_region = mst->par.unmap_flash_region; @@ -249,9 +245,7 @@ static bool master_uses_physmap(const struct registered_master *mst) { #if CONFIG_INTERNAL == 1 - if (mst->buses_supported & BUS_PROG) - return mst->opaque.map_flash_region == physmap; - else if (mst->buses_supported & BUS_SPI) + if (mst->buses_supported & BUS_SPI) return mst->spi.map_flash_region == physmap; else if (mst->buses_supported & BUS_NONSPI) return mst->par.map_flash_region == physmap; diff --git a/include/programmer.h b/include/programmer.h index 4b5bf92..bfce5d6 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -398,8 +398,6 @@ /* opaque.c */ struct opaque_master { - void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len); - void (*unmap_flash_region) (void *virt_addr, size_t len); int max_data_read; int max_data_write; /* Specific functions for this master */ -- To view, visit https://review.coreboot.org/c/flashrom/+/68092 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I36f05154edda371b51f8ff416f019837ff1c243d Gerrit-Change-Number: 68092 Gerrit-PatchSet: 4 Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-MessageType: merged

1 0

[S] Change in flashrom[master]: dummyflasher.c: Remove custom mapper from opaque_master
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68091 ) Change subject: dummyflasher.c: Remove custom mapper from opaque_master ...................................................................... dummyflasher.c: Remove custom mapper from opaque_master Dummy doesn't need a custom mapper on opaque_master; the returned address is not used and the mapper has no side effects. This is the only remaining opaque master with a custom mapper, so this permits removing custom mapper support from opaque masters. Change-Id: I76ae3e0c2e91ecba4fd320941bd1eff038050731 Signed-off-by: Jonathon Hall <jonathon.hall(a)puri.sm> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68091 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M dummyflasher.c 1 file changed, 21 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/dummyflasher.c b/dummyflasher.c index ad734f0..22f392a 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -947,8 +947,6 @@ }; static const struct opaque_master opaque_master_dummyflasher = { - .map_flash_region = dummy_map, - .unmap_flash_region = dummy_unmap, .probe = probe_variable_size, .read = dummy_opaque_read, .write = dummy_opaque_write, -- To view, visit https://review.coreboot.org/c/flashrom/+/68091 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I76ae3e0c2e91ecba4fd320941bd1eff038050731 Gerrit-Change-Number: 68091 Gerrit-PatchSet: 4 Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: ichspi: Do not attempt to map physical memory for hwseq
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68090 ) Change subject: ichspi: Do not attempt to map physical memory for hwseq ...................................................................... ichspi: Do not attempt to map physical memory for hwseq ICH hwseq does not need to actually mmap flash, and this fails for flash chips >16 MB, since only the top 16 MB is mapped into the address space. Test: Read and write flash on ICH hwseq with 32 MB flash chip. Change-Id: Ie698071c3181e988f10b750b0e50c9700efaa1a3 Signed-off-by: Jonathon Hall <jonathon.hall(a)puri.sm> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68090 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M ichspi.c 1 file changed, 21 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/ichspi.c b/ichspi.c index 7e7967e..aefbe2f 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1794,8 +1794,6 @@ }; static const struct opaque_master opaque_master_ich_hwseq = { - .map_flash_region = physmap, - .unmap_flash_region = physunmap, .max_data_read = 64, .max_data_write = 64, .probe = ich_hwseq_probe, -- To view, visit https://review.coreboot.org/c/flashrom/+/68090 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie698071c3181e988f10b750b0e50c9700efaa1a3 Gerrit-Change-Number: 68090 Gerrit-PatchSet: 4 Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: drivers: Move (un)map_flash_region to par/spi/opaque_master
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67695 ) Change subject: drivers: Move (un)map_flash_region to par/spi/opaque_master ...................................................................... drivers: Move (un)map_flash_region to par/spi/opaque_master Move (un)map_flash_region function pointers from programmer_entry to par_master, spi_master, and opaque_master. This enables programmers to specify a different mapper per bus, which is needed for the internal programmer. Mapping is closely tied to the way the memory is accessed using the other functions in the bus master structs. Validate that FWH/LPC programmers provide specialized mapping in register_par_master(); this is needed for chips with FEATURE_REGISTERMAP, which only exist on FWH or LPC buses. programmer.c: Update comment in fallback_map(), NULL return is the desired behavior. Test: Read firmware on SB600 Promontory mainboard (requires physmap) Test: Read firmware externally with ft2232_spi Test: Read firmware on ICH hwseq, verify physmap still occurs Change-Id: I9c3df6ae260bcdb246dfb0cd8e043919609b014b Signed-off-by: Jonathon Hall <jonathon.hall(a)puri.sm> Co-Authored-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67695 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M atavia.c M dummyflasher.c M flashrom.c M ichspi.c M include/flash.h M include/programmer.h M internal.c M it87spi.c M parallel.c M programmer.c M sb600spi.c M serprog.c M wbsio_spi.c 13 files changed, 150 insertions(+), 41 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/atavia.c b/atavia.c index 66bca65..5cbe058 100644 --- a/atavia.c +++ b/atavia.c @@ -133,6 +133,7 @@ } static const struct par_master lpc_master_atavia = { + .map_flash_region = atavia_map, .chip_readb = atavia_chip_readb, .chip_readw = fallback_chip_readw, .chip_readl = fallback_chip_readl, @@ -189,5 +190,4 @@ .type = PCI, .devs.dev = ata_via, .init = atavia_init, - .map_flash_region = atavia_map, }; diff --git a/dummyflasher.c b/dummyflasher.c index b2a41c6..ad734f0 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -920,6 +920,8 @@ } static const struct spi_master spi_master_dummyflasher = { + .map_flash_region = dummy_map, + .unmap_flash_region = dummy_unmap, .features = SPI_MASTER_4BA, .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_UNSPECIFIED, @@ -932,6 +934,8 @@ }; static const struct par_master par_master_dummyflasher = { + .map_flash_region = dummy_map, + .unmap_flash_region = dummy_unmap, .chip_readb = dummy_chip_readb, .chip_readw = dummy_chip_readw, .chip_readl = dummy_chip_readl, @@ -943,6 +947,8 @@ }; static const struct opaque_master opaque_master_dummyflasher = { + .map_flash_region = dummy_map, + .unmap_flash_region = dummy_unmap, .probe = probe_variable_size, .read = dummy_opaque_read, .write = dummy_opaque_write, @@ -1424,6 +1430,4 @@ /* FIXME */ .devs.note = "Dummy device, does nothing and logs all accesses\n", .init = dummy_init, - .map_flash_region = dummy_map, - .unmap_flash_region = dummy_unmap, }; diff --git a/flashrom.c b/flashrom.c index f74b79a..90899f0 100644 --- a/flashrom.c +++ b/flashrom.c @@ -203,27 +203,62 @@ return ret; } -void *programmer_map_flash_region(const char *descr, uintptr_t phys_addr, size_t len) +void *master_map_flash_region(const struct registered_master *mst, + const char *descr, uintptr_t phys_addr, + size_t len) { + /* Check the bus master for a specialized map_flash_region; default to + * fallback if it does not specialize it + */ + void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len) = NULL; + if (mst->buses_supported & BUS_PROG) + map_flash_region = mst->opaque.map_flash_region; + else if (mst->buses_supported & BUS_SPI) + map_flash_region = mst->spi.map_flash_region; + else if (mst->buses_supported & BUS_NONSPI) + map_flash_region = mst->par.map_flash_region; + void *ret; - if (programmer->map_flash_region) - ret = programmer->map_flash_region(descr, phys_addr, len); + if (map_flash_region) + ret = map_flash_region(descr, phys_addr, len); else ret = fallback_map(descr, phys_addr, len); msg_gspew("%s: mapping %s from 0x%0*" PRIxPTR " to 0x%0*" PRIxPTR "\n", - __func__, descr, PRIxPTR_WIDTH, phys_addr, PRIxPTR_WIDTH, (uintptr_t) ret); + __func__, descr, PRIxPTR_WIDTH, phys_addr, PRIxPTR_WIDTH, (uintptr_t) ret); return ret; } -void programmer_unmap_flash_region(void *virt_addr, size_t len) +void master_unmap_flash_region(const struct registered_master *mst, + void *virt_addr, size_t len) { - if (programmer->unmap_flash_region) - programmer->unmap_flash_region(virt_addr, len); + void (*unmap_flash_region) (void *virt_addr, size_t len) = NULL; + if (mst->buses_supported & BUS_PROG) + unmap_flash_region = mst->opaque.unmap_flash_region; + else if (mst->buses_supported & BUS_SPI) + unmap_flash_region = mst->spi.unmap_flash_region; + else if (mst->buses_supported & BUS_NONSPI) + unmap_flash_region = mst->par.unmap_flash_region; + + if (unmap_flash_region) + unmap_flash_region(virt_addr, len); else fallback_unmap(virt_addr, len); msg_gspew("%s: unmapped 0x%0*" PRIxPTR "\n", __func__, PRIxPTR_WIDTH, (uintptr_t)virt_addr); } +static bool master_uses_physmap(const struct registered_master *mst) +{ +#if CONFIG_INTERNAL == 1 + if (mst->buses_supported & BUS_PROG) + return mst->opaque.map_flash_region == physmap; + else if (mst->buses_supported & BUS_SPI) + return mst->spi.map_flash_region == physmap; + else if (mst->buses_supported & BUS_NONSPI) + return mst->par.map_flash_region == physmap; +#endif + return false; +} + void programmer_delay(unsigned int usecs) { if (usecs > 0) { @@ -647,13 +682,13 @@ void unmap_flash(struct flashctx *flash) { if (flash->virtual_registers != (chipaddr)ERROR_PTR) { - programmer_unmap_flash_region((void *)flash->virtual_registers, flash->chip->total_size * 1024); + master_unmap_flash_region(flash->mst, (void *)flash->virtual_registers, flash->chip->total_size * 1024); flash->physical_registers = 0; flash->virtual_registers = (chipaddr)ERROR_PTR; } if (flash->virtual_memory != (chipaddr)ERROR_PTR) { - programmer_unmap_flash_region((void *)flash->virtual_memory, flash->chip->total_size * 1024); + master_unmap_flash_region(flash->mst, (void *)flash->virtual_memory, flash->chip->total_size * 1024); flash->physical_memory = 0; flash->virtual_memory = (chipaddr)ERROR_PTR; } @@ -673,7 +708,7 @@ const chipsize_t size = flash->chip->total_size * 1024; uintptr_t base = flashbase ? flashbase : (0xffffffff - size + 1); - void *addr = programmer_map_flash_region(flash->chip->name, base, size); + void *addr = master_map_flash_region(flash->mst, flash->chip->name, base, size); if (addr == ERROR_PTR) { msg_perr("Could not map flash chip %s at 0x%0*" PRIxPTR ".\n", flash->chip->name, PRIxPTR_WIDTH, base); @@ -687,7 +722,7 @@ * Ignore these problems for now and always report success. */ if (flash->chip->feature_bits & FEATURE_REGISTERMAP) { base = 0xffffffff - size - 0x400000 + 1; - addr = programmer_map_flash_region("flash chip registers", base, size); + addr = master_map_flash_region(flash->mst, "flash chip registers", base, size); if (addr == ERROR_PTR) { msg_pdbg2("Could not map flash chip registers %s at 0x%0*" PRIxPTR ".\n", flash->chip->name, PRIxPTR_WIDTH, base); @@ -831,12 +866,10 @@ msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found", flash->chip->vendor, flash->chip->name, flash->chip->total_size, tmp); free(tmp); -#if CONFIG_INTERNAL == 1 - if (programmer->map_flash_region == physmap) + if (master_uses_physmap(mst)) msg_cinfo("mapped at physical address 0x%0*" PRIxPTR ".\n", PRIxPTR_WIDTH, flash->physical_memory); else -#endif msg_cinfo("on %s.\n", programmer->name); /* Flash registers may more likely not be mapped if the chip was forced. diff --git a/ichspi.c b/ichspi.c index fa15a95..7e7967e 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1773,6 +1773,8 @@ .max_data_write = 64, .command = ich_spi_send_command, .multicommand = ich_spi_send_multicommand, + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, @@ -1783,6 +1785,8 @@ .max_data_write = 64, .command = ich_spi_send_command, .multicommand = ich_spi_send_multicommand, + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, @@ -1790,6 +1794,8 @@ }; static const struct opaque_master opaque_master_ich_hwseq = { + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .max_data_read = 64, .max_data_write = 64, .probe = ich_hwseq_probe, @@ -2199,6 +2205,8 @@ .max_data_write = 16, .command = ich_spi_send_command, .multicommand = ich_spi_send_multicommand, + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, diff --git a/include/flash.h b/include/flash.h index 79aaa64..c696f64 100644 --- a/include/flash.h +++ b/include/flash.h @@ -55,8 +55,11 @@ #define PRIxPTR_WIDTH ((int)(sizeof(uintptr_t)*2)) int register_shutdown(int (*function) (void *data), void *data); -void *programmer_map_flash_region(const char *descr, uintptr_t phys_addr, size_t len); -void programmer_unmap_flash_region(void *virt_addr, size_t len); +struct registered_master; +void *master_map_flash_region(const struct registered_master *mast, + const char *descr, uintptr_t phys_addr, size_t len); +void master_unmap_flash_region(const struct registered_master *mast, + void *virt_addr, size_t len); void programmer_delay(unsigned int usecs); #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) diff --git a/include/programmer.h b/include/programmer.h index 17abfd2..4b5bf92 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -50,9 +50,6 @@ int (*init) (const struct programmer_cfg *cfg); - void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len); - void (*unmap_flash_region) (void *virt_addr, size_t len); - void (*delay) (unsigned int usecs); }; @@ -309,6 +306,8 @@ int (*multicommand)(const struct flashctx *flash, struct spi_command *cmds); /* Optimized functions for this master */ + void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len); + void (*unmap_flash_region) (void *virt_addr, size_t len); int (*read)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int (*write_256)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*write_aai)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); @@ -399,6 +398,8 @@ /* opaque.c */ struct opaque_master { + void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len); + void (*unmap_flash_region) (void *virt_addr, size_t len); int max_data_read; int max_data_write; /* Specific functions for this master */ @@ -424,6 +425,8 @@ /* parallel.c */ struct par_master { + void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len); + void (*unmap_flash_region) (void *virt_addr, size_t len); void (*chip_writeb) (const struct flashctx *flash, uint8_t val, chipaddr addr); void (*chip_writew) (const struct flashctx *flash, uint16_t val, chipaddr addr); void (*chip_writel) (const struct flashctx *flash, uint32_t val, chipaddr addr); diff --git a/internal.c b/internal.c index 2558cde..8c834e2 100644 --- a/internal.c +++ b/internal.c @@ -107,6 +107,8 @@ } static const struct par_master par_master_internal = { + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .chip_readb = internal_chip_readb, .chip_readw = internal_chip_readw, .chip_readl = internal_chip_readl, @@ -347,6 +349,4 @@ .type = OTHER, .devs.note = NULL, .init = internal_init, - .map_flash_region = physmap, - .unmap_flash_region = physunmap, }; diff --git a/it87spi.c b/it87spi.c index 85da150..5acaea6 100644 --- a/it87spi.c +++ b/it87spi.c @@ -315,6 +315,8 @@ .max_data_write = MAX_DATA_UNSPECIFIED, .command = it8716f_spi_send_command, .multicommand = default_spi_send_multicommand, + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .read = it8716f_spi_chip_read, .write_256 = it8716f_spi_chip_write_256, .write_aai = spi_chip_write_1, diff --git a/parallel.c b/parallel.c index 9001fe8..e635088 100644 --- a/parallel.c +++ b/parallel.c @@ -76,6 +76,17 @@ } } + /* Bus masters supporting FWH/LPC cannot use fallback_map(), distinct + * mappings are needed to support chips with FEATURE_REGISTERMAP + */ + if ((buses & (BUS_FWH | BUS_LPC)) && !mst->map_flash_region) { + msg_perr("%s called with incomplete master definition. " + "FWH/LPC masters must provide memory mappings. " + "Please report a bug at flashrom(a)flashrom.org\n", + __func__); + return ERROR_FLASHROM_BUG; + } + if (!mst->chip_writeb || !mst->chip_writew || !mst->chip_writel || !mst->chip_writen || !mst->chip_readb || !mst->chip_readw || !mst->chip_readl || !mst->chip_readn) { diff --git a/programmer.c b/programmer.c index 2b960e6..939d8c2 100644 --- a/programmer.c +++ b/programmer.c @@ -20,7 +20,12 @@ /* Fallback map() for programmers which don't need special handling */ void *fallback_map(const char *descr, uintptr_t phys_addr, size_t len) { - /* FIXME: Should return phys_addr. */ + /* A result of NULL causes mapped addresses to be chip physical + * addresses, assuming only a single region is mapped (the entire flash + * space). Chips with a second region (like a register map) require a + * real memory mapping to distinguish the different ranges. Those chips + * are FWH/LPC, so the bus master provides a real mapping. + */ return NULL; } diff --git a/sb600spi.c b/sb600spi.c index 8b02b51..6020145 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -600,6 +600,8 @@ .max_data_write = FIFO_SIZE_OLD - 3, .command = sb600_spi_send_command, .multicommand = default_spi_send_multicommand, + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, @@ -612,6 +614,8 @@ .max_data_write = FIFO_SIZE_YANGTZE - 3, .command = spi100_spi_send_command, .multicommand = default_spi_send_multicommand, + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, @@ -624,6 +628,8 @@ .max_data_write = FIFO_SIZE_YANGTZE - 3, .command = spi100_spi_send_command, .multicommand = default_spi_send_multicommand, + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .read = promontory_read_memmapped, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, diff --git a/serprog.c b/serprog.c index 12d15d7..a3a3db3 100644 --- a/serprog.c +++ b/serprog.c @@ -438,7 +438,23 @@ return 0; } +static void *serprog_map(const char *descr, uintptr_t phys_addr, size_t len) +{ + /* Serprog transmits 24 bits only and assumes the underlying implementation handles any remaining bits + * correctly (usually setting them to one either in software (for FWH/LPC) or relying on the fact that + * the hardware observes a subset of the address bits only). Combined with the standard mapping of + * flashrom this creates a 16 MB-wide window just below the 4 GB boundary where serprog can operate (as + * needed for non-SPI chips). Below we make sure that the requested range is within this window. */ + if ((phys_addr & 0xFF000000) == 0xFF000000) { + return (void*)phys_addr; + } + msg_pwarn(MSGHEADER "requested mapping %s is incompatible: 0x%zx bytes at 0x%0*" PRIxPTR ".\n", + descr, len, PRIxPTR_WIDTH, phys_addr); + return NULL; +} + static struct spi_master spi_master_serprog = { + .map_flash_region = serprog_map, .features = SPI_MASTER_4BA, .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, @@ -553,6 +569,7 @@ } static const struct par_master par_master_serprog = { + .map_flash_region = serprog_map, .chip_readb = serprog_chip_readb, .chip_readw = fallback_chip_readw, .chip_readl = fallback_chip_readl, @@ -945,27 +962,11 @@ sp_prev_was_write = 0; } -static void *serprog_map(const char *descr, uintptr_t phys_addr, size_t len) -{ - /* Serprog transmits 24 bits only and assumes the underlying implementation handles any remaining bits - * correctly (usually setting them to one either in software (for FWH/LPC) or relying on the fact that - * the hardware observes a subset of the address bits only). Combined with the standard mapping of - * flashrom this creates a 16 MB-wide window just below the 4 GB boundary where serprog can operate (as - * needed for non-SPI chips). Below we make sure that the requested range is within this window. */ - if ((phys_addr & 0xFF000000) == 0xFF000000) { - return (void*)phys_addr; - } - msg_pwarn(MSGHEADER "requested mapping %s is incompatible: 0x%zx bytes at 0x%0*" PRIxPTR ".\n", - descr, len, PRIxPTR_WIDTH, phys_addr); - return NULL; -} - const struct programmer_entry programmer_serprog = { .name = "serprog", .type = OTHER, /* FIXME */ .devs.note = "All programmer devices speaking the serprog protocol\n", .init = serprog_init, - .map_flash_region = serprog_map, .delay = serprog_delay, }; diff --git a/wbsio_spi.c b/wbsio_spi.c index 95ca1e9..661ecbb 100644 --- a/wbsio_spi.c +++ b/wbsio_spi.c @@ -187,6 +187,8 @@ .max_data_write = MAX_DATA_UNSPECIFIED, .command = wbsio_spi_send_command, .multicommand = default_spi_send_multicommand, + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .read = wbsio_spi_read, .write_256 = spi_chip_write_1, .write_aai = spi_chip_write_1, -- To view, visit https://review.coreboot.org/c/flashrom/+/67695 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9c3df6ae260bcdb246dfb0cd8e043919609b014b Gerrit-Change-Number: 67695 Gerrit-PatchSet: 12 Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: drivers: Move (un)map_flash_region to par/spi/opaque_master
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Attention is currently required from: Thomas Heijligen, Jonathon Hall, Nikolai Artemiev. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67695 ) Change subject: drivers: Move (un)map_flash_region to par/spi/opaque_master ...................................................................... Patch Set 11: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67695 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9c3df6ae260bcdb246dfb0cd8e043919609b014b Gerrit-Change-Number: 67695 Gerrit-PatchSet: 11 Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 08 Oct 2022 18:43:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: drivers: Move (un)map_flash_region to par/spi/opaque_master
by Jonathon Hall (Code Review) 08 Oct '22

08 Oct '22

Attention is currently required from: Thomas Heijligen, Angel Pons, Nikolai Artemiev. Jonathon Hall has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67695 ) Change subject: drivers: Move (un)map_flash_region to par/spi/opaque_master ...................................................................... Patch Set 11: (1 comment) File parallel.c: https://review.coreboot.org/c/flashrom/+/67695/comment/45181d55_53aff2de PS10, Line 82: | > nit: spacing needed around `|` Done -- To view, visit https://review.coreboot.org/c/flashrom/+/67695 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9c3df6ae260bcdb246dfb0cd8e043919609b014b Gerrit-Change-Number: 67695 Gerrit-PatchSet: 11 Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 08 Oct 2022 18:42:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Remove custom mappers from opaque_master
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Attention is currently required from: Thomas Heijligen, Jonathon Hall. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68092 ) Change subject: flashrom.c: Remove custom mappers from opaque_master ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68092 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I36f05154edda371b51f8ff416f019837ff1c243d Gerrit-Change-Number: 68092 Gerrit-PatchSet: 3 Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Comment-Date: Sat, 08 Oct 2022 18:41:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: dummyflasher.c: Remove custom mapper from opaque_master
by Angel Pons (Code Review) 08 Oct '22

08 Oct '22

Attention is currently required from: Thomas Heijligen, Jonathon Hall. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68091 ) Change subject: dummyflasher.c: Remove custom mapper from opaque_master ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68091 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I76ae3e0c2e91ecba4fd320941bd1eff038050731 Gerrit-Change-Number: 68091 Gerrit-PatchSet: 3 Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Comment-Date: Sat, 08 Oct 2022 18:41:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2022