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

[Andy Lutomirski]

> On Jan 30, 2020, at 11:22 PM, Xiaoyao Li <xiaoyao.li@xxxxxxxxx> wrote:
> 
> ïOn 1/31/2020 1:16 AM, Andy Lutomirski wrote:
>>>> On Jan 30, 2020, at 8:30 AM, Xiaoyao Li <xiaoyao.li@xxxxxxxxx> wrote:
>>> 
>>> ïOn 1/30/2020 11:18 PM, Andy Lutomirski wrote:
>>>>>> On Jan 30, 2020, at 4:24 AM, Xiaoyao Li <xiaoyao.li@xxxxxxxxx> wrote:
>>>>> 
>>>>> ïThere are two types of #AC can be generated in Intel CPUs:
>>>>> 1. legacy alignment check #AC;
>>>>> 2. split lock #AC;
>>>>> 
>>>>> Legacy alignment check #AC can be injected to guest if guest has enabled
>>>>> alignemnet check.
>>>>> 
>>>>> When host enables split lock detection, i.e., split_lock_detect!=off,
>>>>> guest will receive an unexpected #AC when there is a split_lock happens in
>>>>> guest since KVM doesn't virtualize this feature to guest.
>>>>> 
>>>>> Since the old guests lack split_lock #AC handler and may have split lock
>>>>> buges. To make guest survive from split lock, applying the similar policy
>>>>> as host's split lock detect configuration:
>>>>> - host split lock detect is sld_warn:
>>>>>   warning the split lock happened in guest, and disabling split lock
>>>>>   detect around VM-enter;
>>>>> - host split lock detect is sld_fatal:
>>>>>   forwarding #AC to userspace. (Usually userspace dump the #AC
>>>>>   exception and kill the guest).
>>>> A correct userspace implementation should, with a modern guest kernel, forward the exception. Otherwise youâre introducing a DoS into the guest if the guest kernel is fine but guest userspace is buggy.
>>> 
>>> To prevent DoS in guest, the better solution is virtualizing and advertising this feature to guest, so guest can explicitly enable it by setting split_lock_detect=fatal, if it's a latest linux guest.
>>> 
>>> However, it's another topic, I'll send out the patches later.
>>> 
>> Can we get a credible description of how this would work? I suggest:
>> Intel adds and documents a new CPUID bit or core capability bit that means âsplit lock detection is forced onâ.  If this bit is set, the MSR bit controlling split lock detection is still writable, but split lock detection is on regardless of the value.  Operating systems are expected to set the bit to 1 to indicate to a hypervisor, if present, that they understand that split lock detection is on.
>> This would be an SDM-only change, but it would also be a commitment to certain behavior for future CPUs that donât implement split locks.
> 
> It sounds a PV solution for virtualization that it doesn't need to be defined in Intel-SDM but in KVM document.
> 
> As you suggested, we can define new bit in KVM_CPUID_FEATURES (0x40000001) as KVM_FEATURE_SLD_FORCED and reuse MSR_TEST_CTL or use a new virtualized MSR for guest to tell hypervisor it understand split lock detection is forced on.

Of course KVM can do this. But this missed the point. Intel added a new CPU feature, complete with an enumeration mechanism, that cannot be correctly used if a hypervisor is present. As it stands, without specific hypervisor and guest support of a non-Intel interface, it is *impossible* to give architecturally correct behavior to a guest. If KVM implements your suggestion, *Windows* guests will still malfunction on Linux.

This is Intelâs mess. Intel should have some responsibility for cleaning it up. If Intel puts something like my suggestion into the SDM, all the OSes and hypervisors will implement it the *same* way and will be compatible. Sure, old OSes will still be broken, but at least new guests will work correctly. Without something like this, even new guests are busted.

> 
>> Can one of you Intel folks ask the architecture team about this?
>