From patchwork Fri Jul 12 03:22:28 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tiejun Chen X-Patchwork-Id: 258684 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 64ADD2C0328 for ; Fri, 12 Jul 2013 13:21:51 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755770Ab3GLDVu (ORCPT ); Thu, 11 Jul 2013 23:21:50 -0400 Received: from mail1.windriver.com ([147.11.146.13]:62845 "EHLO mail1.windriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751307Ab3GLDVt (ORCPT ); Thu, 11 Jul 2013 23:21:49 -0400 Received: from ALA-HCA.corp.ad.wrs.com (ala-hca.corp.ad.wrs.com [147.11.189.40]) by mail1.windriver.com (8.14.5/8.14.3) with ESMTP id r6C3LfFd024506 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL); Thu, 11 Jul 2013 20:21:41 -0700 (PDT) Received: from [128.224.162.214] (128.224.162.214) by ALA-HCA.corp.ad.wrs.com (147.11.189.50) with Microsoft SMTP Server (TLS) id 14.2.342.3; Thu, 11 Jul 2013 20:21:39 -0700 Message-ID: <51DF7674.1070906@windriver.com> Date: Fri, 12 Jul 2013 11:22:28 +0800 From: "tiejun.chen" User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130623 Thunderbird/17.0.7 MIME-Version: 1.0 To: Scott Wood , Alexander Graf , Benjamin Herrenschmidt CC: "" , " list" Subject: Re: [v1][PATCH 1/1] KVM: PPC: disable preemption when using hard_irq_disable() References: <1373559480.8183.258@snotra> <1373560585.8183.261@snotra> In-Reply-To: <1373560585.8183.261@snotra> X-Originating-IP: [128.224.162.214] Sender: kvm-ppc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm-ppc@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 Tiejun --- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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. */