Re: [PATCH 2/2] KVM: VMX: Extend VMX's #AC handding

[Andy Lutomirski <luto@amacapital.net>]

> On Feb 1, 2020, at 8:58 AM, Xiaoyao Li <xiaoyao.li@intel.com> wrote:
> 
> On 2/1/2020 5:33 AM, Andy Lutomirski wrote:
>>>> On Jan 31, 2020, at 1:04 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
>>> 
>>> On Fri, Jan 31, 2020 at 12:57:51PM -0800, Andy Lutomirski wrote:
>>>> 
>>>>>> On Jan 31, 2020, at 12:18 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
>>>>> 
>>>>> This is essentially what I proposed a while back.  KVM would allow enabling
>>>>> split-lock #AC in the guest if and only if SMT is disabled or the enable bit
>>>>> is per-thread, *or* the host is in "warn" mode (can live with split-lock #AC
>>>>> being randomly disabled/enabled) and userspace has communicated to KVM that
>>>>> it is pinning vCPUs.
>>>> 
>>>> How about covering the actual sensible case: host is set to fatal?  In this
>>>> mode, the guest gets split lock detection whether it wants it or not. How do
>>>> we communicate this to the guest?
>>> 
>>> KVM doesn't advertise split-lock #AC to the guest and returns -EFAULT to the
>>> userspace VMM if the guest triggers a split-lock #AC.
>>> 
>>> Effectively the same behavior as any other userspace process, just that KVM
>>> explicitly returns -EFAULT instead of the process getting a SIGBUS.
>> Which helps how if the guest is actually SLD-aware?
>> I suppose we could make the argument that, if an SLD-aware guest gets #AC at CPL0, it’s a bug, but it still seems rather nicer to forward the #AC to the guest instead of summarily killing it.
> 
> If KVM does advertise split-lock detection to the guest, then kvm/host can know whether a guest is SLD-aware by checking guest's MSR_TEST_CTRL.SPLIT_LOCK_DETECT bit.
> 
> - If guest's MSR_TEST_CTRL.SPLIT_LOCK_DETECT is set, it indicates guest is SLD-aware so KVM forwards #AC to guest.
> 

I disagree. If you advertise split-lock detection with the current core capability bit, it should *work*.  And it won’t.  The choices you’re actually giving the guest are:

a) Guest understands SLD and wants it on.  The guest gets the same behavior as in bare metal.

b) Guest does not understand. Guest gets killed if it screws up as described below.

> - If not set. It may be a old guest or a malicious guest or a guest without SLD support, and we cannot figure it out. So we have to kill the guest when host is SLD-fatal, and let guest survive when SLD-WARN for old sane buggy guest.

All true, but the result of running a Linux guest in SLD-warn mode will be broken.

> 
> In a word, all the above is on the condition that KVM advertise split-lock detection to guest. But this patch doesn't do this. Maybe I should add that part in v2.

I think you should think the details all the way through, and I think you’re likely to determine that the Intel architecture team needs to do *something* to clean up this mess.

There are two independent problems here.  First, SLD *can’t* be virtualized sanely because it’s per-core not per-thread. Second, most users *won’t want* to virtualize it correctly even if they could: if a guest is allowed to do split locks, it can DoS the system.

So I think there should be an architectural way to tell a guest that SLD is on whether it likes it or not. And the guest, if booted with sld=warn, can print a message saying “haha, actually SLD is fatal” and carry on.

> 
>> ISTM, on an SLD-fatal host with an SLD-aware guest, the host should tell the guest “hey, you may not do split locks — SLD is forced on” and the guest should somehow acknowledge it so that it sees the architectural behavior instead of something we made up.  Hence my suggestion.
>