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

[Simon Glass]

Hi Peter,

On Tue, 28 Sept 2021 at 03:21, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 27 Sept 2021 at 21:12, Simon Glass <sjg@chromium.org> wrote:
> > I think you are misunderstanding my patch and that may be the problem here.
> >
> > Where QEMU is provided with a dtb (-dtb) it uses that and passes it
> > on. This is absolutely fine and I have tested that this works well
> > with U-Boot. No issues.
> >
> > Where QEMU creates its own dtb on the fly the -dtb parameter is
> > actually ignored and there is no way to adjust what QEMU passes on,
> > without recompiling QEMU. It is quite inflexible, actually. Even just
> > creating a new device for development purposes is not possible. That
> > is the problem I am trying to solve.
> >
> > There is certainly no intent to combine two bits of dtb with my patch.
> > We could easily do that externally to QEMU.
>
> 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.

>
> > 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.

>
> > One more question...other than dtb, does QEMU typically add support
> > for data structures needed by particular projects or groups of
> > projects? It looks like dtb was supported for ARM Linux originally? I
> > am looking at supporting bloblist as a way of communicating
> > information between firmware (basically a simple way of packaging
> > multiple blobs).
>
> The answer here is essentially "no, as far as possible". We
> ideally would not have special case support for any particular
> guest code. We do have special handling for "directly boot the
> Linux kernel" for a combination of historical reasons (we've
> always supported it) and it being a massive use case. But even
> that is painful to maintain (it starts off seeming easy but
> gradually requires more weird tweaks and handling as CPU
> architectures evolve). I really strongly don't want to add
> additional categories of guests that QEMU has special case
> support for, which is the main driver behind why I'm so negative
> about this patchset.

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.

>
> 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?

Regards,
Simon