Re: [PATCH v1 7/7] x86/vmx: switch IPT MSRs on vmentry/vmexit

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jun 18, 2020 at 01:07:33PM +0200, Michał Leszczyński wrote:
> ----- 18 cze 2020 o 10:52, Roger Pau Monné roger.pau@xxxxxxxxxx napisał(a):
> 
> > On Wed, Jun 17, 2020 at 08:56:57PM +0200, Michał Leszczyński wrote:
> >> ----- 17 cze 2020 o 17:14, Andrew Cooper andrew.cooper3@xxxxxxxxxx 
> >> napisał(a):
> >> 
> >> > On 17/06/2020 13:51, Roger Pau Monné wrote:
> >> >> On Wed, Jun 17, 2020 at 01:54:45PM +0200, Michał Leszczyński wrote:
> >> >>> ----- 17 cze 2020 o 11:09, Roger Pau Monné roger.pau@xxxxxxxxxx 
> >> >>> napisał(a):
> >> >>>
> >> >>>> 24 Virtual Machine Control Structures -> 24.8 VM-entry Control Fields 
> >> >>>> -> 24.8.1
> >> >>>> VM-Entry Controls
> >> >>>> Software should consult the VMX capability MSRs IA32_VMX_ENTRY_CTLS 
> >> >>>> to determine
> >> >>>> how it should set the reserved bits.
> >> >>> Please look at bit position 18 "Load IA32_RTIT_CTL".
> >> >> I think this is something different from what I was referring to.
> >> >> Those options you refer to (load/clear IA32_RTIT_CTL) deal with
> >> >> loading/storing a specific field on the vmcs that maps to the guest
> >> >> IA32_RTIT_CTL.
> >> >>
> >> >> OTOH MSR load lists can be used to load and store any arbitrary MSR on
> >> >> vmentry/vmexit, see section 26.4 LOADING MSRS on the SDM. There's
> >> >> already infrastructure on Xen to do so, see vmx_{add/del/find}_msr.
> >> > 
> >> > If I remember the historic roadmaps correctly, there are 3 cases.
> >> > 
> >> > The first hardware to support PT (Broadwell?) prohibited its use
> >> > completely in VMX operations.  In this case, we can use it to trace PV
> >> > guests iff we don't enable VMX in hardware to begin with.
> >> > 
> >> > This was relaxed in later hardware (Skylake?) to permit use within VMX
> >> > operations, but without any help in the VMCS.  (i.e. manual context
> >> > switching per this patch, or MSR load lists as noted in the SDM.)
> >> > 
> >> > Subsequent support for "virtualised PT" was added (IceLake?) which adds
> >> > the load/save controls, and the ability to translate the output buffer
> >> > under EPT.
> >> > 
> >> > 
> >> > All of this is from memory so I'm quite possibly wrong with details, but
> >> > I believe this is why the current complexity exists.
> >> > 
> >> > ~Andrew
> >> 
> >> 
> >> I've managed to toggle MSR_IA32_RTIT_CTL values using MSR load lists, as 
> >> in:
> >> 
> >> > 35.5.2.2 Guest-Only Tracing
> >> > "For this usage, VM-entry is programmed to enable trace packet 
> >> > generation, while
> >> > VM-exit is programmed to clear MSR_IA32_RTIT_CTL.TraceEn so as to disable
> >> > trace-packet generation in the host."
> >> 
> >> it actually helped a bit. With patch v1 there were parts of hypervisor 
> >> recorded
> >> in the trace (i.e. the moment between TRACE_EN being set and actual 
> >> vmenter,
> >> and the moment between vmexit and TRACE_EN being unset). Using MSR load 
> >> list
> >> this was eliminated. This change will be reflected in patch v2.
> >> 
> >> 
> >> I can't however implement any working scenario in which all these MSRs are
> >> managed using MSR load lists. As in "35.3.3 Flushing Trace Output": 
> >> packets are
> >> buffered internally and are flushed only when TRACE_EN bit in 
> >> MSR_IA32_RTIT_CTL
> >> is set to 0. The values of remaining registers will be stable after 
> >> everything
> >> is serialized. I think this is too complex for the load lists alone. I 
> >> belive
> >> that currently SDM instructs to use load lists only for toggling this 
> >> single
> >> bit on-or-off.
> > 
> > I think that's exactly what we want: handling TraceEn at
> > vmentry/vmexit, so that no hypervisor packets are recorded. The rest
> > of the MSRs can be handled in VMM mode without issues. Switching those
> > on every vmentry/vmexit would also add more overhead that needed,
> > since I assume they don't need to be modified on every entry/exit?
> 
> 
> Assuming that there is a single DomU per pcpu and they are never migrated 
> between pcpus then you never need to modify the remaining MSRs.
> 
> In case DomUs are floating or there are multiple DomUs per pcpu, we need to 
> read out a few MSRs on vm-exit and restore them on vm-entry. Right now I'm 
> always using this approach as I'm pretty not sure how to optimize it without 
> introducing additional bugs. I will show the implementation in patch v2.

I think you might likely only need to modify the MSRs when doing
context switches of domains, instead of doing it on every
vmentry/vmexit?

Roger.