On 15/09/2023 04:20, “William Roche wrote: > From: William Roche <william.ro...@oracle.com> > > A memory page poisoned from the hypervisor level is no longer readable. > Thus, it is now treated as a zero-page for the ram saving migration phase. > > The migration of a VM will crash Qemu when it tries to read the > memory address space and stumbles on the poisoned page with a similar > stack trace: > > Program terminated with signal SIGBUS, Bus error. > #0 _mm256_loadu_si256 > #1 buffer_zero_avx2 > #2 select_accel_fn > #3 buffer_is_zero > #4 save_zero_page_to_file > #5 save_zero_page > #6 ram_save_target_page_legacy > #7 ram_save_host_page > #8 ram_find_and_save_block > #9 ram_save_iterate > #10 qemu_savevm_state_iterate > #11 migration_iteration_run > #12 migration_thread > #13 qemu_thread_start > > Fix it by considering poisoned pages as if they were zero-pages for > the migration copy. This fix also works with underlying large pages, > taking into account the RAMBlock segment "page-size". > > Standard migration and compressed transfers are handled by this code. > RDMA transfer isn't touched. >
I'm okay with "RDMA isn't touched". BTW, could you share your reproducing program/hacking to poison the page, so that i am able to take a look the RDMA part later when i'm free. Not sure it's suitable to acknowledge a not touched part. Anyway Acked-by: Li Zhijian <lizhij...@fujitsu.com> # RDMA > Signed-off-by: William Roche <william.ro...@oracle.com> > --- > accel/kvm/kvm-all.c | 14 ++++++++++++++ > accel/stubs/kvm-stub.c | 5 +++++ > include/sysemu/kvm.h | 10 ++++++++++ > migration/ram-compress.c | 3 ++- > migration/ram.c | 23 +++++++++++++++++++++-- > migration/ram.h | 2 ++ > 6 files changed, 54 insertions(+), 3 deletions(-) > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index ff1578bb32..7fb13c8a56 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -1152,6 +1152,20 @@ static void kvm_unpoison_all(void *param) > } > } > > +bool kvm_hwpoisoned_page(RAMBlock *block, void *offset) > +{ > + HWPoisonPage *pg; > + ram_addr_t ram_addr = (ram_addr_t) offset; > + > + QLIST_FOREACH(pg, &hwpoison_page_list, list) { > + if ((ram_addr >= pg->ram_addr) && > + (ram_addr - pg->ram_addr < block->page_size)) { > + return true; > + } > + } > + return false; > +} > + > void kvm_hwpoison_page_add(ram_addr_t ram_addr) > { > HWPoisonPage *page; > diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c > index 235dc661bc..c0a31611df 100644 > --- a/accel/stubs/kvm-stub.c > +++ b/accel/stubs/kvm-stub.c > @@ -133,3 +133,8 @@ uint32_t kvm_dirty_ring_size(void) > { > return 0; > } > + > +bool kvm_hwpoisoned_page(RAMBlock *block, void *ram_addr) > +{ > + return false; > +} > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index ee9025f8e9..858688227a 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -570,4 +570,14 @@ bool kvm_arch_cpu_check_are_resettable(void); > bool kvm_dirty_ring_enabled(void); > > uint32_t kvm_dirty_ring_size(void); > + > +/** > + * kvm_hwpoisoned_page - indicate if the given page is poisoned > + * @block: memory block of the given page > + * @ram_addr: offset of the page > + * > + * Returns: true: page is poisoned > + * false: page not yet poisoned > + */ > +bool kvm_hwpoisoned_page(RAMBlock *block, void *ram_addr); > #endif > diff --git a/migration/ram-compress.c b/migration/ram-compress.c > index 06254d8c69..1916ce709d 100644 > --- a/migration/ram-compress.c > +++ b/migration/ram-compress.c > @@ -34,6 +34,7 @@ > #include "qemu/error-report.h" > #include "migration.h" > #include "options.h" > +#include "ram.h" > #include "io/channel-null.h" > #include "exec/target_page.h" > #include "exec/ramblock.h" > @@ -198,7 +199,7 @@ static CompressResult do_compress_ram_page(QEMUFile *f, > z_stream *stream, > > assert(qemu_file_buffer_empty(f)); > > - if (buffer_is_zero(p, page_size)) { > + if (migration_buffer_is_zero(block, offset, page_size)) { > return RES_ZEROPAGE; > } > > diff --git a/migration/ram.c b/migration/ram.c > index 9040d66e61..fd337f7e65 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -1129,6 +1129,26 @@ void ram_release_page(const char *rbname, uint64_t > offset) > ram_discard_range(rbname, offset, TARGET_PAGE_SIZE); > } > > +/** > + * migration_buffer_is_zero: indicate if the page at the given > + * location is entirely filled with zero, or is a poisoned page. > + * > + * @block: block that contains the page > + * @offset: offset inside the block for the page > + * @len: size to consider > + */ > +bool migration_buffer_is_zero(RAMBlock *block, ram_addr_t offset, > + size_t len) > +{ > + uint8_t *p = block->host + offset; > + > + if (kvm_enabled() && kvm_hwpoisoned_page(block, (void *)offset)) { > + return true; > + } > + > + return buffer_is_zero(p, len); > +} > + > /** > * save_zero_page_to_file: send the zero page to the file > * > @@ -1142,10 +1162,9 @@ void ram_release_page(const char *rbname, uint64_t > offset) > static int save_zero_page_to_file(PageSearchStatus *pss, QEMUFile *file, > RAMBlock *block, ram_addr_t offset) > { > - uint8_t *p = block->host + offset; > int len = 0; > > - if (buffer_is_zero(p, TARGET_PAGE_SIZE)) { > + if (migration_buffer_is_zero(block, offset, TARGET_PAGE_SIZE)) { > len += save_page_header(pss, file, block, offset | > RAM_SAVE_FLAG_ZERO); > qemu_put_byte(file, 0); > len += 1; > diff --git a/migration/ram.h b/migration/ram.h > index 145c915ca7..805ea2a211 100644 > --- a/migration/ram.h > +++ b/migration/ram.h > @@ -65,6 +65,8 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t > size); > void ram_transferred_add(uint64_t bytes); > void ram_release_page(const char *rbname, uint64_t offset); > > +bool migration_buffer_is_zero(RAMBlock *block, ram_addr_t offset, size_t > len); > + > int ramblock_recv_bitmap_test(RAMBlock *rb, void *host_addr); > bool ramblock_recv_bitmap_test_byte_offset(RAMBlock *rb, uint64_t > byte_offset); > void ramblock_recv_bitmap_set(RAMBlock *rb, void *host_addr);