qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Clément Chigot" <chigot@adacore.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org, hreitz@redhat.com,  qemu-block@nongnu.org
Subject: Re: [PATCH 5/5] vvfat: add support for "size" options
Date: Tue, 28 Oct 2025 15:54:07 +0100	[thread overview]
Message-ID: <CAJ307EhFGHrTYcVJZghCNgGFmXZf1vTNg2E=7AA41jFvRqtGcg@mail.gmail.com> (raw)
In-Reply-To: <87ms5cmtqh.fsf@pond.sub.org>

On Mon, Oct 27, 2025 at 1:09 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Clément Chigot <chigot@adacore.com> writes:
>
> > On Fri, Oct 24, 2025 at 10:35 AM Markus Armbruster <armbru@redhat.com> wrote:
> >>
> >> Kevin Wolf <kwolf@redhat.com> writes:
> >>
> >> > Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
> >> >> This allows more flexibility to vvfat backend. The value for "Number of
> >> >> Heads" and "Sectors per track" are based on SD specifications Part 2.
> >>
> >> This is too terse to remind me of how vvfat picks cylinders, heads, and
> >> sectors before this patch, so I need to go dig through the source code.
> >> I figure it depends on configuration parameters @floppy and @fat-type
> >> like this:
> >>
> >>     floppy  fat-type    cyls heads secs   cyls*heads*secs*512
> >>     false      12         64    16   63         31.5 MiB
> >>     false      16       1024    16   63        504   MiB
> >>     false      32       1024    16   63        504   MiB
> >>     true       12         80     2   18       1440   KiB
> >>     true       16         80     2   36       2880   KiB
> >>     true       32         80     2   36       2880   KiB
> >>
> >> How exactly does the new parameter @size change this?
> >
> > My prime goal was to create a 256 Mib VVFAT disk. As you can see,
> > today for hard-disks there are only two possibilities: 31.5 Mib or 504
> > Mib. Hence, I've introduced the option `size=xxx` to allow more
> > granular choices.
> > This option changes how cyls, heads and secs parameters are computed
> > to be as closed as possible of its value.
> >
> > I did try to keep it simple. I could have introduced options to select
> > cylinders, heads, etc. But I think "size=xxx" would be more intuitive.
> > There are also approximations made, as not all sizes can be reached. I
> > didn't add errors or warnings for them. I'm fine adding them.
>
> I don't have an opinion on whether we should support more sizes and/or
> provide full control over CHS geometry.
>
> >> >> Some limitations remains, the size parameter is recognized only when
> >> >> "format=vvfat" is passed. In particular, "format=raw,size=xxx" is
> >> >> keeping the previously hardcoded value: 504MB for FAT16 and 32 MB for
> >> >> FAT12. FAT32 has not been adjusted and thus still default to 504MB.
> >>
> >> 31.5MiB unless I'm mistaken.
> >
> > True, I will fix it.
> >
> >> I'm not sure what you're trying to convey in this paragraph.  As far as
> >> I can tell, you're adding a @size parameter to vvfat, so of course it
> >> doesn't affect raw.
> >
> > Yes, but AFAICT, `if=sd,format=raw` will result in vvfat backend being
> > called. I didn't manage to make the new option work with
> > `if=sd,format=raw,size=256Mb`. Thus, when the "size" option is not
> > provided, I keep the previous value (those for your above comment).
> > Hence this paragraph to mostly warn people about the current
> > limitation.
>
> Are you talking about -drive?
>
> Complete examples, please.
>
> I'm confused about the connection between SD (from if=sd here, and "SD
> specification" above) and vvfat.  SD is a frontend.  vvfat is a backend.

Alright, I'll try to explain how I came up with this patch. And sorry
if it's a bit blurry, I made it some months ago hence I don't remember
all the details...
So, first, my prime goal was to access a local folder in a QNX system
running on Raspi 4B emulation.
My usual way to pass such a local folder is through `-drive
file=fat:rw:<host_folder>,format=raw`. For virt, it's usually
connected to virtio-blk-device: `-drive id=disk0,if=none,... -device
virtio-blk-device,drive=disk0`. For the Raspi 4b, adding the
virtio-blk-device is not possible, hence I have to connect it as a SD
card: `-drive if=sd,...`.

However, without any `size=` argument, QEMU will complain that the SD
card has not a valid size:
  | (host) $ qemu-system-aarch64 -M raspi4b -kernel raspi4b-kernel
