Re: [Qemu-devel] [PATCH v6 2/5] qmp: distinguish PC-DIMM and NVDIMM in MemoryDeviceInfoList

[Igor Mammedov]

On Sun, 11 Mar 2018 11:02:12 +0800
Haozhong Zhang <address@hidden> wrote:

> It may need to treat PC-DIMM and NVDIMM differently, e.g., when
> deciding the necessity of non-volatile flag bit in SRAT memory
> affinity structures.
> 
> A new field 'nvdimm' is added to the union type MemoryDeviceInfo for
> such purpose. Its type is currently PCDIMMDeviceInfo and will be
> updated when necessary in the future.
> 
> It also fixes "info memory-devices"/query-memory-devices which
> currently show nvdimm devices as dimm devices since
> object_dynamic_cast(obj, TYPE_PC_DIMM) happily cast nvdimm to
> TYPE_PC_DIMM which it's been inherited from.
> 
> Signed-off-by: Haozhong Zhang <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Igor Mammedov <address@hidden>

> ---
>  hmp.c            | 14 +++++++++++---
>  hw/mem/pc-dimm.c | 10 +++++++++-
>  numa.c           | 19 +++++++++++++------
>  qapi/misc.json   |  6 +++++-
>  4 files changed, 38 insertions(+), 11 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 016cb5c4f1..011a7c6f35 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2421,7 +2421,18 @@ void hmp_info_memory_devices(Monitor *mon, const QDict 
> *qdict)
>              switch (value->type) {
>              case MEMORY_DEVICE_INFO_KIND_DIMM:
>                  di = value->u.dimm.data;
> +                break;
> +
> +            case MEMORY_DEVICE_INFO_KIND_NVDIMM:
> +                di = value->u.nvdimm.data;
> +                break;
> +
> +            default:
> +                di = NULL;
> +                break;
> +            }
>  
> +            if (di) {
>                  monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
>                                 MemoryDeviceInfoKind_str(value->type),
>                                 di->id ? di->id : "");
> @@ -2434,9 +2445,6 @@ void hmp_info_memory_devices(Monitor *mon, const QDict 
> *qdict)
>                                 di->hotplugged ? "true" : "false");
>                  monitor_printf(mon, "  hotpluggable: %s\n",
>                                 di->hotpluggable ? "true" : "false");
> -                break;
> -            default:
> -                break;
>              }
>          }
>      }
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 4d050fe2cd..51350d9c2d 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -20,6 +20,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "hw/mem/pc-dimm.h"
> +#include "hw/mem/nvdimm.h"
>  #include "qapi/error.h"
>  #include "qemu/config-file.h"
>  #include "qapi/visitor.h"
> @@ -250,6 +251,7 @@ MemoryDeviceInfoList *qmp_pc_dimm_device_list(void)
>          MemoryDeviceInfoList *elem = g_new0(MemoryDeviceInfoList, 1);
>          MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1);
>          PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1);
> +        bool is_nvdimm = object_dynamic_cast(obj, TYPE_NVDIMM);
>          DeviceClass *dc = DEVICE_GET_CLASS(obj);
>          DeviceState *dev = DEVICE(obj);
>  
> @@ -265,7 +267,13 @@ MemoryDeviceInfoList *qmp_pc_dimm_device_list(void)
>          di->size = object_property_get_uint(obj, PC_DIMM_SIZE_PROP, NULL);
>          di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem));
>  
> -        info->u.dimm.data = di;
> +        if (!is_nvdimm) {
> +            info->u.dimm.data = di;
> +            info->type = MEMORY_DEVICE_INFO_KIND_DIMM;
> +        } else {
> +            info->u.nvdimm.data = di;
> +            info->type = MEMORY_DEVICE_INFO_KIND_NVDIMM;
> +        }
>          elem->value = info;
>          elem->next = NULL;
>          if (prev) {
> diff --git a/numa.c b/numa.c
> index 94427046ec..1116c90af9 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -529,18 +529,25 @@ static void numa_stat_memory_devices(NumaNodeMem 
> node_mem[])
>  
>          if (value) {
>              switch (value->type) {
> -            case MEMORY_DEVICE_INFO_KIND_DIMM: {
> +            case MEMORY_DEVICE_INFO_KIND_DIMM:
>                  pcdimm_info = value->u.dimm.data;
> +                break;
> +
> +            case MEMORY_DEVICE_INFO_KIND_NVDIMM:
> +                pcdimm_info = value->u.nvdimm.data;
> +                break;
> +
> +            default:
> +                pcdimm_info = NULL;
> +                break;
> +            }
> +
> +            if (pcdimm_info) {
>                  node_mem[pcdimm_info->node].node_mem += pcdimm_info->size;
>                  if (pcdimm_info->hotpluggable && pcdimm_info->hotplugged) {
>                      node_mem[pcdimm_info->node].node_plugged_mem +=
>                          pcdimm_info->size;
>                  }
> -                break;
> -            }
> -
> -            default:
> -                break;
>              }
>          }
>      }
> diff --git a/qapi/misc.json b/qapi/misc.json
> index bcd5d10778..6bf082f612 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -2852,7 +2852,11 @@
>  #
>  # Since: 2.1
>  ##
> -{ 'union': 'MemoryDeviceInfo', 'data': {'dimm': 'PCDIMMDeviceInfo'} }
> +{ 'union': 'MemoryDeviceInfo',
> +  'data': { 'dimm': 'PCDIMMDeviceInfo',
> +            'nvdimm': 'PCDIMMDeviceInfo'
> +          }
> +}
>  
>  ##
>  # @query-memory-devices: