linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Gautham R Shenoy <ego@linux.vnet.ibm.com>
To: Nicholas Piggin <npiggin@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org,
	Balbir Singh <bsingharora@gmail.com>,
	"Shreyas B. Prabhu" <shreyasbp@gmail.com>,
	"Gautham R . Shenoy" <ego@linux.vnet.ibm.com>
Subject: Re: [PATCH] powerpc/64s: relocation, register save fixes for system reset interrupt
Date: Thu, 13 Oct 2016 16:32:21 +0530	[thread overview]
Message-ID: <20161013110221.GA9002@in.ibm.com> (raw)
In-Reply-To: <20161013021714.6624-1-npiggin@gmail.com>

Hi Nick,

[Updated Shreyas's current e-mail address:
	 "Shreyas B. Prabhu" <shreyasbp@gmail.com>]

On Thu, Oct 13, 2016 at 01:17:14PM +1100, Nicholas Piggin wrote:
> This patch does a couple of things. First of all, powernv immediately
> explodes when running a relocated kernel, because the system reset
> exception for handling sleeps does not do correct relocated branches.
> 
> Secondly, the sleep handling code trashes the condition and cfar
> registers, which we would like to preserve for debugging purposes (for
> non-sleep case exception).
> 
> This patch changes the exception to use the standard format that saves
> registers before any tests or branches are made. It adds the test for
> idle-wakeup as an "extra" to break out of the normal exception path.
> Then it branches to a relocated idle handler that calls the various
> idle handling functions.
> 
> After this patch, POWER8 CPU simulator now boots powernv kernel that is
> running at non-zero.

This patch looks good to me. I have tested this on POWER8 and verified
that we are correctly waking up from nap, sleep and winkle.

Acked-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>

