Re: [PATCH] Do not calculate linear rip in emulation failure report

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 13, 2008 at 2:33 PM, Mohammed Gamal <m.gamal005@xxxxxxxxx> wrote:
> On Tue, Jun 10, 2008 at 4:46 PM, Glauber Costa <gcosta@xxxxxxxxxx> 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@xxxxxxxxxx>
>> ---
>>  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
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization

[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux