[PATCH 06/22] KVM: x86: Get RIP from vCPU state when storing it to last_retry_eip

Sean Christopherson posted 22 patches 1 year, 6 months ago
There is a newer version of this series
[PATCH 06/22] KVM: x86: Get RIP from vCPU state when storing it to last_retry_eip
Posted by Sean Christopherson 1 year, 6 months ago
Read RIP from vCPU state instead of pulling it from the emulation context
when filling last_retry_eip, which is part of the anti-infinite-loop
protection used when unprotecting and retrying instructions that hit a
write-protected gfn.

This will allow reusing the anti-infinite-loop protection in flows that
never make it into the emulator.

This is a glorified nop as ctxt->eip is set to kvm_rip_read() in
init_emulate_ctxt(), and EMULTYPE_PF emulation is mutually exclusive with
EMULTYPE_NO_DECODE and EMULTYPE_SKIP, i.e. always goes through
x86_decode_emulated_instruction() and hasn't advanced ctxt->eip (yet).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2072cceac68f..372ed3842732 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8973,7 +8973,7 @@ static bool retry_instruction(struct x86_emulate_ctxt *ctxt,
 	if (!kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(gpa)))
 		return false;
 
-	vcpu->arch.last_retry_eip = ctxt->eip;
+	vcpu->arch.last_retry_eip = kvm_rip_read(vcpu);
 	vcpu->arch.last_retry_addr = cr2_or_gpa;
 	return true;
 }
-- 
2.46.0.76.ge559c4bf1a-goog
Re: [PATCH 06/22] KVM: x86: Get RIP from vCPU state when storing it to last_retry_eip
Posted by Paolo Bonzini 1 year, 5 months ago
On 8/9/24 21:03, Sean Christopherson wrote:
> Read RIP from vCPU state instead of pulling it from the emulation context
> when filling last_retry_eip, which is part of the anti-infinite-loop
> protection used when unprotecting and retrying instructions that hit a
> write-protected gfn.
> 
> This will allow reusing the anti-infinite-loop protection in flows that
> never make it into the emulator.
> 
> This is a glorified nop as ctxt->eip is set to kvm_rip_read() in
> init_emulate_ctxt(), and EMULTYPE_PF emulation is mutually exclusive with
> EMULTYPE_NO_DECODE and EMULTYPE_SKIP, i.e. always goes through
> x86_decode_emulated_instruction() and hasn't advanced ctxt->eip (yet).

This is as much a nit as it can be, but "glorified nop" would be 
interpreted more as "the assignment is not needed at all", or something 
similarly wrong.  Just "This has no functional change because..." will do.

Paolo

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/x86.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2072cceac68f..372ed3842732 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8973,7 +8973,7 @@ static bool retry_instruction(struct x86_emulate_ctxt *ctxt,
>   	if (!kvm_mmu_unprotect_page(vcpu->kvm, gpa_to_gfn(gpa)))
>   		return false;
>   
> -	vcpu->arch.last_retry_eip = ctxt->eip;
> +	vcpu->arch.last_retry_eip = kvm_rip_read(vcpu);
>   	vcpu->arch.last_retry_addr = cr2_or_gpa;
>   	return true;
>   }