On Thu, Oct 27, 2022, Vitaly Kuznetsov wrote: > Sean Christopherson <seanjc@xxxxxxxxxx> writes: > > > On Tue, Oct 18, 2022, Vitaly Kuznetsov wrote: > >> When a new feature gets implemented in KVM, EVMCS1_UNSUPPORTED_* defines > >> need to be adjusted to avoid the situation when the feature is exposed > >> to the guest but there's no corresponding eVMCS field[s] for it. This > >> is not obvious and fragile. > > > > Eh, either way is fragile, the only difference is what goes wrong when it breaks. > > > > At the risk of making this overly verbose, what about requiring developers to > > explicitly define whether or not a new control is support? E.g. keep the > > EVMCS1_UNSUPPORTED_* and then add compile-time assertions to verify that every > > feature that is REQUIRED | OPTIONAL is SUPPORTED | UNSUPPORTED. > > > > That way the eVMCS "supported" controls don't need to include the ALWAYSON > > controls, and anytime someone adds a new control, they'll have to stop and think > > about eVMCS. > > Is this a good thing or a bad one? :-) I'm not against being extra > verbose but adding a new feature to EVMCS1_SUPPORTED_* (even when there > is a corresponding field) requires testing or a > evmcs_has_perf_global_ctrl()-like story may happen and such testing > would require access to Windows/Hyper-V images. This sounds like an > extra burden for contributors. IMO it's OK if new features are > mechanically added to EVMCS1_UNSUPPORTED_* on the grounds that it > wasn't tested but then it's not much different from "unsupported by > default" (my approach). So I'm on the fence here. Yeah, I was hoping the compile-time asserts would buy us full protection, i.e. I was hoping to avoid the sanitization, but I don't see a way to handle the case where Hyper-V starts advertising a feature that was previously unsupported :-( I'm a-ok going with SUPPORTED only, I'm on the fence too.
