Linux Virtualization - Jun 2008 - [PATCH] Do not calculate linear rip in emulation failure report

If we're not gonna do anything (case in which failure is already
reported), we do not need to even bother with calculating the linear rip.

This is a nitpick, but I saw it while doing some testing, so here's
the patch.

Signed-off-by: Glauber Costa <gcosta at redhat.com>
---
 arch/x86/kvm/x86.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 77fb2bd..191fef1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2026,11 +2026,11 @@ void kvm_report_emulation_failure(struct kvm_vcpu *vcpu,
const char *context)
 	unsigned long rip = vcpu->arch.rip;
 	unsigned long rip_linear;
 
-	rip_linear = rip + get_segment_base(vcpu, VCPU_SREG_CS);
-
 	if (reported)
 		return;
 
+	rip_linear = rip + get_segment_base(vcpu, VCPU_SREG_CS);
+
 	emulator_read_std(rip_linear, (void *)opcodes, 4, vcpu);
 
 	printk(KERN_ERR "emulation failed (%s) rip %lx %02x %02x %02x
%02x\n",
-- 
1.5.4.5

Glauber Costa wrote:
> If we're not gonna do anything (case in which failure is already
> reported), we do not need to even bother with calculating the linear rip.
>
> This is a nitpick, but I saw it while doing some testing, so here's
> the patch.
>
>   
Applied, thanks.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

On Tue, Jun 10, 2008 at 4:46 PM, Glauber Costa <gcosta at redhat.com>
wrote:
> If we're not gonna do anything (case in which failure is already
> reported), we do not need to even bother with calculating the linear rip.
>
> This is a nitpick, but I saw it while doing some testing, so here's
> the patch.
>
> Signed-off-by: Glauber Costa <gcosta at redhat.com>
> ---
>  arch/x86/kvm/x86.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 77fb2bd..191fef1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2026,11 +2026,11 @@ void kvm_report_emulation_failure(struct kvm_vcpu
*vcpu, const char *context)
>        unsigned long rip = vcpu->arch.rip;
>        unsigned long rip_linear;
>
> -       rip_linear = rip + get_segment_base(vcpu, VCPU_SREG_CS);
> -
>        if (reported)
>                return;
>
> +       rip_linear = rip + get_segment_base(vcpu, VCPU_SREG_CS);
> +
>        emulator_read_std(rip_linear, (void *)opcodes, 4, vcpu);
>
>        printk(KERN_ERR "emulation failed (%s) rip %lx %02x %02x %02x
%02x\n",
> --
> 1.5.4.5
Why return immediately? Shouldn't we report on failure whenever it
occurs (i.e. by rather removing the if condition)?