svm->next_rip is only used by skip_emulated_instruction() and in case
kvm_set_msr() fails we rightfully don't do that. Move svm->next_rip
advancement to 'else' branch to avoid creating false impression that
it's always advanced.
By the way, rdmsr_interception() has it right already.
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
arch/x86/kvm/svm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 5b2ea34bc9f2..982c6b9bfc90 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4430,13 +4430,13 @@ static int wrmsr_interception(struct vcpu_svm *svm)
msr.index = ecx;
msr.host_initiated = false;
- svm->next_rip = kvm_rip_read(&svm->vcpu) + 2;
if (kvm_set_msr(&svm->vcpu, &msr)) {
trace_kvm_msr_write_ex(ecx, data);
kvm_inject_gp(&svm->vcpu, 0);
return 1;
} else {
trace_kvm_msr_write(ecx, data);
+ svm->next_rip = kvm_rip_read(&svm->vcpu) + 2;
return kvm_skip_emulated_instruction(&svm->vcpu);
}
}
--
2.20.1
On Thu, Jun 20, 2019 at 4:02 AM Vitaly Kuznetsov <[email protected]> wrote:
>
> svm->next_rip is only used by skip_emulated_instruction() and in case
> kvm_set_msr() fails we rightfully don't do that. Move svm->next_rip
> advancement to 'else' branch to avoid creating false impression that
> it's always advanced.
>
> By the way, rdmsr_interception() has it right already.
I think I actually prefer the current placement, because this allows
the code that's common to both kvm-amd.ko and kvm-intel.ko to be
hoisted into the vendor-agnostic kvm module. Also, this hard-coded '2'
should be going away, right?
Jim Mattson <[email protected]> writes:
> On Thu, Jun 20, 2019 at 4:02 AM Vitaly Kuznetsov <[email protected]> wrote:
>>
>> svm->next_rip is only used by skip_emulated_instruction() and in case
>> kvm_set_msr() fails we rightfully don't do that. Move svm->next_rip
>> advancement to 'else' branch to avoid creating false impression that
>> it's always advanced.
>>
>> By the way, rdmsr_interception() has it right already.
>
> I think I actually prefer the current placement, because this allows
> the code that's common to both kvm-amd.ko and kvm-intel.ko to be
> hoisted into the vendor-agnostic kvm module. Also, this hard-coded '2'
> should be going away, right?
This whole change goes away in PATCH5 (with hardcoded '+2'), I added
this patch just to make it clear that RIP advancement we're doing here
is only being used by kvm_skip_emulated_instruction() and
kvm_inject_gp() branch is not affected.
We can throw this patch away from the series.
--
Vitaly