Messages in this thread | | | From | "Li, Xin3" <> | Subject | RE: [PATCH v1 06/23] KVM: VMX: Defer enabling FRED MSRs save/load until after set CPUID | Date | Tue, 14 Nov 2023 02:50:49 +0000 |
| |
> > > 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
| |