> 
> Cc: Balbir Singh <bsingharora@gmail.com>
> Cc: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
> Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/include/asm/exception-64s.h | 16 ++++++++++
>  arch/powerpc/kernel/exceptions-64s.S     | 50 ++++++++++++++++++--------------
>  2 files changed, 45 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
> index 2e4e7d8..84d49b1 100644
> --- a/arch/powerpc/include/asm/exception-64s.h
> +++ b/arch/powerpc/include/asm/exception-64s.h
> @@ -93,6 +93,10 @@
>  	ld	reg,PACAKBASE(r13);	/* get high part of &label */	\
>  	ori	reg,reg,(FIXED_SYMBOL_ABS_ADDR(label))@l;
> 
> +#define __LOAD_HANDLER(reg, label)					\
> +	ld	reg,PACAKBASE(r13);					\
> +	ori	reg,reg,(ABS_ADDR(label))@l;
> +
>  /* Exception register prefixes */
>  #define EXC_HV	H
>  #define EXC_STD
> @@ -208,6 +212,18 @@ END_FTR_SECTION_NESTED(ftr,ftr,943)
>  #define kvmppc_interrupt kvmppc_interrupt_pr
>  #endif
> 
> +#ifdef CONFIG_RELOCATABLE
> +#define BRANCH_TO_COMMON(reg, label)					\
> +	__LOAD_HANDLER(reg, label);					\
> +	mtctr	reg;							\
> +	bctr
> +
> +#else
> +#define BRANCH_TO_COMMON(reg, label)					\
> +	b	label
> +
> +#endif
> +
>  #define __KVM_HANDLER_PROLOG(area, n)					\
>  	BEGIN_FTR_SECTION_NESTED(947)					\
>  	ld	r10,area+EX_CFAR(r13);					\
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 08992f8..e680e84 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -95,19 +95,35 @@ __start_interrupts:
>  /* No virt vectors corresponding with 0x0..0x100 */
>  EXC_VIRT_NONE(0x4000, 0x4100)
> 
> -EXC_REAL_BEGIN(system_reset, 0x100, 0x200)
> -	SET_SCRATCH0(r13)
> +
>  #ifdef CONFIG_PPC_P7_NAP
> -BEGIN_FTR_SECTION
> -	/* Running native on arch 2.06 or later, check if we are
> -	 * waking up from nap/sleep/winkle.
> +	/*
> +	 * If running native on arch 2.06 or later, check if we are waking up
> +	 * from nap/sleep/winkle, and branch to idle handler.
>  	 */
> -	mfspr	r13,SPRN_SRR1
> -	rlwinm.	r13,r13,47-31,30,31
> -	beq	9f
> +#define IDLETEST(n)							\
> +	BEGIN_FTR_SECTION ;						\
> +	mfspr	r10,SPRN_SRR1 ;						\
> +	rlwinm.	r10,r10,47-31,30,31 ;					\
> +	beq-	1f ;							\
> +	cmpwi	cr3,r10,2 ;						\
> +	BRANCH_TO_COMMON(r10, system_reset_idle_common) ;		\
> +1:									\
> +	END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
> +#else
> +#define IDLETEST NOTEST
> +#endif
> 
> -	cmpwi	cr3,r13,2
> -	GET_PACA(r13)
> +EXC_REAL_BEGIN(system_reset, 0x100, 0x200)
> +	SET_SCRATCH0(r13)
> +	EXCEPTION_PROLOG_PSERIES(PACA_EXGEN, system_reset_common, EXC_STD,
> +				 IDLETEST, 0x100)
> +
> +EXC_REAL_END(system_reset, 0x100, 0x200)
> +EXC_VIRT_NONE(0x4100, 0x4200)
> +
> +#ifdef CONFIG_PPC_P7_NAP
> +EXC_COMMON_BEGIN(system_reset_idle_common)
>  	bl	pnv_restore_hyp_resource
> 
>  	li	r0,PNV_THREAD_RUNNING
> @@ -130,14 +146,8 @@ BEGIN_FTR_SECTION
>  	blt	cr3,2f
>  	b	pnv_wakeup_loss
>  2:	b	pnv_wakeup_noloss
> +#endif
> 
> -9:
> -END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
> -#endif /* CONFIG_PPC_P7_NAP */
> -	EXCEPTION_PROLOG_PSERIES(PACA_EXGEN, system_reset_common, EXC_STD,
> -				 NOTEST, 0x100)
> -EXC_REAL_END(system_reset, 0x100, 0x200)
> -EXC_VIRT_NONE(0x4100, 0x4200)
>  EXC_COMMON(system_reset_common, 0x100, system_reset_exception)
> 
>  #ifdef CONFIG_PPC_PSERIES
> @@ -817,10 +827,8 @@ EXC_VIRT(trap_0b, 0x4b00, 0x4c00, 0xb00)
>  TRAMP_KVM(PACA_EXGEN, 0xb00)
>  EXC_COMMON(trap_0b_common, 0xb00, unknown_exception)
> 
> -
> -#define LOAD_SYSCALL_HANDLER(reg)				\
> -	ld	reg,PACAKBASE(r13);				\
> -	ori	reg,reg,(ABS_ADDR(system_call_common))@l;
> +#define LOAD_SYSCALL_HANDLER(reg)					\
> +	__LOAD_HANDLER(reg, system_call_common)
> 
>  /* Syscall routine is used twice, in reloc-off and reloc-on paths */
>  #define SYSCALL_PSERIES_1 					\
> -- 
> 2.9.3
> 

  reply	other threads:[~2016-10-13 11:02 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-13  2:17 [PATCH] powerpc/64s: relocation, register save fixes for system reset interrupt Nicholas Piggin
2016-10-13 11:02 ` Gautham R Shenoy [this message]
2016-10-13 11:54 ` Balbir Singh
2016-10-14  3:16   ` Nicholas Piggin
2016-10-14  5:45     ` Balbir Singh
2016-10-27 11:38 ` Michael Ellerman
2016-10-28 12:01   ` Balbir Singh
2016-11-02  6:04 ` [PATCH] " Mahesh Jagannath Salgaonkar
2016-11-02  6:57   ` Nicholas Piggin
2016-11-02  8:24     ` Mahesh J Salgaonkar
2016-11-02  8:36       ` Nicholas Piggin
2016-11-02  8:45         ` Gautham R Shenoy
2016-11-03  5:21           ` Nicholas Piggin
2016-11-03  5:56             ` Shreyas B. Prabhu
2016-11-03  6:17               ` Nicholas Piggin
2016-11-03  6:32                 ` Shreyas B. Prabhu
2016-11-03  7:02                   ` Nicholas Piggin
2016-11-04  9:04                     ` Gautham R Shenoy
2016-11-04 11:47                       ` Nicholas Piggin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161013110221.GA9002@in.ibm.com \
    --to=ego@linux.vnet.ibm.com \
    --cc=bsingharora@gmail.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=npiggin@gmail.com \
    --cc=shreyasbp@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).