Re: [PATCH v4 2/2] migration: prevent migration when a poisoned page is unknown from the VM

[William Roche <william.roche@oracle.com>]

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.allen@amd.com/
Among other AMD enhancements to better deal with MCE relay.


>  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.  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-axelrasmussen@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-axelrasmussen@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.

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.

Please correct me if I'm wrong.
Thanks again.