On 15/2/23 20:52, Peter Delevoryas wrote:
On Tue, Feb 14, 2023 at 06:18:23PM +0100, Cédric Le Goater wrote:
Currently, when a block backend is attached to a m25p80 device and the
associated file size does not match the flash model, QEMU complains
with the error message "failed to read the initial flash content".
This is confusing for the user.

Use blk_check_size_and_read_all() instead of blk_pread() to improve
the reported error.

Could you reference commit 06f1521795?

Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
Reviewed-by: Peter Delevoryas <pe...@pjd.dev>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Message-Id: <20221115151000.2080833-1-...@kaod.org>
Signed-off-by: Cédric Le Goater <c...@kaod.org>
---

   breakage with commit a4b15a8b9e ("pflash: Only read non-zero parts
   of backend image") when using -snaphot.


I guess it's not obvious to me, what broke?

1. BlockBackend *blk = -drive file=blob.mtd,format=raw,if=mtd,snapshot=on
2. uint8_t *storage = malloc(...)
3. blk_check_size_and_read_all(blk, storage, size)
4. commit a4b15a8b9e
     for sector in blk:
         if any(b != 0 for b in sector):
             memcpy(&storage[...], sector, sizeof(sector))
         else:
             # Skip memcpy of zeroes

But this is probably a regression for us because we actually expect the zeroes
to be copied?

Yes... Commit a4b15a8b9e mostly considered cloud payload optimization.

Since EEPROMs are usually "small", this could be kludged as:

-- >8 --
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -1615,6 +1615,7 @@ static void m25p80_realize(SSIPeripheral *ss, Error **errp)
         trace_m25p80_binding(s);
         s->storage = blk_blockalign(s->blk, s->size);

+        memset(s->storage, ERASED_BYTE, s->size);
         if (blk_pread(s->blk, 0, s->size, s->storage, 0) < 0) {
             error_setg(errp, "failed to read the initial flash content");
             return;
---

On emulation, it is simpler to ask the user to provide a block image
with the same size of the emulated device. See commit 06f1521795
("pflash: Require backend size to match device, improve errors") for
more information.

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 802d2eb021..dc5ffbc4ff 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -24,6 +24,7 @@
  #include "qemu/osdep.h"
  #include "qemu/units.h"
  #include "sysemu/block-backend.h"
+#include "hw/block/block.h"
  #include "hw/qdev-properties.h"
  #include "hw/qdev-properties-system.h"
  #include "hw/ssi/ssi.h"
@@ -1615,8 +1616,7 @@ static void m25p80_realize(SSIPeripheral *ss, Error 
**errp)
          trace_m25p80_binding(s);
          s->storage = blk_blockalign(s->blk, s->size);
- if (blk_pread(s->blk, 0, s->size, s->storage, 0) < 0) {
-            error_setg(errp, "failed to read the initial flash content");
+        if (!blk_check_size_and_read_all(s->blk, s->storage, s->size, errp)) {
              return;
          }
      } else {
--
2.39.1



Reply via email to