On 1/30/25 18:02, David Hildenbrand wrote:
On 27.01.25 22:31, “William Roche wrote:
From: William Roche <william.roche@xxxxxxxxxx>
In case of a large page impacted by a memory error, provide an
information about the impacted large page before the memory
error injection message.
This message would also appear on ras enabled ARM platforms, with
the introduction of an x86 similar error injection message.
In the case of a large page impacted, we now report:
Memory Error on large page from <backend>:<address>+<fd_offset>
+<page_size>
Signed-off-by: William Roche <william.roche@xxxxxxxxxx>
---
accel/kvm/kvm-all.c | 11 +++++++++++
include/exec/cpu-common.h | 9 +++++++++
system/physmem.c | 21 +++++++++++++++++++++
target/arm/kvm.c | 3 +++
4 files changed, 44 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f89568bfa3..08e14f8960 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1296,6 +1296,17 @@ static void kvm_unpoison_all(void *param)
void kvm_hwpoison_page_add(ram_addr_t ram_addr)
{
HWPoisonPage *page;
+ struct RAMBlockInfo rb_info;
+
+ if (qemu_ram_block_location_info_from_addr(ram_addr, &rb_info)) {
+ size_t ps = rb_info.page_size;
+ if (ps > TARGET_PAGE_SIZE) {
+ uint64_t offset = ram_addr - rb_info.offset;
+ error_report("Memory Error on large page from %s:%" PRIx64
+ "+%" PRIx64 " +%zx", rb_info.idstr,
+ QEMU_ALIGN_DOWN(offset, ps),
rb_info.fd_offset, ps);
+ }
+ }
Some smaller nits:
1) I'd call it qemu_ram_block_info_from_addr() -- drop the "_location"
2) Printing the fd_offset only makes sense if there is an fd, right?
You'd have to communicate that information as well.
Apart from that, this series LGTM, thanks!
Thank you David for your feedback.
I'll change the 2 nits above, and add the 2 empty lines missing (in
patch 3/6).
I also removed the fd_offset information in the message from the
qemu_ram_remap() function when we don't have an associated fd (patch 2/6).
You also asked me:
Don't you have to align the fd_offset also down?
fd_offset doesn't need to be aligned as it is used with the value given,
which should already be adapted to the backend needs (when given by the
administrator for example)
I suggest doing the alignment already when calculating "uint64_t offset"
But yes for the offset value itself, it's much better to already align
it when giving it an initial value. Thanks, I've also made the change.
I'm sending a v7 now including all these changes.
I'll also send an update about the kdump behavior on ARM later next week.
Thanks again,
William.
