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.
Which is it ?
thanks
-- PMM
