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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ftdvxtjh.fsf@nanos.tec.linutronix.de>
Date:   Thu, 26 Mar 2020 12:08:34 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Xiaoyao Li <xiaoyao.li@...el.com>, Ingo Molnar <mingo@...hat.com>,
        Borislav Petkov <bp@...en8.de>, hpa@...or.com,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     x86@...nel.org, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Arvind Sankar <nivedita@...m.mit.edu>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Tony Luck <tony.luck@...el.com>
Subject: Re: [PATCH v6 8/8] kvm: vmx: virtualize split lock detection

Xiaoyao Li <xiaoyao.li@...el.com> 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.

   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.

Thanks,

        tglx