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

From: Xiaoyao Li
Date: Wed Oct 16 2019 - 09:51:13 EST


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