All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hw/arm/virt: Allow additions to the generated device tree
@ 2021-09-26 18:34 Simon Glass
  2021-09-26 18:46 ` Peter Maydell
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-09-26 18:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Simon Glass, qemu-arm, Paolo Bonzini

At present qemu creates a device tree automatically with the 'virt' generic
virtual platform. This is very convenient in most cases but there is not
much control over what is generated.

Add a way to provide a device tree binary file with additional properties
to add before booting. This provides flexibility for situations where
Linux needs some tweak to operate correctly. It also allows configuration
information to be passed to U-Boot, supporting verified boot, for example.

The term 'merge' is used here to avoid confusion with device tree overlays,
which are a particular type of format.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 docs/system/arm/virt.rst |  13 +++-
 hw/arm/virt.c            | 135 +++++++++++++++++++++++++++++++++++++++
 hw/core/machine.c        |  20 ++++++
 include/hw/boards.h      |   1 +
 qemu-options.hx          |   8 +++
 softmmu/vl.c             |   3 +
 util/qemu-config.c       |   5 ++
 7 files changed, 183 insertions(+), 2 deletions(-)

diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 850787495b..3191e6eebf 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -146,8 +146,17 @@ Hardware configuration information for bare-metal programming
 The ``virt`` board automatically generates a device tree blob ("dtb")
 which it passes to the guest. This provides information about the
 addresses, interrupt lines and other configuration of the various devices
-in the system. Guest code can rely on and hard-code the following
-addresses:
+in the system.
+
+The optional ``-dtbi`` argument is used to specify a device tree blob to merge
+with this generated device tree, to add any properties required by the guest but
+not included by qemu. Properties are merged after the generated device tree is
+created, so take precedence over generated properties. This can be useful for
+overriding the ``stdout-path`` for Linux, for example, or to add configuration
+information needed by U-Boot. This is intended for simple nodes and properties
+and does not support use of phandles or device tree overlays.
+
+Guest code can rely on and hard-code the following addresses:
 
 - Flash memory starts at address 0x0000_0000
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 1d59f0e59f..1d5c9a911a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -43,6 +43,7 @@
 #include "hw/vfio/vfio-calxeda-xgmac.h"
 #include "hw/vfio/vfio-amd-xgbe.h"
 #include "hw/display/ramfb.h"
+#include <libfdt.h>
 #include "net/net.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/numa.h"
@@ -292,6 +293,135 @@ static void create_fdt(VirtMachineState *vms)
     }
 }
 
+
+/*
+ * From U-Boot v2021.07 (under BSD-2-Clause license)
+ *
+ * This does not use the qemu_fdt interface because that requires node names.
+ * Here we are using offsets.
+ *
+ * overlay_apply_node - Merges a node into the base device tree
+ * @fdt: Base Device Tree blob
+ * @target: Node offset in the base device tree to apply the fragment to
+ * @fdto: Device tree overlay blob
+ * @node: Node offset in the overlay holding the changes to merge
+ *
+ * overlay_apply_node() merges a node into a target base device tree
+ * node pointed.
+ *
+ * This is part of the final step in the device tree overlay
+ * application process, when all the phandles have been adjusted and
+ * resolved and you just have to merge overlay into the base device
+ * tree.
+ *
+ * returns:
+ *      0 on success
+ *      Negative error code on failure
+ */
+static int overlay_apply_node(void *fdt, int target, void *fdto, int node)
+{
+    int property;
+    int subnode;
+
+    fdt_for_each_property_offset(property, fdto, node) {
+        const char *name;
+        const void *prop;
+        int prop_len;
+        int ret;
+
+        prop = fdt_getprop_by_offset(fdto, property, &name, &prop_len);
+        if (prop_len == -FDT_ERR_NOTFOUND) {
+            return -FDT_ERR_INTERNAL;
+        }
+        if (prop_len < 0) {
+            return prop_len;
+        }
+
+        ret = fdt_setprop(fdt, target, name, prop, prop_len);
+        if (ret) {
+            return ret;
+        }
+    }
+
+    fdt_for_each_subnode(subnode, fdto, node) {
+        const char *name = fdt_get_name(fdto, subnode, NULL);
+        int nnode;
+        int ret;
+
+        nnode = fdt_add_subnode(fdt, target, name);
+        if (nnode == -FDT_ERR_EXISTS) {
+            nnode = fdt_subnode_offset(fdt, target, name);
+            if (nnode == -FDT_ERR_NOTFOUND) {
+                return -FDT_ERR_INTERNAL;
+            }
+        }
+
+        if (nnode < 0) {
+            return nnode;
+        }
+
+        ret = overlay_apply_node(fdt, nnode, fdto, subnode);
+        if (ret) {
+            return ret;
+        }
+    }
+
+    return 0;
+}
+
+/* Merge nodes and properties into fdt from fdto */
+static int merge_fdt(void *fdt, void *fdto)
+{
+    int err;
+
+    err = overlay_apply_node(fdt, 0, fdto, 0);
+    if (err) {
+        fprintf(stderr, "Device tree error %s\n", fdt_strerror(err));
+        return -1;
+    }
+
+    return 0;
+}
+
+/* Finish creating the device tree, merging in the -dtbi file if needed */
+static int complete_fdt(VirtMachineState *vms)
+{
+    MachineState *ms = MACHINE(vms);
+
+    if (ms->dtbi) {
+        char *filename;
+        void *fdt;
+        int size;
+        int ret;
+
+        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, ms->dtbi);
+        if (!filename) {
+            fprintf(stderr, "Couldn't open dtbi file %s\n", ms->dtbi);
+            goto fail;
+        }
+
+        fdt = load_device_tree(filename, &size);
+        if (!fdt) {
+            fprintf(stderr, "Couldn't open dtbi file %s\n", filename);
+            g_free(filename);
+            goto fail;
+        }
+
+        ret = merge_fdt(ms->fdt, fdt);
+        g_free(fdt);
+        if (ret) {
+            fprintf(stderr, "Failed to merge in dtbi file %s\n", filename);
+            g_free(filename);
+            goto fail;
+        }
+        g_free(filename);
+    }
+    return 0;
+
+fail:
+    return -1;
+}
+
 static void fdt_add_timer_nodes(const VirtMachineState *vms)
 {
     /* On real hardware these interrupts are level-triggered.
@@ -2102,6 +2232,11 @@ static void machvirt_init(MachineState *machine)
 
     create_platform_bus(vms);
 
+    if (complete_fdt(vms)) {
+        error_report("mach-virt: Failed to complete device tree");
+        exit(1);
+    }
+
     if (machine->nvdimms_state->is_enabled) {
         const struct AcpiGenericAddress arm_virt_nvdimm_acpi_dsmio = {
             .space_id = AML_AS_SYSTEM_MEMORY,
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 067f42b528..b47732fb24 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -306,6 +306,21 @@ static void machine_set_dtb(Object *obj, const char *value, Error **errp)
     ms->dtb = g_strdup(value);
 }
 
+static char *machine_get_dtbi(Object *obj, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    return g_strdup(ms->dtbi);
+}
+
+static void machine_set_dtbi(Object *obj, const char *value, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    g_free(ms->dtbi);
+    ms->dtbi = g_strdup(value);
+}
+
 static char *machine_get_dumpdtb(Object *obj, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
@@ -891,6 +906,11 @@ static void machine_class_init(ObjectClass *oc, void *data)
     object_class_property_set_description(oc, "dtb",
         "Linux kernel device tree file");
 
+    object_class_property_add_str(oc, "dtbi",
+        machine_get_dtbi, machine_set_dtbi);
+    object_class_property_set_description(oc, "dtbi",
+        "Linux kernel device tree file to merge with the generated device tree");
+
     object_class_property_add_str(oc, "dumpdtb",
         machine_get_dumpdtb, machine_set_dumpdtb);
     object_class_property_set_description(oc, "dumpdtb",
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 463a5514f9..1868c4e7b6 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -300,6 +300,7 @@ struct MachineState {
 
     void *fdt;
     char *dtb;
+    char *dtbi;  /* filename of device tree to merge with the generated one */
     char *dumpdtb;
     int phandle_start;
     char *dt_compatible;
diff --git a/qemu-options.hx b/qemu-options.hx
index 8f603cc7e6..73c0e6c998 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3621,6 +3621,14 @@ SRST
     kernel on boot.
 ERST
 
+DEF("dtbi", HAS_ARG, QEMU_OPTION_dtbi, \
+    "-dtbi   file    merge 'file' with generated device tree\n", QEMU_ARCH_ARM)
+SRST
+``-dtbi file``
+    Merge a device tree binary (dtb) file into the generated device tree and
+    pass the result to the kernel on boot.
+ERST
+
 DEFHEADING()
 
 DEFHEADING(Debug/Expert options:)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 55ab70eb97..9f3ea6c510 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2921,6 +2921,9 @@ void qemu_init(int argc, char **argv, char **envp)
             case QEMU_OPTION_dtb:
                 qdict_put_str(machine_opts_dict, "dtb", optarg);
                 break;
+            case QEMU_OPTION_dtbi:
+                qdict_put_str(machine_opts_dict, "dtbi", optarg);
+                break;
             case QEMU_OPTION_cdrom:
                 drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
                 break;
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 436ab63b16..359cfb6a9a 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -186,6 +186,11 @@ static QemuOptsList machine_opts = {
             .name = "dtb",
             .type = QEMU_OPT_STRING,
             .help = "Linux kernel device tree file",
+        },{
+            .name = "dtbi",
+            .type = QEMU_OPT_STRING,
+            .help = "Linux kernel device tree file to merge with the "
+                    "generated device tree",
         },{
             .name = "dumpdtb",
             .type = QEMU_OPT_STRING,
-- 
2.33.0.685.g46640cef36-goog



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-26 18:34 [PATCH] hw/arm/virt: Allow additions to the generated device tree Simon Glass
@ 2021-09-26 18:46 ` Peter Maydell
  2021-09-26 18:55   ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2021-09-26 18:46 UTC (permalink / raw)
  To: Simon Glass; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

