Re: [PATCH v13 08/12] KVM: x86: Add Intel PT context switch for each vcpu

[Thomas Gleixner]

On Mon, 29 Oct 2018, Paolo Bonzini wrote:
> On 24/10/2018 12:13, Alexander Shishkin wrote:
> > Luwei Kang <luwei.kang@xxxxxxxxx> writes:
> >> +	/*
> >> +	 * Set guest state of MSR_IA32_RTIT_CTL MSR (PT will be disabled
> >> +	 * on VM entry when it has been disabled in guest before).
> >> +	 */
> >> +	vmcs_write64(GUEST_IA32_RTIT_CTL, vmx->pt_desc.guest.ctl);
> >> +
> >> +	if (vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) {
> >> +		wrmsrl(MSR_IA32_RTIT_CTL, 0);
> >> +		pt_save_msr(&vmx->pt_desc.host, vmx->pt_desc.addr_range);
> >> +		pt_load_msr(&vmx->pt_desc.guest, vmx->pt_desc.addr_range);
> >> +	}
> >> +}
> > 
> > From my side this is still a NAK, because [1].
> > 
> > [1] https://marc.info/?l=kvm&m=153847567226248&w=2
> 
> Then you should have replied to
> https://marc.info/?l=kvm&m=153865386015249&w=2 instead of having Luwei
> do the work for nothing.
> 
> Quoting from there:
> 
> >> One shouldn't have to enable or disable anything in KVM to stop it from
> >> breaking one's existing workflow. That makes no sense.
> > 
> > If you "have to enable or disable anything" it means you have to
> > override the default.  But the default in this patches is "no change
> > compared to before the patches", leaving tracing of both host and guest
> > entirely to the host, so I don't understand your remark.  What workflow
> > is broken?
> > 
> >> There already are controls in perf that enable/disable guest tracing.
> > 
> > You are confusing "tracing guest from the host" and "the guest can trace
> > itself".  This patchset is adding support for the latter, and that
> > affects directly whether the tracing CPUID leaf can be added to the
> > guest.  Therefore it's not perf that can decide whether to turn it on;
> > KVM must know it when /dev/kvm is opened, which is why it is a module
> > parameter.
> 
> I'd be happier if we found an agreement, but without discussion that
> just won't happen.

So at least we need a way for perf on the host to programmatically detect,
that 'guest traces itself' is enabled, so it can inject that information
into the host data and post processing can tell that. W/o something like
that it's going to be a FAQ.

Thanks,

	tglx