RE: [PATCH] KVM: VMX: Disable Intel PT before VM-entry

Kang, Luwei
2020-03-19T06:13:19Z

> > If the logical processor is operating with Intel PT enabled (
> > IA32_RTIT_CTL.TraceEn = 1) at the time of VM entry, the “load
> > IA32_RTIT_CTL” VM-entry control must be 0(SDM 26.2.1.1).
> >
> > The first disabled the host Intel PT(Clear TraceEn) will make all the
> > buffered packets are flushed out of the processor and it may cause an
> > Intel PT PMI. The host Intel PT will be re-enabled in the host Intel
> > PT PMI handler.
> >
> > handle_pmi_common()
> > -> intel_pt_interrupt()
> > -> pt_config_start()
>
> IIUC, this is only possible when PT "plays nice" with VMX, correct?
> Otherwise pt->vmx_on will be true and pt_config_start() would skip the
> WRMSR.

Yes. The pt_config_start() would skip the WRMSR if the HW doesn't support enable PT across the VMX.

>
> And IPT PMI must be delivered via NMI (though maybe they're always
> delivered via NMI?).

Yes, IPT PMI is an NMI in the host.

>
> In any case, redoing WRMSR doesn't seem safe, and it certainly isn't
> performant, e.g. what prevents the second WRMSR from triggering a second
> IPT PMI?

Twice is enough. There is probably a lot of PT trace not flush out and a PMI will happen after the first WRMSR. There are not so much data between the end of the PMI handler and the second WRMSR.

>
> pt_guest_enter() is called after the switch to the vCPU has already been
> recorded, can't this be handled in the IPT code, e.g. something like this?
>
> diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c index
> 1db7a51d9792..e38ddae9f0d1 100644
> --- a/arch/x86/events/intel/pt.c
> +++ b/arch/x86/events/intel/pt.c
> @@ -405,7 +405,7 @@ static void pt_config_start(struct perf_event *event)
> ctl |= RTIT_CTL_TRACEEN;
> if (READ_ONCE(pt->vmx_on))
> perf_aux_output_flag(&pt->handle, PERF_AUX_FLAG_PARTIAL);
> - else
> + else (!(current->flags & PF_VCPU))
> wrmsrl(MSR_IA32_RTIT_CTL, ctl);
>
> WRITE_ONCE(event->hw.config, ctl);

Thanks. But I am afraid the host perf don’t like any virtualization specific changes. I will try this.

Luwei Kang

>
> > This patch will disable the Intel PT twice to make sure the Intel PT
> > is disabled before VM-Entry.
> >
> > Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> > ---
> > arch/x86/kvm/vmx/vmx.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index
> > 26f8f31..d936a91 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -1095,6 +1095,8 @@ static inline void pt_save_msr(struct pt_ctx
> > *ctx, u32 addr_range)
> >
> > static void pt_guest_enter(struct vcpu_vmx *vmx) {
> > + u64 rtit_ctl;
> > +
> > if (pt_mode == PT_MODE_SYSTEM)
> > return;
> >
> > @@ -1103,8 +1105,14 @@ static void pt_guest_enter(struct vcpu_vmx *vmx)
> > * Save host state before VM entry.
> > */
> > rdmsrl(MSR_IA32_RTIT_CTL, vmx->pt_desc.host.ctl);
> > - if (vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) {
> > + if (vmx->pt_desc.host.ctl & RTIT_CTL_TRACEEN) {
> > wrmsrl(MSR_IA32_RTIT_CTL, 0);
> > + rdmsrl(MSR_IA32_RTIT_CTL, rtit_ctl);
> > + if (rtit_ctl)
> > + wrmsrl(MSR_IA32_RTIT_CTL, 0);
> > + }
> > +
> > + if (vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) {
> > pt_save_msr(&vmx->pt_desc.host, vmx-
> >pt_desc.addr_range);
> > pt_load_msr(&vmx->pt_desc.guest, vmx-
> >pt_desc.addr_range);
> > }
> > --
> > 1.8.3.1
> >