[Qemu-devel] [PATCH v2] 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/1490021158-4469-1-git-send-email-pbonzini@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
There is a newer version of this series
hmp-commands.hx |  32 ++++++++++++++++++
monitor.c       | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 133 insertions(+)
[Qemu-devel] [PATCH v2] 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>
---
 hmp-commands.hx |  32 ++++++++++++++++++
 monitor.c       | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 133 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..216a97a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1421,6 +1421,107 @@ 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);
+    int fd;
+
+    fd = open("/proc/self/pagemap", O_RDONLY);
+    if (fd == -1) {
+        error_setg_errno(errp, errno, "Cannot open /proc/self/pagemap");
+        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 pagemap");
+        goto out;
+    }
+    if ((pinfo & (1ull << 63)) == 0) {
+        error_setg(errp, "Page not present");
+        goto out;
+    }
+    ret = ((pinfo & 0x007fffffffffffffull) * pagesize) | (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 v2] hmp: gpa2hva and gpa2hpa hostaddr command
Posted by Peter Maydell 7 years, 1 month ago
On 20 March 2017 at 14:45, 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>
> ---
>  hmp-commands.hx |  32 ++++++++++++++++++
>  monitor.c       | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 133 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

I have some comments which feel kind of nit-picky, but since this
is a public-facing HMP API I think they need attention since we only
get one chance to get it right.

>      {
> +        .name       = "gpa2hva",
> +        .args_type  = "addr:l",
> +        .params     = "addr",
> +        .help       = "print the host virtual address corresponding to a guest physical address",
> +        .cmd        = hmp_gpa2hva,
> +    },

How does this work for guest CPUs which have more than one physical
address space (eg ARM TrustZone)? There's no ability here to specify
the secure/nonsecure transaction attribute that you need to distinguish
which kind of guest physical address you're talking about.

The command also doesn't let you specify which CPU you care about,
which is bad because they don't all have to have the same address map.

The documentation should also say what happens if the guest physaddr
doesn't correspond to RAM.

> +#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.

...what if you're on a system where host RAM exists at multiple host
physical addresses? What if the RAM happens to be paged out?
(Plus the remarks for gpa2hva apply.)

thanks
-- PMM

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

On 20/03/2017 15:56, Peter Maydell wrote:
> On 20 March 2017 at 14:45, 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>
>> ---
>>  hmp-commands.hx |  32 ++++++++++++++++++
>>  monitor.c       | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 133 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
> 
> I have some comments which feel kind of nit-picky, but since this
> is a public-facing HMP API I think they need attention since we only
> get one chance to get it right.
> 
>>      {
>> +        .name       = "gpa2hva",
>> +        .args_type  = "addr:l",
>> +        .params     = "addr",
>> +        .help       = "print the host virtual address corresponding to a guest physical address",
>> +        .cmd        = hmp_gpa2hva,
>> +    },
> 
> How does this work for guest CPUs which have more than one physical
> address space (eg ARM TrustZone)? There's no ability here to specify
> the secure/nonsecure transaction attribute that you need to distinguish
> which kind of guest physical address you're talking about.
> 
> The command also doesn't let you specify which CPU you care about,
> which is bad because they don't all have to have the same address map.

It just uses address_space_memory currently.  I can make it use the
current CPU address space too, similar to x and xp; it's the same for my
own use.

> The documentation should also say what happens if the guest physaddr
> doesn't correspond to RAM.

An error? :)

>> +#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.
> 
> ...what if you're on a system where host RAM exists at multiple host
> physical addresses?

It gives whatever the host OS thinks it's the corresponding host
physical address.  It's what happens to be in the page tables for the HVA.

> What if the RAM happens to be paged out?
> (Plus the remarks for gpa2hva apply.)

Another error? :)

Paolo

Re: [Qemu-devel] [PATCH v2] 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>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  hmp-commands.hx |  32 ++++++++++++++++++
>  monitor.c       | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 133 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..216a97a 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1421,6 +1421,107 @@ 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);
> +    int fd;
> +
> +    fd = open("/proc/self/pagemap", O_RDONLY);
> +    if (fd == -1) {
> +        error_setg_errno(errp, errno, "Cannot open /proc/self/pagemap");
> +        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 pagemap");
> +        goto out;
> +    }
> +    if ((pinfo & (1ull << 63)) == 0) {
> +        error_setg(errp, "Page not present");
> +        goto out;
> +    }
> +    ret = ((pinfo & 0x007fffffffffffffull) * pagesize) | (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
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2] hmp: gpa2hva and gpa2hpa hostaddr command
Posted by Dr. David Alan Gilbert 7 years 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>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

And queued.

Dave
> ---
>  hmp-commands.hx |  32 ++++++++++++++++++
>  monitor.c       | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 133 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..216a97a 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1421,6 +1421,107 @@ 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);
> +    int fd;
> +
> +    fd = open("/proc/self/pagemap", O_RDONLY);
> +    if (fd == -1) {
> +        error_setg_errno(errp, errno, "Cannot open /proc/self/pagemap");
> +        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 pagemap");
> +        goto out;
> +    }
> +    if ((pinfo & (1ull << 63)) == 0) {
> +        error_setg(errp, "Page not present");
> +        goto out;
> +    }
> +    ret = ((pinfo & 0x007fffffffffffffull) * pagesize) | (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
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK