QEMU, MCE, unpoison memory address across reboot

QEMU, MCE, unpoison memory address across reboot

[Huang Ying]

In Linux kernel HWPoison processing implementation, the virtual
address in processes mapping the error physical memory page is marked
as HWPoison.  So that, the further accessing to the virtual
address will kill corresponding processes with SIGBUS.

If the error physical memory page is used by a KVM guest, the SIGBUS
will be sent to QEMU, and QEMU will simulate a MCE to report that
memory error to the guest OS.  If the guest OS can not recover from
the error (for example, the page is accessed by kernel code), guest OS
will reboot the system.  But because the underlying host virtual
address backing the guest physical memory is still poisoned, if the
guest system accesses the corresponding guest physical memory even
after rebooting, the SIGBUS will still be sent to QEMU and MCE will be
simulated.  That is, guest system can not recover via rebooting.

In fact, across rebooting, the contents of guest physical memory page
need not to be kept.  We can allocate a new host physical page to
back the corresponding guest physical address.

This patch fixes this issue in QEMU-KVM via invoke the unpoison
mechanism implemented in Linux kernel to clear the corresponding page
table entry, so that make it possible to allocate a new page to
recover the issue.

Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 kvm.h                   |    2 ++
 kvm/include/linux/kvm.h |    2 ++
 qemu-kvm.c              |   40 ++++++++++++++++++++++++++++++++++++++++
 target-i386/kvm.c       |    2 ++
 4 files changed, 46 insertions(+)

--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1803,6 +1803,7 @@ int kvm_on_sigbus_vcpu(CPUState *env, in
                 hardware_memory_error();
             }
         }
+        kvm_hwpoison_page_add(vaddr);
         mce.addr = paddr;
         r = kvm_set_mce(env, &mce);
         if (r < 0) {
@@ -1841,6 +1842,7 @@ int kvm_on_sigbus(int code, void *addr)
                     "QEMU itself instead of guest system!: %p\n", addr);
             return 0;
         }
+        kvm_hwpoison_page_add(vaddr);
         status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN
             | MCI_STATUS_MISCV | MCI_STATUS_ADDRV | MCI_STATUS_S
             | 0xc0;