-nographic -no-reboot -append "earlycon=pl011,mmio32,0xfe201000
console=ttyAMA0 noreboot" -dtb bcm2711-rpi-4-b.dtb -initrd
rootfs.cpio.gz  -drive
id=sdcard,file=fat:rw:<host_folder>,format=raw,if=sd
  | qemu-system-aarch64: Invalid SD card size: 504 MiB
  | SD card size has to be a power of 2, e.g. 512 MiB.
  | You can resize disk images with 'qemu-img resize <imagefile> <new-size>'
  | (note that this will lose data if you make the image smaller than
it currently is).

("raspi4b-kernel", the dtb and the rootfs come from
functional/aarch64/test_raspi4.py)

Hence, I've added `size=256M` to reduce the size of that SD card and
make QEMU happy. This allows me to mount my host folder on Linux:
  | (host) $ qemu-system-aarch64 -M raspi4b -kernel raspi4b-kernel
-nographic -no-reboot -append "earlycon=pl011,mmio32,0xfe201000
console=ttyAMA0 noreboot" -dtb bcm2711-rpi-4-b.dtb -initrd
rootfs.cpio.gz  -drive
id=sdcard,file=fat:rw:<host_folder>,format=raw,if=sd,size=256M
  | (QEMU) # fdisk -l /dev/mmcblk1
  | Disk /dev/mmcblk1: 256 MB, 268435456 bytes, 524288 sectors
  | 520 cylinders, 16 heads, 63 sectors/track
  | Units: sectors of 1 * 512 = 512 bytes
  |
  | Device       Boot StartCHS    EndCHS        StartLBA     EndLBA
Sectors  Size Id Type
  | /dev/mmcblk1p1 *  0,1,1       1023,15,63          63    1032191
1032129  503M  6 FAT16

As you can see the "Disk" has the right size (256MB) but the partition
still has a 503M size. However, Linux doesn't seem to care too much
about that as I'm able to mount this partition, and perform IO
operations.
  | (QEMU) # mount /dev/mmcblk1p1 /mnt
  | (QEMU) # ls /mnt
  | file.txt
  | (QEMU) # cat /mnt/file.txt
  | Hello World
  | (QEMU) # echo "OK" > /mnt/test.txt
  | (host) $ cat <host_folder>/test.txt
  | OK

Now, QNX comes into play.
First, the SD card must be connected to another bus. That patch is not
part of this series as I'm considering it a QNX issue. Just FTR here
is the patch:
  | --- a/hw/arm/bcm2838_peripherals.c
  | +++ b/hw/arm/bcm2838_peripherals.c
  | @@ -190,7 +190,7 @@ static void
bcm2838_peripherals_realize(DeviceState *dev, Error **errp)
  |          &s_base->peri_mr, GPIO_OFFSET,
  |          sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->gpio), 0));
  |
  | -    object_property_add_alias(OBJECT(s), "sd-bus",
OBJECT(&s->gpio), "sd-bus");
  | +    object_property_add_alias(OBJECT(s), "sd-bus",
OBJECT(&s->emmc2), "sd-bus");
  |
  |      /* BCM2838 RPiVid ASB must be mapped to prevent kernel crash */

Afterwards, QNX is able to see the SD card, but not able to mount it.
It complains about the filesystem being corrupted. Looking at `fdisk`
output, it shows a mismatch between the "total section" value and the
Cylinders/Heads/etc values.
  | (QEMU) # fdisk /dev/hd0 info
  | Physical disk characteristics: (/dev/hd0)
  |     Disk type        : Direct Access (0)
  |     Cylinders        : 520
  |     Heads            : 16
  |     Sectors/Track    : 63
  |     Total Sectors    : 524288
  |     Block Size       : 512
  |
  | Warning: total sectors field does not agree with
  |            cylinders*sectors/track*heads!! (524288 vs 524160)

The "no-mbr" option introduced in patch 1 is something we (Adacore's
QEMU team) have for a long time. I don't remember the details but we
are using it for other OSes as well (notably RTEMS).
Once added, and the drive command line updated for `-drive
id=sdcard,file=fat:rw:no-mbr:<host_folder>,format=raw,if=sd,size=256M`,
I don't have this warning anymore. Though, I'm still getting the
corrupted filesystem error.

