On Fri, Jun 13, 2008 at 2:33 PM, Mohammed Gamal <[EMAIL PROTECTED]> wrote:
> On Tue, Jun 10, 2008 at 4:46 PM, Glauber Costa <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
>> ---
>>  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)?
>
the kernel ring buffer would be full very quickly, and this is not a
strong enough reason to do that ;-)
Most of the messages would be the same.

What I do think would be useful, and even planned to do (just have
more priority things waiting), is to move
the checking field to the vcpu struct. With the current setup, if you
kill your guest, set things up again, and run it,
you won't see any messages, even if it fails.

So the warning should really be once in the lifetime of the virtual
machine, not once in the lifetime of the kvm module.



-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Reply via email to