From: William Roche <william.roche@oracle.com>
When an entire large page is impacted by an error (hugetlbfs case),
report better the size and location of this large memory hole, so
give a warning message when this page is first hit:
Memory error: Loosing a large page (size: X) at QEMU addr Y and GUEST addr Z
Signed-off-by: William Roche <william.roche@oracle.com>
---
accel/kvm/kvm-all.c | 9 ++++++++-
include/sysemu/kvm_int.h | 4 +++-
target/arm/kvm.c | 2 +-
target/i386/kvm/kvm.c | 2 +-
4 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 6dd06f5edf..a572437115 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1284,7 +1284,7 @@ static void kvm_unpoison_all(void *param)
}
}
-void kvm_hwpoison_page_add(ram_addr_t ram_addr)
+void kvm_hwpoison_page_add(ram_addr_t ram_addr, void *ha, hwaddr gpa)
{
HWPoisonPage *page;
size_t sz = qemu_ram_pagesize_from_addr(ram_addr);
@@ -1301,6 +1301,13 @@ void kvm_hwpoison_page_add(ram_addr_t ram_addr)
page->ram_addr = ram_addr;
page->page_size = sz;
QLIST_INSERT_HEAD(&hwpoison_page_list, page, list);
+
+ if (sz > TARGET_PAGE_SIZE) {
+ gpa = ROUND_DOWN(gpa, sz);
+ ha = (void *)ROUND_DOWN((uint64_t)ha, sz);
+ warn_report("Memory error: Loosing a large page (size: %zu) "
+ "at QEMU addr %p and GUEST addr 0x%" HWADDR_PRIx, sz, ha, gpa);
+ }
}
bool kvm_hwpoisoned_mem(void)
diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
index a1e72763da..ee34f1d225 100644
--- a/include/sysemu/kvm_int.h
+++ b/include/sysemu/kvm_int.h
@@ -178,10 +178,12 @@ void kvm_set_max_memslot_size(hwaddr max_slot_size);
*
* Parameters:
* @ram_addr: the address in the RAM for the poisoned page
+ * @hva: host virtual address aka QEMU addr
+ * @gpa: guest physical address aka GUEST addr
*
* Add a poisoned page to the list
*
* Return: None.
*/
-void kvm_hwpoison_page_add(ram_addr_t ram_addr);
+void kvm_hwpoison_page_add(ram_addr_t ram_addr, void *hva, hwaddr gpa);
#endif
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index f1f1b5b375..aae66dba41 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -2359,7 +2359,7 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
ram_addr = qemu_ram_addr_from_host(addr);
if (ram_addr != RAM_ADDR_INVALID &&
kvm_physical_memory_addr_from_host(c->kvm_state, addr, &paddr)) {
- kvm_hwpoison_page_add(ram_addr);
+ kvm_hwpoison_page_add(ram_addr, addr, paddr);
/*
* If this is a BUS_MCEERR_AR, we know we have been called
* synchronously from the vCPU thread, so we can easily
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index fd9f198892..fd7cd7008e 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -753,7 +753,7 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
ram_addr = qemu_ram_addr_from_host(addr);
if (ram_addr != RAM_ADDR_INVALID &&
kvm_physical_memory_addr_from_host(c->kvm_state, addr, &paddr)) {
- kvm_hwpoison_page_add(ram_addr);
+ kvm_hwpoison_page_add(ram_addr, addr, paddr);
kvm_mce_inject(cpu, paddr, code);
/*
--
2.43.5On 07.11.24 11:21, “William Roche wrote: > From: William Roche <william.roche@oracle.com> > > When an entire large page is impacted by an error (hugetlbfs case), > report better the size and location of this large memory hole, so > give a warning message when this page is first hit: > Memory error: Loosing a large page (size: X) at QEMU addr Y and GUEST addr Z > Hm, I wonder if we really want to special-case hugetlb here. Why not make the warning independent of the underlying page size? -- Cheers, David / dhildenb
On 11/12/24 12:13, David Hildenbrand wrote: > On 07.11.24 11:21, “William Roche wrote: >> From: William Roche <william.roche@oracle.com> >> >> When an entire large page is impacted by an error (hugetlbfs case), >> report better the size and location of this large memory hole, so >> give a warning message when this page is first hit: >> Memory error: Loosing a large page (size: X) at QEMU addr Y and GUEST >> addr Z >> > > Hm, I wonder if we really want to special-case hugetlb here. > > Why not make the warning independent of the underlying page size? We already have a warning provided by Qemu (in kvm_arch_on_sigbus_vcpu()): Guest MCE Memory Error at QEMU addr Y and GUEST addr Z of type BUS_MCEERR_AR/_AO injected The one I suggest is an additional message provided before the above message. Here is an example: qemu-system-x86_64: warning: Memory error: Loosing a large page (size: 2097152) at QEMU addr 0x7fdd7d400000 and GUEST addr 0x11600000 qemu-system-x86_64: warning: Guest MCE Memory Error at QEMU addr 0x7fdd7d400000 and GUEST addr 0x11600000 of type BUS_MCEERR_AO injected According to me, this large page case additional message will help to better understand the probable sudden proliferation of memory errors that can be reported by Qemu on the impacted range. Not only will the machine administrator identify better that a single memory error had this large impact, it can also help us to better measure the impact of fixing the large page memory error support in the field (in the future). These are some reasons why I do think this large page specific message can be useful.
On 12.11.24 19:17, William Roche wrote: > On 11/12/24 12:13, David Hildenbrand wrote: >> On 07.11.24 11:21, “William Roche wrote: >>> From: William Roche <william.roche@oracle.com> >>> >>> When an entire large page is impacted by an error (hugetlbfs case), >>> report better the size and location of this large memory hole, so >>> give a warning message when this page is first hit: >>> Memory error: Loosing a large page (size: X) at QEMU addr Y and GUEST >>> addr Z >>> >> >> Hm, I wonder if we really want to special-case hugetlb here. >> >> Why not make the warning independent of the underlying page size? > > We already have a warning provided by Qemu (in kvm_arch_on_sigbus_vcpu()): > > Guest MCE Memory Error at QEMU addr Y and GUEST addr Z of type > BUS_MCEERR_AR/_AO injected > > The one I suggest is an additional message provided before the above > message. > > Here is an example: > qemu-system-x86_64: warning: Memory error: Loosing a large page (size: > 2097152) at QEMU addr 0x7fdd7d400000 and GUEST addr 0x11600000 > qemu-system-x86_64: warning: Guest MCE Memory Error at QEMU addr > 0x7fdd7d400000 and GUEST addr 0x11600000 of type BUS_MCEERR_AO injected > Hm, I think we should definitely be including the size in the existing one. That code was written without huge pages in mind. We should similarly warn in the arm implementation (where I don't see a similar message yet). > > According to me, this large page case additional message will help to > better understand the probable sudden proliferation of memory errors > that can be reported by Qemu on the impacted range. > Not only will the machine administrator identify better that a single > memory error had this large impact, it can also help us to better > measure the impact of fixing the large page memory error support in the > field (in the future). What about extending the existing one to something like warning: Guest MCE Memory Error at QEMU addr $ADDR and GUEST $PADDR of type BUS_MCEERR_AO and size $SIZE (large page) injected With the "large page" hint you can highlight that this is special. On a related note ...I think we have a problem. Assume we got a SIGBUS on a huge page (e.g., somewhere in a 1 GiB page). We will call kvm_mce_inject(cpu, paddr, code) / acpi_ghes_record_errors(ACPI_HEST_SRC_ID_SEA, paddr) But where is the size information? :// Won't the VM simply assume that there was a MCE on a single 4k page starting at paddr? I'm not sure if we can inject ranges, or if we would have to issue one MCE per page ... hm, what's your take on this? -- Cheers, David / dhildenb
Thanks for the feedback on the patches, I'll send a new version in the coming week. But I just wanted to answer now the questions you asked on this specific one as they are related to the importance of fixing the large page failures handling. On 11/12/24 23:22, David Hildenbrand wrote: > On 12.11.24 19:17, William Roche wrote: >> On 11/12/24 12:13, David Hildenbrand wrote: >>> On 07.11.24 11:21, “William Roche wrote: >>>> From: William Roche <william.roche@oracle.com> >>>> >>>> When an entire large page is impacted by an error (hugetlbfs case), >>>> report better the size and location of this large memory hole, so >>>> give a warning message when this page is first hit: >>>> Memory error: Loosing a large page (size: X) at QEMU addr Y and GUEST >>>> addr Z >>>> >>> >>> Hm, I wonder if we really want to special-case hugetlb here. >>> >>> Why not make the warning independent of the underlying page size? >> >> We already have a warning provided by Qemu (in >> kvm_arch_on_sigbus_vcpu()): >> >> Guest MCE Memory Error at QEMU addr Y and GUEST addr Z of type >> BUS_MCEERR_AR/_AO injected >> >> The one I suggest is an additional message provided before the above >> message. >> >> Here is an example: >> qemu-system-x86_64: warning: Memory error: Loosing a large page (size: >> 2097152) at QEMU addr 0x7fdd7d400000 and GUEST addr 0x11600000 >> qemu-system-x86_64: warning: Guest MCE Memory Error at QEMU addr >> 0x7fdd7d400000 and GUEST addr 0x11600000 of type BUS_MCEERR_AO injected >> > > Hm, I think we should definitely be including the size in the existing > one. That code was written without huge pages in mind. Yes we can do that, and get the page size at this level to pass as a 'page_sise' argument to kvm_hwpoison_page_add(). It would make the message longer as we will have the extra information about the large page on all messages when an error impacts a large page. We could change the messages only when we are dealing with a large page, so that the standard (4k) case isn't modified. > > We should similarly warn in the arm implementation (where I don't see a > similar message yet). Ok, I'll also add a message for the ARM platform. >> >> According to me, this large page case additional message will help to >> better understand the probable sudden proliferation of memory errors >> that can be reported by Qemu on the impacted range. >> Not only will the machine administrator identify better that a single >> memory error had this large impact, it can also help us to better >> measure the impact of fixing the large page memory error support in the >> field (in the future). > > What about extending the existing one to something like > > warning: Guest MCE Memory Error at QEMU addr $ADDR and GUEST $PADDR of > type BUS_MCEERR_AO and size $SIZE (large page) injected > > > With the "large page" hint you can highlight that this is special. Right, we can do it that way. It also gives the impression that we somehow inject errors on a large range of the memory. Which is not the case. I'll send a proposal with a different formulation, so that you can choose. > On a related note ...I think we have a problem. Assume we got a SIGBUS > on a huge page (e.g., somewhere in a 1 GiB page). > > We will call kvm_mce_inject(cpu, paddr, code) / > acpi_ghes_record_errors(ACPI_HEST_SRC_ID_SEA, paddr) > > But where is the size information? :// Won't the VM simply assume that > there was a MCE on a single 4k page starting at paddr? This is absolutely right ! It's exactly what happens: The VM kernel received the information and considers that only the impacted page has to be poisoned. That's also the reason why Qemu repeats the error injections every time the poisoned large page is accessed (for all other touched 4k pages located on this "memory hole"). > > I'm not sure if we can inject ranges, or if we would have to issue one > MCE per page ... hm, what's your take on this? I don't know of any size information about a memory error reported by the hardware. The kernel doesn't seem to expect any such information. It explains why there is no impact/blast size information provided when an error is relayed to the VM. We could take the "memory hole" size into account in Qemu, but repeating error injections is not going to help a lot either: We'd need to give the VM some time to deal with an error injection before producing a new error for the next page etc... in the case (x86 only) where an asynchronous error is relayed with BUS_MCEERR_AO, we would also have to repeat the error for all the 4k pages located on the lost large page too. We can see that the Linux kernel has some mechanisms to deal with a seldom 4k page loss, but a larger blast is very likely to crash the VM (which is fine). And as a significant part of the memory is no longer accessible, dealing with the error itself can be impaired and we increase the risk of loosing data, even though most of the memory on the large page could still be used. Now if we can recover the 'still valid' memory of the impacted large page, we can significantly reduce this blast and give a much better chance to the VM to survive the incident or crash more gracefully. I've looked at the project you indicated me, which is not ready to be adopted: https://lore.kernel.org/linux-mm/20240924043924.3562257-2-jiaqiyan@google.com/T/ But we see that, this large page enhancement is needed, sometimes just to give a chance to the VM to survive a little longer before being terminated or moved. Injecting multiple MCEs or ACPI error records doesn't help, according to me. William.
>> Hm, I think we should definitely be including the size in the existing >> one. That code was written without huge pages in mind. > > Yes we can do that, and get the page size at this level to pass as a > 'page_sise' argument to kvm_hwpoison_page_add(). > > It would make the message longer as we will have the extra information > about the large page on all messages when an error impacts a large page. > We could change the messages only when we are dealing with a large page, > so that the standard (4k) case isn't modified. Right. And likely we should call it "huge page" instead, which is the Linux term for anything larger than a single page. [...] >> >> With the "large page" hint you can highlight that this is special. > > Right, we can do it that way. It also gives the impression that we > somehow inject errors on a large range of the memory. Which is not the > case. I'll send a proposal with a different formulation, so that you can > choose. > Make sense. > > >> On a related note ...I think we have a problem. Assume we got a SIGBUS >> on a huge page (e.g., somewhere in a 1 GiB page). >> >> We will call kvm_mce_inject(cpu, paddr, code) / >> acpi_ghes_record_errors(ACPI_HEST_SRC_ID_SEA, paddr) >> >> But where is the size information? :// Won't the VM simply assume that >> there was a MCE on a single 4k page starting at paddr? > > This is absolutely right ! > It's exactly what happens: The VM kernel received the information and > considers that only the impacted page has to be poisoned. > > That's also the reason why Qemu repeats the error injections every time > the poisoned large page is accessed (for all other touched 4k pages > located on this "memory hole"). :/ So we always get from Linux the full 1Gig range and always report the first 4k page essentially, on any such access, right? BTW, should we handle duplicates in our poison list? > >> >> I'm not sure if we can inject ranges, or if we would have to issue one >> MCE per page ... hm, what's your take on this? > > I don't know of any size information about a memory error reported by > the hardware. The kernel doesn't seem to expect any such information. > It explains why there is no impact/blast size information provided when > an error is relayed to the VM. > > We could take the "memory hole" size into account in Qemu, but repeating > error injections is not going to help a lot either: We'd need to give > the VM some time to deal with an error injection before producing a new > error for the next page etc... in the case (x86 only) where an I had the same thoughts. > asynchronous error is relayed with BUS_MCEERR_AO, we would also have to > repeat the error for all the 4k pages located on the lost large page too. > > We can see that the Linux kernel has some mechanisms to deal with a > seldom 4k page loss, but a larger blast is very likely to crash the VM > (which is fine). Right, and that will inevitably happen when we get a MVE on a 1GiG hugetlb page, correct? The whole thing will be inaccessible. > And as a significant part of the memory is no longer > accessible, dealing with the error itself can be impaired and we > increase the risk of loosing data, even though most of the memory on the > large page could still be used. > > Now if we can recover the 'still valid' memory of the impacted large > page, we can significantly reduce this blast and give a much better > chance to the VM to survive the incident or crash more gracefully. Right. That cannot be sorted out in user space alone, unfortunately. > > I've looked at the project you indicated me, which is not ready to be > adopted: > https://lore.kernel.org/linux-mm/20240924043924.3562257-2-jiaqiyan@google.com/T/ > Yes, that goes into a better direction, though. > But we see that, this large page enhancement is needed, sometimes just > to give a chance to the VM to survive a little longer before being > terminated or moved. > Injecting multiple MCEs or ACPI error records doesn't help, according to me. I suspect that in most cases, when we get an MCE on a 1Gig page in the hypervisor, our running Linux guest will soon crash, because it really lost 1 Gig of contiguous memory. :( -- Cheers, David / dhildenb
© 2016 - 2024 Red Hat, Inc.