> Thus it would be good to
> have kvm-unit-tests for all what is checked here.
I'll try to implement unit tests a bit later. The fixed patch based on
comments and critics from this thread is following.
> Indeed. Better have function than accepts the field index and that has
> some translation table to derive the name for printing the message.
Is having translation table for such simple case an overkill? Maybe
printing VMCS field indexes would be enough?
> Is white-listing a safe approach for this? Wouldn't it be better to
> save/restore only a known set of MSRs, possibly adding an option to
> ignore others (with a warning) instead of returning an error (similar to
> what we do when L1 tries to write to an unknown MSR)?
MSRs are written via kvm_set_msr() path and MSR set restricting is done there.
If ignored or unhandled MSR attempted to write then whole MSR loading
transaction fails.
This check (vmx_msr_switch_is_protected_msr) adds more strict check on
u-code-related MSRs (this model-specified checks are specified in SDM
26.4, 27.4 and 35). kvm_set_msr() simply ignores write attempts to
these MSRs (MSR_IA32_UCODE_WRITE,
MSR_IA32_UCODE_REV)) and returns 0 (success). Instead we should fail
here according to SDM.
> Same comment as above.
kvm_{get/set}_msr() are safe, aren't they?
>>> + msr = nested_vmx_load_msrs(vcpu, vmcs12->vm_entry_msr_load_count,
>>> + vmcs12->vm_entry_msr_load_addr);
>>> + if (msr)
>>> + nested_vmx_entry_failure(vcpu, vmcs12,
>>> + EXIT_REASON_MSR_LOAD_FAILURE, msr);
>
> Don't you have to terminate the nested run attempt here as well -> return 1?
Fixed.
--
Eugene
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
