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)
Attachment:
signature.asc
Description: This is a digitally signed message part
