Re: [SeaBIOS] [PATCH 2/2] Boot Linux using QEMU fw_cfg DMA interface

[Laszlo Ersek]

On 08/06/15 17:46, Kevin O'Connor wrote:
> On Thu, Aug 06, 2015 at 02:52:16PM +0200, Marc Marí wrote:
> > On Thu, 6 Aug 2015 13:38:33 +0100
> > Stefan Hajnoczi  wrote:
> > > On Thu, Aug 6, 2015 at 12:02 PM, Marc Marí  wrote:
> > > >  case IPL_TYPE_BEV:
> > > >    boot_rom(ie->vector);
> > > >    if (qemu_cfg_dma_enabled()) {
> > > >        boot_linux_cfg_dma();
> > > >    } else {
> > > >        boot_rom(ie->vector);
> > > >    }
> > > >    break;
> > >
> > > I thought the fw_cfg DMA interface is equivalent to the PIO interface?
> > >
> > > Therefore, your Linux loading code completely replaces the
> > > linuxboot.rom.  Why would you want to keep using linuxboot.rom in the
> > > PIO case if SeaBIOS now has to code to launch the kernel?
> > >
> > > By the way, I think this change is incorrect.  We shouldn't hijack the
> > > BEV.  Instead fw_cfg should simply add a higher priority IPL entry
> > > when the kernel file is present.  That way we just don't execute the
> > > option ROM.
> >
> > linuxboot.rom is doing all the work of reading and copying the kernel
> > to the correct position through the PIO interface, in assembly. If we
> > want to speed up the kernel copying with DMA, it is necessary to change
> > it, and implementing fw_cfg DMA in C is a lot easier.
> >
> > This new boot is independent of PIO or DMA, so now linuxboot.rom would
> > not be necessary. I don't know if this is desirable.
> >
> > In any case, I agree with adding a new IPL.
>
> One can look at boot.c:boot_cbfs() (and its call chain) as an example
> of how to add a new IPL.
>
> As an aside - if ultimately the goal is to change QEMU so that it no
> longer uses linuxboot,

I don't think that's the ultimate / primary goal (well, certainly not
the reason why I'm interested in this work).
> I wonder if it makes sense to clean up the
> QEMU<->firmware interface for it.  It seems that linuxboot uses 12
> old-style fw_cfg entries to pass the kernel.  It seems that could be
> replaced with three new-style fw_cfg "file" entries - "/vmlinuz",
> "/initrd", and "/cmdline" - and QEMU could avoid having to parse and
> manipulate the vmlinuz contents.  I'm guessing all the old-style
> fw_cfg entries were implemented to make the linuxboot assembler
> easier.  That shouldn't be necessary with SeaBIOS though.  Just
> something to think about - it shouldn't be a pre-requisite for the
> fw_cfg dma stuff.

Both OVMF (UEFI for x86 guests, built from edk2) and AAVMF (UEFI for ARM
guests, built from edk2) depend on those old-style fw_cfg entries. I
didn't like those numeric keys when I had to write code for them, but
replacing them with named fw_cfg files at this point would create
compatibility problems.

New firmware would have to probe for the files, and if they are not
there, fall back to the old keys (because QEMU could be old).

Older firmware would not find the old keys (and wouldn't know to probe
for the new files), so it would break. In turn, if QEMU preserved the
old keys too, for compatibility with old firmware, then new firmware
wouldn't bother to look for the new files either.

I think we should stick with these handful old numeric keys forever, and
add new entries only as named files.

... Linuxboot is bundled with QEMU, so I guess it could be updated in
sync with QEMU, but OVMF and AAVMF are not bundled.

Thanks
Laszlo