Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 10/16/2019 7:58 PM, Paolo Bonzini wrote:
> On 16/10/19 13:49, Thomas Gleixner wrote:
>> On Wed, 16 Oct 2019, Paolo Bonzini wrote:
>>> Yes it does.  But Sean's proposal, as I understand it, leads to the
>>> guest receiving #AC when it wasn't expecting one.  So for an old guest,
>>> as soon as the guest kernel happens to do a split lock, it gets an
>>> unexpected #AC and crashes and burns.  And then, after much googling and
>>> gnashing of teeth, people proceed to disable split lock detection.
>>
>> I don't think that this was what he suggested/intended.
> 
> Xiaoyao's reply suggests that he also understood it like that.
>

Actually, what I replied is a little different from what you stated 
above that guest won't receive #AC when it wasn't expecting one but the 
userspace receives this #AC.

>>> In all of these cases, the common final result is that split-lock
>>> detection is disabled on the host.  So might as well go with the
>>> simplest one and not pretend to virtualize something that (without core
>>> scheduling) is obviously not virtualizable.
>>
>> You are completely ignoring any argument here and just leave it behind your
>> signature (instead of trimming your reply).
> 
> I am not ignoring them, I think there is no doubt that this is the
> intended behavior.  I disagree that Sean's patches achieve it, however.
> 
>>>> 1) Sane guest
>>>>
>>>> Guest kernel has #AC handler and you basically prevent it from
>>>> detecting malicious user space and killing it. You also prevent #AC
>>>> detection in the guest kernel which limits debugability.
>>
>> That's a perfectly fine situation. Host has #AC enabled and exposes the
>> availability of #AC to the guest. Guest kernel has a proper handler and
>> does the right thing. So the host _CAN_ forward #AC to the guest and let it
>> deal with it. For that to work you need to expose the MSR so you know the
>> guest state in the host.
>>
>> Your lazy 'solution' just renders #AC completely useless even for
>> debugging.
>>
>>>> 2) Malicious guest
>>>>
>>>> Trigger #AC to disable the host detection and then carry out the DoS
>>>> attack.
>>
>> With your proposal you render #AC useless even on hosts which have SMT
>> disabled, which is just wrong. There are enough good reasons to disable
>> SMT.
> 
> My lazy "solution" only applies to SMT enabled.  When SMT is either not
> supported, or disabled as in "nosmt=force", we can virtualize it like
> the posted patches have done so far.
> 

Do we really need to divide it into two cases of SMT enabled and SMT 
disabled?

>> I agree that with SMT enabled the situation is truly bad, but we surely can
>> be smarter than just disabling it globally unconditionally and forever.
>>
>> Plus we want a knob which treats guests triggering #AC in the same way as
>> we treat user space, i.e. kill them with SIGBUS.
> 
> Yes, that's a valid alternative.  But if SMT is possible, I think the
> only sane possibilities are global disable and SIGBUS.  SIGBUS (or
> better, a new KVM_RUN exit code) can be acceptable for debugging guests too.

If SIGBUS, why need to globally disable?

When there is an #AC due to split-lock in guest, KVM only has below two 
choices:
1) inject back into guest.
    - If kvm advertise this feature to guest, and guest kernel is 
latest, and guest kernel must enable it too. It's the happy case that 
guest can handler it on its own purpose.
    - Any other cases, guest get an unexpected #AC and crash.
2) report to userspace (I think the same like a SIGBUS)

So for simplicity, we can do what Paolo suggested that don't advertise 
this feature and report #AC to userspace when an #AC due to split-lock 
in guest *but* we never disable the host's split-lock detection due to 
guest's split-lock.

> Paolo
>