[Qemu-devel] [PATCH for 2.9?] hmp: gpa2hva and gpa2hpa hostaddr command

Paolo Bonzini posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1489512045-49206-1-git-send-email-pbonzini@redhat.com
Test checkpatch failed
Test docker passed
There is a newer version of this series
hmp-commands.hx |  32 ++++++++++++++++++
monitor.c       | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 135 insertions(+)
[Qemu-devel] [PATCH for 2.9?] hmp: gpa2hva and gpa2hpa hostaddr command
Posted by Paolo Bonzini 7 years, 1 month ago
These commands are useful when testing machine-check passthrough.
gpa2hva is useful to inject a MADV_HWPOISON madvise from gdb, while
gpa2hpa is useful to inject an error with the mce-inject kernel
module.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	Technically a feature, but on the other hand it is pretty
	much impossible to test without it, and the people testing
	the feature are using it as an out-of-tree patch.  Opinions?

 hmp-commands.hx |  32 ++++++++++++++++++
 monitor.c       | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8819281..0aca984 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -526,6 +526,38 @@ Dump 80 16 bit values at the start of the video memory.
 ETEXI
 
     {
+        .name       = "gpa2hva",
+        .args_type  = "addr:l",
+        .params     = "addr",
+        .help       = "print the host virtual address corresponding to a guest physical address",
+        .cmd        = hmp_gpa2hva,
+    },
+
+STEXI
+@item gpa2hva @var{addr}
+@findex gpa2hva
+Print the host virtual address at which the guest's physical address @var{addr}
+is mapped.
+ETEXI
+
+#ifdef CONFIG_LINUX
+    {
+        .name       = "gpa2hpa",
+        .args_type  = "addr:l",
+        .params     = "addr",
+        .help       = "print the host physical address corresponding to a guest physical address",
+        .cmd        = hmp_gpa2hpa,
+    },
+#endif
+
+STEXI
+@item gpa2hpa @var{addr}
+@findex gpa2hpa
+Print the host physical address at which the guest's physical address @var{addr}
+is mapped.
+ETEXI
+
+    {
         .name       = "p|print",
         .args_type  = "fmt:/,val:l",
         .params     = "/fmt expr",
diff --git a/monitor.c b/monitor.c
index be282ec..1ac8c5f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1421,6 +1421,109 @@ static void hmp_physical_memory_dump(Monitor *mon, const QDict *qdict)
     memory_dump(mon, count, format, size, addr, 1);
 }
 
