RE: [PATCH v1 06/23] KVM: VMX: Defer enabling FRED MSRs save/load until after set CPUID

Li, Xin3
2023-11-14T02:50:55Z

> > > Clearing VM_EXIT_ACTIVATE_SECONDARY_CONTROLS may be problematic when
> > > new bits are added to secondary vmcs controls. Why not keep
> > > VM_EXIT_ACTIVATE_SECONDARY_CONTROLS always on if it is supported? or
> > > you see any perf impact?
> >
> > I think it from the other way, why keeps hw loading it on every
> > vmentry even if it's not used by a guest?
>
> Oh, yeesh, this is clearing the activation control. Yeah, NAK to that, just leave it
> set. If it weren't for the fact that there is apparently a metrict ton of FRED state (I
> thought the whole point of FRED was to kill off legacy crap like
> CPL1 and CPL2 stacks?) _and_ that KVM needs to toggle MSR intercepts, I'd
> probably push back on toggling even the FRED controls.

To me, FRED is to _architecturally_ do the right thing for x86 event handling.

I don't think FRED's major purpose is to kill legacy craps, but longer term
it paves the way for that.

Yeah, I would like to discuss whether to toggle FRED controls.

>
> > Different CPUs may implement it in different ways, which we can't assume.
>
> Implement what in a different way? The VMCS fields and FRED are architectural.
> The internal layout of the VMCS is uarch specific, but the encodings and semantics
> absolutely cannot change without breaking software. And if Intel does something
> asinine like make a control active-low then we have far, far bigger problems.

I should have made it clear that I wasn't talking at the ISA level. And
of course CPU uarch implementations should be transparent to software.

I mean a CPU uarch could choose to check the activation bit in the VM exit
controls first and then decide whether to load the 2nd VM exit controls.
While if resources allow, a CPU uarch could always load the 2nd VM exit
controls.

BTW, I believe the active-low controls are really gone with new features.
All new controls are all 0s by default.

> > Other features needing it should set it separately, say with a refcount.
>
> Absolutely not. Unless Intel screwed up the implementation, the cost of leaving
> VM_EXIT_ACTIVATE_SECONDARY_CONTROLS set when it's supported shouldn't
> even be measurable.

I do hope so. However, I don't know whether this is guaranteed or not on
all uarch implementations.

A decision to leave it set is good enough for now.

Thanks!
Xin