linux-kernel - Re: [PATCH 01/17] x86: spec_ctrl: fix SPEC

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]

Date:   Mon, 23 Sep 2019 13:34:21 -0400
From:   Andrea Arcangeli <aarcange@...hat.com>
To:     Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        "Dr. David Alan Gilbert" <dgilbert@...hat.com>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        Peter Xu <peterx@...hat.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/17] x86: spec_ctrl: fix SPEC_CTRL initialization after
 kexec

Hello,

On Mon, Sep 23, 2019 at 08:30:57AM -0700, Sean Christopherson wrote:
> On Mon, Sep 23, 2019 at 12:22:23PM +0200, Paolo Bonzini wrote:
> > On 20/09/19 23:24, Andrea Arcangeli wrote:
> > > We can't assume the SPEC_CTRL msr is zero at boot because it could be
> > > left enabled by a previous kernel booted with
> > > spec_store_bypass_disable=on.
> > > 
> > > Without this fix a boot with spec_store_bypass_disable=on followed by
> > > a kexec boot with spec_store_bypass_disable=off would erroneously and
> > > unexpectedly leave bit 2 set in SPEC_CTRL.
> > > 
> > > Signed-off-by: Andrea Arcangeli <aarcange@...hat.com>
> > 
> > Can you send this out separately, so that Thomas et al. can pick it up
> > as a bug fix?

As specified in the cover letter 1/17 was already intended to be
merged separately. I just keep this included in case people had the
idea of using kexec to benchmark this work, because I was bitten by
that bug myself and it wasted a few days worth of benchmarks.

> Can all off the patches that are not directly related to the monolithic
> conversion be sent separately?  AFAICT, patches 01, 03, 07, 08, 14, 15, 16
> and 17 are not required or dependent on the conversion to a monolithic
> module.  That's almost half the series...

03 07 08 are directly related to the monolithic conversion as the
subject of the patch clarifies. In fact I should try to reorder 7/8 in
front to make things more bisectable under all config options.

Per subject of the patch, 14 is also an optimization that while not a
strict requirement, is somewhat related to the monolithic conversion
because in fact it may naturally disappear if I rename the vmx/svm
functions directly.

15 16 17 don't have the monolithic tag in the subject of the patch and
they're obviously unrelated to the monolithic conversion, but when I
did the first research on this idea of dropping kvm.ko a couple of
months ago, things didn't really work well until I got rid of those
few last retpolines too. If felt as if the large retpoline regression
wasn't linear with the number of retpolines executed for each vmexit,
and that it was more linear with the percentage of vmexits that hit on
any number of retpolines. So while they're not part of the monolithic
conversion I assumed they're required to run any meaningful benchmark.

I can drop 15 16 17 from further submits of course, after clarifying
benchmark should be only run on the v1 full set I posted earlier, or
they wouldn't be meaningful.

Thanks,
Andrea