Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree

[Andrew Jones]

On Wed, Sep 29, 2021 at 09:09:59AM -0600, Simon Glass wrote:
> Hi Peter,
> 
> On Wed, 29 Sept 2021 at 03:10, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Wed, 29 Sept 2021 at 04:01, Simon Glass <sjg@chromium.org> wrote:
> > > On Tue, 28 Sept 2021 at 03:21, Peter Maydell <peter.maydell@linaro.org> 
> > > wrote:
> > > > So what *is* this patch doing? The subject says "Allow additions to
> > > > the generated device tree", and the patch adds an option to
> > > > "Merge a device tree binary (dtb) file into the generated device tree".
> > > > That sounds exactly like "combine two bits of dtb" -- the one the
> > > > user provides to the -dtbi option and the one QEMU generates
> > > > internally.
> > >
> > > Yes that's right, but as you say one of them is generated by QEMU. It
> > > is fixing a problem caused by QEMU's generation of the device tree,
> > > since it provides no wat to augment the device tree without my patch.
> >
> > You can augment it in the guest if you need to add guest-specific stuff.
> 
> This was also discussed, see below.
> 
> >
> > > > > The only current working option is to just pass the U-Boot dtb through
> > > > > and not use QEMU's on-the-fly-generated dtb at all. But I am assuming
> > > > > there is a reason why QEMU generates that dtb, so that would not be
> > > > > desirable?
> > > >
> > > > QEMU generates the dtb to tell the guest what hardware is
> > > > present and where it is. We don't guarantee that that hardware
> > > > arrangement is necessarily stable between QEMU versions (in
> > > > practice there are generally additions and not deletions or
> > > > moves, but in theory there might be). All the guest is supposed
> > > > to assume is the location of RAM and flash; everything else it
> > > > must look in the dtb to determine.
> > >
> > > Yes, so that is going to severely limit the usefulness of the virtio
> > > support, for example. But we can just ignore it for CI, I suppose,
> > > since we can use fixed parameters to QEMU, if you don't want to allow
> > > more control of the device tree.
> >
> > virtio-mmio devices should already be correctly indicated in the
> > device tree. virtio-pci devices can be found by probing the PCI
> > bus in the usual way (and you can find out where the PCI controller
> > is by looking in the dtb).
> 
> Which device tree? The one generated by QEMU or the one we would
> actually use, in U-Boot?
> 
> >
> > > This patch is really just augmenting what QEMU is already doing and
> > > solving a problem that it has. I don't see it as creating a new boot
> > > mechanism or being a weird tweak. If anything, it puts the tweaking in
> > > the hands of the user. It seems like something you would be keen on,
> > > from that POV.
> >
> > I absolutely see it as a weird tweak. It's something that
> > only u-boot seems to care about, and it's adding an extra
> > user-facing command line option that is basically
> > "if you need to boot u-boot you need this". The user should
> > not need to "control" the dtb, because QEMU should already
> > be creating a dtb which describes the hardware it is exposing
> > to the guest.
> 
> I don't even know how to respond to that. Anything that uses
> devicetree for control will need to have its bindings respected. It is
> QEMU that is confused here, with its idea that if it just creates
> whatever it feels like, it will be suitable for the payload. It may be
> suitable for linux because it was coded up that way, but it is not
> suitable in general. Each project can have its own device tree
> bindings, yes?
> 
> See for example barebox which has the same problem:
> https://www.barebox.org/doc/latest/devicetree/bindings/barebox/barebox,environment.html
> 
> >
> > > > Guest software running on the "virt" board needs to know it is
> > > > running on the "virt" board and deal with the interface and
> > > > information that QEMU provides. (For instance, Arm Trusted
> > > > Firmware and UEFI both have done this.)
> > >
> > > Well unfortunately this is in conflict, based on what you have said in
> > > this thread. We can either have virtio support (use QEMU-generated
> > > device tree) or have U-Boot work correctly (use -dtb). Do you have any
> > > other ideas?
> >
> > I've already suggested what I think you should do: have u-boot
> > combine the dtb it gets from QEMU with the dtb-extras stuff
> > it wants for its own purposes, with the latter supplied in
> > any of a bunch of workable ways (extra file loaded in memory,
> > concatenate the dtb blob with the u-boot binary at compile
> > time, whatever; rough analogy with however u-boot gets the
> > full dtb on hardware platforms where there is none).
> 
> Unfortunately this is not practical for reasons I explained
> previously. To recap:
> 
> - this is not how real boards work (they expect their dtb to be provided)
> - It adds code to the early stage of U-Boot which would not be there
> in a real system
> - this code takes a long time to run, particularly with the cache off or in 
> SPL
> - we don't have access to a second dtb anyway
> 
> Another option is to run QEMU twice, like:
> 
> arm/qemu-system-arm -M virt -nographic  -M dumpdtb=out.dtb
> (run a tool to merge them)
> arm/qemu-system-arm -M virt -nographic -bios
> /tmp/b/qemu_arm/u-boot-nodtb.bin -dtb merged.dtb
> 
> If we really need the virtio devices then that could be one option.
>

FWIW, this approach was the first thing that popped into my mind when I
saw the posting of this patch. I'm not sure why doing the merging in
QEMU is much better, considering the complexity of the QEMU command line
more or less requires scripting or running through libvirt anyway.

Thanks,
drew