| Message ID | 20161010003423.4333-9-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/09/2016 07:34 PM, Haozhong Zhang wrote: > Xen uses this command to get the backend resource, guest SPA and size of > NVDIMM devices so as to map them to guest. > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > --- > Cc: Markus Armbruster <armbru@redhat.com> > +++ b/docs/qmp-commands.txt > @@ -3800,3 +3800,39 @@ Example for pc machine type started with > "props": {"core-id": 0, "socket-id": 0, "thread-id": 0} > } > ]} > + > +EQMP > + > + { > + .name = "query-nvdimms", > + .args_type = "", > + .mhandler.cmd_new = qmp_marshal_query_nvdimms, Needs rebasing - we no longer need SQMP/EQMP sections or callouts to the initializers, now that commit bd6092e4 has automated the mapping of QAPI to command registration. > + }, > + > +SQMP > +Show plugged NVDIMM devices > +--------------------------- > + > +Arguments: None. > + > +Example for pc machine type started with > +-object memory-backend-file,id=mem1,mem-path=/path/to/nvm1,size=4G > +-device nvdimm,id=nvdimm1,memdev=mem1 > +-object memory-backend-file,id=mem2,mem-path=/path/to/nvm2,size=8G > +-device nvdimm,id=nvdimm2,memdev=mem2: > + > +-> { "execute": "query-nvdimms" } > +<- { "returns": [ > + { > + "mem-path": "/path/to/nvm1", > + "slot": 0, TAB damage; please fix. > + "spa": 17179869184, > + "length": 4294967296 > + }, > + { > + "mem-path": "/path/to/nvm2", > + "slot": 1, > + "spa": 21474836480, > + "length": 8589934592 > + } > + ]} > +++ b/qapi-schema.json > @@ -4646,3 +4646,32 @@ > # Since: 2.7 > ## > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } > + > +## > +# @NvdimmInfo > +# > +# Information about an NVDIMM device. > +# > +# @mem-path: the backend file of the NVDIMM device > +# > +# @slot: the slot index of the NVDIMM device > +# > +# @spa: the 64-bit SPA base address of the NVDIMM device > +# > +# @length: the 64-bit size in bytes of the NVDIMM device > +# > +# Since 2.8 > +## > +{ 'struct': 'NvdimmInfo', > + 'data': {'mem-path' : 'str', 'slot': 'int', 'spa': 'int', 'length': 'int'} } > + > +## > +# @query-nvdimms: > +# > +# Returns information about each NVDIMM device > +# > +# Returns: a list of @NvdimmInfo for each device > +# > +# Since: 2.8 > +## > +{ 'command': 'query-nvdimms', 'returns': ['NvdimmInfo'] } > Is this something that can be added to the existing query-memdev or query-memory-devices command, instead of needing a new command?
On 10/10/16 14:16, Eric Blake wrote: > On 10/09/2016 07:34 PM, Haozhong Zhang wrote: > > Xen uses this command to get the backend resource, guest SPA and size of > > NVDIMM devices so as to map them to guest. > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > --- > > Cc: Markus Armbruster <armbru@redhat.com> > > > +++ b/docs/qmp-commands.txt > > @@ -3800,3 +3800,39 @@ Example for pc machine type started with > > "props": {"core-id": 0, "socket-id": 0, "thread-id": 0} > > } > > ]} > > + > > +EQMP > > + > > + { > > + .name = "query-nvdimms", > > + .args_type = "", > > + .mhandler.cmd_new = qmp_marshal_query_nvdimms, > > Needs rebasing - we no longer need SQMP/EQMP sections or callouts to the > initializers, now that commit bd6092e4 has automated the mapping of QAPI > to command registration. > good to know this, will change in next version > > + }, > > + > > +SQMP > > +Show plugged NVDIMM devices > > +--------------------------- > > + > > +Arguments: None. > > + > > +Example for pc machine type started with > > +-object memory-backend-file,id=mem1,mem-path=/path/to/nvm1,size=4G > > +-device nvdimm,id=nvdimm1,memdev=mem1 > > +-object memory-backend-file,id=mem2,mem-path=/path/to/nvm2,size=8G > > +-device nvdimm,id=nvdimm2,memdev=mem2: > > + > > +-> { "execute": "query-nvdimms" } > > +<- { "returns": [ > > + { > > + "mem-path": "/path/to/nvm1", > > + "slot": 0, > > TAB damage; please fix. > will fix > > + "spa": 17179869184, > > + "length": 4294967296 > > + }, > > + { > > + "mem-path": "/path/to/nvm2", > > + "slot": 1, > > + "spa": 21474836480, > > + "length": 8589934592 > > + } > > + ]} > > > +++ b/qapi-schema.json > > @@ -4646,3 +4646,32 @@ > > # Since: 2.7 > > ## > > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } > > + > > +## > > +# @NvdimmInfo > > +# > > +# Information about an NVDIMM device. > > +# > > +# @mem-path: the backend file of the NVDIMM device > > +# > > +# @slot: the slot index of the NVDIMM device > > +# > > +# @spa: the 64-bit SPA base address of the NVDIMM device > > +# > > +# @length: the 64-bit size in bytes of the NVDIMM device > > +# > > +# Since 2.8 > > +## > > +{ 'struct': 'NvdimmInfo', > > + 'data': {'mem-path' : 'str', 'slot': 'int', 'spa': 'int', 'length': 'int'} } > > + > > +## > > +# @query-nvdimms: > > +# > > +# Returns information about each NVDIMM device > > +# > > +# Returns: a list of @NvdimmInfo for each device > > +# > > +# Since: 2.8 > > +## > > +{ 'command': 'query-nvdimms', 'returns': ['NvdimmInfo'] } > > > > Is this something that can be added to the existing query-memdev or > query-memory-devices command, instead of needing a new command? > Both query-memdev and query-memory-devices do not tell whether a device is NVDIMM. If adding an optional argument to existing a qmp command does not break backwards compatibility, I propose to add an argument "nvdimm" to query-memory-devices which controls whether this command to return all memory devices or only NVDIMM devices. Thanks, Haozhong
Haozhong Zhang <haozhong.zhang@intel.com> writes: > On 10/10/16 14:16, Eric Blake wrote: >> On 10/09/2016 07:34 PM, Haozhong Zhang wrote: >> > Xen uses this command to get the backend resource, guest SPA and size of >> > NVDIMM devices so as to map them to guest. >> > >> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> >> > --- >> > Cc: Markus Armbruster <armbru@redhat.com> >> >> > +++ b/docs/qmp-commands.txt >> > @@ -3800,3 +3800,39 @@ Example for pc machine type started with >> > "props": {"core-id": 0, "socket-id": 0, "thread-id": 0} >> > } >> > ]} >> > + >> > +EQMP >> > + >> > + { >> > + .name = "query-nvdimms", >> > + .args_type = "", >> > + .mhandler.cmd_new = qmp_marshal_query_nvdimms, >> >> Needs rebasing - we no longer need SQMP/EQMP sections or callouts to the >> initializers, now that commit bd6092e4 has automated the mapping of QAPI >> to command registration. >> > > good to know this, will change in next version > >> > + }, >> > + >> > +SQMP >> > +Show plugged NVDIMM devices >> > +--------------------------- >> > + >> > +Arguments: None. >> > + >> > +Example for pc machine type started with >> > +-object memory-backend-file,id=mem1,mem-path=/path/to/nvm1,size=4G >> > +-device nvdimm,id=nvdimm1,memdev=mem1 >> > +-object memory-backend-file,id=mem2,mem-path=/path/to/nvm2,size=8G >> > +-device nvdimm,id=nvdimm2,memdev=mem2: >> > + >> > +-> { "execute": "query-nvdimms" } >> > +<- { "returns": [ >> > + { >> > + "mem-path": "/path/to/nvm1", >> > + "slot": 0, >> >> TAB damage; please fix. >> > > will fix > >> > + "spa": 17179869184, >> > + "length": 4294967296 >> > + }, >> > + { >> > + "mem-path": "/path/to/nvm2", >> > + "slot": 1, >> > + "spa": 21474836480, >> > + "length": 8589934592 >> > + } >> > + ]} >> >> > +++ b/qapi-schema.json >> > @@ -4646,3 +4646,32 @@ >> > # Since: 2.7 >> > ## >> > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } >> > + >> > +## >> > +# @NvdimmInfo >> > +# >> > +# Information about an NVDIMM device. >> > +# >> > +# @mem-path: the backend file of the NVDIMM device >> > +# >> > +# @slot: the slot index of the NVDIMM device >> > +# >> > +# @spa: the 64-bit SPA base address of the NVDIMM device >> > +# >> > +# @length: the 64-bit size in bytes of the NVDIMM device >> > +# >> > +# Since 2.8 >> > +## >> > +{ 'struct': 'NvdimmInfo', >> > + 'data': {'mem-path' : 'str', 'slot': 'int', 'spa': 'int', 'length': 'int'} } >> > + >> > +## >> > +# @query-nvdimms: >> > +# >> > +# Returns information about each NVDIMM device >> > +# >> > +# Returns: a list of @NvdimmInfo for each device >> > +# >> > +# Since: 2.8 >> > +## >> > +{ 'command': 'query-nvdimms', 'returns': ['NvdimmInfo'] } >> > >> >> Is this something that can be added to the existing query-memdev or >> query-memory-devices command, instead of needing a new command? >> > > Both query-memdev and query-memory-devices do not tell whether a > device is NVDIMM. If adding an optional argument to existing a qmp > command does not break backwards compatibility, I propose to add an > argument "nvdimm" to query-memory-devices which controls whether this > command to return all memory devices or only NVDIMM devices. query-memory-devices returns a list of MemoryDeviceInfo: ## # @MemoryDeviceInfo: # # Union containing information about a memory device # # Since: 2.1 ## { 'union': 'MemoryDeviceInfo', 'data': {'dimm': 'PCDIMMDeviceInfo'} } This is a union, designed to be extended for other types of memory device frontends. Sadly, it's a "simple" union (I dislike those). You could add a new member to be used for the NVDIMM case. It would probably duplicate some or all of PCDIMMDeviceInfo, though. If it needs all of PCDIMMDeviceInfo, you could make the new member's type have PCDIMMDeviceInfo as base. If we can identify information *any* memory device frontend should have, the appropriate design would be a flat union with common information common, and type-specific information in union branches. Could MemoryDeviceInfo be backward-compatibly morphed into such a type? We also have query-memdev, which is about memory backends. PCDIMMDeviceInfo member memdev links from the frontend to the backend. Aside: calling the backends "memdev" and the frontends "memory-devices" is very bad taste. It's also very traditional for QEMU: we call everything "device" or maybe "driver".
On 10/11/16 10:22, Markus Armbruster wrote: > Haozhong Zhang <haozhong.zhang@intel.com> writes: > > > On 10/10/16 14:16, Eric Blake wrote: > >> On 10/09/2016 07:34 PM, Haozhong Zhang wrote: > >> > Xen uses this command to get the backend resource, guest SPA and size of > >> > NVDIMM devices so as to map them to guest. > >> > > >> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > >> > --- > >> > Cc: Markus Armbruster <armbru@redhat.com> > >> > >> > +++ b/docs/qmp-commands.txt > >> > @@ -3800,3 +3800,39 @@ Example for pc machine type started with > >> > "props": {"core-id": 0, "socket-id": 0, "thread-id": 0} > >> > } > >> > ]} > >> > + > >> > +EQMP > >> > + > >> > + { > >> > + .name = "query-nvdimms", > >> > + .args_type = "", > >> > + .mhandler.cmd_new = qmp_marshal_query_nvdimms, > >> > >> Needs rebasing - we no longer need SQMP/EQMP sections or callouts to the > >> initializers, now that commit bd6092e4 has automated the mapping of QAPI > >> to command registration. > >> > > > > good to know this, will change in next version > > > >> > + }, > >> > + > >> > +SQMP > >> > +Show plugged NVDIMM devices > >> > +--------------------------- > >> > + > >> > +Arguments: None. > >> > + > >> > +Example for pc machine type started with > >> > +-object memory-backend-file,id=mem1,mem-path=/path/to/nvm1,size=4G > >> > +-device nvdimm,id=nvdimm1,memdev=mem1 > >> > +-object memory-backend-file,id=mem2,mem-path=/path/to/nvm2,size=8G > >> > +-device nvdimm,id=nvdimm2,memdev=mem2: > >> > + > >> > +-> { "execute": "query-nvdimms" } > >> > +<- { "returns": [ > >> > + { > >> > + "mem-path": "/path/to/nvm1", > >> > + "slot": 0, > >> > >> TAB damage; please fix. > >> > > > > will fix > > > >> > + "spa": 17179869184, > >> > + "length": 4294967296 > >> > + }, > >> > + { > >> > + "mem-path": "/path/to/nvm2", > >> > + "slot": 1, > >> > + "spa": 21474836480, > >> > + "length": 8589934592 > >> > + } > >> > + ]} > >> > >> > +++ b/qapi-schema.json > >> > @@ -4646,3 +4646,32 @@ > >> > # Since: 2.7 > >> > ## > >> > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } > >> > + > >> > +## > >> > +# @NvdimmInfo > >> > +# > >> > +# Information about an NVDIMM device. > >> > +# > >> > +# @mem-path: the backend file of the NVDIMM device > >> > +# > >> > +# @slot: the slot index of the NVDIMM device > >> > +# > >> > +# @spa: the 64-bit SPA base address of the NVDIMM device > >> > +# > >> > +# @length: the 64-bit size in bytes of the NVDIMM device > >> > +# > >> > +# Since 2.8 > >> > +## > >> > +{ 'struct': 'NvdimmInfo', > >> > + 'data': {'mem-path' : 'str', 'slot': 'int', 'spa': 'int', 'length': 'int'} } > >> > + > >> > +## > >> > +# @query-nvdimms: > >> > +# > >> > +# Returns information about each NVDIMM device > >> > +# > >> > +# Returns: a list of @NvdimmInfo for each device > >> > +# > >> > +# Since: 2.8 > >> > +## > >> > +{ 'command': 'query-nvdimms', 'returns': ['NvdimmInfo'] } > >> > > >> > >> Is this something that can be added to the existing query-memdev or > >> query-memory-devices command, instead of needing a new command? > >> > > > > Both query-memdev and query-memory-devices do not tell whether a > > device is NVDIMM. If adding an optional argument to existing a qmp > > command does not break backwards compatibility, I propose to add an > > argument "nvdimm" to query-memory-devices which controls whether this > > command to return all memory devices or only NVDIMM devices. > > query-memory-devices returns a list of MemoryDeviceInfo: > > ## > # @MemoryDeviceInfo: > # > # Union containing information about a memory device > # > # Since: 2.1 > ## > { 'union': 'MemoryDeviceInfo', 'data': {'dimm': 'PCDIMMDeviceInfo'} } > > This is a union, designed to be extended for other types of memory > device frontends. > > Sadly, it's a "simple" union (I dislike those). > > You could add a new member to be used for the NVDIMM case. It would > probably duplicate some or all of PCDIMMDeviceInfo, though. > Cool, I didn't notice it's a union. > If it needs all of PCDIMMDeviceInfo, you could make the new member's > type have PCDIMMDeviceInfo as base. Information so far queried in my patch is just a subset of PCDIMMDeviceInfo. Additional information (e.g. location of NVDIMM label area) may be needed in the future, so I would like to add a new type NVDIMMDeviceInfo with PCDIMMDeviceInfo as base. Thanks, Haozhong > > If we can identify information *any* memory device frontend should have, > the appropriate design would be a flat union with common information > common, and type-specific information in union branches. Could > MemoryDeviceInfo be backward-compatibly morphed into such a type? > > We also have query-memdev, which is about memory backends. > PCDIMMDeviceInfo member memdev links from the frontend to the backend. > > Aside: calling the backends "memdev" and the frontends "memory-devices" > is very bad taste. It's also very traditional for QEMU: we call > everything "device" or maybe "driver". >
On 10/11/2016 03:22 AM, Markus Armbruster wrote: > query-memory-devices returns a list of MemoryDeviceInfo: > > ## > # @MemoryDeviceInfo: > # > # Union containing information about a memory device > # > # Since: 2.1 > ## > { 'union': 'MemoryDeviceInfo', 'data': {'dimm': 'PCDIMMDeviceInfo'} } > > This is a union, designed to be extended for other types of memory > device frontends. > > Sadly, it's a "simple" union (I dislike those). > > You could add a new member to be used for the NVDIMM case. It would > probably duplicate some or all of PCDIMMDeviceInfo, though. > > If it needs all of PCDIMMDeviceInfo, you could make the new member's > type have PCDIMMDeviceInfo as base. > > If we can identify information *any* memory device frontend should have, > the appropriate design would be a flat union with common information > common, and type-specific information in union branches. Could > MemoryDeviceInfo be backward-compatibly morphed into such a type? Yes, conversion to a flat union is possible without breaking existing QMP usage. It would look something like this (with anonymous base and branch classes, which is still one of my pending qapi patches to post): { 'enum': 'MemoryDeviceType', 'data': [ 'dimm', 'nvdimm' ] } { 'union': 'MemoryDeviceInfo', 'base': { 'type': 'MemoryDeviceType' }, 'discriminator': 'type', 'data': { 'dimm': { 'data': 'PCIMMDeviceInfo' }, 'nvdimm': 'whatever_type_here' } } We would still have back-compatible: { "type": "dimm", "data": { "addr":..., "size":..., ... } } for dimm, and for nvdimm, we would have { "type": "nvdimm", whatever fields we want here } whether we want all the fields to be flattened in the nvdimm case, vs. nested (for back-compat) under a 'data' dict in the 'dimm' case, or whether we want both uses to be nested under a 'data' dict for consistency, is a matter of taste.
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt index e0adceb..90e9fb6 100644 --- a/docs/qmp-commands.txt +++ b/docs/qmp-commands.txt @@ -3800,3 +3800,39 @@ Example for pc machine type started with "props": {"core-id": 0, "socket-id": 0, "thread-id": 0} } ]} + +EQMP + + { + .name = "query-nvdimms", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_query_nvdimms, + }, + +SQMP +Show plugged NVDIMM devices +--------------------------- + +Arguments: None. + +Example for pc machine type started with +-object memory-backend-file,id=mem1,mem-path=/path/to/nvm1,size=4G +-device nvdimm,id=nvdimm1,memdev=mem1 +-object memory-backend-file,id=mem2,mem-path=/path/to/nvm2,size=8G +-device nvdimm,id=nvdimm2,memdev=mem2: + +-> { "execute": "query-nvdimms" } +<- { "returns": [ + { + "mem-path": "/path/to/nvm1", + "slot": 0, + "spa": 17179869184, + "length": 4294967296 + }, + { + "mem-path": "/path/to/nvm2", + "slot": 1, + "spa": 21474836480, + "length": 8589934592 + } + ]} diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c index 4cfb94d..eedc128 100644 --- a/hw/acpi/nvdimm.c +++ b/hw/acpi/nvdimm.c @@ -57,7 +57,7 @@ static int nvdimm_plugged_device_list(Object *obj, void *opaque) * Note: it is the caller's responsibility to free the list to avoid * memory leak. */ -static GSList *nvdimm_get_plugged_device_list(void) +GSList *nvdimm_get_plugged_device_list(void) { GSList *list = NULL; diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c index d25993b..99d0cc9 100644 --- a/hw/mem/nvdimm.c +++ b/hw/mem/nvdimm.c @@ -26,6 +26,7 @@ #include "qapi/error.h" #include "qapi/visitor.h" #include "hw/mem/nvdimm.h" +#include "qmp-commands.h" static void nvdimm_get_label_size(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) @@ -180,3 +181,37 @@ static void nvdimm_register_types(void) } type_init(nvdimm_register_types) + +NvdimmInfoList *qmp_query_nvdimms(Error **errp) +{ + NvdimmInfoList *info_list = NULL; + NvdimmInfoList *info; + GSList *device_list = nvdimm_get_plugged_device_list(); + + while (device_list) { + DeviceState *dev = device_list->data; + PCDIMMDevice *parent = PC_DIMM(OBJECT(dev)); + const char *mem_path; + + info = g_new0(NvdimmInfoList, 1); + info->value = g_new0(NvdimmInfo, 1); + + mem_path = object_property_get_str(OBJECT(parent->hostmem), + "mem-path", NULL); + info->value->mem_path = mem_path ? strdup(mem_path) : NULL; + + info->value->slot = object_property_get_int(OBJECT(dev), + PC_DIMM_SLOT_PROP, NULL); + info->value->spa = object_property_get_int(OBJECT(dev), + PC_DIMM_ADDR_PROP, NULL); + info->value->length = object_property_get_int(OBJECT(dev), + PC_DIMM_SIZE_PROP, NULL); + + info->next = info_list; + info_list = info; + device_list = device_list->next; + } + + g_slist_free(device_list); + return info_list; +} diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h index 1cfe9e0..6be269e 100644 --- a/include/hw/mem/nvdimm.h +++ b/include/hw/mem/nvdimm.h @@ -113,4 +113,14 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io, FWCfgState *fw_cfg, Object *owner); void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, BIOSLinker *linker, GArray *dsm_dma_arrea); + +/* + * Inquire plugged NVDIMM devices and link them into the list which is + * returned to the caller. + * + * Note: it is the caller's responsibility to free the list to avoid + * memory leak. + */ +GSList *nvdimm_get_plugged_device_list(void); + #endif diff --git a/qapi-schema.json b/qapi-schema.json index c3dcf11..6246255 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4646,3 +4646,32 @@ # Since: 2.7 ## { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } + +## +# @NvdimmInfo +# +# Information about an NVDIMM device. +# +# @mem-path: the backend file of the NVDIMM device +# +# @slot: the slot index of the NVDIMM device +# +# @spa: the 64-bit SPA base address of the NVDIMM device +# +# @length: the 64-bit size in bytes of the NVDIMM device +# +# Since 2.8 +## +{ 'struct': 'NvdimmInfo', + 'data': {'mem-path' : 'str', 'slot': 'int', 'spa': 'int', 'length': 'int'} } + +## +# @query-nvdimms: +# +# Returns information about each NVDIMM device +# +# Returns: a list of @NvdimmInfo for each device +# +# Since: 2.8 +## +{ 'command': 'query-nvdimms', 'returns': ['NvdimmInfo'] }
Xen uses this command to get the backend resource, guest SPA and size of NVDIMM devices so as to map them to guest. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- Cc: Markus Armbruster <armbru@redhat.com> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Eric Blake <eblake@redhat.com> --- docs/qmp-commands.txt | 36 ++++++++++++++++++++++++++++++++++++ hw/acpi/nvdimm.c | 2 +- hw/mem/nvdimm.c | 35 +++++++++++++++++++++++++++++++++++ include/hw/mem/nvdimm.h | 10 ++++++++++ qapi-schema.json | 29 +++++++++++++++++++++++++++++ 5 files changed, 111 insertions(+), 1 deletion(-)