mirror of
https://review.coreboot.org/flashrom.git
synced 2025-04-26 22:52:34 +02:00
Correct get_flash_region() to use inclusive upper bounds
get_flash_region() emits a struct flash_region, which uses chipoff_t for the start and end addresses of a region. chipoff_t is defined as a valid flash address, so it was wrong to be setting the end address to start + len; this is clearly wrong in the case where there is a single region because setting end to the flash size generates an address that is beyond the end of the chip (due to zero-indexing). This changes the one actual implementation of .get_region in ichspi.c to use inclusive upper bounds, and corrects all callers of get_flash_region() to treat the upper bounds as inclusive. Overall this reduces complexity slightly by removing more downward adjustments by 1 than it needs to add upward adjustments. TEST=on yaviks, `flashrom -V -x` prints equivalent messages about "x region (0xZZZZ..0xZZZZ) is readable" before and after this patch. Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec Signed-off-by: Peter Marheine <pmarheine@chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82496 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Hsuan-ting Chen <roccochen@google.com> Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
This commit is contained in:
parent
d7e4240263
commit
140d65b4dd
@ -318,13 +318,13 @@ int erase_write(struct flashctx *const flashctx, chipoff_t region_start, chipoff
|
||||
struct flash_region region;
|
||||
get_flash_region(flashctx, addr, ®ion);
|
||||
|
||||
len = min(start_addr + block_len, region.end) - addr;
|
||||
len = min(start_addr + block_len, region.end + 1) - addr;
|
||||
|
||||
if (region.write_prot) {
|
||||
msg_gdbg("%s: cannot erase inside %s "
|
||||
"region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n",
|
||||
__func__, region.name,
|
||||
region.start, region.end - 1,
|
||||
region.start, region.end,
|
||||
addr, addr + len - 1);
|
||||
free(region.name);
|
||||
continue;
|
||||
@ -333,7 +333,7 @@ int erase_write(struct flashctx *const flashctx, chipoff_t region_start, chipoff
|
||||
msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is "
|
||||
"writable, erasing range (%#08x..%#08x).\n",
|
||||
__func__, region.name,
|
||||
region.start, region.end - 1,
|
||||
region.start, region.end,
|
||||
addr, addr + len - 1);
|
||||
free(region.name);
|
||||
|
||||
|
34
flashrom.c
34
flashrom.c
@ -379,7 +379,7 @@ void get_flash_region(const struct flashctx *flash, int addr, struct flash_regio
|
||||
} else {
|
||||
region->name = strdup("");
|
||||
region->start = 0;
|
||||
region->end = flashrom_flash_getsize(flash);
|
||||
region->end = flashrom_flash_getsize(flash) - 1;
|
||||
region->read_prot = false;
|
||||
region->write_prot = false;
|
||||
}
|
||||
@ -388,12 +388,12 @@ void get_flash_region(const struct flashctx *flash, int addr, struct flash_regio
|
||||
int check_for_unwritable_regions(const struct flashctx *flash, unsigned int start, unsigned int len)
|
||||
{
|
||||
struct flash_region region;
|
||||
for (unsigned int addr = start; addr < start + len; addr = region.end) {
|
||||
for (unsigned int addr = start; addr < start + len; addr = region.end + 1) {
|
||||
get_flash_region(flash, addr, ®ion);
|
||||
|
||||
if (region.write_prot) {
|
||||
msg_gerr("%s: cannot write/erase inside %s region (%#08"PRIx32"..%#08"PRIx32").\n",
|
||||
__func__, region.name, region.start, region.end - 1);
|
||||
__func__, region.name, region.start, region.end);
|
||||
free(region.name);
|
||||
return -1;
|
||||
}
|
||||
@ -596,14 +596,14 @@ int read_flash(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigne
|
||||
struct flash_region region;
|
||||
get_flash_region(flash, addr, ®ion);
|
||||
|
||||
read_len = min(start + len, region.end) - addr;
|
||||
read_len = min(start + len, region.end + 1) - addr;
|
||||
uint8_t *rbuf = buf + addr - start;
|
||||
|
||||
if (region.read_prot) {
|
||||
if (flash->flags.skip_unreadable_regions) {
|
||||
msg_gdbg("%s: cannot read inside %s region (%#08"PRIx32"..%#08"PRIx32"), "
|
||||
"filling (%#08x..%#08x) with erased value instead.\n",
|
||||
__func__, region.name, region.start, region.end - 1,
|
||||
__func__, region.name, region.start, region.end,
|
||||
addr, addr + read_len - 1);
|
||||
free(region.name);
|
||||
|
||||
@ -612,12 +612,12 @@ int read_flash(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigne
|
||||
}
|
||||
|
||||
msg_gerr("%s: cannot read inside %s region (%#08"PRIx32"..%#08"PRIx32").\n",
|
||||
__func__, region.name, region.start, region.end - 1);
|
||||
__func__, region.name, region.start, region.end);
|
||||
free(region.name);
|
||||
return -1;
|
||||
}
|
||||
msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is readable, reading range (%#08x..%#08x).\n",
|
||||
__func__, region.name, region.start, region.end - 1, addr, addr + read_len - 1);
|
||||
__func__, region.name, region.start, region.end, addr, addr + read_len - 1);
|
||||
free(region.name);
|
||||
|
||||
read_func_t *read_func = lookup_read_func_ptr(flash->chip);
|
||||
@ -665,25 +665,25 @@ int verify_range(struct flashctx *flash, const uint8_t *cmpbuf, unsigned int sta
|
||||
for (size_t addr = start; addr < start + len; addr += read_len) {
|
||||
struct flash_region region;
|
||||
get_flash_region(flash, addr, ®ion);
|
||||
read_len = min(start + len, region.end) - addr;
|
||||
read_len = min(start + len, region.end + 1) - addr;
|
||||
|
||||
if ((region.write_prot && flash->flags.skip_unwritable_regions) ||
|
||||
(region.read_prot && flash->flags.skip_unreadable_regions)) {
|
||||
msg_gdbg("%s: Skipping verification of %s region (%#08"PRIx32"..%#08"PRIx32")\n",
|
||||
__func__, region.name, region.start, region.end - 1);
|
||||
__func__, region.name, region.start, region.end);
|
||||
free(region.name);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (region.read_prot) {
|
||||
msg_gerr("%s: Verification imposible because %s region (%#08"PRIx32"..%#08"PRIx32") is unreadable.\n",
|
||||
__func__, region.name, region.start, region.end - 1);
|
||||
__func__, region.name, region.start, region.end);
|
||||
free(region.name);
|
||||
goto out_free;
|
||||
}
|
||||
|
||||
msg_gdbg("%s: Verifying %s region (%#08"PRIx32"..%#08"PRIx32")\n",
|
||||
__func__, region.name, region.start, region.end - 1);
|
||||
__func__, region.name, region.start, region.end);
|
||||
free(region.name);
|
||||
|
||||
ret = read_flash(flash, readbuf, addr, read_len);
|
||||
@ -1046,18 +1046,18 @@ int write_flash(struct flashctx *flash, const uint8_t *buf,
|
||||
struct flash_region region;
|
||||
get_flash_region(flash, addr, ®ion);
|
||||
|
||||
write_len = min(start + len, region.end) - addr;
|
||||
write_len = min(start + len, region.end + 1) - addr;
|
||||
const uint8_t *rbuf = buf + addr - start;
|
||||
|
||||
if (region.write_prot) {
|
||||
msg_gdbg("%s: cannot write inside %s region (%#08"PRIx32"..%#08"PRIx32"), skipping (%#08x..%#08x).\n",
|
||||
__func__, region.name, region.start, region.end - 1, addr, addr + write_len - 1);
|
||||
__func__, region.name, region.start, region.end, addr, addr + write_len - 1);
|
||||
free(region.name);
|
||||
continue;
|
||||
}
|
||||
|
||||
msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is writable, writing range (%#08x..%#08x).\n",
|
||||
__func__, region.name, region.start, region.end - 1, addr, addr + write_len - 1);
|
||||
__func__, region.name, region.start, region.end, addr, addr + write_len - 1);
|
||||
|
||||
write_func_t *write_func = lookup_write_func_ptr(flash->chip);
|
||||
int ret = write_func(flash, rbuf, addr, write_len);
|
||||
@ -1511,17 +1511,17 @@ static int erase_block(struct flashctx *const flashctx,
|
||||
struct flash_region region;
|
||||
get_flash_region(flashctx, addr, ®ion);
|
||||
|
||||
len = min(info->erase_start + erase_len, region.end) - addr;
|
||||
len = min(info->erase_start + erase_len, region.end + 1) - addr;
|
||||
|
||||
if (region.write_prot) {
|
||||
msg_gdbg("%s: cannot erase inside %s region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n",
|
||||
__func__, region.name, region.start, region.end - 1, addr, addr + len - 1);
|
||||
__func__, region.name, region.start, region.end, addr, addr + len - 1);
|
||||
free(region.name);
|
||||
continue;
|
||||
}
|
||||
|
||||
msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is writable, erasing range (%#08x..%#08x).\n",
|
||||
__func__, region.name, region.start, region.end - 1, addr, addr + len - 1);
|
||||
__func__, region.name, region.start, region.end, addr, addr + len - 1);
|
||||
free(region.name);
|
||||
|
||||
if (erasefn(flashctx, addr, len))
|
||||
|
4
ichspi.c
4
ichspi.c
@ -1436,7 +1436,7 @@ static void ich_get_region(const struct flashctx *flash, unsigned int addr, stru
|
||||
region->read_prot = false;
|
||||
region->write_prot = false;
|
||||
region->start = 0;
|
||||
region->end = flashrom_flash_getsize(flash);
|
||||
region->end = flashrom_flash_getsize(flash) - 1;
|
||||
|
||||
for (ssize_t i = 0; i < nr; i++) {
|
||||
uint32_t base = fd_regions[i].base;
|
||||
@ -1459,7 +1459,7 @@ static void ich_get_region(const struct flashctx *flash, unsigned int addr, stru
|
||||
/* fd_regions[i] contains addr, copy to *region. */
|
||||
name = fd_regions[i].name;
|
||||
region->start = base;
|
||||
region->end = limit + 1;
|
||||
region->end = limit;
|
||||
region->read_prot = (level == LOCKED) || (level == READ_PROT);
|
||||
region->write_prot = (level == LOCKED) || (level == WRITE_PROT);
|
||||
break;
|
||||
|
Loading…
x
Reference in New Issue
Block a user