+static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
+{
+    MemoryRegionSection mrs = memory_region_find(get_system_memory(),
+                                                 addr, 1);
+
+    if (!mrs.mr) {
+        error_setg(errp, "No memory is mapped at address 0x%" HWADDR_PRIx, addr);
+        return NULL;
+    }
+
+    if (!memory_region_is_ram(mrs.mr) && !memory_region_is_romd(mrs.mr)) {
+        error_setg(errp, "Memory at address 0x%" HWADDR_PRIx "is not RAM", addr);
+        memory_region_unref(mrs.mr);
+        return NULL;
+    }
+
+    *p_mr = mrs.mr;
+    return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
+}
+
+static void hmp_gpa2hva(Monitor *mon, const QDict *qdict)
+{
+    hwaddr addr = qdict_get_int(qdict, "addr");
+    Error *local_err = NULL;
+    MemoryRegion *mr;
+    void *ptr;
+
+    ptr = gpa2hva(&mr, addr, &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+        return;
+    }
+
+    monitor_printf(mon, "Host virtual address for 0x%" HWADDR_PRIx
+                   " (%s) is %p\n",
+                   addr, mr->name, ptr);
+
+    memory_region_unref(mr);
+}
+
+#ifdef CONFIG_LINUX
+static uint64_t vtop(void *ptr, Error **errp)
+{
+    uint64_t pinfo;
+    uint64_t ret = -1;
+    uintptr_t addr = (uintptr_t) ptr;
+    uintptr_t pagesize = getpagesize();
+    off_t offset = addr / pagesize * sizeof (pinfo);
+    char pagemapname[64];
+    int fd;
+
+    sprintf(pagemapname, "/proc/%d/pagemap", getpid());
+    fd = open(pagemapname, O_RDONLY);
+    if (fd == -1) {
+        error_setg_errno(errp, errno, "Cannot open %s", pagemapname);
+        return -1;
+    }
+
+    /* Force copy-on-write if necessary.  */
+    atomic_add((uint8_t *)ptr, 0);
+
+    if (pread(fd, &pinfo, sizeof(pinfo), offset) != sizeof(pinfo)) {
+        error_setg_errno(errp, errno, "Cannot read from %s", pagemapname);
+        goto out;
+    }
+    if ((pinfo & (1ull << 63)) == 0) {
+        error_setg(errp, "Page not present");
+        goto out;
+    }
+    ret = ((pinfo & 0x007fffffffffffffull) << 12) | (addr & (pagesize - 1));
+
+out:
+    close(fd);
+    return ret;
+}
+
+static void hmp_gpa2hpa(Monitor *mon, const QDict *qdict)
+{
+    hwaddr addr = qdict_get_int(qdict, "addr");
+    Error *local_err = NULL;
+    MemoryRegion *mr;
+    void *ptr;
+    uint64_t physaddr;
+
+    ptr = gpa2hva(&mr, addr, &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+        return;
+    }
+
+    physaddr = vtop(ptr, &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+    } else {
+        monitor_printf(mon, "Host physical address for 0x%" HWADDR_PRIx
+                       " (%s) is 0x%" PRIx64 "\n",
+                       addr, mr->name, (uint64_t) physaddr);
+    }
+
+    memory_region_unref(mr);
+}
+#endif
+
 static void do_print(Monitor *mon, const QDict *qdict)
 {
     int format = qdict_get_int(qdict, "format");
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH for 2.9?] hmp: gpa2hva and gpa2hpa hostaddr command
Posted by no-reply@patchew.org 7 years, 1 month ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH for 2.9?] hmp: gpa2hva and gpa2hpa hostaddr command
Message-id: 1489512045-49206-1-git-send-email-pbonzini@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/1489507437-32301-1-git-send-email-kwolf@redhat.com -> patchew/1489507437-32301-1-git-send-email-kwolf@redhat.com
 - [tag update]      patchew/1489507451-32356-1-git-send-email-kwolf@redhat.com -> patchew/1489507451-32356-1-git-send-email-kwolf@redhat.com
 * [new tag]         patchew/1489512045-49206-1-git-send-email-pbonzini@redhat.com -> patchew/1489512045-49206-1-git-send-email-pbonzini@redhat.com
Switched to a new branch 'test'
34854e1 hmp: gpa2hva and gpa2hpa hostaddr command

=== OUTPUT BEGIN ===
Checking PATCH 1/1: hmp: gpa2hva and gpa2hpa hostaddr command...
WARNING: line over 80 characters
#71: FILE: monitor.c:1429:
+        error_setg(errp, "No memory is mapped at address 0x%" HWADDR_PRIx, addr);

WARNING: line over 80 characters
#76: FILE: monitor.c:1434:
+        error_setg(errp, "Memory at address 0x%" HWADDR_PRIx "is not RAM", addr);

ERROR: space prohibited between function name and open parenthesis '('
#112: FILE: monitor.c:1470:
+    off_t offset = addr / pagesize * sizeof (pinfo);

total: 1 errors, 2 warnings, 147 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH for 2.9?] hmp: gpa2hva and gpa2hpa hostaddr command
Posted by Dr. David Alan Gilbert 7 years, 1 month ago
* Paolo Bonzini (pbonzini@redhat.com) wrote:
> These commands are useful when testing machine-check passthrough.
> gpa2hva is useful to inject a MADV_HWPOISON madvise from gdb, while
> gpa2hpa is useful to inject an error with the mce-inject kernel
> module.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	Technically a feature, but on the other hand it is pretty
> 	much impossible to test without it, and the people testing
> 	the feature are using it as an out-of-tree patch.  Opinions?

