On 9 June 2018 at 15:23, Eric Auger <eric.au...@redhat.com> wrote:
> When running dtc on the guest /proc/device-tree we get the
> following warning: Warning (unit_address_vs_reg): Node /memory
> has a reg or ranges property, but no unit name".
>
> Let's fix that by adding the unit address to the node name. We also
> don't create the /memory node anymore in create_fdt(). We directly
> create it in load_dtb. /chosen still needs to be created in create_fdt
> as the uart needs it. In case the user provided his own dtb, either
> the bank is added to the existing /memory node or if this latter is
> not found we create a new separate memory node.
>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
> ---
> hw/arm/boot.c | 20 ++++++++++++++------
> hw/arm/virt.c | 6 ------
> 2 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 1e2be20..2054670 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -593,24 +593,32 @@ static int load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo,
> g_free(nodename);
> }
> } else {
I think you need also to change the "if" half of this if..else,
which deals with NUMA, because it assumes a "/memory" node was
created by the virt.c code and now it will not be.
> + char *nodename = g_strdup("/memory");
> Error *err = NULL;
>
> - rc = fdt_path_offset(fdt, "/memory");
> + /* If there is an existing /memory node (user provided dtb), we add
> the
> + * new bank into it, otherwise we create a /memory@addr node
> + */
> + rc = fdt_path_offset(fdt, nodename);
If we create /memory@addr nodes then we should also look for them in
an input dtb, I think. Otherwise we'll break the usecase where the
user asks QEMU to dump the generated dtb to a file, edits it and
then passes it back to a subsequent QEMU invocation, because we won't
find the memory node we created.
thanks
-- PMM
