From mboxrd@z Thu Jan 1 00:00:00 1970 From: "tiejun.chen" Subject: Re: [v1][PATCH 1/1] KVM: PPC: disable preemption when using hard_irq_disable() Date: Fri, 12 Jul 2013 10:13:39 +0800 Message-ID: <51DF6653.7010902@windriver.com> References: <1373436139-27998-1-git-send-email-tiejun.chen@windriver.com> <62E2724C-EC17-4E36-AC9E-C9FFEDF5C5B7@suse.de> <51DE1CE9.7060406@windriver.com> <1373545684.19894.80.camel@pasglop> <3B36D92F-CD11-49A0-A0F5-CD3E49BD6793@suse.de> <1373547293.19894.99.camel@pasglop> <1373588345.19894.126.camel@pasglop> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: Alexander Graf , "" , " list" To: Benjamin Herrenschmidt Return-path: In-Reply-To: <1373588345.19894.126.camel@pasglop> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 07/12/2013 08:19 AM, Benjamin Herrenschmidt wrote: > On Thu, 2013-07-11 at 15:07 +0200, Alexander Graf wrote: >> Ok, let me quickly explain the problem. >> >> We are leaving host context, switching slowly into guest context. >> During that transition we call get_paca() indirectly (apparently by >> another call to hard_disable() which sounds bogus, but that's another >> story). >> >> get_paca() warns when we're preemptible. We're only not preemptible >> when either preempt is disabled or irqs are disabled. Irqs are >> disabled, but arch_irqs_disabled() doesn't know, because it only >> checks for soft disabled IRQs. >> >> So we can fix this either by setting IRQs as soft disabled as well or >> by disabling preemption until we enter the guest for real. Any >> preferences? > > Well, if you hard disable first (ie, direct transition from full enabled > to hard disabled), you know you have nothing lazy pending in > irq_pending, then it's ok to mess around with local_paca->soft_enabled > to make it "look disabled". > > IE. Call hard_irq_disable(), then only turn local_paca->soft_enabled > back on late in the process, some time before the final rfi(d). > > That works as long as you had a transition from full enabled to full > disabled and don't hard re-enable in the process. IE, You are certain > that there is nothing pending in irq_happened. > > HOWEVER ! > > If you do that, you *ALSO* need to clear irq_happened. You must *NEVER* > leave PACA_IRQ_HARD_DIS set in irq_happened if you are soft-enabled, and > since the above means that you *will* be seen as soft enabled on the way > out of the guest, you can kaboom. > > BTW. I'm fine with a patch that does: > > #define hard_irq_disable() do { \ > u8 _was_enabled = get_paca()->soft_enabled; \ Current problem I met is issued from the above line. > __hard_irq_disable(); \ > - get_paca()->soft_enabled = 0; \ Not here. If I'm misunderstanding what you guys means, please correct me since this is a long discussion thread. I have to reread that carefully. Tiejun > + local_paca->soft_enabled = 0; \ > > In fact we should probably do it anyway. > > Cheers, > Ben. > > > >