On Mon, Feb 7, 2022 at 10:06 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
[...]
> > +#define KVM_X86_QUIRK_TWEAK_VMX_CTRL_MSRS (1 << 5)
>
> I'd prefer we include msr_ia32_feature_control_valid_bits in this quirk, it should
> be relatively easy to do since most of the modifications stem from
> vmx_vcpu_after_set_cpuid(). vmx_setup_mce() is a bit odd, but IMO it's worth
> excising as much crud as we can.
>
Sure, this is a good opportunity to rip out the crud.
msr_ia32_feature_control_valid_bits is a bit messy, since the default
value does not contain all the bits we support. At least with
IA32_VM_TRUE_{ENTRY,EXIT}_CTLS we slim down the hardware values to get
the default value.
Not at all objecting, but it looks like we will need to populate some
bits in the default value of the IA32_FEAT_CTL mask, otherwise with
the quirk enabled guests could never set any of the bits in the MSR.
> > #define KVM_STATE_NESTED_FORMAT_VMX 0
> > #define KVM_STATE_NESTED_FORMAT_SVM 1
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 395787b7e7ac..60b1b76782e1 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -7231,6 +7231,9 @@ void nested_vmx_entry_exit_ctls_update(struct kvm_vcpu *vcpu)
> > {
> > struct vcpu_vmx *vmx = to_vmx(vcpu);
> >
> > + if (!kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_TWEAK_VMX_CTRL_MSRS))
> > + return;
>
>
> Probably worth calling out that nested_vmx_cr_fixed1_bits_update() is intentionally
> exempt from this "rule":
Agreed.
--
Thanks,
Oliver