I've no real objection except for the freeze; they're reasonable
debug commands.

>  hmp-commands.hx |  32 ++++++++++++++++++
>  monitor.c       | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 135 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8819281..0aca984 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -526,6 +526,38 @@ Dump 80 16 bit values at the start of the video memory.
>  ETEXI
>  
>      {
> +        .name       = "gpa2hva",
> +        .args_type  = "addr:l",
> +        .params     = "addr",
> +        .help       = "print the host virtual address corresponding to a guest physical address",
> +        .cmd        = hmp_gpa2hva,
> +    },
> +
> +STEXI
> +@item gpa2hva @var{addr}
> +@findex gpa2hva
> +Print the host virtual address at which the guest's physical address @var{addr}
> +is mapped.
> +ETEXI
> +
> +#ifdef CONFIG_LINUX
> +    {
> +        .name       = "gpa2hpa",
> +        .args_type  = "addr:l",
> +        .params     = "addr",
> +        .help       = "print the host physical address corresponding to a guest physical address",
> +        .cmd        = hmp_gpa2hpa,
> +    },
> +#endif
> +
> +STEXI
> +@item gpa2hpa @var{addr}
> +@findex gpa2hpa
> +Print the host physical address at which the guest's physical address @var{addr}
> +is mapped.
> +ETEXI
> +
> +    {
>          .name       = "p|print",
>          .args_type  = "fmt:/,val:l",
>          .params     = "/fmt expr",
> diff --git a/monitor.c b/monitor.c
> index be282ec..1ac8c5f 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1421,6 +1421,109 @@ static void hmp_physical_memory_dump(Monitor *mon, const QDict *qdict)
>      memory_dump(mon, count, format, size, addr, 1);
>  }
>  
> +static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
> +{
> +    MemoryRegionSection mrs = memory_region_find(get_system_memory(),
> +                                                 addr, 1);
> +
> +    if (!mrs.mr) {
> +        error_setg(errp, "No memory is mapped at address 0x%" HWADDR_PRIx, addr);
> +        return NULL;
> +    }
> +
> +    if (!memory_region_is_ram(mrs.mr) && !memory_region_is_romd(mrs.mr)) {
> +        error_setg(errp, "Memory at address 0x%" HWADDR_PRIx "is not RAM", addr);
> +        memory_region_unref(mrs.mr);
> +        return NULL;
> +    }
> +
> +    *p_mr = mrs.mr;
> +    return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
> +}
> +
> +static void hmp_gpa2hva(Monitor *mon, const QDict *qdict)
> +{
> +    hwaddr addr = qdict_get_int(qdict, "addr");
> +    Error *local_err = NULL;
> +    MemoryRegion *mr;
> +    void *ptr;
> +
> +    ptr = gpa2hva(&mr, addr, &local_err);
> +    if (local_err) {
> +        error_report_err(local_err);
> +        return;
> +    }
> +
> +    monitor_printf(mon, "Host virtual address for 0x%" HWADDR_PRIx
> +                   " (%s) is %p\n",
> +                   addr, mr->name, ptr);
> +
> +    memory_region_unref(mr);
> +}
> +
> +#ifdef CONFIG_LINUX
> +static uint64_t vtop(void *ptr, Error **errp)
> +{
> +    uint64_t pinfo;
> +    uint64_t ret = -1;

An odd value for a uint.

> +    uintptr_t addr = (uintptr_t) ptr;
> +    uintptr_t pagesize = getpagesize();
> +    off_t offset = addr / pagesize * sizeof (pinfo);
> +    char pagemapname[64];
> +    int fd;
> +
> +    sprintf(pagemapname, "/proc/%d/pagemap", getpid());
> +    fd = open(pagemapname, O_RDONLY);

/proc/self/pagemap would seem easier.

> +    if (fd == -1) {
> +        error_setg_errno(errp, errno, "Cannot open %s", pagemapname);
> +        return -1;
> +    }
> +
> +    /* Force copy-on-write if necessary.  */
> +    atomic_add((uint8_t *)ptr, 0);
> +
> +    if (pread(fd, &pinfo, sizeof(pinfo), offset) != sizeof(pinfo)) {
> +        error_setg_errno(errp, errno, "Cannot read from %s", pagemapname);
> +        goto out;
> +    }
> +    if ((pinfo & (1ull << 63)) == 0) {
> +        error_setg(errp, "Page not present");
> +        goto out;
> +    }
> +    ret = ((pinfo & 0x007fffffffffffffull) << 12) | (addr & (pagesize - 1));

Is that an x86 magic 12 ?
(life would seem to get messy if pagesize != 1<<12 for that line)

> +out:
> +    close(fd);
> +    return ret;
> +}
> +
> +static void hmp_gpa2hpa(Monitor *mon, const QDict *qdict)
> +{
> +    hwaddr addr = qdict_get_int(qdict, "addr");
> +    Error *local_err = NULL;
> +    MemoryRegion *mr;
> +    void *ptr;
> +    uint64_t physaddr;
> +
> +    ptr = gpa2hva(&mr, addr, &local_err);
> +    if (local_err) {
> +        error_report_err(local_err);
> +        return;
> +    }
> +
> +    physaddr = vtop(ptr, &local_err);
> +    if (local_err) {
> +        error_report_err(local_err);
> +    } else {
> +        monitor_printf(mon, "Host physical address for 0x%" HWADDR_PRIx
> +                       " (%s) is 0x%" PRIx64 "\n",
> +                       addr, mr->name, (uint64_t) physaddr);

> +    }
> +
> +    memory_region_unref(mr);
> +}
> +#endif
> +
>  static void do_print(Monitor *mon, const QDict *qdict)
>  {
>      int format = qdict_get_int(qdict, "format");

Dave

> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH for 2.9?] hmp: gpa2hva and gpa2hpa hostaddr command
Posted by Paolo Bonzini 7 years, 1 month ago

