From: marcandre.lureau@redhat.com
To: qemu-devel@nongnu.org
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
qiaonuohan@cn.fujitsu.com,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Peter Maydell" <peter.maydell@linaro.org>,
"David Hildenbrand" <david@redhat.com>,
"Stefan Berger" <stefanb@linux.vnet.ibm.com>
Subject: [PATCH 2/2] dump: fix kdump to work over non-aligned blocks
Date: Thu, 25 Aug 2022 17:21:10 +0400 [thread overview]
Message-ID: <20220825132110.1500330-3-marcandre.lureau@redhat.com> (raw)
In-Reply-To: <20220825132110.1500330-1-marcandre.lureau@redhat.com>
From: Marc-André Lureau <marcandre.lureau@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
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
dump/dump.c | 89 ++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 64 insertions(+), 25 deletions(-)
diff --git a/dump/dump.c b/dump/dump.c
index 18f06cffe2..9aeed88688 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -1094,49 +1094,79 @@ static uint64_t dump_pfn_to_paddr(DumpState *s, uint64_t pfn)
}
/*
- * exam every page and return the page frame number and the address of the page.
- * bufptr can be NULL. note: the blocks here is supposed to reflect guest-phys
- * blocks, so block->target_start and block->target_end should be interal
- * multiples of the target page size.
+ * Return the page frame number and the page content in *bufptr.
+ * bufptr and allocptr can be NULL. If alloced, *bufptr must be freed.
*/
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);
- 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);
+ buf = hbuf;
+ break;
+ } else {
+ buf = g_malloc0(page_size);
+ alloced = true;
+ }
+ }
+
+ memcpy(buf + addr % page_size, hbuf, n);
+ addr += n;
+ if (addr % page_size == 0) {
+ /* we filled up the alloc page */
+ break;
+ }
+ } else {
+ /* the next page is in the next block */
+ *blockptr = block = QTAILQ_NEXT(block, next);
+ if (!block) {
+ break;
+ }
+
+ addr = block->target_start;
+ /* are we still in the same page? */
+ if (dump_paddr_to_pfn(s, addr) != *pfnptr) {
+ if (alloced) {
+ /* no, but we already filled something earlier, return it */
+ break;
+ } else {
+ /* else continue from there */
+ *pfnptr = dump_paddr_to_pfn(s, addr);
+ }
+ }
}
- addr = block->target_start;
- buf = block->host_addr;
}
- /* those checks are going away next */
- assert((block->target_start & ~target_page_mask) == 0);
- assert((block->target_end & ~target_page_mask) == 0);
- *pfnptr = dump_paddr_to_pfn(s, addr);
if (bufptr) {
*bufptr = buf;
+ *allocptr = alloced;
}
- return true;
+ return buf != NULL;
}
static void write_dump_bitmap(DumpState *s, Error **errp)
@@ -1159,7 +1189,7 @@ static void write_dump_bitmap(DumpState *s, Error **errp)
* exam memory page by page, and set the bit in dump_bitmap corresponded
* to the existing page.
*/
- while (get_next_page(&block_iter, &pfn, NULL, s)) {
+ while (get_next_page(&block_iter, &pfn, NULL, NULL, s)) {
ret = set_dump_bitmap(last_pfn, pfn, true, dump_bitmap_buf, s);
if (ret < 0) {
error_setg(errp, "dump: failed to set dump_bitmap");
@@ -1274,6 +1304,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
uint8_t *buf;
GuestPhysBlock *block_iter = NULL;
uint64_t pfn_iter;
+ bool freebuf = false;
/* get offset of page_desc and page_data in dump file */
offset_desc = s->offset_page;
@@ -1314,7 +1345,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
* dump memory to vmcore page by page. zero page will all be resided in the
* first page of page section
*/
- while (get_next_page(&block_iter, &pfn_iter, &buf, s)) {
+ while (get_next_page(&block_iter, &pfn_iter, &buf, &freebuf, s)) {
/* check zero page */
if (buffer_is_zero(buf, s->dump_info.page_size)) {
ret = write_cache(&page_desc, &pd_zero, sizeof(PageDescriptor),
@@ -1403,6 +1434,10 @@ static void write_dump_pages(DumpState *s, Error **errp)
error_setg(errp, "dump: failed to write page desc");
goto out;
}
+ if (freebuf) {
+ g_free(buf);
+ freebuf = false;
+ }
}
s->written_size += s->dump_info.page_size;
}
@@ -1419,6 +1454,10 @@ static void write_dump_pages(DumpState *s, Error **errp)
}
out:
+ if (freebuf) {
+ g_free(buf);
+ }
+
free_data_cache(&page_desc);
free_data_cache(&page_data);
--
2.37.2
next prev parent reply other threads:[~2022-08-25 13:26 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-25 13:21 [PATCH 0/2] Fix dumping in kdump format with non-aligned memory marcandre.lureau
2022-08-25 13:21 ` [PATCH 1/2] dump: simplify a bit kdump get_next_page() marcandre.lureau
2022-08-26 9:45 ` David Hildenbrand
2022-08-26 9:56 ` Marc-André Lureau
2022-08-26 9:58 ` David Hildenbrand
2022-08-25 13:21 ` marcandre.lureau [this message]
2022-08-25 13:34 ` [PATCH 2/2] dump: fix kdump to work over non-aligned blocks Peter Maydell
2022-08-25 13:38 ` Marc-André Lureau
2022-08-26 9:57 ` David Hildenbrand
2022-08-26 10:02 ` Marc-André Lureau
2022-08-26 10:07 ` David Hildenbrand
2022-08-26 10:10 ` David Hildenbrand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220825132110.1500330-3-marcandre.lureau@redhat.com \
--to=marcandre.lureau@redhat.com \
--cc=david@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=qiaonuohan@cn.fujitsu.com \
--cc=stefanb@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).