Alex Bennée <[email protected]> writes: > Markus Armbruster <[email protected]> writes: > >> From: Alex Bennée <[email protected]> >> >> It looks like there was going to be code to check we had some sort of >> alignment so let's replace it with an actual check. Reject undersized >> images with "device needs N bytes, backing file provides only M >> bytes". This is a bit more useful than the enigmatic "failed to read >> the initial flash content" when we attempt to read the number of bytes >> the device should have. Continue to accept oversized images, but warn >> "device needs N bytes, rest ignored". >> >> Signed-off-by: Alex Bennée <[email protected]> >> Reviewed-by: Laszlo Ersek <[email protected]> >> Signed-off-by: Markus Armbruster <[email protected]> >> --- >> hw/block/pflash_cfi01.c | 29 ++++++++++++++++++++++------- >> 1 file changed, 22 insertions(+), 7 deletions(-) >> >> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c >> index 9d1c356eb6..75ce8ef489 100644 >> --- a/hw/block/pflash_cfi01.c >> +++ b/hw/block/pflash_cfi01.c >> @@ -45,6 +45,7 @@ >> #include "qemu/bitops.h" >> #include "qemu/host-utils.h" >> #include "qemu/log.h" >> +#include "qemu/error-report.h" >> #include "hw/sysbus.h" >> #include "sysemu/sysemu.h" >> #include "trace.h" >> @@ -730,13 +731,6 @@ static void pflash_cfi01_realize(DeviceState *dev, >> Error **errp) >> } >> device_len = sector_len_per_device * blocks_per_device; >> >> - /* XXX: to be fixed */ >> -#if 0 >> - if (total_len != (8 * 1024 * 1024) && total_len != (16 * 1024 * 1024) && >> - total_len != (32 * 1024 * 1024) && total_len != (64 * 1024 * 1024)) >> - return NULL; >> -#endif >> - >> memory_region_init_rom_device( >> &pfl->mem, OBJECT(dev), >> &pflash_cfi01_ops, >> @@ -763,6 +757,27 @@ static void pflash_cfi01_realize(DeviceState *dev, >> Error **errp) >> } >> >> if (pfl->blk) { >> + /* >> + * Validate the backing store is the right size for pflash >> + * devices. If the user supplies a larger file we ignore the >> + * tail. >> + */ >> + int64_t backing_len = blk_getlength(pfl->blk); >> + if (backing_len < 0) { >> + error_setg(errp, "unable to check size of backing file"); >> + return; >> + } >> + >> + if (backing_len < total_len) { >> + error_setg(errp, "device needs %" PRIu64 " bytes, " >> + "backing file provides only %" PRIu64 " bytes", >> + total_len, backing_len); >> + return; >> + } else if (backing_len > total_len) { >> + warn_report("device needs %" PRIu64 " bytes, rest ignored", >> + total_len); >> + } > > As discussed elsewhere I'm happy for this just to error out with a > useful error message.
I'll do that in the next spin. Thanks! >> + >> /* read the initial flash content */ >> ret = blk_pread(pfl->blk, 0, pfl->storage, total_len); > > > -- > Alex Bennée
