On Tue, Oct 17, 2023 at 02:38:48AM +0200, William Roche wrote:
> On 10/16/23 18:48, Peter Xu wrote:
> > On Fri, Oct 13, 2023 at 03:08:39PM +0000, “William Roche wrote:
> > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > > index 5e95c496bb..e8db6380c1 100644
> > > --- a/target/arm/kvm64.c
> > > +++ b/target/arm/kvm64.c
> > > @@ -1158,7 +1158,6 @@ 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);
> > > /*
> > > * If this is a BUS_MCEERR_AR, we know we have been called
> > > * synchronously from the vCPU thread, so we can easily
> > > @@ -1169,7 +1168,12 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int
> > > code, void *addr)
> > > * called synchronously from the vCPU thread, or a bit
> > > * later from the main thread, so doing the injection of
> > > * the error would be more complicated.
> > > + * In this case, BUS_MCEERR_AO errors are unknown from the
> > > + * guest, and we will prevent migration as long as this
> > > + * poisoned page hasn't generated a BUS_MCEERR_AR error
> > > + * that the guest takes into account.
> > > */
> > > + kvm_hwpoison_page_add(ram_addr, (code == BUS_MCEERR_AR));
> >
> > I'm curious why ARM doesn't forward this event to guest even if it's AO.
> > X86 does it, and makes more sense to me.
>
> I agree that forwarding this error is the best option to implement.
> But an important note about this aspect is that only Intel architecture
> handles the AO error forwarding correctly; currently an AMD VM crashes
> when an AO error relay is attempted.
>
> That's why we've submitted the following kvm patch:
> https://lore.kernel.org/all/20230912211824.90952-3-john.al...@amd.com/
> Among other AMD enhancements to better deal with MCE relay.
I see.
>
>
> > Not familiar with arm, do you
> > know the reason?
>
> I can't answer this question as I don't know anything about the specific
> 'complications' mentioned in the comment above. Maybe something around
> the injection through ACPI GHES and its interrupt mechanism ??
> But note also that ignoring AO errors is just a question of relying on
> the Hypervisor kernel to generate an AR error when the asynchronously
> poisoned page is touched later. Which can be acceptable -- when the
> system guaranties the AR fault on the page.
>
> >
> > I think this patch needs review from ARM and/or KVM side. Do you want to
> > have the 1st patch merged, or rather wait for the whole set?
>
> I think that integrating the first patch alone is not an option
> as we would introduce the silent data corruption possibility I
> described.
I asked because I think patch 1 itself is still an improvement, which
avoids src VM from crashing when hitting poisoned pages. Especially IIUC
on some arch (Intel?) it's a complete fix.
But for sure we can keep them as a whole series if you want, but then it'll
be good you add some more reviewers; at least some ARM/AMD developers,
perhaps.
> It would be better to integrate the two of them as a whole
> set. But the use of the kernel feature you indicated me can change all
> of that !
>
> >
> > Another thing to mention: feel free to look at a recent addition of ioctl
> > from userfault, where it can inject poisoned ptes:
> >
> > https://lore.kernel.org/r/20230707215540.2324998-1-axelrasmus...@google.com
> >
> > I'm wondering if that'll be helpful to qemu too, where we can migrate
> > hwpoison_page_list and enforce the poisoning on dest. Then even for AO
> > when accessed by guest it'll generated another MCE on dest.
>
> I could be missing something, but Yes, this is exactly how I understand
> this kernel feature use case with its description in:
> https://lore.kernel.org/all/20230707215540.2324998-5-axelrasmus...@google.com/
>
> vvvvvv
> So the basic way to use this new feature is:
>
> - On the new host, the guest's memory is registered with userfaultfd, in
> either MISSING or MINOR mode (doesn't really matter for this purpose).
> - On any first access, we get a userfaultfd event. At this point we can
> communicate with the old host to find out if the page was poisoned.
> - If so, we can respond with a UFFDIO_POISON - this places a swap marker
> so any future accesses will SIGBUS. Because the pte is now "present",
> future accesses won't generate more userfaultfd events, they'll just
> SIGBUS directly.
> ^^^^^^
>
> Thank you for letting me know about this kernel functionality.
>
> I need to take some time to investigate it, to see how I could use it.
One more hint, please double check though: in QEMU's use case (e.g. precopy
only, while not using postcopy) I think you may even be able to install the
poisoned pte without MISSING (or any other uffd) mode registered.
You can try creating one uffd descriptor (which will bind the desc with the
current mm context; in this case we need it to happen only on dest qemu),
then try injecting poison ptes anywhere in the guest address ranges.
>
> The solution I'm suggesting here doesn't cover as many cases as the
> UFFDIO_POISON use could help to implement.
> But it gives us a possibility to live migrate VMs that already
> experienced memory errors, trusting the VM kernel to correctly deal with
> these past errors.
>
> AFAIK, currently, a standard qemu VM that has experienced a memory error
> can't be live migrated at all.
I suppose here you meant AO errors only.
IIUC the major issue regarding migration is AO errors will become ARs on
src qemu when vcpu accessed, which means AOs are all fine if not forwarded
to guest. However after migration that is not guaranteed. Poisoned ptes
properly installed on dest basically grants QEMU the ability to "migrate a
poisoned page", meanwhile without really wasting a physical page on dest,
making sure those AO error addrs keep generating ARs even after migration.
It seems the 1st patch is still needed even in this case?
Thanks,
--
Peter Xu
