qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 08/11] hw/arm/boot: Expose the PC-DIMM nodes


From: Shameerali Kolothum Thodi
Subject: Re: [Qemu-devel] [PATCH v2 08/11] hw/arm/boot: Expose the PC-DIMM nodes in the DT
Date: Tue, 12 Mar 2019 16:39:22 +0000


> -----Original Message-----
> From: Auger Eric [mailto:address@hidden
> Sent: 12 March 2019 15:38
> To: Shameerali Kolothum Thodi <address@hidden>;
> Igor Mammedov <address@hidden>
> Cc: address@hidden; address@hidden;
> address@hidden; address@hidden; Linuxarm
> <address@hidden>; address@hidden; xuwei (O)
> <address@hidden>; address@hidden
> Subject: Re: [Qemu-devel] [PATCH v2 08/11] hw/arm/boot: Expose the
> PC-DIMM nodes in the DT
> 
> Hi Shameer,
> 
> On 3/12/19 4:13 PM, Shameerali Kolothum Thodi wrote:
> >
> >
> >> -----Original Message-----
> >> From: Igor Mammedov [mailto:address@hidden
> >> Sent: 11 March 2019 14:59
> >> To: Shameerali Kolothum Thodi <address@hidden>
> >> Cc: address@hidden; address@hidden;
> >> address@hidden; address@hidden;
> >> address@hidden; address@hidden;
> >> address@hidden; Linuxarm <address@hidden>; xuwei (O)
> >> <address@hidden>
> >> Subject: Re: [PATCH v2 08/11] hw/arm/boot: Expose the PC-DIMM nodes in
> the
> >> DT
> >>
> >> On Fri, 8 Mar 2019 11:42:15 +0000
> >> Shameer Kolothum <address@hidden> wrote:
> >>
> >>> This patch adds memory nodes corresponding to PC-DIMM regions.
> >>>
> >>> NVDIMM and ACPI_NVDIMM configs are not yet set for ARM so we
> >>> don't need to care about NVDIMM at this stage.
> >>>
> >>> Signed-off-by: Shameer Kolothum
> <address@hidden>
> >>> Signed-off-by: Eric Auger <address@hidden>
> >>> ---
> >>>  hw/arm/boot.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 42 insertions(+)
> >>>
> >>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> >>> index a830655..4caaf91 100644
> >>> --- a/hw/arm/boot.c
> >>> +++ b/hw/arm/boot.c
> >>> @@ -19,6 +19,7 @@
> >>>  #include "sysemu/numa.h"
> >>>  #include "hw/boards.h"
> >>>  #include "hw/loader.h"
> >>> +#include "hw/mem/memory-device.h"
> >>>  #include "elf.h"
> >>>  #include "sysemu/device_tree.h"
> >>>  #include "qemu/config-file.h"
> >>> @@ -522,6 +523,41 @@ static void fdt_add_psci_node(void *fdt)
> >>>      qemu_fdt_setprop_cell(fdt, "/psci", "migrate", migrate_fn);
> >>>  }
> >>>
> >>> +static int fdt_add_hotpluggable_memory_nodes(void *fdt,
> >>> +                                             uint32_t acells,
> >> uint32_t scells) {
> >>> +    MemoryDeviceInfoList *info, *info_list = qmp_memory_device_list();
> >>> +    MemoryDeviceInfo *mi;
> >>> +    int ret = 0;
> >>> +
> >>> +    for (info = info_list; info != NULL; info = info->next) {
> >>> +        mi = info->value;
> >>> +        switch (mi->type) {
> >>> +        case MEMORY_DEVICE_INFO_KIND_DIMM:
> >>> +        {
> >>> +            PCDIMMDeviceInfo *di = mi->u.dimm.data;
> >>> +
> >>> +            ret = fdt_add_memory_node(fdt, acells, di->addr,
> >>> +                                      scells, di->size, di->node);
> >>> +            if (ret) {
> >>> +                fprintf(stderr,
> >>> +                        "couldn't add PCDIMM
> /address@hidden"PRIx64"
> >> node\n",
> >>> +                        di->addr);
> >>> +                goto out;
> >>> +            }
> >>> +            break;
> >>> +        }
> >>> +        default:
> >>> +            fprintf(stderr, "%s memory nodes are not yet
> supported\n",
> >>> +                    MemoryDeviceInfoKind_str(mi->type));
> >>> +            ret = -ENOENT;
> >>> +            goto out;
> >>> +        }
> >>> +    }
> >>> +out:
> >>> +    qapi_free_MemoryDeviceInfoList(info_list);
> >>> +    return ret;
> >>> +}
> >>> +
> >>>  int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
> >>>                   hwaddr addr_limit, AddressSpace *as)
> >>>  {
> >>> @@ -621,6 +657,12 @@ int arm_load_dtb(hwaddr addr, const struct
> >> arm_boot_info *binfo,
> >>>          }
> >>>      }
> >>>
> >>> +    rc = fdt_add_hotpluggable_memory_nodes(fdt, acells, scells);
> >> According to Eric's test, guest kernel picks this up. So it probably
> >> should be an opt-in feature to avoid conflict with ACPI definition in DSDT.
> >
> > Ok. My take away from that discussion was that if we properly describe the
> > PNP0C80 (which is what #06 of this series does) then we don't have to worry
> > about DT entries as ACPI will "override" it.  But it looks like that is not 
> > the
> case
> > and describing DT will anyway cause the memory being end up in UEFI
> > GeTMemoryMap() and results in being used as normal memory early at boot.
> >
> > Hi Eric,
> >
> > How do we verify that this is actually happening? I tried running the
> /proc/meminfo
> > on a Qemu build upto #06 of this series, and it reported memory including 
> > the
> > cold plugged dimm memory.
> >
> > But not sure how to verify that this dimm mem is being treated as normal
> mem on
> > boot time or not? Please let me know.
> 
> At the time I tested I did not have PNP0C80 build. I confirm that if you
> remove the dt description in ACPI mode, /proc/meminfo does not report
> the DIMMs. Now that you have #06, if you remove the DT  hotpluggables
> memory mode generation in ACPI mode, you should see them and if I
> understand correctly this is what you confirm. I have not tested
> anything else at the moment.

Yes, that is what I confirmed. But the concern now is having DT memory node
generation will result in the problem described above - UEFI GetMemoryMap().

I was looking for a way to check that is a valid issue or not. I will try to 
look for 
a way to verify that. Please update if you come across anything.

Thanks,
Shameer

> Thanks
> 
> Eric
> >
> > Thanks,
> > Shameer
> >
> >
> >>> +    if (rc < 0) {
> >>> +        fprintf(stderr, "couldn't add hotpluggable memory nodes\n");
> >>> +        goto fail;
> >>> +    }
> >>> +
> >>>      rc = fdt_path_offset(fdt, "/chosen");
> >>>      if (rc < 0) {
> >>>          qemu_fdt_add_subnode(fdt, "/chosen");
> >
> >

reply via email to

[Prev in Thread] Current Thread [Next in Thread]