On Sun, 26 Sept 2021 at 19:34, Simon Glass <sjg@chromium.org> wrote:
>
> At present qemu creates a device tree automatically with the 'virt' generic
> virtual platform. This is very convenient in most cases but there is not
> much control over what is generated.
>
> Add a way to provide a device tree binary file with additional properties
> to add before booting. This provides flexibility for situations where
> Linux needs some tweak to operate correctly. It also allows configuration
> information to be passed to U-Boot, supporting verified boot, for example.

So, I'm strongly inclined to say "nope" here. The device
tree virt generates is supposed to entirely describe the
virtual hardware we pass to the guest. If a guest doesn't
work with that, then either we're generating bogus dtb info
or the guest is not handling the dtb we pass it, and one
or the other should be fixed.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-26 18:46 ` Peter Maydell
@ 2021-09-26 18:55   ` Simon Glass
  2021-09-27  8:48     ` Peter Maydell
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-09-26 18:55 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

Hi Peter,

On Sun, 26 Sept 2021 at 12:47, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sun, 26 Sept 2021 at 19:34, Simon Glass <sjg@chromium.org> wrote:
> >
> > At present qemu creates a device tree automatically with the 'virt' generic
> > virtual platform. This is very convenient in most cases but there is not
> > much control over what is generated.
> >
> > Add a way to provide a device tree binary file with additional properties
> > to add before booting. This provides flexibility for situations where
> > Linux needs some tweak to operate correctly. It also allows configuration
> > information to be passed to U-Boot, supporting verified boot, for example.
>
> So, I'm strongly inclined to say "nope" here. The device
> tree virt generates is supposed to entirely describe the
> virtual hardware we pass to the guest. If a guest doesn't
> work with that, then either we're generating bogus dtb info
> or the guest is not handling the dtb we pass it, and one
> or the other should be fixed.

Thanks for the response.

In the case of U-Boot at least, it uses the devicetree for
configuration (it is a boot loader, so there is no user space to
provide configuration). So the current setup is not sufficient to boot
it correctly in all cases. On the other hand, the 'virt' feature is
very useful for testing U-Boot, so it would be great to be able to
support this.

I also forgot to ask about testing, but am happy to add a test if you
can give me some pointers.

Regards,
Simon


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-26 18:55   ` Simon Glass
@ 2021-09-27  8:48     ` Peter Maydell
  2021-09-27 15:17       ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2021-09-27  8:48 UTC (permalink / raw)
  To: Simon Glass; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

On Sun, 26 Sept 2021 at 19:55, Simon Glass <sjg@chromium.org> wrote:
> In the case of U-Boot at least, it uses the devicetree for
> configuration (it is a boot loader, so there is no user space to
> provide configuration). So the current setup is not sufficient to boot
> it correctly in all cases. On the other hand, the 'virt' feature is
> very useful for testing U-Boot, so it would be great to be able to
> support this.

So what is missing in the QEMU-provided DTB that it needs?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-27  8:48     ` Peter Maydell
@ 2021-09-27 15:17       ` Simon Glass
  2021-09-27 15:45         ` Peter Maydell
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-09-27 15:17 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

