From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH] kvm/ppc/booke64: Hard disable interrupts when entering the guest Date: Mon, 6 May 2013 18:53:38 -0500 Message-ID: <1367884418.3398.10@snotra> References: <1367787788.11982.58.camel@pasglop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="Flowed"; DelSp="Yes" Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Alexander Graf , kvm-ppc@vger.kernel.org, Mihai Caraman , Paul Mackerras , linuxppc-dev@lists.ozlabs.org To: Benjamin Herrenschmidt Return-path: In-Reply-To: <1367787788.11982.58.camel@pasglop> (from benh@kernel.crashing.org on Sun May 5 16:03:08 2013) Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" List-Id: kvm.vger.kernel.org On 05/05/2013 04:03:08 PM, Benjamin Herrenschmidt wrote: > On Fri, 2013-05-03 at 18:45 -0500, Scott Wood wrote: > > kvmppc_lazy_ee_enable() was causing interrupts to be soft-enabled > > (albeit hard-disabled) in kvmppc_restart_interrupt(). This led to > > warnings, and possibly breakage if the interrupt state was later > saved > > and then restored (leading to interrupts being hard-and-soft enabled > > when they should be at least soft-disabled). > > > > Simply removing kvmppc_lazy_ee_enable() leaves interrupts only > > soft-disabled when we enter the guest, but they will be > hard-disabled > > when we exit the guest -- without PACA_IRQ_HARD_DIS ever being set, > so > > the local_irq_enable() fails to hard-enable. > > > > While we could just set PACA_IRQ_HARD_DIS after an exit to > compensate, > > instead hard-disable interrupts before entering the guest. This > way, > > we won't have to worry about interactions if we take an interrupt > > during the guest entry code. While I don't see any obvious > > interactions, it could change in the future (e.g. it would be bad if > > the non-hv code were used on 64-bit or if 32-bit guest lazy > interrupt > > disabling, since the non-hv code changes IVPR among other things). > > Shouldn't the interrupts be marked soft-enabled (even if hard > disabled) > when entering the guest ? > > Ie. The last stage of entry will hard enable, so they should be > soft-enabled too... if not, latency trackers will consider the whole > guest periods as "interrupt disabled"... OK... I guess we already have that problem on 32-bit as well? > Now, kvmppc_lazy_ee_enable() seems to be clearly bogus to me. It will > unconditionally set soft_enabled and clear irq_happened from a > soft-disabled state, thus potentially losing a pending event. > > Book3S "HV" seems to be keeping interrupts fully enabled all the way > until the asm hard disables, which would be fine except that I'm > worried > we are racy vs. need_resched & signals. > > One thing you may be able to do is call prep_irq_for_idle(). This will > tell you if something happened, giving you a chance to abort/re-enable > before you go the guest. As long as we go straight from IRQs fully enabled to hard-disabled, before we check for signals and such, I don't think we need that (and using it would raise the question of what to do on 32-bit). What if we just take this patch, and add trace_hardirqs_on() just before entering the guest? This would be similar to what the 32-bit non-KVM exception return code does (except it would be in C code). Perhaps we could set soft_enabled as well, but then we'd have to clear it again before calling kvmppc_restart_interrupt() -- since the KVM exception handlers don't actually care about soft_enabled (it would just be for consistency), I'd rather just leave soft_enabled off. We also don't want PACA_IRQ_HARD_DIS to be cleared the way prep_irq_for_idle() does, because that's what lets the local_irq_enable() do the hard-enabling after we exit the guest. -Scott