[edk2-devel] [PATCH 1/1] OvmfPkg/VirtioFsDxe: fix SimpleFileOpen

[Gerd Hoffmann]

  Hi,

> > -  if (!VirtioFsFile->IsDirectory) {
> > +  if (!VirtioFsFile->IsDirectory && FileName[0] != '\\') {

> It's nice to see this topic pop up on edk2-devel; apparently you started
> testing shim on top of virtio-fs. :)

Indeed.  For starters just passed the host /boot/efi into the guest to
see how far it goes.

Amazingly ranking virtiofs first in bootorder JustWorks[tm].

Next was shim trapping into this when trying to read BOOTX64.CSV (i.e.
probably it actually was fallback.efi) and a boot loop.

> >     strictly since the beginning, and a few months ago I reported USWG Mantis
> >     ticket #2367 [1] too, for clearing up the related confusion in the UEFI
> >     spec.

I have to admit I didn't check the what the spec says yet.  Looking ...

Hmm,  EFI_FILE_PROTOCOL.Open() description says "This would typically be
an open handle to a directory".  I don't read that as "MUST be a
directory".

With an absolute path the only thing we need from the Handle passed in
is the reference to the Filesystem root, the actual file/directory is
not needed, so accepting anything and not only directories makes sense
to me.  And most existing implementations apparently agree, otherwise we
would see much more fallout from this.

Updating the spec to clearly document expected behavior in that corner
case would be good.

> (2) With this modification in place, shim is happy, but grub isn't. When
> I realized that, I looked relatively deeply into making grub work on top
> of virtio-fs as well -- and my findings were horrendous.
> 
> I wrote up my findings in a private email to some colleagues; you were
> among the recipients.

Yes, I remember.

Didn't try anything in grub yet.  It boots to the grub shell prompt, but
there is no kernel on the host ESP anyway.  And no real grub.cfg either,
only a stub pointing to the real grub.cfg on a different filesystem.

But, yes, that is probably a dead end.  IIRC grub completely ignores the
efi filesystem drivers and uses its own exclusively (using its own
interfaces, not registering them in efi).

Trying to fix that opens the can of worms where you can have both grub
and efi filesystem drivers being active and stomping on each others
feet.

> If you have a use case where you rely on shim but *not* on Grub
> (UKIs?), then I'm OK relaxing the strictness of VirtioFsDxe.

No concrete plans yet.  There are ideas/plans to allow booting container
images as VM.  In that case the root fileystem would be virtiofs, and
given that OVMF has a driver it IMHO makes sense to use that to boot the
system.  And, yes, using UKIs for that makes sense, although the exact
workflow is far from being clear.

I think one possible option would be to simply have an /EFI subdirectory
on the root filesystem, place the files you would have on the ESP there,
so the rootfs becomes EFI-bootable.  No need to have a boot filesystem
or directory, given that EFI can read the complete filesystem anyway you
can slurp the UKIs directly from /lib/modules/$kernel/vmlinuz-virt.efi.

> In that case, I'd prefer upstreaming my above patch, from April,
> rather than taking yours.  What do you think about that?

Fine with me.

The UEFI spec is not clear and I don't feel like bikeshedding whenever
shim behavior is a bug or not.  Once the UEFI spec has been fixed the
one way or other we can revisit this.

> Here's a further (independent) caveat: if you are using VirtioFsDxe with
> the rust language virtiofsd, then you might experience hangs in
> VirtioFsInit. For fixing that, you need the following *qemu* patch set:
> 
>   [PATCH v3 0/7] vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously
>   https://patchew.org/QEMU/20231002203221.17241-1-lersek@redhat.com/
> 
> (This patch set has been on qemu-devel for nearly 2 months now, counting
> from v1; I'm going to ping MST again. It's been ready for merging for
> weeks now!)

Good to know, I think I tapped into this when I tried to pass the
complete host filesystem into the guest.

IIRC mst tends to do large pulls in large intervals, I wouldn't worry
too much yet.  Pinging doesn't hurt though.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109746): https://edk2.groups.io/g/devel/message/109746
Mute This Topic: https://groups.io/mt/102036263/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-