From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH]: Fix memory corruption in-kernel IOAPIC emulation Date: Wed, 30 Jan 2008 18:54:34 +0200 Message-ID: <47A0ABCA.4020206@qumranet.com> References: <479FB5C6.6060204@redhat.com> <47A04BB3.7020302@qumranet.com> <47A0A830.8040900@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Chris Lalancette Return-path: In-Reply-To: <47A0A830.8040900-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Chris Lalancette wrote: > Avi Kivity wrote: > >> Excellent catch, but the fix is wrong. Instead of partially restoring >> the ioapic state in the kernel, you should fully save it in qemu. >> >> This is a trap that many fall into: considering kvm and qemu as one >> entity and making sure they work well together. We need to make sure >> that kvm and libkvm are useful for other userspace programs as well. >> >> > > Actually, let me ask a question here. It seems to me that there are two > ways I could go about this: > > 1) Change the save protocol so that it saves the relevant information > (i.e. base_address and irr), and then the restore protocol so it > actually pulls this stuff off the wire. This solution seems more > "right" to me, but it has the downside that we are changing the > over-the-wire information, which will break migration between old and > new versions of qemu. > > qemu supports backwards compatible changes to the protocol via the version field. > 2) Just change the restore protocol so that we properly fill in the > missing fields with real, hard-coded values instead of just random > memory. This has the benefit that it doesn't change the protocol, but > has the downside that we won't reflect changes to base_address or irr > 100% properly. In practice, this doesn't seem like a big deal since > there doesn't currently seem to be a way to change base_address anyway, > and losing the irr doesn't seem to be catastrophic (although I'm not > 100% certain about that). > > The attached patch implements 2); does anyone have an opinion on which > way to go here? > I prefer doing a full save/restore, since we don't know what guests depend on. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/