Markus Armbruster <[email protected]> writes:
> From: Alex Bennée <[email protected]>
>
> We reject undersized images. As of the previous commit, even with a
> decent error message. Still, this is a potentially confusing
> stumbling block when you move from using -bios to using -drive
> if=pflash,file=blob,format=raw,readonly for loading your firmware
> code. To mitigate that we automatically pad in the read-only case and
> warn the user when we have performed magic to enable things to Just
> Work (tm).
>
> Signed-off-by: Alex Bennée <[email protected]>
> Reviewed-by: Laszlo Ersek <[email protected]>
> Signed-off-by: Markus Armbruster <[email protected]>
I think this convenience feature is a bad idea, and this patch should
not be applied. Here are my reasons.
1. As I explained in my disccussion of v5[*], there is no single true
way to pad. This patch pads with 0xFF. When working with physical
devices, you'd sometimes pad that way, but at other times, you'd pad
differently.
2. Except this patch doesn't *actually* pad with 0xFF. The block layer
silently pads with zeros up to the next multiple of 512. Evidence:
$ yes | dd of=4090b.img bs=1 count=4090
4090+0 records in
4090+0 records out
4090 bytes (4.1 kB, 4.0 KiB) copied, 0.0186459 s, 219 kB/s
$ qemu-io -f raw -c 'read -v 4000 96' 4090b.img
00000fa0: 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a y.y.y.y.y.y.y.y.
00000fb0: 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a y.y.y.y.y.y.y.y.
00000fc0: 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a y.y.y.y.y.y.y.y.
00000fd0: 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a y.y.y.y.y.y.y.y.
00000fe0: 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a y.y.y.y.y.y.y.y.
00000ff0: 79 0a 79 0a 79 0a 79 0a 79 0a 00 00 00 00 00 00 y.y.y.y.y.......
read 96/96 bytes at offset 4000
96 bytes, 1 ops; 0.0001 sec (694.444 KiB/sec and 7407.4074 ops/sec)
This patch then pads some more with 0xFF to the flash memory size.
Because of that the way the magic padding works makes no sense, to be
frank. Going back to v3's zero padding would at least be explainable
without blushing.
I consider the block layer's padding a misfeature here, but right now
we got to play the hand we've been dealt.
3. Convenience magic has bitten us in the posterior so many times. Just
say no unless there's a really compelling use case. Where's the use
case for this one? We've rejected undersized images for ages, and
nobody complained. Why add convenience magic now?
[*] Message-ID: <[email protected]>
https://lists.nongnu.org/archive/html/qemu-devel/2019-03/msg01115.html
