Re: [PATCH v2] x86/kvm: Disable KVM_ASYNC_PF_SEND_ALWAYS

[Andy Lutomirski <luto@kernel.org>]

Hi, Tony.  I'm adding you because, despite the fact that everyone in
this thread is allergic to #MC, this seems to be one of your favorite
topics :)

> On Apr 9, 2020, at 8:17 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 09/04/20 17:03, Andy Lutomirski wrote:
>>> No, I think we wouldn't use a paravirt #VE at this point, we would
>>> use the real thing if available.
>>>
>>> It would still be possible to switch from the IST to the main
>>> kernel stack before writing 0 to the reentrancy word.
>>
>> Almost but not quite. We do this for NMI-from-usermode, and it’s
>> ugly. But we can’t do this for NMI-from-kernel or #VE-from-kernel
>> because there might not be a kernel stack.  Trying to hack around
>> this won’t be pretty.
>>
>> Frankly, I think that we shouldn’t even try to report memory failure
>> to the guest if it happens with interrupts off. Just kill the guest
>> cleanly and keep it simple. Or inject an intentionally unrecoverable
>> IST exception.
>
> But it would be nice to use #VE for all host-side page faults, not just
> for memory failure.
>
> So the solution would be the same as for NMIs, duplicating the stack
> frame and patching the outer handler's stack from the recursive #VE
> (https://lwn.net/Articles/484932/).  It's ugly but it's a known ugliness.
>
>

Believe me, I know all about how ugly it is, since I’m the one who
fixed most of the bugs in the first few implementations.  And, before
I wrote or ack any such thing for #VE, I want to know why.  What,
exactly, is a sufficiently strong motivation for using #VE *at all*
that Linux should implement a #VE handler?

As I see it, #VE has several downsides:

1. Intel only.

2. Depending on precisely what it's used for, literally any memory
access in the kernel can trigger it as a fault.  This means that it
joins NMI and #MC (and, to a limited extent, #DB) in the horrible
super-atomic-happens-in-bad-contexts camp.  IST is mandatory, and IST
is not so great.

3. Just like NMI and MCE, it comes with a fundamentally broken
don't-recurse-me mechanism.

If we do support #VE, I would suggest we do it roughly like this.  The
#VE handler is a regular IST entry -- there's a single IST stack, and
#VE from userspace stack-switches to the regular kernel stack.  The C
handler (do_virtualization_exception?) is permitted to panic if
something is horribly wrong, but is *not* permitted to clear the word
at byte 4 to re-enable #VE.  Instead, it does something to trigger a
deferred re-enable.  For example, it sends IPI to self and the IPI
handler clears the magic re-enable flag.

There are definitely horrible corner cases here.  For example, suppose
user code mmaps() some kind of failable memory (due to NVDIMM hardware
failures, truncation, whatever).  Then user code points RBP at it and
we get a perf NMI.  Now the perf code tries to copy_from_user_nmi()
the user stack and hits the failure.  It gets #MC or #VE or some
paravirt thing.  Now we're in a situation where we got an IST
exception in the middle of NMI processing and we're expected to do
something intelligent about it.  Sure, we can invoke the extable
handler, but it's not really clear how to recover if we somehow hit
the same type of failure again in the same NMI.

A model that could actually work, perhaps for #VE and #MC, is to have
the extable code do the re-enabling.  So ex_handler_uaccess() and
ex_handler_mcsafe() will call something like rearm_memory_failure(),
and that will do whatever is needed to re-arm the specific memory
failure reporting mechanism in use.

But, before I touch that with a ten-foot pole, I want to know *why*.
What's the benefit?  Why do we want convertible EPT violations?