On Mon, Jun 21, 2021, Paolo Bonzini wrote:
> On 18/06/21 23:46, Sean Christopherson wrote:
> > Calculate the max VMCS index for vmcs12 by walking the array to find the
> > actual max index. Hardcoding the index is prone to bitrot, and the
> > calculation is only done on KVM bringup (albeit on every CPU, but there
> > aren't _that_ many null entries in the array).
> >
> > Fixes: 3c0f99366e34 ("KVM: nVMX: Add a TSC multiplier field in VMCS12")
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > ---
> >
> > Note, the vmx test in kvm-unit-tests will still fail using stock QEMU,
> > as QEMU also hardcodes and overwrites the MSR. The test passes if I
> > hack KVM to ignore userspace (it was easier than rebuilding QEMU).
>
> Queued, thanks. Without having checked the kvm-unit-tests sources very
> thoroughly, this might be a configuration issue in kvm-unit-tests; in theory
> "-cpu host" (unlike "-cpu host,migratable=no") should not enable TSC
> scaling.
As noted in the code comments, KVM allows VMREAD/VMWRITE to all defined fields,
whether or not the field should actually exist for the vCPU model doesn't enter
into the equation. That's technically wrong as there are a number of fields
that the SDM explicitly states exist iff a certain feature is supported. To fix
that we'd need to add a "feature flag" to vmcs_field_to_offset_table that is
checked against the vCPU model, though updating the MSR would probably fall onto
userspace's shoulders?
And FWIW, this is the QEMU code:
#define VMCS12_MAX_FIELD_INDEX (0x17)
static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
{
...
/*
* Just to be safe, write these with constant values. The CRn_FIXED1
* MSRs are generated by KVM based on the vCPU's CPUID.
*/
kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR0_FIXED0,
CR0_PE_MASK | CR0_PG_MASK | CR0_NE_MASK);
kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR4_FIXED0,
CR4_VMXE_MASK);
kvm_msr_entry_add(cpu, MSR_IA32_VMX_VMCS_ENUM,
VMCS12_MAX_FIELD_INDEX << 1);
}