Hi Peter,

On Mon, 27 Sept 2021 at 02:48, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sun, 26 Sept 2021 at 19:55, Simon Glass <sjg@chromium.org> wrote:
> > In the case of U-Boot at least, it uses the devicetree for
> > configuration (it is a boot loader, so there is no user space to
> > provide configuration). So the current setup is not sufficient to boot
> > it correctly in all cases. On the other hand, the 'virt' feature is
> > very useful for testing U-Boot, so it would be great to be able to
> > support this.
>
> So what is missing in the QEMU-provided DTB that it needs?

Quite a lot. Here are some examples:

U-Boot has limited pre-relocation memory so tries to avoid
binding/probing devices that are not used before relocation:

https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#pre-relocation-support

There is a configuration node (which is likely to change form in
future releases, but will still be there)

https://github.com/u-boot/u-boot/blob/master/doc/device-tree-bindings/config.txt

Then there are various features which put things in U-Boot's control
dtb, such as verified boot, which adds public keys during signing:

https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L135

More generally, the U-Boot tree has hundreds of files which add
properties for each board, since we try to keep the U-Boot-specific
things out of the Linux tree:

$ find . -name *u-boot.dtsi |wc -l
398

Quite a bit of this is to do with SPL and so far it seems that QEMU
mostly runs U-Boot proper only, although I see that SPL is starting to
creep in too in the U-Boot CI.

So at present QEMU is not able to support U-Boot fully. It would be
great to add this as we use QEMU heavily in CI testing, e.g. see the
second column here:

https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/9260

Regards,
Simon


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-27 15:17       ` Simon Glass
@ 2021-09-27 15:45         ` Peter Maydell
  2021-09-27 16:04           ` Simon Glass
  2021-11-03 11:39           ` Alex Bennée
  0 siblings, 2 replies; 18+ messages in thread
From: Peter Maydell @ 2021-09-27 15:45 UTC (permalink / raw)
  To: Simon Glass; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

On Mon, 27 Sept 2021 at 16:18, Simon Glass <sjg@chromium.org> wrote:
> On Mon, 27 Sept 2021 at 02:48, Peter Maydell <peter.maydell@linaro.org> wrote:
> > So what is missing in the QEMU-provided DTB that it needs?
>
> Quite a lot. Here are some examples:
>
> U-Boot has limited pre-relocation memory so tries to avoid
> binding/probing devices that are not used before relocation:
>
> https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#pre-relocation-support

It's up to u-boot to decide what it wants to touch and
what it does not. QEMU tells u-boot what all the available
devices are; I don't think we should have extra stuff saying
"and if you are u-boot, do something odd".

> There is a configuration node (which is likely to change form in
> future releases, but will still be there)
>
> https://github.com/u-boot/u-boot/blob/master/doc/device-tree-bindings/config.txt

I think u-boot should be storing this kind of thing somewhere
else (e.g. as part of the binary blob that is u-boot itself,
or stored in flash or RAM as a separate blob).

> Then there are various features which put things in U-Boot's control
> dtb, such as verified boot, which adds public keys during signing:
>
> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L135
>
> More generally, the U-Boot tree has hundreds of files which add
> properties for each board, since we try to keep the U-Boot-specific
> things out of the Linux tree:
>
> $ find . -name *u-boot.dtsi |wc -l
> 398

If any of this is actual information about the hardware then you
should sort out getting the bindings documented officially
(which I think is still in the Linux tree), and then QEMU can
provide them.

> Quite a bit of this is to do with SPL and so far it seems that QEMU
> mostly runs U-Boot proper only, although I see that SPL is starting to
> creep in too in the U-Boot CI.
>
> So at present QEMU is not able to support U-Boot fully.

My take is that this is u-boot doing weird custom things with
the DTB that aren't "describe the hardware". You should be able
to boot u-boot by putting those custom DTB extra things in a
separate blob and having u-boot combine that with the
actual DTB when it starts.

-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-27 15:45         ` Peter Maydell
@ 2021-09-27 16:04           ` Simon Glass
  2021-09-27 16:49             ` Peter Maydell
  2021-11-03 11:39           ` Alex Bennée
  1 sibling, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-09-27 16:04 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

Hi Peter,

On Mon, 27 Sept 2021 at 09:46, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 27 Sept 2021 at 16:18, Simon Glass <sjg@chromium.org> wrote:
> > On Mon, 27 Sept 2021 at 02:48, Peter Maydell <peter.maydell@linaro.org> wrote:
> > > So what is missing in the QEMU-provided DTB that it needs?
> >
> > Quite a lot. Here are some examples:
> >
> > U-Boot has limited pre-relocation memory so tries to avoid
> > binding/probing devices that are not used before relocation:
> >
> > https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#pre-relocation-support
>
> It's up to u-boot to decide what it wants to touch and
> what it does not. QEMU tells u-boot what all the available
> devices are; I don't think we should have extra stuff saying
> "and if you are u-boot, do something odd".

Do you have familiarity with U-Boot and the space constraints of
embedded systems?

>
> > There is a configuration node (which is likely to change form in
> > future releases, but will still be there)
> >
> > https://github.com/u-boot/u-boot/blob/master/doc/device-tree-bindings/config.txt
>
> I think u-boot should be storing this kind of thing somewhere
> else (e.g. as part of the binary blob that is u-boot itself,
> or stored in flash or RAM as a separate blob).
>
> > Then there are various features which put things in U-Boot's control
> > dtb, such as verified boot, which adds public keys during signing:
> >
> > https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L135
> >
> > More generally, the U-Boot tree has hundreds of files which add
> > properties for each board, since we try to keep the U-Boot-specific
> > things out of the Linux tree:
> >
> > $ find . -name *u-boot.dtsi |wc -l
> > 398
>
> If any of this is actual information about the hardware then you
> should sort out getting the bindings documented officially
> (which I think is still in the Linux tree), and then QEMU can
> provide them.
>
> > Quite a bit of this is to do with SPL and so far it seems that QEMU
> > mostly runs U-Boot proper only, although I see that SPL is starting to
> > creep in too in the U-Boot CI.
> >
> > So at present QEMU is not able to support U-Boot fully.
>
> My take is that this is u-boot doing weird custom things with
> the DTB that aren't "describe the hardware". You should be able
> to boot u-boot by putting those custom DTB extra things in a
> separate blob and having u-boot combine that with the
> actual DTB when it starts.