Afterwards, it's a bit blurry but I think by trial and errors we ended
up removing the SD size error and realize that `-drive
id=sdcard,file=fat:rw:no-mbr:<host_folder>,format=raw,if=sd` was
working. However, `size=256M` still results in a corrupted filesystem.
As a comment in vvfat.c states that it either creates a "32MB or 504
MB disk". I decided to check if I can adapt, hence this patch.

I didn't find any VFAT documentation explaining the relation between
the size and the cylinders, heads, sector per track values. However,
the SD documentation was giving some recommandations, hence I used it
as a base.

I was unable to make `vvfat.c` recognize the "size" argument passed
along `format=raw`, even if hardcoding the value in `vvfat.c` did make
a difference. And that's why I'm adding the warning in the commit
message.

I've also realized that following my patch, the mismatch in between
the disk and the partition in Linux was going away when using `-drive
format=vvfat,size=xxx`. Making it not just QNX-oriented.
  | (host) $ qemu-system-aarch64 -M raspi4b -kernel raspi4b-kernel
-nographic -no-reboot -append "earlycon=pl011,mmio32,0xfe201000
console=ttyAMA0 noreboot" -dtb bcm2711-rpi-4-b.dtb -initrd
rootfs.cpio.gz  -drive
id=sdcard,file=fat:rw:<host_folder>,format=vvfat,if=sd,size=256M
  | (QEMU) # fdisk -l /dev/mmcblk1
  | Disk /dev/mmcblk1: 256 MB, 268435456 bytes, 524288 sectors
  | 1024 cylinders, 16 heads, 32 sectors/track
  | Units: sectors of 1 * 512 = 512 bytes
  |
  | Device       Boot StartCHS    EndCHS        StartLBA     EndLBA
Sectors  Size Id Type
  | /dev/mmcblk1p1 *  0,1,32      1023,15,32          63     524287
 524225  255M  6 FAT16

I probably missed a few things, but I hope this is clearer to you why
and how this series was made.


  reply	other threads:[~2025-10-28 14:55 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-03  7:57 [PATCH 0/5] block/vvfat: introduce "size" option Clément Chigot
2025-09-03  7:57 ` [PATCH 1/5] vvfat: introduce no-mbr option Clément Chigot
2025-10-23 18:20   ` Kevin Wolf
2025-10-29  8:37     ` Clément Chigot
2025-10-29 10:56       ` Kevin Wolf
2025-10-29 13:44         ` Clément Chigot
2025-09-03  7:57 ` [PATCH 2/5] vvfat: move fat_type check prior to size setup Clément Chigot
2025-10-23 18:39   ` Kevin Wolf
2025-10-29 13:48     ` Clément Chigot
2025-10-29 13:58       ` BALATON Zoltan
2025-10-29 16:05         ` Kevin Wolf
2025-09-03  7:57 ` [PATCH 3/5] vvfat: add a define for SECTOR_SIZE Clément Chigot
2025-10-23 18:47   ` Kevin Wolf
2025-09-03  7:57 ` [PATCH 4/5] vvfat: move size parameters within driver structure Clément Chigot
2025-09-03  7:57 ` [PATCH 5/5] vvfat: add support for "size" options Clément Chigot
2025-10-23 19:29   ` Kevin Wolf
2025-10-24  8:30     ` Markus Armbruster
2025-10-24  9:23       ` Clément Chigot
2025-10-27 12:09         ` Markus Armbruster
2025-10-28 14:54           ` Clément Chigot [this message]
2025-10-31  7:46             ` Markus Armbruster
2025-10-31  9:47               ` Clément Chigot
2025-10-31 11:56                 ` Kevin Wolf
2025-10-31 13:07                   ` Clément Chigot
2025-11-05  7:06                     ` Markus Armbruster
2025-11-05  7:08     ` Markus Armbruster
2025-11-05  9:35       ` Kevin Wolf
2025-11-05 10:15         ` Clément Chigot
2025-11-07  8:06         ` Markus Armbruster
2025-09-15  8:47 ` [PATCH 0/5] block/vvfat: introduce "size" option Clément Chigot
2025-10-07  7:43 ` Clément Chigot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJ307EhFGHrTYcVJZghCNgGFmXZf1vTNg2E=7AA41jFvRqtGcg@mail.gmail.com' \
    --to=chigot@adacore.com \
    --cc=armbru@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).