linux-kernel - Re: [PATCH] Move VMEnter and VMExit tracepoints closer to the actual event

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: <5e6ad92a72e139877fa0e7a1d77682a075060d16.camel@suse.com>
Date:   Fri, 21 May 2021 09:51:14 +0200
From:   Dario Faggioli <dfaggioli@...e.com>
To:     Sean Christopherson <seanjc@...gle.com>,
        Stefano De Venuto <stefano.devenuto99@...il.com>
Cc:     linux-kernel@...r.kernel.org, pbonzini@...hat.com,
        vkuznets@...hat.com, wanpengli@...cent.com, jmattson@...gle.com,
        x86@...nel.org, hpa@...or.com, kvm@...r.kernel.org,
        rostedt@...dmis.org, y.karadz@...il.com
Subject: Re: [PATCH] Move VMEnter and VMExit tracepoints closer to the
 actual event

Hi Sean,

On Thu, 2021-05-20 at 15:32 +0000, Sean Christopherson wrote:
> On Wed, May 19, 2021, Stefano De Venuto wrote:
> > 
> > This patch moves the tracepoints closer to the events, for both
> > Intel
> > and AMD, so that a combined host-guest trace will offer a more
> > realistic representation of what is really happening, as shown
> > here:
> > 
> >            trace.dat:  CPU 0/KVM-2553  [000]  2.190290: write_msr:
> > 48, value 0
> 
> I'm not sure this is a good thing, as it's not clear to me that
> invoking tracing
> with the guest's SPEC_CTRL loaded is desirable.  Maybe it's a non-
> issue, but it
> should be explicitly called out and discussed.
> 
Oh, this is actually a very good point. Considering the rest of the
review comments, it looks like we're not putting the tracepoint past
the SPEC_CTRL msr-write, not for now at least. :-) But I agree that it
must be explicitly considered and thought about, if/when trying to do
it again.

> And to some extent, the current behavior is _more_ accurate because
> it shows that
> KVM started its VM-Enter sequence and then the WRMSR occured as part
> of that
> sequence.  It is writing the guest's value after all.  Ditto for
> XCR0, XSS, PKRU,
> Intel PT, etc...
> 
Yaah, I guess it comes to what we want/assume the meaning of the
VMEnter/Exit tracepoints to be. I.e., is it the beginning of the
sequence of operations necessary to enter a guest, or the exact point
in time where we switch to it (and vice-versa, for exit)?

In my view and in my experience of trying to trace host and guest at
the same time, I find the latter more useful, but I appreciate that the
former is valid too especially considering that, as you say, some
operations are altering the guest's state already.

> A more concrete example would be perf; on VMX, if a perf NMI happens
> after KVM
> invokes atomic_switch_perf_msrs() then I absolutely want to see that
> reflected
> in the trace, e.g. to help debug the PEBS mess[*].  If the VM-Enter
> tracepoint
> is moved closer to VM-Enter, that may or may not hold true depending
> on where the
> NMI lands.
> 
> On VMX, I think the tracepoint can be moved below the VMWRITEs
> without much
> contention (though doing so is likely a nop), but moving it below
> kvm_load_guest_xsave_state() requires a bit more discussion.
> 
Ok, well, IMO closer is better, even if no past XSAVE state handling.
:-) Let us look into this a little bit.

> I 100% agree that the current behavior can be a bit confusing, but I
> wonder if
> we'd be better off "solving" that problem through documentation.
> 
Indeed. So, do you happen to have in mind what could be the best place
and the best way for documenting this?

Thanks and Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)