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 11:22:28 +0800 Message-ID: <51DF7674.1070906@windriver.com> References: <1373559480.8183.258@snotra> <1373560585.8183.261@snotra> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: "" , " list" To: Scott Wood , Alexander Graf , Benjamin Herrenschmidt Return-path: In-Reply-To: <1373560585.8183.261@snotra> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 07/12/2013 12:36 AM, Scott Wood wrote: > On 07/11/2013 11:30:41 AM, Alexander Graf wrote: >> >> On 11.07.2013, at 18:18, Scott Wood wrote: >> >> > On 07/11/2013 08:07:30 AM, Alexander Graf wrote: >> >> 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 >> > >> > If we set IRQs as soft-disabled prior to calling hard_irq_disable(), then >> hard_irq_disable() will fail to call trace_hardirqs_off(). >> >> Right... > > Plus we'd have the same problem trying to set soft_enabled to 0. > >> >> Any preferences? >> > >> > Use arch_local_save_flags() in hard_irq_disable() instead of reading >> soft_enabled with C code. >> >> That only operates on the soft_enabled bit. We also need to access irq_happened. > > OK, so we'll need more inline asm. If so, why not to remove directly hard_irq_disable() inside kvmppc_handle_exit() by reverting that commit, "kvm/ppc/booke64: Fix lazy ee handling in kvmppc_handle_exit()"? Then we can use SOFT_DISABLE_INTS() explicitly before call kvmppc_handle_exit() like this: KVM: PPC: Book3E HV: call SOFT_DISABLE_INTS to sync the software state We enter with interrupts disabled in hardware, but we need to call SOFT_DISABLE_INTS anyway to ensure that the software state is kept in sync. Signed-off-by: Tiejun Chen diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..b521d21 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -33,6 +33,8 @@ #ifdef CONFIG_64BIT #include +#include +#include #else #include "../kernel/head_booke.h" /* for THREAD_NORMSAVE() */ #endif @@ -469,6 +471,14 @@ _GLOBAL(kvmppc_resume_host) PPC_LL r3, HOST_RUN(r1) mr r5, r14 /* intno */ mr r14, r4 /* Save vcpu pointer. */ +#ifdef CONFIG_64BIT + /* + * We enter with interrupts disabled in hardware, but + * we need to call SOFT_DISABLE_INTS anyway to ensure + * that the software state is kept in sync. + */ + SOFT_DISABLE_INTS(r7,r8) +#endif bl kvmppc_handle_exit /* Restore vcpu pointer and the nonvolatiles we used. */ Tiejun