Jim Mattson <jmattson@xxxxxxxxxx> writes:
> Take the following code in rdmsr_interception, for example.
>
> svm->next_rip = kvm_rip_read(&svm->vcpu) + 2;
>
> Yes, the canonical rdmsr instruction is two bytes. However, there is
> nothing in the architectural specification prohibiting useless or
> redundant prefixes. So, for instance, 65 66 67 67 67 0f 32 is a
> perfectly valid 7-byte rdmsr instruction.
(I don't know much about why this was added but nobody else commented
so in case I'm not terribly mistaken):
This looks ugly, it is likely an over-optimization: we seem to only
advance svm->next_rip to be able to avoid doing
kvm_emulate_instruction() in skip_emulated_instruction(). With NRIP_SAVE
feature (appeared long ago) we don't use the advanced value as we
already know the next RIP:
if (svm->vmcb->control.next_rip != 0) {
WARN_ON_ONCE(!static_cpu_has(X86_FEATURE_NRIPS));
svm->next_rip = svm->vmcb->control.next_rip;
}
IMO, always doing kvm_emulate_instruction(vcpu, EMULTYPE_SKIP) in !NRIPS
case would be the correct way. I tried throwing away these advancements
and nothing broke, with and without NRIPS.
I can try sending a patch removing the manual advancement to see if
anyone has any objections.
--
Vitaly
