Re: [RFC PATCH 0/4] generic loader FDT support (for direct Xen boot)

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Mon, Oct 12, 2020 at 05:02:57PM +0100, Alex Bennée wrote:
> 
> Alistair Francis <alistair23@gmail.com> writes:
> 
> > On Fri, Oct 9, 2020 at 10:07 AM Alex Bennée <alex.bennee@linaro.org> wrote:
> >>
> >> Hi,
> >>
> >> This series adds the ability to append FDT data for blobs loaded by
> >> the generic loader. My principle use-case was to be able to directly
> >> boot Xen with a kernel image which avoided having to:
> >>
> >>   - get the kernel image into my system image
> >>   - deal with bugs in FDT munging by -bios firmware and/or grub
> >>
> >> as such this currently a developer hack that makes *my* life easy and
> >> is thus presented as an RFC for discussion. While I've tested it with
> >> Xen I'm sure the approach would be applicable to other hypervisors or
> >> firmware which expect to consume FDT data pointing at various blobs.
> >
> > An interesting idea. I think this comes up enough that it's worth
> > thinking about.
> >
> >>
> >> An example command line that launches this is (magic from -kernel):
> >>
> >>   ./aarch64-softmmu/qemu-system-aarch64 -cpu cortex-a57 \
> >>     -machine type=virt,virtualization=on -display none \
> >>     -serial mon:stdio \
> >>     -netdev user,id=unet,hostfwd=tcp::2222-:22 \
> >>     -device virtio-net-pci,netdev=unet,id=virt-net,disable-legacy=on \
> >>     -device virtio-scsi-pci,id=virt-scsi,disable-legacy=on \
> >>     -blockdev driver=raw,node-name=hd,discard=unmap,file.driver=host_device,file.filename=/dev/zen-disk/debian-buster-arm64 \
> >>     -device scsi-hd,drive=hd,id=virt-scsi-hd \
> >>     -smp 4 -m 4096 \
> >>     -kernel ~/lsrc/xen.git/xen/xen \
> >>     -append "dom0_mem=1G,max:1G loglvl=all guest_loglvl=all" \
> >>     -device loader,addr=0x47000000,file=$HOME/lsrc/linux.git/builds/arm64/arch/arm64/boot/Image,len-fdt-compat=2,fdt-compat[0]='multiboot,,module',fdt-compat[1]='multiboot,,kernel',fdt-bootargs="root=/dev/sda2 ro console=hvc0 earlyprintk=xen"
> >
> > You are loading the kernel `Image` file, but changing the device tree
> > that was generated by QEMU and is being loaded in memory. As a user
> > that is really confusing.
> 
> Well in this case the "kernel" is Xen which helpfully comes with a
> kernel compatible header that the kernel loaded understand. The blob
> could be any Dom0 kernel - it just happens to be a Linux kernel in this
> case.
> 
> >
> >>
> >> So a couple of choices I've made doing this:
> >>
> >> Promoting *fdt to MachineState
> >>
> >> This seemed the simplest approach to making the fdt available to the
> >> global state, especially as MachineState already has a *dtb pointer.
> >> I've converted the ARM virt machine and a RISCV machine but if this is
> >> acceptable I can convert the others.
> >
> > This seems fine to me.
> >
> >>
> >> "Polluting" the generic loader arguments
> >>
> >> This was mainly out of convenience as the loader already knows the
> >> size of the blob being loaded. However you could certainly argue it
> >> makes sense to have a more generic "FDT expander" virtual device that
> >> could for example query the QOM model somehow to find the details it
> >> needs.
> >
> > That seems like a better option. Why not have a generic way to modify
> > the device tree with a specific argument? It could either be -device
> > loader,file=fdt,... or -fdt ...
> 
> Well it comes down to how much of a special case this is? Pretty much
> all FDT (and ACPI for the matter) is basically down to the board level
> models - and not all of them just the funky configurable ones. For other
> board models we just expect the user to pass the FDT they got with their
> kernel blob.
> 
> For modern VirtIO systems the only thing you really need to expose is
> the root PCI bus because the rest of the devices are discover-able
> there.
> 
> So the real question is are there any other -devices that we want to be
> able to graft FDT entries on or is the generic loader enough of a
> special case that we keep all the logic in there?
>

Hi,

Another option is to allow the user to pass along a DTB overlay with the
generic loader option (or with a separate option as Alistair suggested).
With overlways we wouldn't need all the command-line options to enable
construction of dtb fragments, it would be more or less transparent to
QEMU. There may be limitations with the overlay flows that I'm not aware
of though...

Cheers,
Edgar