* [Qemu-devel] [PATCH V3 1/1] Add pmemsave command for both monitor and qmp, which could be useful to have qemu-softmmu as a cross debugger to load guest physical memory.
@ 2014-04-08 19:30 Baojun Wang
2014-04-08 19:42 ` Eric Blake
0 siblings, 1 reply; 6+ messages in thread
From: Baojun Wang @ 2014-04-08 19:30 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, wangbj
I found this could be useful to have qemu-softmmu as a cross debugger (launch
with -s -S command line option), then if we can have a command to load guest
physical memory, we can use cross gdb to do some target debug which gdb cannot
do directly.
Many thanks to Eric Blake for review the patch.
---
cpus.c | 24 ++++++++++++++++++++++++
hmp-commands.hx | 13 +++++++++++++
hmp.c | 11 +++++++++++
hmp.h | 1 +
qapi-schema.json | 20 ++++++++++++++++++++
qmp-commands.hx | 27 +++++++++++++++++++++++++++
6 files changed, 96 insertions(+)
diff --git a/cpus.c b/cpus.c
index 1104d61..03d1277 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1467,6 +1467,30 @@ exit:
fclose(f);
}
+void qmp_pmemload(int64_t addr, int64_t size, const char *filename,
+ Error **errp)
+{
+ FILE *f;
+ uint32_t l;
+ uint8_t buf[1024];
+
+ f = fopen(filename, "rb");
+ if (!f) {
+ error_setg_file_open(errp, errno, filename);
+ return;
+ }
+
+ while (size != 0) {
+ l = fread(buf, 1, sizeof(buf), f);
+ if (l > size)
+ l = size;
+ cpu_physical_memory_rw(addr, buf, l, 1);
+ addr += l;
+ size -= l;
+ }
+
+ fclose(f);
+}
void qmp_inject_nmi(Error **errp)
{
#if defined(TARGET_I386)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f3fc514..18604a6 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -809,6 +809,19 @@ save to disk physical memory dump starting at @var{addr} of size @var{size}.
ETEXI
{
+ .name = "pmemload",
+ .args_type = "val:l,size:i,filename:s",
+ .params = "addr size file",
+ .help = "load from disk physical memory dump starting at 'addr' of size 'size'",
+ .mhandler.cmd = hmp_pmemload,
+ },
+
+STEXI
+@item pmemload @var{addr} @var{size} @var{file}
+@findex pmemload
+load from disk physical memory dump starting at @var{addr} of size @var{size}.
+ETEXI
+ {
.name = "boot_set",
.args_type = "bootdevice:s",
.params = "bootdevice",
diff --git a/hmp.c b/hmp.c
index 2f279c4..6e932f9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -767,6 +767,17 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &errp);
}
+void hmp_pmemload(Monitor *mon, const QDict *qdict)
+{
+ uint32_t size = qdict_get_int(qdict, "size");
+ const char *filename = qdict_get_str(qdict, "filename");
+ uint64_t addr = qdict_get_int(qdict, "val");
+ Error *errp = NULL;
+
+ qmp_pmemload(addr, size, filename, &errp);
+ hmp_handle_error(mon, &errp);
+}
+
void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
{
const char *chardev = qdict_get_str(qdict, "device");
diff --git a/hmp.h b/hmp.h
index ed58f0e..f5f2a16 100644
--- a/hmp.h
+++ b/hmp.h
@@ -44,6 +44,7 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
void hmp_cpu(Monitor *mon, const QDict *qdict);
void hmp_memsave(Monitor *mon, const QDict *qdict);
void hmp_pmemsave(Monitor *mon, const QDict *qdict);
+void hmp_pmemload(Monitor *mon, const QDict *qdict);
void hmp_ringbuf_write(Monitor *mon, const QDict *qdict);
void hmp_ringbuf_read(Monitor *mon, const QDict *qdict);
void hmp_cont(Monitor *mon, const QDict *qdict);
diff --git a/qapi-schema.json b/qapi-schema.json
index 391356f..f251f5d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1708,6 +1708,26 @@
'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
##
+# @pmemload:
+#
+# Load a portion of guest physical memory from a file.
+#
+# @val: the physical address of the guest to start from
+#
+# @size: the size of memory region to load
+#
+# @filename: the file to load the memory from as binary data
+#
+# Returns: Nothing on success
+#
+# Since: 2.1
+#
+# Notes: Errors were not reliably returned until 2.1
+##
+{ 'command': 'pmemload',
+ 'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
+
+##
# @cont:
#
# Resume guest VCPU execution.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ed3ab92..584d6cf 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -468,6 +468,33 @@ Example:
EQMP
{
+ .name = "pmemload",
+ .args_type = "val:l,size:i,filename:s",
+ .mhandler.cmd_new = qmp_marshal_input_pmemload,
+ },
+
+SQMP
+pmemload
+--------
+
+load from disk physical memory dump starting at 'val' of size 'size'.
+
+Arguments:
+
+- "val": the starting address (json-int)
+- "size": the memory size, in bytes (json-int)
+- "filename": file path (json-string)
+
+Example:
+
+-> { "execute": "pmemload",
+ "arguments": { "val": 10,
+ "size": 100,
+ "filename": "/tmp/physical-mem-dump" } }
+<- { "return": {} }
+
+EQMP
+ {
.name = "inject-nmi",
.args_type = "",
.mhandler.cmd_new = qmp_marshal_input_inject_nmi,
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH V3 1/1] Add pmemsave command for both monitor and qmp, which could be useful to have qemu-softmmu as a cross debugger to load guest physical memory.
2014-04-08 19:30 [Qemu-devel] [PATCH V3 1/1] Add pmemsave command for both monitor and qmp, which could be useful to have qemu-softmmu as a cross debugger to load guest physical memory Baojun Wang
@ 2014-04-08 19:42 ` Eric Blake
2014-04-08 23:29 ` Baojun Wang
0 siblings, 1 reply; 6+ messages in thread
From: Eric Blake @ 2014-04-08 19:42 UTC (permalink / raw)
To: Baojun Wang, qemu-devel; +Cc: qemu-trivial
[-- Attachment #1: Type: text/plain, Size: 3365 bytes --]
On 04/08/2014 01:30 PM, Baojun Wang wrote:
Your subject line is extremely long. In general, we strive for
something less than 60 characters, so that 'git shortlog -30' can
display the entire line in an 80-column terminal. Also, the subject
mentions pmemsave, but your commit mentions pmemload - it's very
misleading to provide a patch where the commit message doesn't even
mention the name of the command the patch is adding. I would suggest a
subject line of:
qmp: add pmemload command
> I found this could be useful to have qemu-softmmu as a cross debugger (launch
> with -s -S command line option), then if we can have a command to load guest
> physical memory, we can use cross gdb to do some target debug which gdb cannot
> do directly.
>
Your patch is lacking a Signed-off-by designation, therefore we cannot
accept it yet.
> Many thanks to Eric Blake for review the patch.
This comment may be useful to reviewers, but is not part of the commit
itself, so it belongs better...
>
> ---
...here, after the --- separator.
>
> +void qmp_pmemload(int64_t addr, int64_t size, const char *filename,
> + Error **errp)
> +{
> + FILE *f;
> + uint32_t l;
> + uint8_t buf[1024];
> +
> + f = fopen(filename, "rb");
Rather than using fopen() and FILE* operations, I'd prefer you use
qemu_open() and lower-level read() operations. In particular, this will
automatically make it possible for management applications to pass in
'/dev/fdset/1' to reuse a file descriptor that they passed in to qemu,
rather than forcing qemu to directly open the file.
> + if (!f) {
> + error_setg_file_open(errp, errno, filename);
> + return;
> + }
> +
> + while (size != 0) {
> + l = fread(buf, 1, sizeof(buf), f);
> + if (l > size)
> + l = size;
This is lousy at error detection. Again, you should be using read(),
not fread(), and properly erroring out on read failures rather than
silently ignoring them.
> + .name = "pmemload",
> + .args_type = "val:l,size:i,filename:s",
Would it make sense to put filename as the FIRST argument, with val and
size as optional arguments (defaulting to reading in at address 0 and
for the size determined by the length of the input file), rather than
forcing the user to pass in a size up front?
> +++ b/qapi-schema.json
> @@ -1708,6 +1708,26 @@
> 'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
>
> ##
> +# @pmemload:
> +#
> +# Load a portion of guest physical memory from a file.
> +#
> +# @val: the physical address of the guest to start from
> +#
> +# @size: the size of memory region to load
> +#
> +# @filename: the file to load the memory from as binary data
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 2.1
> +#
> +# Notes: Errors were not reliably returned until 2.1
Delete this line. I guess I didn't make it clear in my first review
that this line is bogus copy and paste. You don't need it. pmemsave
needed it, because pmemsave went through some iterations where earlier
versions were buggy. But your pmemload command is brand new, and should
not have bugs worth worrying about.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH V3 1/1] Add pmemsave command for both monitor and qmp, which could be useful to have qemu-softmmu as a cross debugger to load guest physical memory.
2014-04-08 19:42 ` Eric Blake
@ 2014-04-08 23:29 ` Baojun Wang
2014-04-09 16:54 ` [Qemu-devel] [PATCH V4 1/1] qmp: add pmemload command Baojun Wang
0 siblings, 1 reply; 6+ messages in thread
From: Baojun Wang @ 2014-04-08 23:29 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-trivial, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5013 bytes --]
(Re-send because missing reply-to-all)
Really appreciate your time and valuable input Eric. I almost didn't submit
any patches, so there is quite a lot to learn about it.
I'd agree with you on this, but memsave/pmemsave function are using stdio,
thus I was trying to be consistent with them.
> > + if (!f) {
> > + error_setg_file_open(errp, errno, filename);
> > + return;
> > + }
> > +
> > + while (size != 0) {
> > + l = fread(buf, 1, sizeof(buf), f);
> > + if (l > size)
> > + l = size;
>
> How about error handling as below, the same error handling is used by
memsave/pmemsave.
l = fread(buf, 1, sizeof(buf), f);
if (l ! = sizeof(buf)) {
error_set(errp, QERR_IO_ERROR);
goto eixt;
}
> This is lousy at error detection. Again, you should be using read(),
> not fread(), and properly erroring out on read failures rather than
> silently ignoring them.
>
>
> > + .name = "pmemload",
> > + .args_type = "val:l,size:i,filename:s",
>
> Would it make sense to put filename as the FIRST argument, with val and
> size as optional arguments (defaulting to reading in at address 0 and
> for the size determined by the length of the input file), rather than
> forcing the user to pass in a size up front?
>
> Same reason as above, I though it could be easier if pmemload takes the
same arguments as memsave/pmemsave.
Again, really thanks for your patient.
Best Regards
On Tue, Apr 8, 2014 at 12:42 PM, Eric Blake <eblake@redhat.com> wrote:
> On 04/08/2014 01:30 PM, Baojun Wang wrote:
>
> Your subject line is extremely long. In general, we strive for
> something less than 60 characters, so that 'git shortlog -30' can
> display the entire line in an 80-column terminal. Also, the subject
> mentions pmemsave, but your commit mentions pmemload - it's very
> misleading to provide a patch where the commit message doesn't even
> mention the name of the command the patch is adding. I would suggest a
> subject line of:
>
> qmp: add pmemload command
>
> > I found this could be useful to have qemu-softmmu as a cross debugger
> (launch
> > with -s -S command line option), then if we can have a command to load
> guest
> > physical memory, we can use cross gdb to do some target debug which gdb
> cannot
> > do directly.
> >
>
> Your patch is lacking a Signed-off-by designation, therefore we cannot
> accept it yet.
>
> > Many thanks to Eric Blake for review the patch.
>
> This comment may be useful to reviewers, but is not part of the commit
> itself, so it belongs better...
>
> >
> > ---
>
> ...here, after the --- separator.
>
>
> >
> > +void qmp_pmemload(int64_t addr, int64_t size, const char *filename,
> > + Error **errp)
> > +{
> > + FILE *f;
> > + uint32_t l;
> > + uint8_t buf[1024];
> > +
> > + f = fopen(filename, "rb");
>
> Rather than using fopen() and FILE* operations, I'd prefer you use
> qemu_open() and lower-level read() operations. In particular, this will
> automatically make it possible for management applications to pass in
> '/dev/fdset/1' to reuse a file descriptor that they passed in to qemu,
> rather than forcing qemu to directly open the file.
>
> > + if (!f) {
> > + error_setg_file_open(errp, errno, filename);
> > + return;
> > + }
> > +
> > + while (size != 0) {
> > + l = fread(buf, 1, sizeof(buf), f);
> > + if (l > size)
> > + l = size;
>
> This is lousy at error detection. Again, you should be using read(),
> not fread(), and properly erroring out on read failures rather than
> silently ignoring them.
>
>
> > + .name = "pmemload",
> > + .args_type = "val:l,size:i,filename:s",
>
> Would it make sense to put filename as the FIRST argument, with val and
> size as optional arguments (defaulting to reading in at address 0 and
> for the size determined by the length of the input file), rather than
> forcing the user to pass in a size up front?
>
> > +++ b/qapi-schema.json
> > @@ -1708,6 +1708,26 @@
> > 'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
> >
> > ##
> > +# @pmemload:
> > +#
> > +# Load a portion of guest physical memory from a file.
> > +#
> > +# @val: the physical address of the guest to start from
> > +#
> > +# @size: the size of memory region to load
> > +#
> > +# @filename: the file to load the memory from as binary data
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 2.1
> > +#
> > +# Notes: Errors were not reliably returned until 2.1
>
> Delete this line. I guess I didn't make it clear in my first review
> that this line is bogus copy and paste. You don't need it. pmemsave
> needed it, because pmemsave went through some iterations where earlier
> versions were buggy. But your pmemload command is brand new, and should
> not have bugs worth worrying about.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>
[-- Attachment #2: Type: text/html, Size: 7054 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH V4 1/1] qmp: add pmemload command
2014-04-08 23:29 ` Baojun Wang
@ 2014-04-09 16:54 ` Baojun Wang
2014-04-09 17:02 ` Eric Blake
0 siblings, 1 reply; 6+ messages in thread
From: Baojun Wang @ 2014-04-09 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Baojun Wang
Signed-off-by: Baojun Wang <wangbj@gmail.com>
---
cpus.c | 29 +++++++++++++++++++++++++++++
hmp-commands.hx | 13 +++++++++++++
hmp.c | 11 +++++++++++
hmp.h | 1 +
qapi-schema.json | 18 ++++++++++++++++++
qmp-commands.hx | 27 +++++++++++++++++++++++++++
6 files changed, 99 insertions(+)
diff --git a/cpus.c b/cpus.c
index 1104d61..9617d43 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1467,6 +1467,35 @@ exit:
fclose(f);
}
+void qmp_pmemload(int64_t addr, int64_t size, const char *filename,
+ Error **errp)
+{
+ FILE *f;
+ uint32_t l;
+ uint8_t buf[1024];
+
+ f = fopen(filename, "rb");
+ if (!f) {
+ error_setg_file_open(errp, errno, filename);
+ return;
+ }
+
+ while (size != 0) {
+ l = fread(buf, 1, sizeof(buf), f);
+ if (l != sizeof(buf)) {
+ error_set(errp, QERR_IO_ERROR);
+ goto exit;
+ }
+ if (l > size)
+ l = size;
+ cpu_physical_memory_rw(addr, buf, l, 1);
+ addr += l;
+ size -= l;
+ }
+
+exit:
+ fclose(f);
+}
void qmp_inject_nmi(Error **errp)
{
#if defined(TARGET_I386)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f3fc514..18604a6 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -809,6 +809,19 @@ save to disk physical memory dump starting at @var{addr} of size @var{size}.
ETEXI
{
+ .name = "pmemload",
+ .args_type = "val:l,size:i,filename:s",
+ .params = "addr size file",
+ .help = "load from disk physical memory dump starting at 'addr' of size 'size'",
+ .mhandler.cmd = hmp_pmemload,
+ },
+
+STEXI
+@item pmemload @var{addr} @var{size} @var{file}
+@findex pmemload
+load from disk physical memory dump starting at @var{addr} of size @var{size}.
+ETEXI
+ {
.name = "boot_set",
.args_type = "bootdevice:s",
.params = "bootdevice",
diff --git a/hmp.c b/hmp.c
index 2f279c4..6e932f9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -767,6 +767,17 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &errp);
}
+void hmp_pmemload(Monitor *mon, const QDict *qdict)
+{
+ uint32_t size = qdict_get_int(qdict, "size");
+ const char *filename = qdict_get_str(qdict, "filename");
+ uint64_t addr = qdict_get_int(qdict, "val");
+ Error *errp = NULL;
+
+ qmp_pmemload(addr, size, filename, &errp);
+ hmp_handle_error(mon, &errp);
+}
+
void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
{
const char *chardev = qdict_get_str(qdict, "device");
diff --git a/hmp.h b/hmp.h
index ed58f0e..f5f2a16 100644
--- a/hmp.h
+++ b/hmp.h
@@ -44,6 +44,7 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
void hmp_cpu(Monitor *mon, const QDict *qdict);
void hmp_memsave(Monitor *mon, const QDict *qdict);
void hmp_pmemsave(Monitor *mon, const QDict *qdict);
+void hmp_pmemload(Monitor *mon, const QDict *qdict);
void hmp_ringbuf_write(Monitor *mon, const QDict *qdict);
void hmp_ringbuf_read(Monitor *mon, const QDict *qdict);
void hmp_cont(Monitor *mon, const QDict *qdict);
diff --git a/qapi-schema.json b/qapi-schema.json
index 391356f..60f9782 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1708,6 +1708,24 @@
'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
##
+# @pmemload:
+#
+# Load a portion of guest physical memory from a file.
+#
+# @val: the physical address of the guest to start from
+#
+# @size: the size of memory region to load
+#
+# @filename: the file to load the memory from as binary data
+#
+# Returns: Nothing on success
+#
+# Since: 2.1
+##
+{ 'command': 'pmemload',
+ 'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
+
+##
# @cont:
#
# Resume guest VCPU execution.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ed3ab92..584d6cf 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -468,6 +468,33 @@ Example:
EQMP
{
+ .name = "pmemload",
+ .args_type = "val:l,size:i,filename:s",
+ .mhandler.cmd_new = qmp_marshal_input_pmemload,
+ },
+
+SQMP
+pmemload
+--------
+
+load from disk physical memory dump starting at 'val' of size 'size'.
+
+Arguments:
+
+- "val": the starting address (json-int)
+- "size": the memory size, in bytes (json-int)
+- "filename": file path (json-string)
+
+Example:
+
+-> { "execute": "pmemload",
+ "arguments": { "val": 10,
+ "size": 100,
+ "filename": "/tmp/physical-mem-dump" } }
+<- { "return": {} }
+
+EQMP
+ {
.name = "inject-nmi",
.args_type = "",
.mhandler.cmd_new = qmp_marshal_input_inject_nmi,
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH V4 1/1] qmp: add pmemload command
2014-04-09 16:54 ` [Qemu-devel] [PATCH V4 1/1] qmp: add pmemload command Baojun Wang
@ 2014-04-09 17:02 ` Eric Blake
2014-04-09 17:49 ` [Qemu-devel] [PATCH V5 " Baojun Wang
0 siblings, 1 reply; 6+ messages in thread
From: Eric Blake @ 2014-04-09 17:02 UTC (permalink / raw)
To: Baojun Wang, qemu-devel; +Cc: qemu-trivial
[-- Attachment #1: Type: text/plain, Size: 1714 bytes --]
On 04/09/2014 10:54 AM, Baojun Wang wrote:
> Signed-off-by: Baojun Wang <wangbj@gmail.com>
You lost this part of your commit message, which gives more details
about the 'why' (the subject line covers the 'what', but it is often the
'why' that is most needed when reviewing a commit later). Your earlier
mail had:
I found this could be useful to have qemu-softmmu as a cross debugger
(launch
with -s -S command line option), then if we can have a command to load guest
physical memory, we can use cross gdb to do some target debug which gdb
cannot
do directly.
>
> +void qmp_pmemload(int64_t addr, int64_t size, const char *filename,
> + Error **errp)
> +{
> + FILE *f;
> + uint32_t l;
> + uint8_t buf[1024];
> +
> + f = fopen(filename, "rb");
I still think that fopen()/fread() is wrong, and that you should be
using qemu_open()/read() - that way, libvirt could pass in a file
descriptor and use qemu's already-existing /dev/fdset/nnn support rather
than forcing qemu to open something from the filesystem.
> +++ b/hmp-commands.hx
> @@ -809,6 +809,19 @@ save to disk physical memory dump starting at @var{addr} of size @var{size}.
> ETEXI
>
> {
> + .name = "pmemload",
> + .args_type = "val:l,size:i,filename:s",
> + .params = "addr size file",
> + .help = "load from disk physical memory dump starting at 'addr' of size 'size'",
> + .mhandler.cmd = hmp_pmemload,
And I still think that you should list filename first, with val and size
optional at the end.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH V5 1/1] qmp: add pmemload command
2014-04-09 17:02 ` Eric Blake
@ 2014-04-09 17:49 ` Baojun Wang
0 siblings, 0 replies; 6+ messages in thread
From: Baojun Wang @ 2014-04-09 17:49 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Baojun Wang
I found this could be useful to have qemu-softmmu as a cross debugger (launch
with -s -S command line option), then if we can have a command to load guest
physical memory, we can use cross gdb to do some target debug which gdb cannot
do directly.
Many thanks to Eric Blake for review the patch and suggestions.
Signed-off-by: Baojun Wang <wangbj@gmail.com>
---
cpus.c | 29 +++++++++++++++++++++++++++++
hmp-commands.hx | 13 +++++++++++++
hmp.c | 11 +++++++++++
hmp.h | 1 +
qapi-schema.json | 18 ++++++++++++++++++
qmp-commands.hx | 27 +++++++++++++++++++++++++++
6 files changed, 99 insertions(+)
diff --git a/cpus.c b/cpus.c
index 1104d61..230664f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1467,6 +1467,35 @@ exit:
fclose(f);
}
+void qmp_pmemload(const char* filename, int64_t addr, int64_t size,
+ Error **errp)
+{
+ int fd;
+ ssize_t l;
+ uint8_t buf[4096];
+
+ fd = qemu_open(filename, O_RDONLY | O_BINARY);
+ if (fd < 0) {
+ error_setg_file_open(errp, errno, filename);
+ return;
+ }
+
+ while (size != 0) {
+ l = read(fd, buf, sizeof(buf));
+ if (l != sizeof(buf)) {
+ error_set(errp, QERR_IO_ERROR);
+ goto exit;
+ }
+ if (l > size)
+ l = size;
+ cpu_physical_memory_rw(addr, buf, l, 1);
+ addr += l;
+ size -= l;
+ }
+
+exit:
+ qemu_close(fd);
+}
void qmp_inject_nmi(Error **errp)
{
#if defined(TARGET_I386)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f3fc514..1eae6e3 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -809,6 +809,19 @@ save to disk physical memory dump starting at @var{addr} of size @var{size}.
ETEXI
{
+ .name = "pmemload",
+ .args_type = "filename:s,val:l,size:i",
+ .params = "file addr size",
+ .help = "load from disk physical memory dump starting at 'addr' of size 'size'",
+ .mhandler.cmd = hmp_pmemload,
+ },
+
+STEXI
+@item pmemload @var{file} @var{addr} @var{size}
+@findex pmemload
+load from disk physical memory dump starting at @var{addr} of size @var{size}.
+ETEXI
+ {
.name = "boot_set",
.args_type = "bootdevice:s",
.params = "bootdevice",
diff --git a/hmp.c b/hmp.c
index 2f279c4..745c087 100644
--- a/hmp.c
+++ b/hmp.c
@@ -767,6 +767,17 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &errp);
}
+void hmp_pmemload(Monitor *mon, const QDict *qdict)
+{
+ uint32_t size = qdict_get_int(qdict, "size");
+ const char *filename = qdict_get_str(qdict, "filename");
+ uint64_t addr = qdict_get_int(qdict, "val");
+ Error *errp = NULL;
+
+ qmp_pmemload(filename, addr, size, &errp);
+ hmp_handle_error(mon, &errp);
+}
+
void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
{
const char *chardev = qdict_get_str(qdict, "device");
diff --git a/hmp.h b/hmp.h
index ed58f0e..f5f2a16 100644
--- a/hmp.h
+++ b/hmp.h
@@ -44,6 +44,7 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
void hmp_cpu(Monitor *mon, const QDict *qdict);
void hmp_memsave(Monitor *mon, const QDict *qdict);
void hmp_pmemsave(Monitor *mon, const QDict *qdict);
+void hmp_pmemload(Monitor *mon, const QDict *qdict);
void hmp_ringbuf_write(Monitor *mon, const QDict *qdict);
void hmp_ringbuf_read(Monitor *mon, const QDict *qdict);
void hmp_cont(Monitor *mon, const QDict *qdict);
diff --git a/qapi-schema.json b/qapi-schema.json
index 391356f..7198242 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1708,6 +1708,24 @@
'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
##
+# @pmemload:
+#
+# Load a portion of guest physical memory from a file.
+#
+# @filename: the file to load the memory from as binary data
+#
+# @val: the physical address of the guest to start from
+#
+# @size: the size of memory region to load
+#
+# Returns: Nothing on success
+#
+# Since: 2.1
+##
+{ 'command': 'pmemload',
+ 'data': {'filename': 'str', 'val': 'int', 'size': 'int'} }
+
+##
# @cont:
#
# Resume guest VCPU execution.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ed3ab92..2831038 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -468,6 +468,33 @@ Example:
EQMP
{
+ .name = "pmemload",
+ .args_type = "filename:s,val:l,size:i",
+ .mhandler.cmd_new = qmp_marshal_input_pmemload,
+ },
+
+SQMP
+pmemload
+--------
+
+load from disk physical memory dump starting at 'val' of size 'size'.
+
+Arguments:
+
+- "filename": file path (json-string)
+- "val": the starting address (json-int)
+- "size": the memory size, in bytes (json-int)
+
+Example:
+
+-> { "execute": "pmemload",
+ "arguments": { "filename": "/tmp/physical-mem-dump",
+ "val": 10,
+ "size": 100 } }
+<- { "return": {} }
+
+EQMP
+ {
.name = "inject-nmi",
.args_type = "",
.mhandler.cmd_new = qmp_marshal_input_inject_nmi,
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-09 17:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-08 19:30 [Qemu-devel] [PATCH V3 1/1] Add pmemsave command for both monitor and qmp, which could be useful to have qemu-softmmu as a cross debugger to load guest physical memory Baojun Wang
2014-04-08 19:42 ` Eric Blake
2014-04-08 23:29 ` Baojun Wang
2014-04-09 16:54 ` [Qemu-devel] [PATCH V4 1/1] qmp: add pmemload command Baojun Wang
2014-04-09 17:02 ` Eric Blake
2014-04-09 17:49 ` [Qemu-devel] [PATCH V5 " Baojun Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).