From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCHv1 dont apply] RFC: kvm eoi PV using shared memory Date: Mon, 16 Apr 2012 20:37:37 +0300 Message-ID: <20120416173737.GA5541@redhat.com> References: <20120410132756.GA14101@redhat.com> <20120415161857.GA8710@redhat.com> <20120416100807.GO11918@redhat.com> <20120416110919.GA11605@redhat.com> <20120416112446.GQ11918@redhat.com> <20120416121824.GD11605@redhat.com> <20120416123047.GS11918@redhat.com> <20120416131328.GF11605@redhat.com> <20120416151011.GB18613@redhat.com> <20120416172414.GA19544@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org To: "Michael S. Tsirkin" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:21915 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752699Ab2DPSeh (ORCPT ); Mon, 16 Apr 2012 14:34:37 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q3GIYXh8010765 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 16 Apr 2012 14:34:37 -0400 Content-Disposition: inline In-Reply-To: <20120416172414.GA19544@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Apr 16, 2012 at 08:24:16PM +0300, Michael S. Tsirkin wrote: > On Mon, Apr 16, 2012 at 06:10:11PM +0300, Gleb Natapov wrote: > > > > lapic changes should be minimal. > > > > > > Exactly my motivation. > > > > > My patch removes 13 lines more :) > > Haven't checked whether your patch is correct yet > but I see it's checking the eoi register on each exit. > Only if eoi.pending == true on exit. > I think it's clear this would make code a bit shorter > (not necessarily clearer as we are relying on ) but > as I said even though the extra overhead is likely > negligeable I have a feeling it's a wrong approach since > this won't scale as we add more features. > > Let's see what do others think. > What I do not like about not calling eoi here is that we have to reason about all the paths into apic and check that we clear isr on all of them. And that is not all. eoi handler set event request, is it OK to skip it? May be, or may be not. > > > I also find the logic easier to follow as is - > > > it is contained in lapic.c without relying > > > on being called from x86.c as just the right moment. > > > > > See the patch. It change nothing outside of lapic.c. > > True but you rely on x86.c to call kvm_sync_lapic_from_vapic > at the right time before injecting interrupts. Not before injecting interrupts, but on vmexit. > I haven't checked whether that is always the case but > to me, this makes the code less clear and more fragile. > We call a lot of apic functions from x86.c. That's were interrupt injection happens. > Again, it appears to be a matter of taste. > > -- > MST -- Gleb.