From: Haozhong Zhang <haozhong.zhang@intel.com> To: Markus Armbruster <armbru@redhat.com> Cc: Eric Blake <eblake@redhat.com>, xen-devel@lists.xensource.com, Xiao Guangrong <guangrong.xiao@linux.intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, qemu-devel@nongnu.org, Igor Mammedov <imammedo@redhat.com> Subject: Re: [Qemu-devel] [RFC QEMU PATCH 8/8] qmp: add a qmp command 'query-nvdimms' to get plugged NVDIMM devices Date: Tue, 11 Oct 2016 18:12:27 +0800 [thread overview] Message-ID: <20161011101227.vyk4lpw63csphl7f@hz-desktop> (raw) In-Reply-To: <87d1j7pa62.fsf@dusky.pond.sub.org> 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". >
WARNING: multiple messages have this Message-ID (diff)
From: Haozhong Zhang <haozhong.zhang@intel.com> To: Markus Armbruster <armbru@redhat.com> Cc: xen-devel@lists.xensource.com, Xiao Guangrong <guangrong.xiao@linux.intel.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, "Michael S. Tsirkin" <mst@redhat.com>, qemu-devel@nongnu.org, Igor Mammedov <imammedo@redhat.com> Subject: Re: [RFC QEMU PATCH 8/8] qmp: add a qmp command 'query-nvdimms' to get plugged NVDIMM devices Date: Tue, 11 Oct 2016 18:12:27 +0800 [thread overview] Message-ID: <20161011101227.vyk4lpw63csphl7f@hz-desktop> (raw) In-Reply-To: <87d1j7pa62.fsf@dusky.pond.sub.org> 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". >
next prev parent reply other threads:[~2016-10-11 10:12 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-10-10 0:34 [Qemu-devel] [RFC QEMU PATCH 0/8] Implement vNVDIMM for Xen HVM guest Haozhong Zhang 2016-10-10 0:34 ` Haozhong Zhang 2016-10-10 0:34 ` [Qemu-devel] [RFC QEMU PATCH 1/8] nvdimm: do not initialize label_data if label_size is zero Haozhong Zhang 2016-10-10 0:34 ` Haozhong Zhang 2017-02-15 22:18 ` [Qemu-devel] " Konrad Rzeszutek Wilk 2017-02-15 22:18 ` Konrad Rzeszutek Wilk 2016-10-10 0:34 ` [Qemu-devel] [RFC QEMU PATCH 2/8] xen-hvm: add a function to copy ACPI to guest Haozhong Zhang 2016-10-10 0:34 ` Haozhong Zhang 2016-10-10 0:34 ` [Qemu-devel] [RFC QEMU PATCH 3/8] nvdimm acpi: do not use fw_cfg on Xen Haozhong Zhang 2016-10-10 0:34 ` Haozhong Zhang 2016-10-10 0:34 ` [Qemu-devel] [RFC QEMU PATCH 4/8] nvdimm acpi: build and copy NFIT to guest " Haozhong Zhang 2016-10-10 0:34 ` Haozhong Zhang 2016-10-10 0:34 ` [Qemu-devel] [RFC QEMU PATCH 5/8] nvdimm acpi: build and copy NVDIMM namespace devices " Haozhong Zhang 2016-10-10 0:34 ` Haozhong Zhang 2016-10-10 0:34 ` [Qemu-devel] [RFC QEMU PATCH 6/8] hostmem: add a host memory backend for Xen Haozhong Zhang 2016-10-10 0:34 ` Haozhong Zhang 2016-10-10 0:34 ` [Qemu-devel] [RFC QEMU PATCH 7/8] xen-hvm: create hotplug memory region for HVM guest Haozhong Zhang 2016-10-10 0:34 ` Haozhong Zhang 2016-10-10 0:34 ` [Qemu-devel] [RFC QEMU PATCH 8/8] qmp: add a qmp command 'query-nvdimms' to get plugged NVDIMM devices Haozhong Zhang 2016-10-10 0:34 ` Haozhong Zhang 2016-10-10 19:16 ` [Qemu-devel] " Eric Blake 2016-10-10 19:16 ` Eric Blake 2016-10-11 6:31 ` [Qemu-devel] " Haozhong Zhang 2016-10-11 6:31 ` Haozhong Zhang 2016-10-11 8:22 ` [Qemu-devel] " Markus Armbruster 2016-10-11 8:22 ` Markus Armbruster 2016-10-11 10:12 ` Haozhong Zhang [this message] 2016-10-11 10:12 ` Haozhong Zhang 2016-10-11 14:45 ` [Qemu-devel] " Eric Blake 2016-10-11 14:45 ` Eric Blake 2016-10-10 16:52 ` [Qemu-devel] [RFC QEMU PATCH 0/8] Implement vNVDIMM for Xen HVM guest no-reply 2016-10-10 16:52 ` no-reply
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20161011101227.vyk4lpw63csphl7f@hz-desktop \ --to=haozhong.zhang@intel.com \ --cc=armbru@redhat.com \ --cc=eblake@redhat.com \ --cc=guangrong.xiao@linux.intel.com \ --cc=imammedo@redhat.com \ --cc=konrad.wilk@oracle.com \ --cc=mst@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=xen-devel@lists.xensource.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.