Hi On Thu, Aug 25, 2022 at 5:35 PM Peter Maydell <peter.mayd...@linaro.org> wrote: > > On Thu, 25 Aug 2022 at 14:21, <marcandre.lur...@redhat.com> wrote: > > > > From: Marc-André Lureau <marcandre.lur...@redhat.com> > > > > Rewrite get_next_page() to work over non-aligned blocks. When it > > encounters non aligned addresses, it will allocate a zero-page and try > > to fill it. > > > > This solves a kdump crash with "tpm-crb-cmd" RAM memory region, > > qemu-kvm: ../dump/dump.c:1162: _Bool get_next_page(GuestPhysBlock **, > > uint64_t *, uint8_t **, DumpState *): Assertion `(block->target_start & > > ~target_page_mask) == 0' failed. > > > > because: > > guest_phys_block_add_section: target_start=00000000fed40080 > > target_end=00000000fed41000: added (count: 4) > > > > Fixes: > > https://bugzilla.redhat.com/show_bug.cgi?id=2120480 > > > > static bool get_next_page(GuestPhysBlock **blockptr, uint64_t *pfnptr, > > - uint8_t **bufptr, DumpState *s) > > + uint8_t **bufptr, bool *allocptr, DumpState *s) > > { > > GuestPhysBlock *block = *blockptr; > > - hwaddr addr, target_page_mask = ~((hwaddr)s->dump_info.page_size - 1); > > In the old code, we treated the dump_info.page size as an indication > of the target's page size... > > > - uint8_t *buf; > > + uint32_t page_size = s->dump_info.page_size; > > + bool alloced = false; > > + uint8_t *buf = NULL, *hbuf; > > + hwaddr addr; > > > > /* block == NULL means the start of the iteration */ > > if (block == NULL) { > > *blockptr = block = QTAILQ_FIRST(&s->guest_phys_blocks.head); > > addr = block->target_start; > > + *pfnptr = dump_paddr_to_pfn(s, addr); > > } else { > > - addr = dump_pfn_to_paddr(s, *pfnptr + 1); > > + assert(block != NULL); > > + *pfnptr += 1; > > + addr = dump_pfn_to_paddr(s, *pfnptr); > > } > > assert(block != NULL); > > > > - if ((addr >= block->target_start) && > > - (addr + s->dump_info.page_size <= block->target_end)) { > > - buf = block->host_addr + (addr - block->target_start); > > - } else { > > - /* the next page is in the next block */ > > - *blockptr = block = QTAILQ_NEXT(block, next); > > - if (!block) { > > - return false; > > + while (1) { > > + if (addr >= block->target_start && addr < block->target_end) { > > + size_t n = MIN(block->target_end - addr, page_size - addr % > > page_size); > > + hbuf = block->host_addr + (addr - block->target_start); > > + if (!alloced) { > > + if (n == page_size) { > > + /* this is a whole host page, go for it */ > > + assert(addr % page_size == 0); > > ...but here we're claiming in this comment that it is the host's > page size.
+ uint32_t page_size = s->dump_info.page_size; bad comment, will be fixed, thanks > > Which is it ? > > thanks > -- PMM >
