From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Subject: Re: [v1][PATCH 1/1] KVM: PPC: disable preemption when using
 hard_irq_disable()
Date: Thu, 11 Jul 2013 09:13:36 -0500
Message-ID: <1373552016.8183.242@snotra>
References: <1373436139-27998-1-git-send-email-tiejun.chen@windriver.com>
 <1373483740.8183.223@snotra> <51DE1FD1.90609@windriver.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="Flowed"; DelSp="Yes"
Content-Transfer-Encoding: 7bit
Cc: linuxppc-dev@lists.ozlabs.org, agraf@suse.de, kvm-ppc@vger.kernel.org,
 kvm@vger.kernel.org
To: tiejun.chen <tiejun.chen@windriver.com>
Return-path: <linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org>
In-Reply-To: <51DE1FD1.90609@windriver.com> (from tiejun.chen@windriver.com
 on Wed Jul 10 22:00:33 2013)
Content-Disposition: inline
List-Unsubscribe: <https://lists.ozlabs.org/options/linuxppc-dev>,
 <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=unsubscribe>
List-Archive: <http://lists.ozlabs.org/pipermail/linuxppc-dev/>
List-Post: <mailto:linuxppc-dev@lists.ozlabs.org>
List-Help: <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=help>
List-Subscribe: <https://lists.ozlabs.org/listinfo/linuxppc-dev>,
 <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=subscribe>
Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org
Sender: "Linuxppc-dev"
 <linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org>
List-Id: kvm.vger.kernel.org

On 07/10/2013 10:00:33 PM, tiejun.chen wrote:
> On 07/11/2013 03:15 AM, Scott Wood wrote:
>> On 07/10/2013 01:02:19 AM, Tiejun Chen wrote:
>>> We should ensure the preemption cannot occur while calling  
>>> get_paca()
>>> insdide hard_irq_disable(), otherwise the paca_struct may be the
>>> wrong one just after. And btw, we may update timing stats in this  
>>> case.
>> 
>> The soft-ee mechanism depends on accessing the PACA directly via r13  
>> to avoid
>> this.  We probably should be using inline asm to read was_enabled  
>> rather than
> 
> Yes.
> 
>> hoping the compiler doesn't do anything silly.
> 
> Do you recommend I should directly replace get_paca() with local_paca  
> inside hard_irq_disable()?
> 
> #define hard_irq_disable()      do {                    \
>         u8 _was_enabled = get_paca()->soft_enabled;     \
> 
> ->	u8 _was_enabled = local_paca->soft_enabled;
> 
> But is this safe for all scenarios?

get_paca() is just a #define for local_paca.  It won't make a  
difference, other than to evade the debug check.

In practice, it's unlikely that GCC would do anything other than a load  
directly from r13, but to be sure we should use inline asm to do the  
load, just like arch_local_save_flags and arch_local_irq_disable do.

-Scott