On 14/03/2017 18:57, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>> These commands are useful when testing machine-check passthrough.
>> gpa2hva is useful to inject a MADV_HWPOISON madvise from gdb, while
>> gpa2hpa is useful to inject an error with the mce-inject kernel
>> module.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> 	Technically a feature, but on the other hand it is pretty
>> 	much impossible to test without it, and the people testing
>> 	the feature are using it as an out-of-tree patch.  Opinions?
> 
> I've no real objection except for the freeze; they're reasonable
> debug commands.
> 
>>  hmp-commands.hx |  32 ++++++++++++++++++
>>  monitor.c       | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 135 insertions(+)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 8819281..0aca984 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -526,6 +526,38 @@ Dump 80 16 bit values at the start of the video memory.
>>  ETEXI
>>  
>>      {
>> +        .name       = "gpa2hva",
>> +        .args_type  = "addr:l",
>> +        .params     = "addr",
>> +        .help       = "print the host virtual address corresponding to a guest physical address",
>> +        .cmd        = hmp_gpa2hva,
>> +    },
>> +
>> +STEXI
>> +@item gpa2hva @var{addr}
>> +@findex gpa2hva
>> +Print the host virtual address at which the guest's physical address @var{addr}
>> +is mapped.
>> +ETEXI
>> +
>> +#ifdef CONFIG_LINUX
>> +    {
>> +        .name       = "gpa2hpa",
>> +        .args_type  = "addr:l",
>> +        .params     = "addr",
>> +        .help       = "print the host physical address corresponding to a guest physical address",
>> +        .cmd        = hmp_gpa2hpa,
>> +    },
>> +#endif
>> +
>> +STEXI
>> +@item gpa2hpa @var{addr}
>> +@findex gpa2hpa
>> +Print the host physical address at which the guest's physical address @var{addr}
>> +is mapped.
>> +ETEXI
>> +
>> +    {
>>          .name       = "p|print",
>>          .args_type  = "fmt:/,val:l",
>>          .params     = "/fmt expr",
>> diff --git a/monitor.c b/monitor.c
>> index be282ec..1ac8c5f 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1421,6 +1421,109 @@ static void hmp_physical_memory_dump(Monitor *mon, const QDict *qdict)
>>      memory_dump(mon, count, format, size, addr, 1);
>>  }
>>  
>> +static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
>> +{
>> +    MemoryRegionSection mrs = memory_region_find(get_system_memory(),
>> +                                                 addr, 1);
>> +
>> +    if (!mrs.mr) {
>> +        error_setg(errp, "No memory is mapped at address 0x%" HWADDR_PRIx, addr);
>> +        return NULL;
>> +    }
>> +
>> +    if (!memory_region_is_ram(mrs.mr) && !memory_region_is_romd(mrs.mr)) {
>> +        error_setg(errp, "Memory at address 0x%" HWADDR_PRIx "is not RAM", addr);
>> +        memory_region_unref(mrs.mr);
>> +        return NULL;
>> +    }
>> +
>> +    *p_mr = mrs.mr;
>> +    return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
>> +}
>> +
>> +static void hmp_gpa2hva(Monitor *mon, const QDict *qdict)
>> +{
>> +    hwaddr addr = qdict_get_int(qdict, "addr");
>> +    Error *local_err = NULL;
>> +    MemoryRegion *mr;
>> +    void *ptr;
>> +
>> +    ptr = gpa2hva(&mr, addr, &local_err);
>> +    if (local_err) {
>> +        error_report_err(local_err);
>> +        return;
>> +    }
>> +
>> +    monitor_printf(mon, "Host virtual address for 0x%" HWADDR_PRIx
>> +                   " (%s) is %p\n",
>> +                   addr, mr->name, ptr);
>> +
>> +    memory_region_unref(mr);
>> +}
>> +
>> +#ifdef CONFIG_LINUX
>> +static uint64_t vtop(void *ptr, Error **errp)
>> +{
>> +    uint64_t pinfo;
>> +    uint64_t ret = -1;
> 
> An odd value for a uint.

Call it an invalid physical address. :)

