Re: [Qemu-devel] [RFC v3 08/15] hw/arm/boot: introduce fdt_add_memory_node helper

[Igor Mammedov]

On Wed, 8 Aug 2018 11:44:14 +0200
Auger Eric <address@hidden> wrote:

> Hi Igor,
> 
> On 07/18/2018 04:04 PM, Igor Mammedov wrote:
> > On Tue,  3 Jul 2018 09:19:51 +0200
> > Eric Auger <address@hidden> wrote:
> >   
> >> From: Shameer Kolothum <address@hidden>
> >>
> >> We introduce an helper to create a memory node.
> >>
> >> Signed-off-by: Eric Auger <address@hidden>
> >> Signed-off-by: Shameer Kolothum <address@hidden>
> >>
> >> ---
> >>
> >> v1 -> v2:
> >> - nop of existing /memory nodes was already handled
> >> ---
> >>  hw/arm/boot.c | 54 ++++++++++++++++++++++++++++++++++--------------------
> >>  1 file changed, 34 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> >> index e09201c..5243a25 100644
> >> --- a/hw/arm/boot.c
> >> +++ b/hw/arm/boot.c
> >> @@ -413,6 +413,36 @@ static void set_kernel_args_old(const struct 
> >> arm_boot_info *info,
> >>      }
> >>  }
> >>  
> >> +static int fdt_add_memory_node(void *fdt, uint32_t acells, hwaddr 
> >> mem_base,
> >> +                               uint32_t scells, hwaddr mem_len,
> >> +                               int numa_node_id)
> >> +{
> >> +    char *nodename = NULL;
> >> +    int ret;
> >> +
> >> +    nodename = g_strdup_printf("/address@hidden" PRIx64, mem_base);
> >> +    qemu_fdt_add_subnode(fdt, nodename);
> >> +    qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory");
> >> +    ret = qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", acells, 
> >> mem_base,
> >> +                                       scells, mem_len);
> >> +    if (ret < 0) {
> >> +        fprintf(stderr, "couldn't set %s/reg\n", nodename);
> >> +        goto out;
> >> +    }
> >> +    if (numa_node_id < 0) {
> >> +        goto out;
> >> +    }
> >> +
> >> +    ret = qemu_fdt_setprop_cell(fdt, nodename, "numa-node-id", 
> >> numa_node_id);
> >> +    if (ret < 0) {
> >> +        fprintf(stderr, "couldn't set %s/numa-node-id\n", nodename);
> >> +    }
> >> +
> >> +out:
> >> +    g_free(nodename);
> >> +    return ret;
> >> +}
> >> +  
> > 
> > not related question from hotplug POV,
> > is entry size fixed?  
> Sorry I don't get what entry you are referring to?
> > can we estimate exact size for #slots number of dimms and reserve it in 
> > advance
> > in FDT 'rom'?  
> Not sure I get your drift either.
> 
> patch "[RFC v3 09/15] hw/arm/boot: Expose the PC-DIMM nodes in the DT"
> builds the DT nodes for each node, by enumerating the MemoryDeviceInfoList.

In case of hotplug we don not care about adding DTB node at runtime
(guest won't see it anyways). However if we reboot machine it's reasonable
to regenerate DTB on reboot so guest would see previously hotplugged DIMMs.
Problem is that DTB is stored in fixed size 'rom' that's copied into guest's
RAM, so we should reserve a space for possible slots in advance or switch
to another mechanism to provide DTB to guest. (it could be a memory region
mapped outside of RAM)

[...]