Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756055AbdDMGzu convert rfc822-to-8bit (ORCPT ); Thu, 13 Apr 2017 02:55:50 -0400 Received: from ozlabs.org ([103.22.144.67]:60521 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751159AbdDMGzr (ORCPT ); Thu, 13 Apr 2017 02:55:47 -0400 Message-ID: <1492066545.4624.52.camel@neuling.org> Subject: Re: [PATCH 2/3] powernv:idle: Decouple TB restore & Per-core SPRs restore From: Michael Neuling To: "Gautham R. Shenoy" , Michael Ellerman , Benjamin Herrenschmidt , "Shreyas B. Prabhu" , Shilpasri G Bhat , Vaidyanathan Srinivasan , Anton Blanchard , Balbir Singh , Akshay Adiga , Nicholas Piggin , Mahesh J Salgaonkar , "Aneesh Kumar K.V" Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Date: Thu, 13 Apr 2017 16:55:45 +1000 In-Reply-To: <27511c1235ee58c66eabdea65b39f5dd35d91426.1491996797.git.ego@linux.vnet.ibm.com> References: <27511c1235ee58c66eabdea65b39f5dd35d91426.1491996797.git.ego@linux.vnet.ibm.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT X-Mailer: Evolution 3.22.3-0ubuntu0.1 Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1841 Lines: 57 On Wed, 2017-04-12 at 17:16 +0530, Gautham R. Shenoy wrote: > From: "Gautham R. Shenoy" > > The idle-exit code assumes that if Timebase is not lost, then neither > are the per-core hypervisor resources lost. Double negative! How about: The idle-exit code assumes that if the timebase is restored, then the per-core hypervisor resources are also restored. > This was true on POWER8 > where fast-sleep lost only TB but not per-core resources, and winkle > lost both. > > This assumption is not true for POWER9 however, since there can be > states which do not lose timebase but can lose per-core SPRs. > > Hence check if we need to restore the per-core hypervisor state even > if timebase is not lost. I think I understand what you're doing, just seems awkwardly worded. Is this actually what the patch is doing? It seem to be just changing one branch. Mikey > > Signed-off-by: Gautham R. Shenoy > --- >  arch/powerpc/kernel/idle_book3s.S | 7 ++++--- >  1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/kernel/idle_book3s.S > b/arch/powerpc/kernel/idle_book3s.S > index 9b747e9..6a9bd28 100644 > --- a/arch/powerpc/kernel/idle_book3s.S > +++ b/arch/powerpc/kernel/idle_book3s.S > @@ -723,13 +723,14 @@ timebase_resync: >    * Use cr3 which indicates that we are waking up with atleast partial >    * hypervisor state loss to determine if TIMEBASE RESYNC is needed. >    */ > - ble cr3,clear_lock > + ble cr3,.Ltb_resynced >   /* Time base re-sync */ >   bl opal_resync_timebase; >   /* > -  * If waking up from sleep, per core state is not lost, skip to > -  * clear_lock. > +  * If waking up from sleep (POWER8), per core state > +  * is not lost, skip to clear_lock. >    */ > +.Ltb_resynced: >   blt cr4,clear_lock >   >   /*