>> +    uintptr_t addr = (uintptr_t) ptr;
>> +    uintptr_t pagesize = getpagesize();
>> +    off_t offset = addr / pagesize * sizeof (pinfo);
>> +    char pagemapname[64];
>> +    int fd;
>> +
>> +    sprintf(pagemapname, "/proc/%d/pagemap", getpid());
>> +    fd = open(pagemapname, O_RDONLY);
> 
> /proc/self/pagemap would seem easier.

Ok.

>> +    if (fd == -1) {
>> +        error_setg_errno(errp, errno, "Cannot open %s", pagemapname);
>> +        return -1;
>> +    }
>> +
>> +    /* Force copy-on-write if necessary.  */
>> +    atomic_add((uint8_t *)ptr, 0);
>> +
>> +    if (pread(fd, &pinfo, sizeof(pinfo), offset) != sizeof(pinfo)) {
>> +        error_setg_errno(errp, errno, "Cannot read from %s", pagemapname);
>> +        goto out;
>> +    }
>> +    if ((pinfo & (1ull << 63)) == 0) {
>> +        error_setg(errp, "Page not present");
>> +        goto out;
>> +    }
>> +    ret = ((pinfo & 0x007fffffffffffffull) << 12) | (addr & (pagesize - 1));
> 
> Is that an x86 magic 12 ?
> (life would seem to get messy if pagesize != 1<<12 for that line)

Yes, it is.

Paolo