[PATCH v4 32/45] fs: fat: Support reading from a larger block size

[Simon Glass]

Hi Bin,

On Wed, 2 Aug 2023 at 07:18, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Wed, Aug 2, 2023 at 8:50 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Tue, 1 Aug 2023 at 07:47, Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Aug 1, 2023 at 9:26 PM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Mon, 31 Jul 2023 at 04:54, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > >
> > > > > On Mon, Jul 31, 2023 at 6:33 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Fri, Jul 28, 2023 at 7:56 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Wed, 19 Jul 2023 at 01:58, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > On Wed, Jul 19, 2023 at 9:11 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > > > > >
> > > > > > > > > Hi Bin,
> > > > > > > > >
> > > > > > > > > On Sun, 16 Jul 2023 at 09:19, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Simon,
> > > > > > > > > >
> > > > > > > > > > On Sun, Jul 16, 2023 at 7:42 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Bin,
> > > > > > > > > > >
> > > > > > > > > > > On Thu, 13 Jul 2023 at 04:49, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Simon,
> > > > > > > > > > > >
> > > > > > > > > > > > On Mon, Jun 19, 2023 at 8:02 PM Simon Glass <sjg at chromium.org> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > At present it is not possible to read from some CDROM drives since the
> > > > > > > > > > > > > FAT sector size does not match the media's block size. Add a conversion
> > > > > > > > > > > > > option for this, so that reading is possible.
> > > > > > > > > > > >
> > > > > > > > > > > > I am completely confused. The CDROM uses iso9660 file system. This has
> > > > > > > > > > > > nothing to do with FAT.
> > > > > > > > > > >
> > > > > > > > > > > It actually can use both - this is the -cdrom option in QEMU which can
> > > > > > > > > > > emulate an old-style CDROM, with a FAT filesystem on it!
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > What QEMU command line is this to enable a CDROM with FAT file system?
> > > > > > > > > >
> > > > > > > > > > If that is the case, what you changed in this commit only works with
> > > > > > > > > > QEMU, not with any real-world devices, I believe?
> > > > > > > > > >
> > > > > > > > > > Could you use iso9660 instead?
> > > > > > > > >
> > > > > > > > > Sure, but the image I am using has an MSDOS partition name, an EFI
> > > > > > > > > partition table, a FAT filesystem and an ISO9660 filesystem! I was
> > > > > > > > > trying to look at the FAT filesystem via the MSDOS partition table.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Is that a released installer iso that I can download somewhere?
> > > > > > > >
> > > > > > > > Which QEMU command line should I use for reproducing the issue?
> > > > > > >
> > > > > > > Here is what I did:
> > > > > > >
> > > > > > > qemu-system-x86_64 -m 2G  -smp 4 -bios /tmp/b/qemu-x86/u-boot.rom
> > > > > > > -cdrom /vid/software/linux/debian/debian-11.5.0-i386-DVD-1.iso -serial
> > > > > > > mon:stdio
> > > > > > > => part set ide 2 iso
> > > > > > >
> > > > > > > Partition Map for IDE device 2  --   Partition Type: ISO
> > > > > > >
> > > > > > > Part   Start     Sect x Size Type
> > > > > > >   1     6912        1   2048 U-Boot
> > > > > > >   2     5912     1000   2048 U-Boot
> > > > > > > => ls ide 2:2
> > > > > > > FAT sector size mismatch (fs=512, dev=2048): translating for read-only
> > > > > > >             efi/
> > > > > > >       160   ubootefi.var
> > > > > > >
> > > > > > > 1 file(s), 1 dir(s)
> > > > > > >
> > > > > > > =>
> > > > > > >
> > > > > > > Without this patch we cannot enable translation and it is not possible
> > > > > > > to read the files.
> > > > > >
> > > > > > I cannot reproduce this issue.
> > > > > >
> > > > > > I investigated this a little bit, and it looks the iso image you are
> > > > > > using is of the so-called "isohybrid" [1] type. Almost all recent
> > > > > > Linux distros release their CDs using the isohyrbrid format.
> > > > > >
> > > > > > So with isohybrid I think you can directly boot this using the "-drive
> > > > > > file=xxx.iso,if=virtio" without this patch, no?
> > > >
> > > > Yes, that's right.
> > > >
> > > > > >
> > > > > > [1] https://wiki.syslinux.org/wiki/index.php?title=Isohybrid
> > > > > >
> > > > >
> > > > > I don't think QEMU's -cdrom should work with the isohybrid format
> > > > > images. The isohybrid image is a hack, according to the Rufus author
> > > > > [1].
> > > > >
> > > > > The isohybrid images are nothing different from the normal harddisk
> > > > > images, so it should only work with the disk controllers that expect
> > > > > the 512 byte sector size.
> > > > >
> > > > > The -cdrom should work with the original iso image, for example,
> > > > > distros 10 years ago, like Ubuntu 10.04 [2]
> > > > >
> > > > > [1] https://superuser.com/a/1527373/241386
> > > > > [2] https://old-releases.ubuntu.com/releases/10.04.0/ubuntu-10.04-desktop-amd64.iso
> > > >
> > > > I think this patch is needed when virtio is not used, but for virtio
> > > > it is not needed.
> > >
> > > For other block interfaces like if=nvme, if=sd, they are not needed too.
> > >
> > > As I said in the comments, only -cdrom needs this patch to work with
> > > IsoHybrid images, but I don't think that's a valid use case.
> >
> > OK, should we document that -cdrom is not supported by U-Boot? It is
> > quite confusing since when I tried it, since the built-in thing
> > (seabios) work fine.
> >
>
> Yeah we need to document -cdrom is supposed to work with the original
> iso format images, not the recently invented iso hybrid image.
>
> If SeaBIOS works with -cdrom + isohybrid then it must have done some
> hack as well.

OK I sent a doc patch.

Regards,
Simon