From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] Do not calculate linear rip in emulation failure report Date: Fri, 13 Jun 2008 22:46:27 +0300 Message-ID: <4852CE93.9020104@qumranet.com> References: <1213105613-8571-1-git-send-email-gcosta@redhat.com> <52d4a3890806131033g56ec2bc0o43442d2c290401f2@mail.gmail.com> <5d6222a80806131037u245c66ev902fadaa6d5c4605@mail.gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Mohammed Gamal , Glauber Costa , kvm@vger.kernel.org, virtualization@lists.linux-foundation.org To: Glauber Costa Return-path: Received: from il.qumranet.com ([212.179.150.194]:26190 "EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750832AbYFMTqc (ORCPT ); Fri, 13 Jun 2008 15:46:32 -0400 In-Reply-To: <5d6222a80806131037u245c66ev902fadaa6d5c4605@mail.gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: 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.