Well this is how U-Boot works. Since it doesn't have a user-space
program to provide configuration / policy, nor a command line to
provide parameters (except with sandbox[1]), device tree is what it
uses. All of its driver model and configuration comes from there The
'describe the hardware' thing has been discussed to death but U-Boot
needs board- and arch-specific policy information about the hardware
so it can actually boot successfully on real systems.

It has been like this since U-Boot started using device tree, some 9
years ago! I can't imagine it changing.

As to a separate blob, isn't that what I am suggesting with this
patch? QEMU doesn't support passing two separate dtb blobs to U-Boot,
nor is there an API for that. Even if we did that it would require
code very early in U-Boot to process, which would make it infeasible
for anything other than QEMU. Ideally QEMU should work the same way as
other boards.

As a related point, I am looking at how we pass things between
firmware components.  If we wanted to pass in some initiate state in
some sort of blob, is it possible to set that up in memory (along with
the binary) for when QEMU starts emulating? The code and RAM might be
quite a long way apart so using a single image would involve a lot of
zeroes.

Regards,
Simon

[1] https://u-boot.readthedocs.io/en/latest/arch/sandbox.html


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-27 16:04           ` Simon Glass
@ 2021-09-27 16:49             ` Peter Maydell
  2021-09-27 20:12               ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2021-09-27 16:49 UTC (permalink / raw)
  To: Simon Glass; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

On Mon, 27 Sept 2021 at 17:04, Simon Glass <sjg@chromium.org> wrote:
> On Mon, 27 Sept 2021 at 09:46, Peter Maydell <peter.maydell@linaro.org> wrote:

> > My take is that this is u-boot doing weird custom things with
> > the DTB that aren't "describe the hardware". You should be able
> > to boot u-boot by putting those custom DTB extra things in a
> > separate blob and having u-boot combine that with the
> > actual DTB when it starts.
>
> Well this is how U-Boot works. Since it doesn't have a user-space
> program to provide configuration / policy, nor a command line to
> provide parameters (except with sandbox[1]), device tree is what it
> uses. All of its driver model and configuration comes from there The
> 'describe the hardware' thing has been discussed to death but U-Boot
> needs board- and arch-specific policy information about the hardware
> so it can actually boot successfully on real systems.
>
> It has been like this since U-Boot started using device tree, some 9
> years ago! I can't imagine it changing.

> As to a separate blob, isn't that what I am suggesting with this
> patch? QEMU doesn't support passing two separate dtb blobs to U-Boot,
> nor is there an API for that.

You're suggesting "QEMU should have machinery for taking two
blobs and combining them and passing one blob to the guest".
I'm suggesting "the guest should combine them" (and the second
blob could be provided via several different existing mechanisms
that amount to 'QEMU provides some ways to load data into guest
ROM or RAM'), because as far as I know no other guest has this
"combine two different bits of dtb for me" requirement.

> Even if we did that it would require
> code very early in U-Boot to process, which would make it infeasible
> for anything other than QEMU. Ideally QEMU should work the same way as
> other boards.

Well, real hardware doesn't provide device tree blobs of any
form to u-boot, right? u-boot is just compiled into flash, or
perhaps launched from some other boot ROM, as I understand it.
Where does it get its dtb from then ?

> As a related point, I am looking at how we pass things between
> firmware components.  If we wanted to pass in some initiate state in
> some sort of blob, is it possible to set that up in memory (along with
> the binary) for when QEMU starts emulating? The code and RAM might be
> quite a long way apart so using a single image would involve a lot of
> zeroes.

The generic-loader is quite good for this sort of thing:
https://qemu-project.gitlab.io/qemu/system/generic-loader.html
You can load raw data files to specific addresses; or you can
load ELF files (which can have multiple segments which get loaded
as the ELF header specifies). You can specify -device generic-loader,...
as many times as you need to to load multiple blobs.

-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-27 16:49             ` Peter Maydell
@ 2021-09-27 20:12               ` Simon Glass
  2021-09-28  9:20                 ` Peter Maydell
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

Hi Peter,

On Mon, 27 Sept 2021 at 10:50, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 27 Sept 2021 at 17:04, Simon Glass <sjg@chromium.org> wrote:
> > On Mon, 27 Sept 2021 at 09:46, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> > > My take is that this is u-boot doing weird custom things with
> > > the DTB that aren't "describe the hardware". You should be able
> > > to boot u-boot by putting those custom DTB extra things in a
> > > separate blob and having u-boot combine that with the
> > > actual DTB when it starts.
> >
> > Well this is how U-Boot works. Since it doesn't have a user-space
> > program to provide configuration / policy, nor a command line to
> > provide parameters (except with sandbox[1]), device tree is what it
> > uses. All of its driver model and configuration comes from there The
> > 'describe the hardware' thing has been discussed to death but U-Boot
> > needs board- and arch-specific policy information about the hardware
> > so it can actually boot successfully on real systems.
> >
> > It has been like this since U-Boot started using device tree, some 9
> > years ago! I can't imagine it changing.
>
> > As to a separate blob, isn't that what I am suggesting with this
> > patch? QEMU doesn't support passing two separate dtb blobs to U-Boot,
> > nor is there an API for that.
>
> You're suggesting "QEMU should have machinery for taking two
> blobs and combining them and passing one blob to the guest".
> I'm suggesting "the guest should combine them" (and the second
> blob could be provided via several different existing mechanisms
> that amount to 'QEMU provides some ways to load data into guest
> ROM or RAM'), because as far as I know no other guest has this
> "combine two different bits of dtb for me" requirement.

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.

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?

>
> > Even if we did that it would require
> > code very early in U-Boot to process, which would make it infeasible
> > for anything other than QEMU. Ideally QEMU should work the same way as
> > other boards.
>
> Well, real hardware doesn't provide device tree blobs of any
> form to u-boot, right? u-boot is just compiled into flash, or
> perhaps launched from some other boot ROM, as I understand it.
> Where does it get its dtb from then ?

The dtb is compiled as part of the U-Boot build. but exists as a
separate file. The mechanism for providing the dtb to U-Boot at
runtime is somewhat board-specific and we are working on standardising
it more. On the rpi for example, it is provided by a FAT file system
and first-stage firmware loads that and passes it along to U-Boot.
Some systems use TF-A which does a similar thing. Some use U-Boot as a
first-stage loader in which case SPL may select a DTB (out of many
built by the build) to pass to U-Boot proper.

There is also a tool called binman which packages the firmware as it
is getting quite complicated:

https://u-boot.readthedocs.io/en/latest/develop/package/binman.html

>
> > As a related point, I am looking at how we pass things between
> > firmware components.  If we wanted to pass in some initiate state in
> > some sort of blob, is it possible to set that up in memory (along with
> > the binary) for when QEMU starts emulating? The code and RAM might be
> > quite a long way apart so using a single image would involve a lot of
> > zeroes.
>
> The generic-loader is quite good for this sort of thing:
> https://qemu-project.gitlab.io/qemu/system/generic-loader.html
> You can load raw data files to specific addresses; or you can
> load ELF files (which can have multiple segments which get loaded
> as the ELF header specifies). You can specify -device generic-loader,...
> as many times as you need to to load multiple blobs.

OK great, thank you, that looks very useful.

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

https://github.com/ni/u-boot/blob/master/doc/README.bloblist

Regards,
Simon


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-27 20:12               ` Simon Glass
@ 2021-09-28  9:20                 ` Peter Maydell
  2021-09-29  3:01                   ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2021-09-28  9:20 UTC (permalink / raw)
  To: Simon Glass; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

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.

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

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

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

-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-28  9:20                 ` Peter Maydell
@ 2021-09-29  3:01                   ` Simon Glass
  2021-09-29  9:09                     ` Peter Maydell
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-09-29  3:01 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-29  3:01                   ` Simon Glass
@ 2021-09-29  9:09                     ` Peter Maydell
  2021-09-29 15:09                       ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2021-09-29  9:09 UTC (permalink / raw)
  To: Simon Glass; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

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.

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

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

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

