Re: QEMU, MCE, unpoison memory address across reboot

2010-12-28 Thread 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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-28 Thread 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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-28 Thread 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


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-28 Thread 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


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-28 Thread 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.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-28 Thread 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.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-27 Thread 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


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-27 Thread 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.


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-27 Thread 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.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-26 Thread 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

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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-23 Thread 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


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-23 Thread 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


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-23 Thread 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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: QEMU, MCE, unpoison memory address across reboot

2010-12-23 Thread 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 

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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html