--- a/kvm/include/linux/kvm.h
+++ b/kvm/include/linux/kvm.h
@@ -663,6 +663,8 @@ struct kvm_clock_data {
 /* Available with KVM_CAP_PIT_STATE2 */
 #define KVM_GET_PIT2              _IOR(KVMIO,  0x9f, struct kvm_pit_state2)
 #define KVM_SET_PIT2              _IOW(KVMIO,  0xa0, struct kvm_pit_state2)
+#define KVM_PPC_GET_PVINFO        _IOW(KVMIO,  0xa1, struct kvm_ppc_pvinfo)
+#define KVM_UNPOISON_ADDRESS      _IO(KVMIO,  0xa2)
 
 /*
  * ioctls for vcpu fds
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1619,6 +1619,45 @@ int kvm_arch_init_irq_routing(void)
 }
 #endif
 
+struct HWPoisonPage;
+typedef struct HWPoisonPage HWPoisonPage;
+struct HWPoisonPage
+{
+    void *vaddr;
+    QLIST_ENTRY(HWPoisonPage) list;
+};
+
+static QLIST_HEAD(hwpoison_page_list, HWPoisonPage) hwpoison_page_list =
+    QLIST_HEAD_INITIALIZER(hwpoison_page_list);
+
+static void kvm_unpoison_all(void *param)
+{
+    HWPoisonPage *page, *next_page;
+    unsigned long address;
+    KVMState *s = param;
+
+    QLIST_FOREACH_SAFE(page, &hwpoison_page_list, list, next_page) {
+        address = (unsigned long)page->vaddr;
+        QLIST_REMOVE(page, list);
+        kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
+        qemu_free(page);
+    }
+}
+
+void kvm_hwpoison_page_add(void *vaddr)
+{
+    HWPoisonPage *page;
+
+    QLIST_FOREACH(page, &hwpoison_page_list, list) {
+        if (page->vaddr == vaddr)
+            return;
+    }
+
+    page = qemu_malloc(sizeof(HWPoisonPage));
+    page->vaddr = vaddr;
+    QLIST_INSERT_HEAD(&hwpoison_page_list, page, list);
+}
+
 extern int no_hpet;
 
 static int kvm_create_context(void)
@@ -1703,6 +1742,7 @@ static int kvm_create_context(void)
         }
 #endif
     }
+    qemu_register_reset(kvm_unpoison_all, kvm_state);
 
     return 0;
 }
--- a/kvm.h
+++ b/kvm.h
@@ -221,4 +221,6 @@ int kvm_irqchip_in_kernel(void);
 
 int kvm_set_irq(int irq, int level, int *status);
 
+void kvm_hwpoison_page_add(void *vaddr);
+
 #endif

Re: QEMU, MCE, unpoison memory address across reboot

[Marcelo Tosatti]

On Wed, Dec 22, 2010 at 10:52:51AM +0800, Huang Ying wrote:
> In Linux kernel HWPoison processing implementation, the virtual
> address in processes mapping the error physical memory page is marked
> as HWPoison.  So that, the further accessing to the virtual
> address will kill corresponding processes with SIGBUS.
> 
> If the error physical memory page is used by a KVM guest, the SIGBUS
> will be sent to QEMU, and QEMU will simulate a MCE to report that
> memory error to the guest OS.  If the guest OS can not recover from
> the error (for example, the page is accessed by kernel code), guest OS
> will reboot the system.  But because the underlying host virtual
> address backing the guest physical memory is still poisoned, if the
> guest system accesses the corresponding guest physical memory even
> after rebooting, the SIGBUS will still be sent to QEMU and MCE will be
> simulated.  That is, guest system can not recover via rebooting.
> 
> In fact, across rebooting, the contents of guest physical memory page
> need not to be kept.  We can allocate a new host physical page to
> back the corresponding guest physical address.
> 
> This patch fixes this issue in QEMU-KVM via invoke the unpoison
> mechanism implemented in Linux kernel to clear the corresponding page
> table entry, so that make it possible to allocate a new page to
> recover the issue.
> 
> Signed-off-by: Huang Ying <ying.huang@intel.com>

> +struct HWPoisonPage;
> +typedef struct HWPoisonPage HWPoisonPage;
> +struct HWPoisonPage
> +{
> +    void *vaddr;
> +    QLIST_ENTRY(HWPoisonPage) list;
> +};
> +
> +static QLIST_HEAD(hwpoison_page_list, HWPoisonPage) hwpoison_page_list =
> +    QLIST_HEAD_INITIALIZER(hwpoison_page_list);
> +
> +static void kvm_unpoison_all(void *param)
> +{
> +    HWPoisonPage *page, *next_page;
> +    unsigned long address;
> +    KVMState *s = param;
> +
> +    QLIST_FOREACH_SAFE(page, &hwpoison_page_list, list, next_page) {
> +        address = (unsigned long)page->vaddr;
> +        QLIST_REMOVE(page, list);
> +        kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
> +        qemu_free(page);
> +    }
> +}

Can't you free and reallocate all guest memory instead, on reboot, if
there's a hwpoisoned page? Then you don't need this interface.

Re: QEMU, MCE, unpoison memory address across reboot

[Andi Kleen]

> Can't you free and reallocate all guest memory instead, on reboot, if
> there's a hwpoisoned page? Then you don't need this interface.

I think that would be more efficient. You can potentially save a lot
of memory if the new guest doesn't need as much as the old one.


-Andi

Re: QEMU, MCE, unpoison memory address across reboot

[Huang Ying]

Hi, Andi,

On Fri, 2010-12-24 at 00:57 +0800, Andi Kleen wrote:
> > Can't you free and reallocate all guest memory instead, on reboot, if
> > there's a hwpoisoned page? Then you don't need this interface.
> 
> I think that would be more efficient. You can potentially save a lot
> of memory if the new guest doesn't need as much as the old one.

So you suggest to free/reallocate all guest memory across every reboot
regardless whether there's hwpoisoned page?

Best Regards,
Huang Ying

Re: QEMU, MCE, unpoison memory address across reboot

[Huang Ying]

On Thu, 2010-12-23 at 22:28 +0800, Marcelo Tosatti wrote:
> Can't you free and reallocate all guest memory instead, on reboot, if
> there's a hwpoisoned page? Then you don't need this interface.

Consider about this method.  It seems that some guest RAMs are not
allocated in qemu_ram_alloc_from_ptr(), that is, host parameter is
allocated elsewhere and passed in.  I found two:

- assigned_dev_register_regions() in hw/device-assignment.c
- create_shared_memory_BAR() and ivshmem_read() in hw/ivshmem.c

There is no general method to reallocate these memory so far.  We may
need a flag in struct RAMBlock to track these memory, and ignore them
during reallocation.  But if there are hwpoisoned pages in these memory,
we can not recover.  Do you think that is OK?

Best Regards,
Huang Ying

Re: QEMU, MCE, unpoison memory address across reboot

[Marcelo Tosatti]

On Fri, Dec 24, 2010 at 11:30:37AM +0800, Huang Ying wrote:
> On Thu, 2010-12-23 at 22:28 +0800, Marcelo Tosatti wrote:
> > Can't you free and reallocate all guest memory instead, on reboot, if
> > there's a hwpoisoned page? Then you don't need this interface.
> 
> Consider about this method.  It seems that some guest RAMs are not
> allocated in qemu_ram_alloc_from_ptr(), that is, host parameter is
> allocated elsewhere and passed in.  I found two:
> 
> - assigned_dev_register_regions() in hw/device-assignment.c
> - create_shared_memory_BAR() and ivshmem_read() in hw/ivshmem.c
> 
> There is no general method to reallocate these memory so far.  We may
> need a flag in struct RAMBlock to track these memory, and ignore them
> during reallocation.  But if there are hwpoisoned pages in these memory,
> we can not recover.  Do you think that is OK?

Yes, these are not guest RAM so there should be no MCE for them.

Re: QEMU, MCE, unpoison memory address across reboot

[Avi Kivity]

On 12/23/2010 04:28 PM, Marcelo Tosatti wrote:
> On Wed, Dec 22, 2010 at 10:52:51AM +0800, Huang Ying wrote:
> >  In Linux kernel HWPoison processing implementation, the virtual
> >  address in processes mapping the error physical memory page is marked
> >  as HWPoison.  So that, the further accessing to the virtual
> >  address will kill corresponding processes with SIGBUS.
> >
> >  If the error physical memory page is used by a KVM guest, the SIGBUS
> >  will be sent to QEMU, and QEMU will simulate a MCE to report that
> >  memory error to the guest OS.  If the guest OS can not recover from
> >  the error (for example, the page is accessed by kernel code), guest OS
> >  will reboot the system.  But because the underlying host virtual
> >  address backing the guest physical memory is still poisoned, if the
> >  guest system accesses the corresponding guest physical memory even
> >  after rebooting, the SIGBUS will still be sent to QEMU and MCE will be
> >  simulated.  That is, guest system can not recover via rebooting.
> >
> >  In fact, across rebooting, the contents of guest physical memory page
> >  need not to be kept.  We can allocate a new host physical page to
> >  back the corresponding guest physical address.
> >
> >  This patch fixes this issue in QEMU-KVM via invoke the unpoison
> >  mechanism implemented in Linux kernel to clear the corresponding page
> >  table entry, so that make it possible to allocate a new page to
> >  recover the issue.
> >
> >  Signed-off-by: Huang Ying<ying.huang@intel.com>
>
> >  +struct HWPoisonPage;
> >  +typedef struct HWPoisonPage HWPoisonPage;
> >  +struct HWPoisonPage
> >  +{
> >  +    void *vaddr;
> >  +    QLIST_ENTRY(HWPoisonPage) list;
> >  +};
> >  +
> >  +static QLIST_HEAD(hwpoison_page_list, HWPoisonPage) hwpoison_page_list =
> >  +    QLIST_HEAD_INITIALIZER(hwpoison_page_list);
> >  +
> >  +static void kvm_unpoison_all(void *param)
> >  +{
> >  +    HWPoisonPage *page, *next_page;
> >  +    unsigned long address;
> >  +    KVMState *s = param;
> >  +
> >  +    QLIST_FOREACH_SAFE(page,&hwpoison_page_list, list, next_page) {
> >  +        address = (unsigned long)page->vaddr;
> >  +        QLIST_REMOVE(page, list);
> >  +        kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
> >  +        qemu_free(page);
> >  +    }
> >  +}
>
> Can't you free and reallocate all guest memory instead, on reboot, if
> there's a hwpoisoned page? Then you don't need this interface.
>

Alternatively, MADV_DONTNEED?  We already use it for ballooning.

-- 
error compiling committee.c: too many arguments to function

Re: QEMU, MCE, unpoison memory address across reboot

[Marcelo Tosatti]

On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
> >>  +static void kvm_unpoison_all(void *param)
> >>  +{
> >>  +    HWPoisonPage *page, *next_page;
> >>  +    unsigned long address;
> >>  +    KVMState *s = param;
> >>  +
> >>  +    QLIST_FOREACH_SAFE(page,&hwpoison_page_list, list, next_page) {
> >>  +        address = (unsigned long)page->vaddr;
> >>  +        QLIST_REMOVE(page, list);
> >>  +        kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
> >>  +        qemu_free(page);
> >>  +    }
> >>  +}
> >
> >Can't you free and reallocate all guest memory instead, on reboot, if
> >there's a hwpoisoned page? Then you don't need this interface.
> >
> 
> Alternatively, MADV_DONTNEED?  We already use it for ballooning.

Does not work for hugetlbfs.

Re: QEMU, MCE, unpoison memory address across reboot

[Huang Ying]

On Tue, 2010-12-28 at 05:27 +0800, Marcelo Tosatti wrote:
> On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
> > >>  +static void kvm_unpoison_all(void *param)
> > >>  +{
> > >>  +    HWPoisonPage *page, *next_page;
> > >>  +    unsigned long address;
> > >>  +    KVMState *s = param;
> > >>  +
> > >>  +    QLIST_FOREACH_SAFE(page,&hwpoison_page_list, list, next_page) {
> > >>  +        address = (unsigned long)page->vaddr;
> > >>  +        QLIST_REMOVE(page, list);
> > >>  +        kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
> > >>  +        qemu_free(page);
> > >>  +    }
> > >>  +}
> > >
> > >Can't you free and reallocate all guest memory instead, on reboot, if
> > >there's a hwpoisoned page? Then you don't need this interface.
> > >
> > 
> > Alternatively, MADV_DONTNEED?  We already use it for ballooning.
> 
> Does not work for hugetlbfs.

Yes.  And I think zap the page range is just the implementation detail
but semantics of MADV_DONTNEED.

But on the other hand, whether qemu_vmalloc is implemented via
posix_memalign on Linux?  If it is, we can not guarantee that
corresponding page table is zapped after qemu_vfree and qemu_vmalloc?
That is glibc implementation details.

Best Regards,
Huang Ying

Re: QEMU, MCE, unpoison memory address across reboot

[Avi Kivity]

On 12/27/2010 11:27 PM, Marcelo Tosatti wrote:
> On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
> >  >>   +static void kvm_unpoison_all(void *param)
> >  >>   +{
> >  >>   +    HWPoisonPage *page, *next_page;
> >  >>   +    unsigned long address;
> >  >>   +    KVMState *s = param;
> >  >>   +
> >  >>   +    QLIST_FOREACH_SAFE(page,&hwpoison_page_list, list, next_page) {
> >  >>   +        address = (unsigned long)page->vaddr;
> >  >>   +        QLIST_REMOVE(page, list);
> >  >>   +        kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
> >  >>   +        qemu_free(page);
> >  >>   +    }
> >  >>   +}
> >  >
> >  >Can't you free and reallocate all guest memory instead, on reboot, if
> >  >there's a hwpoisoned page? Then you don't need this interface.
> >  >
> >
> >  Alternatively, MADV_DONTNEED?  We already use it for ballooning.
>
> Does not work for hugetlbfs.
>

True.  We can munmap() the page (extending it to the huge page size in 
effect), and then mmap() it back in.  The kernel should merge the new 
vma with its neighbors.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: QEMU, MCE, unpoison memory address across reboot

[Huang Ying]

On Tue, 2010-12-28 at 16:11 +0800, Avi Kivity wrote:
> On 12/27/2010 11:27 PM, Marcelo Tosatti wrote:
> > On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
> > >  >>   +static void kvm_unpoison_all(void *param)
> > >  >>   +{
> > >  >>   +    HWPoisonPage *page, *next_page;
> > >  >>   +    unsigned long address;
> > >  >>   +    KVMState *s = param;
> > >  >>   +
> > >  >>   +    QLIST_FOREACH_SAFE(page,&hwpoison_page_list, list, next_page) {
> > >  >>   +        address = (unsigned long)page->vaddr;
> > >  >>   +        QLIST_REMOVE(page, list);
> > >  >>   +        kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
> > >  >>   +        qemu_free(page);
> > >  >>   +    }
> > >  >>   +}
> > >  >
> > >  >Can't you free and reallocate all guest memory instead, on reboot, if
> > >  >there's a hwpoisoned page? Then you don't need this interface.
> > >  >
> > >
> > >  Alternatively, MADV_DONTNEED?  We already use it for ballooning.
> >
> > Does not work for hugetlbfs.
> >
> 
> True.  We can munmap() the page (extending it to the huge page size in 
> effect), and then mmap() it back in.  The kernel should merge the new 
> vma with its neighbors.

To merge the new vma with its neighbors, we need all information about
the original mmap (when doing allocating).  It appears that some
information is hided in glibc (posix_memalign).

Best Regards,
Huang Ying

Re: QEMU, MCE, unpoison memory address across reboot

[Avi Kivity]

On 12/28/2010 10:32 AM, Huang Ying wrote:
> On Tue, 2010-12-28 at 16:11 +0800, Avi Kivity wrote:
> >  On 12/27/2010 11:27 PM, Marcelo Tosatti wrote:
> >  >  On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
> >  >  >   >>    +static void kvm_unpoison_all(void *param)
> >  >  >   >>    +{
> >  >  >   >>    +    HWPoisonPage *page, *next_page;
> >  >  >   >>    +    unsigned long address;
> >  >  >   >>    +    KVMState *s = param;
> >  >  >   >>    +
> >  >  >   >>    +    QLIST_FOREACH_SAFE(page,&hwpoison_page_list, list, next_page) {
> >  >  >   >>    +        address = (unsigned long)page->vaddr;
> >  >  >   >>    +        QLIST_REMOVE(page, list);
> >  >  >   >>    +        kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
> >  >  >   >>    +        qemu_free(page);
> >  >  >   >>    +    }
> >  >  >   >>    +}
> >  >  >   >
> >  >  >   >Can't you free and reallocate all guest memory instead, on reboot, if
> >  >  >   >there's a hwpoisoned page? Then you don't need this interface.
> >  >  >   >
> >  >  >
> >  >  >   Alternatively, MADV_DONTNEED?  We already use it for ballooning.
> >  >
> >  >  Does not work for hugetlbfs.
> >  >
> >
> >  True.  We can munmap() the page (extending it to the huge page size in
> >  effect), and then mmap() it back in.  The kernel should merge the new
> >  vma with its neighbors.
>
> To merge the new vma with its neighbors, we need all information about
> the original mmap (when doing allocating).  It appears that some
> information is hided in glibc (posix_memalign).
>

It's likely just an anonymous mmap().

Even if it's not merged, it isn't the end of the world.  Poisoned pages 
should be very rare, and nothing bad happens from having two extra VMAs.

It is a little more complicated, we have to reissue MADV_MERGABLE etc, 
but it's better than adding new interfaces IMO.

-- 
error compiling committee.c: too many arguments to function

Re: QEMU, MCE, unpoison memory address across reboot

[Gleb Natapov]

On Mon, Dec 27, 2010 at 07:27:54PM -0200, Marcelo Tosatti wrote:
> On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
> > >>  +static void kvm_unpoison_all(void *param)
> > >>  +{
> > >>  +    HWPoisonPage *page, *next_page;
> > >>  +    unsigned long address;
> > >>  +    KVMState *s = param;
> > >>  +
> > >>  +    QLIST_FOREACH_SAFE(page,&hwpoison_page_list, list, next_page) {
> > >>  +        address = (unsigned long)page->vaddr;
> > >>  +        QLIST_REMOVE(page, list);
> > >>  +        kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
> > >>  +        qemu_free(page);
> > >>  +    }
> > >>  +}
> > >
> > >Can't you free and reallocate all guest memory instead, on reboot, if
> > >there's a hwpoisoned page? Then you don't need this interface.
> > >
> > 
> > Alternatively, MADV_DONTNEED?  We already use it for ballooning.
> 
> Does not work for hugetlbfs.
> 
Don't we break huge page to 4k pages during poisoning?

--
			Gleb.

Re: QEMU, MCE, unpoison memory address across reboot

[Huang Ying]

On Tue, 2010-12-28 at 16:27 +0800, Gleb Natapov wrote:
> On Mon, Dec 27, 2010 at 07:27:54PM -0200, Marcelo Tosatti wrote:
> > On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
> > > >>  +static void kvm_unpoison_all(void *param)
> > > >>  +{
> > > >>  +    HWPoisonPage *page, *next_page;
> > > >>  +    unsigned long address;
> > > >>  +    KVMState *s = param;
> > > >>  +
> > > >>  +    QLIST_FOREACH_SAFE(page,&hwpoison_page_list, list, next_page) {
> > > >>  +        address = (unsigned long)page->vaddr;
> > > >>  +        QLIST_REMOVE(page, list);
> > > >>  +        kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
> > > >>  +        qemu_free(page);
> > > >>  +    }
> > > >>  +}
> > > >
> > > >Can't you free and reallocate all guest memory instead, on reboot, if
> > > >there's a hwpoisoned page? Then you don't need this interface.
> > > >
> > > 
> > > Alternatively, MADV_DONTNEED?  We already use it for ballooning.
> > 
> > Does not work for hugetlbfs.
> > 
> Don't we break huge page to 4k pages during poisoning?

Yes.  That has not been implemented yet.  So in fact, we can not deal
with hwpoison+hugetlb in kvm now.

Best Regards,
Huang Ying

Re: QEMU, MCE, unpoison memory address across reboot

[Avi Kivity]

On 12/28/2010 10:35 AM, Huang Ying wrote:
> >  >
> >  Don't we break huge page to 4k pages during poisoning?
>
> Yes.  That has not been implemented yet.  So in fact, we can not deal
> with hwpoison+hugetlb in kvm now.

Should be a lot easier to deal with using transparent hugepages, since 
the break-apart function is already implemented.

-- 
error compiling committee.c: too many arguments to function