-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-29  9:09                     ` Peter Maydell
@ 2021-09-29 15:09                       ` Simon Glass
  2021-09-29 19:44                         ` Andrew Jones
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-09-29 15:09 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, qemu-arm, QEMU Developers, Eduardo Habkost

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.

Regards,
Simon


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-29 15:09                       ` Simon Glass
@ 2021-09-29 19:44                         ` Andrew Jones
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Jones @ 2021-09-29 19:44 UTC (permalink / raw)
  To: Simon Glass
  Cc: Peter Maydell, qemu-arm, QEMU Developers, Eduardo Habkost, Paolo Bonzini

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



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-09-27 15:45         ` Peter Maydell
  2021-09-27 16:04           ` Simon Glass
@ 2021-11-03 11:39           ` Alex Bennée
  2021-11-03 13:17             ` François Ozog
  1 sibling, 1 reply; 18+ messages in thread
From: Alex Bennée @ 2021-11-03 11:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: François Ozog, Eduardo Habkost, Simon Glass,
	QEMU Developers, qemu-arm, Paolo Bonzini


Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 27 Sept 2021 at 16:18, Simon Glass <sjg@chromium.org> wrote:
>> On Mon, 27 Sept 2021 at 02:48, Peter Maydell <peter.maydell@linaro.org> wrote:
>> > So what is missing in the QEMU-provided DTB that it needs?
>>
>> Quite a lot. Here are some examples:
>>
>> U-Boot has limited pre-relocation memory so tries to avoid
>> binding/probing devices that are not used before relocation:
>>
>> https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#pre-relocation-support
>
> It's up to u-boot to decide what it wants to touch and
> what it does not. QEMU tells u-boot what all the available
> devices are; I don't think we should have extra stuff saying
> "and if you are u-boot, do something odd".
>
>> There is a configuration node (which is likely to change form in
>> future releases, but will still be there)
>>
>> https://github.com/u-boot/u-boot/blob/master/doc/device-tree-bindings/config.txt
>
> I think u-boot should be storing this kind of thing somewhere
> else (e.g. as part of the binary blob that is u-boot itself,
> or stored in flash or RAM as a separate blob).
>
>> Then there are various features which put things in U-Boot's control
>> dtb, such as verified boot, which adds public keys during signing:
>>
>> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L135
>>
>> More generally, the U-Boot tree has hundreds of files which add
>> properties for each board, since we try to keep the U-Boot-specific
>> things out of the Linux tree:
>>
>> $ find . -name *u-boot.dtsi |wc -l
>> 398
>
> If any of this is actual information about the hardware then you
> should sort out getting the bindings documented officially
> (which I think is still in the Linux tree), and then QEMU can
> provide them.
>
>> Quite a bit of this is to do with SPL and so far it seems that QEMU
>> mostly runs U-Boot proper only, although I see that SPL is starting to
>> creep in too in the U-Boot CI.
>>
>> So at present QEMU is not able to support U-Boot fully.
>
> My take is that this is u-boot doing weird custom things with
> the DTB that aren't "describe the hardware". You should be able
> to boot u-boot by putting those custom DTB extra things in a
> separate blob and having u-boot combine that with the
> actual DTB when it starts.

It's not entirely without precedent - for SPL (which I hope is secondary
program loading) we have things like the guest loader which expands the
plain HW DTB with some information needed by the bootloader and the
primary OS to load additional blobs which have been put into memory.

In effect the DTB is being expanded as a signalling mechanism similar to
things like fw_cfg and other things we use to control boot up. Whether
this affects the "purity" of DTB as a "just the HW" description is
probably a philosophical question.

I agree with Peter that just allowing the merging of arbitrary data into
the QEMU generated DTB is going to lead to confusion and breakages.
Indeed I wrote the guest-loader because instructions for booting Xen up
until that point involved dumpdtb and hand hacking the data which was
silly because this is stuff QEMU already knew about.

>
> -- PMM


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-11-03 11:39           ` Alex Bennée
@ 2021-11-03 13:17             ` François Ozog
  2021-11-05 15:50               ` François Ozog
  0 siblings, 1 reply; 18+ messages in thread
From: François Ozog @ 2021-11-03 13:17 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Peter Maydell, Eduardo Habkost, Simon Glass, QEMU Developers,
	qemu-arm, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 6168 bytes --]

Hi,

Thanks Alex to patch me in.

I'd like to present another perspective on the motivation as I can't really
comment on the actual "how".

On real Arm boards, firmware is often assembled into a FIP.
That FIP can contain quite a good deal of things, including an
NT_FW_CONFIG, NonTrusted_FirmWare_CONFIGuration (NT_FW = BL33 which is
U-Boot in our case).
So the expected typical content for that section is a DT fragment/overlay.
That section is to be used in different ways but one is
https://trustedfirmware-a.readthedocs.io/en/latest/components/fconf/index.html
.
For SystemReady systems we will almost inevitably put a device tree
fragment in this section and have BL2 merge it with the board DT before
handing it over to BL33 (U-Boot is one of them).
In some real world examples based on carrier board + som for instance, it
may contain SerDes configuration for U-Boot that will result in appropriate
PCI lanes or MDIO lanes for the booted OS.

So I could say there is precedence in Simon's effort.

In any case, when we will have made the changes in TFA for the SystemReady
boards we work on, booting the full SystemReady stack (TFA, OP-TEE, U-Boot)
on Qemu will allow that late merge based through the FIP.

Other boot flows such as VBE (without TFA but with TPL/SPL/U-Boot proper)
need a similar facility.

Hence I am supporting Simon's proposal at least on the intent. On the how
exactly, that is outside my skillset.

future comments below


On Wed, 3 Nov 2021 at 12:48, Alex Bennée <alex.bennee@linaro.org> wrote:

>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Mon, 27 Sept 2021 at 16:18, Simon Glass <sjg@chromium.org> wrote:
> >> On Mon, 27 Sept 2021 at 02:48, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >> > So what is missing in the QEMU-provided DTB that it needs?
> >>
> >> Quite a lot. Here are some examples:
> >>
> >> U-Boot has limited pre-relocation memory so tries to avoid
> >> binding/probing devices that are not used before relocation:
> >>
> >>
> https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#pre-relocation-support
> >
> > It's up to u-boot to decide what it wants to touch and
> > what it does not. QEMU tells u-boot what all the available
> > devices are; I don't think we should have extra stuff saying
> > "and if you are u-boot, do something odd".
> >
> >> There is a configuration node (which is likely to change form in
> >> future releases, but will still be there)
> >>
> >>
> https://github.com/u-boot/u-boot/blob/master/doc/device-tree-bindings/config.txt
> >
> > I think u-boot should be storing this kind of thing somewhere
> > else (e.g. as part of the binary blob that is u-boot itself,
> > or stored in flash or RAM as a separate blob).
> >
> >> Then there are various features which put things in U-Boot's control
> >> dtb, such as verified boot, which adds public keys during signing:
> >>
> >>
> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L135
> >>
> >> More generally, the U-Boot tree has hundreds of files which add
> >> properties for each board, since we try to keep the U-Boot-specific
> >> things out of the Linux tree:
> >>
> >> $ find . -name *u-boot.dtsi |wc -l
> >> 398
> >
> > If any of this is actual information about the hardware then you
> > should sort out getting the bindings documented officially
> > (which I think is still in the Linux tree), and then QEMU can
> > provide them.
> >
> >> Quite a bit of this is to do with SPL and so far it seems that QEMU
> >> mostly runs U-Boot proper only, although I see that SPL is starting to
> >> creep in too in the U-Boot CI.
> >>
> >> So at present QEMU is not able to support U-Boot fully.
> >
> > My take is that this is u-boot doing weird custom things with
> > the DTB that aren't "describe the hardware". You should be able
> > to boot u-boot by putting those custom DTB extra things in a
> > separate blob and having u-boot combine that with the
> > actual DTB when it starts.
>
> It's not entirely without precedent - for SPL (which I hope is secondary
> program loading) we have things like the guest loader which expands the
> plain HW DTB with some information needed by the bootloader and the
> primary OS to load additional blobs which have been put into memory.
>
> In effect the DTB is being expanded as a signalling mechanism similar to
> things like fw_cfg and other things we use to control boot up. Whether
> this affects the "purity" of DTB as a "just the HW" description is
> probably a philosophical question.
>
> More than a philosophical question: a key aspect of supply chain that need
change from
quite inflexible and tightly coupled to loosely coupled.
 A key aspect of it is to maintain "pure" hardware description DTBs at rest.
Composition of DTBs at build time (for products) or runtime (for
development boards) should be a simple thing.
Another aspect to take into account is System Device Trees. U-Boot only
deal with Cortex-As on a platform,
so there are multiple device trees for each compute domain. Communities are
working on System Device Tree
to define the overall platform with its power and clock domains. A tool
"lopper" is being developed to slide the SDT into diverse domain DTs.
One of them being included into the FIP as the basis for the computing
element (Carrier, SoM...).
Those attempts to cleanup passed DTBs from configuration data (drivers,
bootloaders...) is not incompatible
with merging fragments at runtime (for dev boards) or build time (for
products).

> I agree with Peter that just allowing the merging of arbitrary data into
> the QEMU generated DTB is going to lead to confusion and breakages.
> Indeed I wrote the guest-loader because instructions for booting Xen up
> until that point involved dumpdtb and hand hacking the data which was
> silly because this is stuff QEMU already knew about.
>
> >
> > -- PMM
>
>
> --
> Alex Bennée
>


-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

