On 7/24/20 11:50 PM, Jason Wessel wrote:
> While using u-boot with qemu's virtio driver I stumbled across a
> problem reading files less than sector size. On the real hardware the
> block reader seems ok with reading zero blocks, and while we could fix
> the virtio host side of qemu to deal with a zero block read instead of
> crashing, the u-boot fat driver should not be doing zero block reads
> in the first place. If you ask hardware to read zero blocks you are
> just going to get zero data. There may also be other hardware that
> responds similarly to the virtio interface so this is worth fixing.
>
> Without the patch I get the following and have to restart qemu because
> it dies.
Our block drivers all have a different view on this:
mmc_read_blocks() is reading one block even if 0 blocks are requested.
scsi_setup_read16() happily passes the 0 to the controller.
I think blk_read_devnum(), blk_write_devnum(), blk_dread(), and
blk_dwrite() is where we need the block count check.
> ---------------------------------
> => fatls virtio 0:1
> 30 cmdline.txt
> => fatload virtio 0:1 ${loadaddr} cmdline.txt
> qemu-system-aarch64: virtio: zero sized buffers are not allowed
> ---------------------------------
>
> With the patch I get the expected results.
> ---------------------------------
> => fatls virtio 0:1
> 30 cmdline.txt
> => fatload virtio 0:1 ${loadaddr} cmdline.txt
> 30 bytes read in 11 ms (2 KiB/s)
> => md.b ${loadaddr} 0x1E
> 40080000: 64 77 63 5f 6f 74 67 2e 6c 70 6d 5f 65 6e 61 62 dwc_otg.lpm_enab
> 40080010: 6c 65 3d 30 20 72 6f 6f 74 77 61 69 74 0a le=0 rootwait.
>
> ---------------------------------
>
> Signed-off-by: Jason Wessel <jason.wes...@windriver.com>
> Reviewed-by: Tom Rini <tr...@konsulko.com>
> ---
> fs/fat/fat.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 9578b74bae..28aa5aaa9f 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -278,7 +278,10 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8
> *buffer, unsigned long size)
> }
> } else {
> idx = size / mydata->sect_size;
> - ret = disk_read(startsect, idx, buffer);
> + if (idx == 0)
> + ret = 0;
> + else
> + ret = disk_read(startsect, idx, buffer);
Using idx as variable name here for a block count is quite misleading.
Best regards
Heinrich
> if (ret != idx) {
> debug("Error reading data (got %d)\n", ret);
> return -1;
>
