From mboxrd@z Thu Jan  1 00:00:00 1970
From: "tiejun.chen" <tiejun.chen@windriver.com>
Subject: Re: [v1][PATCH 1/1] KVM: PPC: disable preemption when using hard_irq_disable()
Date: Mon, 15 Jul 2013 10:25:52 +0800
Message-ID: <51E35DB0.4090804@windriver.com>
References: <1373651433.8183.276@snotra>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Alexander Graf <agraf@suse.de>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	"<kvm-ppc@vger.kernel.org>" <kvm-ppc@vger.kernel.org>,
	"<kvm@vger.kernel.org> list" <kvm@vger.kernel.org>
To: Scott Wood <scottwood@freescale.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail1.windriver.com ([147.11.146.13]:56512 "EHLO
	mail1.windriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753819Ab3GOCZI (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sun, 14 Jul 2013 22:25:08 -0400
In-Reply-To: <1373651433.8183.276@snotra>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 07/13/2013 01:50 AM, Scott Wood wrote:
> On 07/11/2013 10:22:28 PM, tiejun.chen wrote:
>> 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.chen@windriver.com>
>>
>> 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 <asm/exception-64e.h>
>> +#include <asm/hw_irq.h>
>> +#include <asm/irqflags.h>
>>  #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
>
> This will clobber the arguments we want to pass to kvmppc_handle_exit.  That can
> be fixed by moving SOFT_DISABLE_INTS[1] earlier, and maybe it's more idiomatic

Okay. Once we have a final name to replace SOFT_DISABLE_INTS, I can regenerate 
this as you comment.

> to use SOFT_DISABLE_INTS rather than what we currently do, but we still want to
> fix hard_irq_disable().  There are other places where we call hard_irq_disable()
> where interrupts (and I believe preemption) were previously enabled.

Yes, I had a preliminary change ACKed by Ben, and I guess you also saw :) so 
I'll send that firstly.

Thanks,

Tiejun