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

[Avi Kivity <avi@qumranet.com>]

Glauber Costa wrote:
>>>        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.
>
>
>   

I've changed it to use printk_ratelimit().

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