Re: [PATCH v6 8/8] kvm: vmx: virtualize split lock detection

From: Xiaoyao Li
Date: Thu Mar 26 2020 - 08:31:09 EST


On 3/26/2020 7:08 PM, Thomas Gleixner wrote:
Xiaoyao Li <xiaoyao.li@xxxxxxxxx> writes:
On 3/25/2020 9:41 AM, Thomas Gleixner wrote:
If you really want to address that scenario, then why are you needing
any of those completely backwards interfaces at all?

Just because your KVM exception trap uses the host handling function
which sets TIF_SLD?
Yes. just because KVM use the host handling function.

If you disallow me to touch codes out of kvm. It can be achieved with

Who said you cannot touch code outside of KVM?

Obviously re-use TIF_SLD flag to automatically switch MSR_TEST_CTRL.SLD
bit when switch to/from vcpu thread is better.

What's better about that?

TIF_SLD has very well defined semantics. It's used to denote that the
SLD bit needs to be cleared for the task when its scheduled in.

So now you overload it by clearing it magically and claim that this is
better.

vCPU-thread

user space (qemu)
triggers #AC
-> exception
set TIF_SLD

iotctl()
vcpu_run()
-> clear TIF_SLD

It's not better, it's simply wrong and inconsistent.

And to virtualize SLD feature as full as possible for guest, we have to
implement the backwards interface. If you really don't want that
interface, we have to write code directly in kvm to modify TIF_SLD flag
and MSR_TEST_CTRL.SLD bit.

Wrong again. KVM has absolutely no business in fiddling with TIF_SLD and
the function to flip the SLD bit is simply sld_update_msr(bool on) which
does not need any KVMism at all.

There are two options to handle SLD for KVM:

1) Follow strictly the host rules

If user space or guest triggered #AC then TIF_SLD is set and that
task is excluded from ever setting SLD again.

Obviously, it's not about to virtualize SLD for guest. So we don't pick this one.

2) Track KVM guest state separately

vcpu_run()
if (current_has(TIF_SLD) && guest_sld_on())
sld_update_msr(true);
else if (!current_has(TIF_SLD) && !guest_sld_on())
sld_update_msr(false);
vmenter()
....
vmexit()
if (current_has(TIF_SLD) && guest_sld_on())
sld_update_msr(false);
else if (!current_has(TIF_SLD) && !guest_sld_on())
sld_update_msr(true);

If the guest triggers #AC then this solely affects guest state
and does not fiddle with TIF_SLD.


OK. So when host is sld_warn, guest's SLD value can be loaded into hardware MSR when vmenter.

I'll go with this option.