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

[Xiaoyao Li]

On 1/31/2020 11:37 PM, Andy Lutomirski wrote:
> > 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. 

Why it cannot be correctly used if a hypervisor is present?
Because it needs to disable split lock detection when running a vcpu for 
guest as this patch wants to do?

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

Actually, KVM don't need to implement my suggestion. It can just 
virtualize and expose this feature (MSR_IA32_CORE_CAPABILITIES and 
MSR_TEST_CTRL) to guest, (but it may have some requirement that HT is 
disabled and host is sld_off) then guest can use it architecturally.

If we don't virtualize and expose this feature to guest as now, both old 
and modern guest should think there is no feature split lock detection, 
and of course there is no #AC when it executes a split lock.

However, the problem here is the scope of host split lock detection, 
i.e., whether or not host's enabling of split lock detection takes 
effect on guest.

> 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?