[-- Attachment #2: Type: text/html, Size: 9579 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-11-03 13:17             ` François Ozog
@ 2021-11-05 15:50               ` François Ozog
  2023-02-07 18:39                 ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: François Ozog @ 2021-11-05 15:50 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Peter Maydell, Eduardo Habkost, Simon Glass, QEMU Developers,
	qemu-arm, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 6823 bytes --]

Hi,

On Wed, 3 Nov 2021 at 14:17, François Ozog <francois.ozog@linaro.org> wrote:

> Hi,
>
> Thanks Alex to patch me in.
>
> I'd like to present another perspective on the motivation as I can't
> really comment on the actual "how".
>
> On real Arm boards, firmware is often assembled into a FIP.
> That FIP can contain quite a good deal of things, including an
> NT_FW_CONFIG, NonTrusted_FirmWare_CONFIGuration (NT_FW = BL33 which is
> U-Boot in our case).
> So the expected typical content for that section is a DT fragment/overlay.
> That section is to be used in different ways but one is
> https://trustedfirmware-a.readthedocs.io/en/latest/components/fconf/index.html
> .
> For SystemReady systems we will almost inevitably put a device tree
> fragment in this section and have BL2 merge it with the board DT before
> handing it over to BL33 (U-Boot is one of them).
> In some real world examples based on carrier board + som for instance, it
> may contain SerDes configuration for U-Boot that will result in appropriate
> PCI lanes or MDIO lanes for the booted OS.
>
> So I could say there is precedence in Simon's effort.
>
> In any case, when we will have made the changes in TFA for the SystemReady
> boards we work on, booting the full SystemReady stack (TFA, OP-TEE, U-Boot)
> on Qemu will allow that late merge based through the FIP.
>
> Other boot flows such as VBE (without TFA but with TPL/SPL/U-Boot proper)
> need a similar facility.
>
>
Hence I am supporting Simon's proposal at least on the intent. On the how
> exactly, that is outside my skillset.
>
Responding to my own comment:
As the boot flow TFA+U-Boot works, it looks like a cleaner option is to
leave QEMU alone and have
U-Boot SPL do the same work as with TFA: use a section for the FIT for the
DT to be merged.



>
future comments below
>
>
> On Wed, 3 Nov 2021 at 12:48, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>> > On Mon, 27 Sept 2021 at 16:18, Simon Glass <sjg@chromium.org> wrote:
>> >> On Mon, 27 Sept 2021 at 02:48, Peter Maydell <peter.maydell@linaro.org>
>> wrote:
>> >> > So what is missing in the QEMU-provided DTB that it needs?
>> >>
>> >> Quite a lot. Here are some examples:
>> >>
>> >> U-Boot has limited pre-relocation memory so tries to avoid
>> >> binding/probing devices that are not used before relocation:
>> >>
>> >>
>> https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#pre-relocation-support
>> >
>> > It's up to u-boot to decide what it wants to touch and
>> > what it does not. QEMU tells u-boot what all the available
>> > devices are; I don't think we should have extra stuff saying
>> > "and if you are u-boot, do something odd".
>> >
>> >> There is a configuration node (which is likely to change form in
>> >> future releases, but will still be there)
>> >>
>> >>
>> https://github.com/u-boot/u-boot/blob/master/doc/device-tree-bindings/config.txt
>> >
>> > I think u-boot should be storing this kind of thing somewhere
>> > else (e.g. as part of the binary blob that is u-boot itself,
>> > or stored in flash or RAM as a separate blob).
>> >
>> >> Then there are various features which put things in U-Boot's control
>> >> dtb, such as verified boot, which adds public keys during signing:
>> >>
>> >>
>> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L135
>> >>
>> >> More generally, the U-Boot tree has hundreds of files which add
>> >> properties for each board, since we try to keep the U-Boot-specific
>> >> things out of the Linux tree:
>> >>
>> >> $ find . -name *u-boot.dtsi |wc -l
>> >> 398
>> >
>> > If any of this is actual information about the hardware then you
>> > should sort out getting the bindings documented officially
>> > (which I think is still in the Linux tree), and then QEMU can
>> > provide them.
>> >
>> >> Quite a bit of this is to do with SPL and so far it seems that QEMU
>> >> mostly runs U-Boot proper only, although I see that SPL is starting to
>> >> creep in too in the U-Boot CI.
>> >>
>> >> So at present QEMU is not able to support U-Boot fully.
>> >
>> > My take is that this is u-boot doing weird custom things with
>> > the DTB that aren't "describe the hardware". You should be able
>> > to boot u-boot by putting those custom DTB extra things in a
>> > separate blob and having u-boot combine that with the
>> > actual DTB when it starts.
>>
>> It's not entirely without precedent - for SPL (which I hope is secondary
>> program loading) we have things like the guest loader which expands the
>> plain HW DTB with some information needed by the bootloader and the
>> primary OS to load additional blobs which have been put into memory.
>>
>> In effect the DTB is being expanded as a signalling mechanism similar to
>> things like fw_cfg and other things we use to control boot up. Whether
>> this affects the "purity" of DTB as a "just the HW" description is
>> probably a philosophical question.
>>
>> More than a philosophical question: a key aspect of supply chain that
> need change from
> quite inflexible and tightly coupled to loosely coupled.
>  A key aspect of it is to maintain "pure" hardware description DTBs at
> rest.
> Composition of DTBs at build time (for products) or runtime (for
> development boards) should be a simple thing.
> Another aspect to take into account is System Device Trees. U-Boot only
> deal with Cortex-As on a platform,
> so there are multiple device trees for each compute domain. Communities
> are working on System Device Tree
> to define the overall platform with its power and clock domains. A tool
> "lopper" is being developed to slide the SDT into diverse domain DTs.
> One of them being included into the FIP as the basis for the computing
> element (Carrier, SoM...).
> Those attempts to cleanup passed DTBs from configuration data (drivers,
> bootloaders...) is not incompatible
> with merging fragments at runtime (for dev boards) or build time (for
> products).
>
>> I agree with Peter that just allowing the merging of arbitrary data into
>> the QEMU generated DTB is going to lead to confusion and breakages.
>> Indeed I wrote the guest-loader because instructions for booting Xen up
>> until that point involved dumpdtb and hand hacking the data which was
>> silly because this is stuff QEMU already knew about.
>>
>> >
>> > -- PMM
>>
>>
>> --
>> Alex Bennée
>>
>
>
> --
> François-Frédéric Ozog | *Director Business Development*
> T: +33.67221.6485
> francois.ozog@linaro.org | Skype: ffozog
>
>

-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

[-- Attachment #2: Type: text/html, Size: 12480 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
  2021-11-05 15:50               ` François Ozog
@ 2023-02-07 18:39                 ` Simon Glass
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2023-02-07 18:39 UTC (permalink / raw)
  To: François Ozog
  Cc: Alex Bennée, Peter Maydell, Paolo Bonzini, QEMU Developers,
	Marcel Apfelbaum, Eduardo Habkost, qemu-arm

Hi,

On Fri, 5 Nov 2021 at 09:51, François Ozog <francois.ozog@linaro.org> wrote:
>
> Hi,
>
> On Wed, 3 Nov 2021 at 14:17, François Ozog <francois.ozog@linaro.org> wrote:
>>
>> Hi,
>>
>> Thanks Alex to patch me in.
>>
>> I'd like to present another perspective on the motivation as I can't really comment on the actual "how".
>>
>> On real Arm boards, firmware is often assembled into a FIP.
>> That FIP can contain quite a good deal of things, including an NT_FW_CONFIG, NonTrusted_FirmWare_CONFIGuration (NT_FW = BL33 which is U-Boot in our case).
>> So the expected typical content for that section is a DT fragment/overlay.
>> That section is to be used in different ways but one is https://trustedfirmware-a.readthedocs.io/en/latest/components/fconf/index.html.
>> For SystemReady systems we will almost inevitably put a device tree fragment in this section and have BL2 merge it with the board DT before handing it over to BL33 (U-Boot is one of them).
>> In some real world examples based on carrier board + som for instance, it may contain SerDes configuration for U-Boot that will result in appropriate PCI lanes or MDIO lanes for the booted OS.
>>
>> So I could say there is precedence in Simon's effort.
>>
>> In any case, when we will have made the changes in TFA for the SystemReady boards we work on, booting the full SystemReady stack (TFA, OP-TEE, U-Boot) on Qemu will allow that late merge based through the FIP.
>>
>> Other boot flows such as VBE (without TFA but with TPL/SPL/U-Boot proper) need a similar facility.
>>
>>
>> Hence I am supporting Simon's proposal at least on the intent. On the how exactly, that is outside my skillset.
>
> Responding to my own comment:
> As the boot flow TFA+U-Boot works, it looks like a cleaner option is to leave QEMU alone and have
> U-Boot SPL do the same work as with TFA: use a section for the FIT for the DT to be merged.
>
>
>>
>>
>> future comments below
>>
>>
>> On Wed, 3 Nov 2021 at 12:48, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>
>>>
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>
>>> > On Mon, 27 Sept 2021 at 16:18, Simon Glass <sjg@chromium.org> wrote:
>>> >> On Mon, 27 Sept 2021 at 02:48, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> >> > So what is missing in the QEMU-provided DTB that it needs?
>>> >>
>>> >> Quite a lot. Here are some examples:
>>> >>
>>> >> U-Boot has limited pre-relocation memory so tries to avoid
>>> >> binding/probing devices that are not used before relocation:
>>> >>
>>> >> https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#pre-relocation-support
>>> >
>>> > It's up to u-boot to decide what it wants to touch and
>>> > what it does not. QEMU tells u-boot what all the available
>>> > devices are; I don't think we should have extra stuff saying
>>> > "and if you are u-boot, do something odd".
>>> >
>>> >> There is a configuration node (which is likely to change form in
>>> >> future releases, but will still be there)
>>> >>
>>> >> https://github.com/u-boot/u-boot/blob/master/doc/device-tree-bindings/config.txt
>>> >
>>> > I think u-boot should be storing this kind of thing somewhere
>>> > else (e.g. as part of the binary blob that is u-boot itself,
>>> > or stored in flash or RAM as a separate blob).
>>> >
>>> >> Then there are various features which put things in U-Boot's control
>>> >> dtb, such as verified boot, which adds public keys during signing:
>>> >>
>>> >> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L135
>>> >>
>>> >> More generally, the U-Boot tree has hundreds of files which add
>>> >> properties for each board, since we try to keep the U-Boot-specific
>>> >> things out of the Linux tree:
>>> >>
>>> >> $ find . -name *u-boot.dtsi |wc -l
>>> >> 398
>>> >
>>> > If any of this is actual information about the hardware then you
>>> > should sort out getting the bindings documented officially
>>> > (which I think is still in the Linux tree), and then QEMU can
>>> > provide them.
>>> >
>>> >> Quite a bit of this is to do with SPL and so far it seems that QEMU
>>> >> mostly runs U-Boot proper only, although I see that SPL is starting to
>>> >> creep in too in the U-Boot CI.
>>> >>
>>> >> So at present QEMU is not able to support U-Boot fully.
>>> >
>>> > My take is that this is u-boot doing weird custom things with
>>> > the DTB that aren't "describe the hardware". You should be able
>>> > to boot u-boot by putting those custom DTB extra things in a
>>> > separate blob and having u-boot combine that with the
>>> > actual DTB when it starts.
>>>
>>> It's not entirely without precedent - for SPL (which I hope is secondary
>>> program loading) we have things like the guest loader which expands the
>>> plain HW DTB with some information needed by the bootloader and the
>>> primary OS to load additional blobs which have been put into memory.
>>>
>>> In effect the DTB is being expanded as a signalling mechanism similar to
>>> things like fw_cfg and other things we use to control boot up. Whether
>>> this affects the "purity" of DTB as a "just the HW" description is
>>> probably a philosophical question.
>>>
>> More than a philosophical question: a key aspect of supply chain that need change from
>> quite inflexible and tightly coupled to loosely coupled.
>>  A key aspect of it is to maintain "pure" hardware description DTBs at rest.
>> Composition of DTBs at build time (for products) or runtime (for development boards) should be a simple thing.
>> Another aspect to take into account is System Device Trees. U-Boot only deal with Cortex-As on a platform,
>> so there are multiple device trees for each compute domain. Communities are working on System Device Tree
>> to define the overall platform with its power and clock domains. A tool "lopper" is being developed to slide the SDT into diverse domain DTs.
>> One of them being included into the FIP as the basis for the computing element (Carrier, SoM...).
>> Those attempts to cleanup passed DTBs from configuration data (drivers, bootloaders...) is not incompatible
>> with merging fragments at runtime (for dev boards) or build time (for products).
>>>
>>> I agree with Peter that just allowing the merging of arbitrary data into
>>> the QEMU generated DTB is going to lead to confusion and breakages.
>>> Indeed I wrote the guest-loader because instructions for booting Xen up
>>> until that point involved dumpdtb and hand hacking the data which was
>>> silly because this is stuff QEMU already knew about.
>>>
>>> >
>>> > -- PMM
>>>
>>>
>>> --
>>> Alex Bennée
>>
>>
>>
>> --
>> François-Frédéric Ozog | Director Business Development
>> T: +33.67221.6485
>> francois.ozog@linaro.org | Skype: ffozog
>>
>
>
> --
> François-Frédéric Ozog | Director Business Development
> T: +33.67221.6485
> francois.ozog@linaro.org | Skype: ffozog
>

I am pinging this thread again as it has come up on the U-Boot mailing list[1].

Please can you reconsider this patch. It would greatly help the
collaboration between QEMU and U-Boot.

Regards,
Simon

[1] https://lore.kernel.org/u-boot/CAFEAcA-TH96CG1gi4toAQXpnxs4kxsPCSSfZrXbo2QF8CoE=XA@mail.gmail.com/T/#m1b41b54e3ca5dda1fcb8fdd189ac472ec7e4a96d


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-02-07 18:40 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-26 18:34 [PATCH] hw/arm/virt: Allow additions to the generated device tree Simon Glass
2021-09-26 18:46 ` Peter Maydell
2021-09-26 18:55   ` Simon Glass
2021-09-27  8:48     ` Peter Maydell
2021-09-27 15:17       ` Simon Glass
2021-09-27 15:45         ` Peter Maydell
2021-09-27 16:04           ` Simon Glass
2021-09-27 16:49             ` Peter Maydell
2021-09-27 20:12               ` Simon Glass
2021-09-28  9:20                 ` Peter Maydell
2021-09-29  3:01                   ` Simon Glass
2021-09-29  9:09                     ` Peter Maydell
2021-09-29 15:09                       ` Simon Glass
2021-09-29 19:44                         ` Andrew Jones
2021-11-03 11:39           ` Alex Bennée
2021-11-03 13:17             ` François Ozog
2021-11-05 15:50               ` François Ozog
2023-02-07 18:39                 ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.