Skip to contentSkip to conversation

QEMU, MCE, unpoison memory address across reboot

PATCH Huang Ying 5 participants 15 messages Dec 22, 2010 – Dec 28, 2010 KVM

lore
Share & tools
lore

Huang Ying

author
#0 lore raw

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 <[email protected]>
---
 kvm.h                   |    2 ++
 kvm/include/linux/kvm.h |    2 ++
 qemu-kvm.c              |   40 ++++++++++++++++++++++++++++++++++++++++
 target-i386/kvm.c       |    2 ++
 4 files changed, 46 insertions(+)
target-i386/kvm.c +2 -0
@@ -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;
kvm/include/linux/kvm.h +2 -0
@@ -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
qemu-kvm.c +40 -0
@@ -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;
 }
kvm.h +2 -0
@@ -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
#1 lore raw
Quoted 40 lines · 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 <[email protected]>
qemu-kvm.c +14 -0
+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;

qemu-kvm.c int kvm_arch_init_irq_routing(void) · line 1645

Quoted 10 lines
qemu-kvm.c +10 -0
+    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.

#2 lore raw
Quoted 3 lines

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

author
#3 lore raw

Hi, Andi,

Quoted 5 lines · Andi Kleen
Quoted 2 lines

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

author
#4 lore raw
In reply to Marcelo Tosatti
Quoted 2 lines · Marcelo Tosatti

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

#5 lore raw
Quoted 8 lines · Huang Ying
Quoted 2 lines · Marcelo Tosatti

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:

Quoted 7 lines

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

#6 lore raw
In reply to Marcelo Tosatti
Quoted 52 lines · Marcelo Tosatti
Quoted 48 lines · 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<[email protected]>

+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);
Quoted 2 lines

+ }
+}

Quoted 3 lines

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
#7 lore raw
Quoted 14 lines · Avi Kivity
Quoted 14 lines
Quoted 13 lines

+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);

+ }
+}

Quoted 5 lines
Quoted 3 lines

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.

author
#8 lore raw
Quoted 15 lines · Marcelo Tosatti
Quoted 14 lines · Avi Kivity
Quoted 14 lines
Quoted 13 lines

+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);

+ }
+}

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

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

#9 lore raw
In reply to Marcelo Tosatti
Quoted 15 lines · Marcelo Tosatti
Quoted 14 lines · Avi Kivity
Quoted 14 lines
Quoted 13 lines

+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);

+ }
+}

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

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.

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
author
#10 lore raw
Quoted 20 lines · Avi Kivity
Quoted 19 lines · Marcelo Tosatti
Quoted 18 lines · Avi Kivity
Quoted 17 lines
Quoted 11 lines

+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);
Quoted 2 lines

+ }
+}

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.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

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

#11 lore raw
Quoted 26 lines · Huang Ying
Quoted 25 lines · Avi Kivity
Quoted 23 lines · Marcelo Tosatti
Quoted 14 lines · Avi Kivity
Quoted 14 lines
Quoted 13 lines

+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);

+ }
+}

Quoted 5 lines
Quoted 3 lines

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.

Quoted 8 lines
Quoted 3 lines

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
#12 lore raw
In reply to Marcelo Tosatti
Quoted 15 lines · Marcelo Tosatti
Quoted 14 lines · Avi Kivity
Quoted 14 lines
Quoted 13 lines

+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);

+ }
+}

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

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.

author
#13 lore raw
Quoted 20 lines · Gleb Natapov
Quoted 19 lines · Marcelo Tosatti
Quoted 18 lines · Avi Kivity
Quoted 17 lines
Quoted 11 lines

+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);
Quoted 2 lines

+ }
+}

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.

Quoted 5 lines
Quoted 4 lines
Quoted 1 line

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

#14 lore raw
Quoted 5 lines · Huang Ying
Quoted 2 lines
Quoted 1 line

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.

Signature · 1 line
error compiling committee.c: too many arguments to function