LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 7/9] fsl: add EPU FSM configuration for deep sleep
From: Scott Wood @ 2014-03-14 22:51 UTC (permalink / raw)
  To: Chenhui Zhao; +Cc: linuxppc-dev, linux-kernel, Jason.Jin
In-Reply-To: <20140312083410.GF4706@localhost.localdomain>

On Wed, 2014-03-12 at 16:34 +0800, Chenhui Zhao wrote:
> On Tue, Mar 11, 2014 at 07:08:43PM -0500, Scott Wood wrote:
> > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
> > > From: Hongbo Zhang <hongbo.zhang@freescale.com>
> > > 
> > > In the last stage of deep sleep, software will trigger a Finite
> > > State Machine (FSM) to control the hardware precedure, such as
> > > board isolation, killing PLLs, removing power, and so on.
> > > 
> > > When the system is waked up by an interrupt, the FSM controls the
> > > hardware to complete the early resume precedure.
> > > 
> > > This patch configure the EPU FSM preparing for deep sleep.
> > > 
> > > Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
> > > Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com>
> > 
> > Couldn't this be part of qoriq_pm.c?
> 
> Put the code in drivers/platform/fsl/ so that LS1 can share these code.

How can LS1 share it if it's got hardcoded T1040 values?

> > > diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
> > > index 09fde58..6539e6d 100644
> > > --- a/drivers/platform/Kconfig
> > > +++ b/drivers/platform/Kconfig
> > > @@ -6,3 +6,7 @@ source "drivers/platform/goldfish/Kconfig"
> > >  endif
> > >  
> > >  source "drivers/platform/chrome/Kconfig"
> > > +
> > > +if FSL_SOC
> > > +source "drivers/platform/fsl/Kconfig"
> > > +endif
> > 
> > Chrome doesn't need an ifdef -- why does this?
> 
> Don't wish other platform see these options, and the X86 and GOLDFISH have
> ifdefs.

The point is you can implement the dependency inside
drivers/platform/fsl/Kconfig.

> > > diff --git a/drivers/platform/fsl/Makefile b/drivers/platform/fsl/Makefile
> > > new file mode 100644
> > > index 0000000..d99ca0e
> > > --- /dev/null
> > > +++ b/drivers/platform/fsl/Makefile
> > > @@ -0,0 +1,5 @@
> > > +#
> > > +# Makefile for linux/drivers/platform/fsl
> > > +# Freescale Specific Power Management Drivers
> > > +#
> > > +obj-$(CONFIG_FSL_SLEEP_FSM)	+= sleep_fsm.o
> > 
> > Why is this here while the other stuff is in arch/powerpc/sysdev?
> > 
> > > +/* Block offsets */
> > > +#define	RCPM_BLOCK_OFFSET	0x00022000
> > > +#define	EPU_BLOCK_OFFSET	0x00000000
> > > +#define	NPC_BLOCK_OFFSET	0x00001000
> > 
> > Why don't these block offsets come from the device tree?
> 
> Have maped DCSR registers. Don't wish to remap them.

We don't wish to have hardcoded CCSR/DCSR offsets in the kernel source.
Sorry.
 
> > > +	/* Configure the EPU Counters */
> > > +	epu_write(EPCCR15, 0x92840000);
> > > +	epu_write(EPCCR14, 0x92840000);
> > > +	epu_write(EPCCR12, 0x92840000);
> > > +	epu_write(EPCCR11, 0x92840000);
> > > +	epu_write(EPCCR10, 0x92840000);
> > > +	epu_write(EPCCR9, 0x92840000);
> > > +	epu_write(EPCCR8, 0x92840000);
> > > +	epu_write(EPCCR5, 0x92840000);
> > > +	epu_write(EPCCR4, 0x92840000);
> > > +	epu_write(EPCCR2, 0x92840000);
> > > +
> > > +	/* Configure the SCUs Inputs */
> > > +	epu_write(EPSMCR15, 0x76000000);
> > > +	epu_write(EPSMCR14, 0x00000031);
> > > +	epu_write(EPSMCR13, 0x00003100);
> > > +	epu_write(EPSMCR12, 0x7F000000);
> > > +	epu_write(EPSMCR11, 0x31740000);
> > > +	epu_write(EPSMCR10, 0x65000030);
> > > +	epu_write(EPSMCR9, 0x00003000);
> > > +	epu_write(EPSMCR8, 0x64300000);
> > > +	epu_write(EPSMCR7, 0x30000000);
> > > +	epu_write(EPSMCR6, 0x7C000000);
> > > +	epu_write(EPSMCR5, 0x00002E00);
> > > +	epu_write(EPSMCR4, 0x002F0000);
> > > +	epu_write(EPSMCR3, 0x2F000000);
> > > +	epu_write(EPSMCR2, 0x6C700000);
> > 
> > Where do these magic numbers come from?  Which chips are they valid for?
> 
> They are for T1040. Can be found in the RCPM chapter of T1040RM.

Then put in a comment to that effect, including what part of the RCPM
chapter.

How do you plan to handle the addition of another SoC with different
values?

-Scott

^ permalink raw reply

* [PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
From: Laura Abbott @ 2014-03-14 22:58 UTC (permalink / raw)
  To: Russell King, David S. Miller, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, James E.J. Bottomley, Fenghua Yu, Tony Luck,
	Benjamin Herrenschmidt, Paul Mackerras, Martin Schwidefsky,
	Heiko Carstens, Andrew Morton
  Cc: linux-s390, Laura Abbott, linux-scsi, linux-kernel, linux390,
	sparclinux, linux-ia64, linuxppc-dev, linux-arm-kernel

Rather than have architectures #define ARCH_HAS_SG_CHAIN in an architecture
specific scatterlist.h, make it a proper Kconfig option and use that
instead. At same time, remove the header files are are now mostly
useless and just include asm-generic/scatterlist.h.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Benjamin Herrenschmidt <benh@kernel.crashing.org>
Paul Mackerras <paulus@samba.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
 arch/arm/Kconfig                       |  1 +
 arch/arm/include/asm/Kbuild            |  1 +
 arch/arm/include/asm/scatterlist.h     | 12 ------------
 arch/arm64/Kconfig                     |  1 +
 arch/ia64/Kconfig                      |  1 +
 arch/ia64/include/asm/Kbuild           |  1 +
 arch/ia64/include/asm/scatterlist.h    |  7 -------
 arch/powerpc/Kconfig                   |  1 +
 arch/powerpc/include/asm/Kbuild        |  1 +
 arch/powerpc/include/asm/scatterlist.h | 17 -----------------
 arch/s390/Kconfig                      |  1 +
 arch/s390/include/asm/Kbuild           |  1 +
 arch/s390/include/asm/scatterlist.h    |  3 ---
 arch/sparc/Kconfig                     |  1 +
 arch/sparc/include/asm/Kbuild          |  1 +
 arch/sparc/include/asm/scatterlist.h   |  8 --------
 arch/x86/Kconfig                       |  1 +
 arch/x86/include/asm/Kbuild            |  1 +
 arch/x86/include/asm/scatterlist.h     |  8 --------
 include/linux/scatterlist.h            |  2 +-
 include/scsi/scsi.h                    |  2 +-
 lib/Kconfig                            |  7 +++++++
 lib/scatterlist.c                      |  4 ++--
 23 files changed, 24 insertions(+), 59 deletions(-)
 delete mode 100644 arch/arm/include/asm/scatterlist.h
 delete mode 100644 arch/ia64/include/asm/scatterlist.h
 delete mode 100644 arch/powerpc/include/asm/scatterlist.h
 delete mode 100644 arch/s390/include/asm/scatterlist.h
 delete mode 100644 arch/sparc/include/asm/scatterlist.h
 delete mode 100644 arch/x86/include/asm/scatterlist.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1594945..8122294 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -82,6 +82,7 @@ config ARM
 	  <http://www.arm.linux.org.uk/>.
 
 config ARM_HAS_SG_CHAIN
+	select ARCH_HAS_SG_CHAIN
 	bool
 
 config NEED_SG_DMA_LENGTH
diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
index 3278afe..2357ed6 100644
--- a/arch/arm/include/asm/Kbuild
+++ b/arch/arm/include/asm/Kbuild
@@ -18,6 +18,7 @@ generic-y += param.h
 generic-y += parport.h
 generic-y += poll.h
 generic-y += resource.h
+generic-y += scatterlist.h
 generic-y += sections.h
 generic-y += segment.h
 generic-y += sembuf.h
diff --git a/arch/arm/include/asm/scatterlist.h b/arch/arm/include/asm/scatterlist.h
deleted file mode 100644
index cefdb8f..0000000
--- a/arch/arm/include/asm/scatterlist.h
+++ /dev/null
@@ -1,12 +0,0 @@
-#ifndef _ASMARM_SCATTERLIST_H
-#define _ASMARM_SCATTERLIST_H
-
-#ifdef CONFIG_ARM_HAS_SG_CHAIN
-#define ARCH_HAS_SG_CHAIN
-#endif
-
-#include <asm/memory.h>
-#include <asm/types.h>
-#include <asm-generic/scatterlist.h>
-
-#endif /* _ASMARM_SCATTERLIST_H */
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 27bbcfc..f2f95f4 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -2,6 +2,7 @@ config ARM64
 	def_bool y
 	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
 	select ARCH_USE_CMPXCHG_LOCKREF
+	select ARCH_HAS_SG_CHAIN
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 0c8e553..13e2e8b 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -44,6 +44,7 @@ config IA64
 	select HAVE_MOD_ARCH_SPECIFIC
 	select MODULES_USE_ELF_RELA
 	select ARCH_USE_CMPXCHG_LOCKREF
+	select ARCH_HAS_SG_CHAIN
 	default y
 	help
 	  The Itanium Processor Family is Intel's 64-bit successor to
diff --git a/arch/ia64/include/asm/Kbuild b/arch/ia64/include/asm/Kbuild
index 283a831..3906865 100644
--- a/arch/ia64/include/asm/Kbuild
+++ b/arch/ia64/include/asm/Kbuild
@@ -2,6 +2,7 @@
 generic-y += clkdev.h
 generic-y += exec.h
 generic-y += kvm_para.h
+generic-y += scatterlist.h
 generic-y += trace_clock.h
 generic-y += preempt.h
 generic-y += vtime.h
diff --git a/arch/ia64/include/asm/scatterlist.h b/arch/ia64/include/asm/scatterlist.h
deleted file mode 100644
index 08fd93b..0000000
--- a/arch/ia64/include/asm/scatterlist.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef _ASM_IA64_SCATTERLIST_H
-#define _ASM_IA64_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-#define ARCH_HAS_SG_CHAIN
-
-#endif /* _ASM_IA64_SCATTERLIST_H */
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 957bf34..659aee2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -141,6 +141,7 @@ config PPC
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK
 	select ARCH_USE_CMPXCHG_LOCKREF if PPC64
+	select ARCH_HAS_SG_CHAIN
 
 config GENERIC_CSUM
 	def_bool CPU_LITTLE_ENDIAN
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index 6c0a955..ca596ec 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -1,6 +1,7 @@
 
 generic-y += clkdev.h
 generic-y += rwsem.h
+generic-y += scatterlist.h
 generic-y += trace_clock.h
 generic-y += preempt.h
 generic-y += vtime.h
diff --git a/arch/powerpc/include/asm/scatterlist.h b/arch/powerpc/include/asm/scatterlist.h
deleted file mode 100644
index de1f620..0000000
--- a/arch/powerpc/include/asm/scatterlist.h
+++ /dev/null
@@ -1,17 +0,0 @@
-#ifndef _ASM_POWERPC_SCATTERLIST_H
-#define _ASM_POWERPC_SCATTERLIST_H
-/*
- * Copyright (C) 2001 PPC64 Team, IBM Corp
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <asm/dma.h>
-#include <asm-generic/scatterlist.h>
-
-#define ARCH_HAS_SG_CHAIN
-
-#endif /* _ASM_POWERPC_SCATTERLIST_H */
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 65a0775..d6c2059 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -142,6 +142,7 @@ config S390
 	select SYSCTL_EXCEPTION_TRACE
 	select VIRT_CPU_ACCOUNTING
 	select VIRT_TO_BUS
+	select ARCH_HAS_SG_CHAIN
 
 config SCHED_OMIT_FRAME_POINTER
 	def_bool y
diff --git a/arch/s390/include/asm/Kbuild b/arch/s390/include/asm/Kbuild
index 8386a4a..14be6d0 100644
--- a/arch/s390/include/asm/Kbuild
+++ b/arch/s390/include/asm/Kbuild
@@ -4,3 +4,4 @@ generic-y += clkdev.h
 generic-y += trace_clock.h
 generic-y += preempt.h
 generic-y += hash.h
+generic-y += scatterlist.h
diff --git a/arch/s390/include/asm/scatterlist.h b/arch/s390/include/asm/scatterlist.h
deleted file mode 100644
index 6d45ef6..0000000
--- a/arch/s390/include/asm/scatterlist.h
+++ /dev/null
@@ -1,3 +0,0 @@
-#include <asm-generic/scatterlist.h>
-
-#define ARCH_HAS_SG_CHAIN
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 7d8b7e9..7a179fe 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -42,6 +42,7 @@ config SPARC
 	select MODULES_USE_ELF_RELA
 	select ODD_RT_SIGACTION
 	select OLD_SIGSUSPEND
+	select ARCH_HAS_SG_CHAIN
 
 config SPARC32
 	def_bool !64BIT
diff --git a/arch/sparc/include/asm/Kbuild b/arch/sparc/include/asm/Kbuild
index 4b60a0c..437f0c6 100644
--- a/arch/sparc/include/asm/Kbuild
+++ b/arch/sparc/include/asm/Kbuild
@@ -18,3 +18,4 @@ generic-y += types.h
 generic-y += word-at-a-time.h
 generic-y += preempt.h
 generic-y += hash.h
+generic-y += scatterlist.h
diff --git a/arch/sparc/include/asm/scatterlist.h b/arch/sparc/include/asm/scatterlist.h
deleted file mode 100644
index 92bb638..0000000
--- a/arch/sparc/include/asm/scatterlist.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef _SPARC_SCATTERLIST_H
-#define _SPARC_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-
-#define ARCH_HAS_SG_CHAIN
-
-#endif /* !(_SPARC_SCATTERLIST_H) */
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0af5250..76997dc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -127,6 +127,7 @@ config X86
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
 	select HAVE_CC_STACKPROTECTOR
+	select ARCH_HAS_SG_CHAIN
 
 config INSTRUCTION_DECODER
 	def_bool y
diff --git a/arch/x86/include/asm/Kbuild b/arch/x86/include/asm/Kbuild
index 7f66985..9b3d749 100644
--- a/arch/x86/include/asm/Kbuild
+++ b/arch/x86/include/asm/Kbuild
@@ -5,3 +5,4 @@ genhdr-y += unistd_64.h
 genhdr-y += unistd_x32.h
 
 generic-y += clkdev.h
+generic-y += scatterlist.h
diff --git a/arch/x86/include/asm/scatterlist.h b/arch/x86/include/asm/scatterlist.h
deleted file mode 100644
index 4240878..0000000
--- a/arch/x86/include/asm/scatterlist.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef _ASM_X86_SCATTERLIST_H
-#define _ASM_X86_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-
-#define ARCH_HAS_SG_CHAIN
-
-#endif /* _ASM_X86_SCATTERLIST_H */
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index a964f72..4b152c8 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -136,7 +136,7 @@ static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
 static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
 			    struct scatterlist *sgl)
 {
-#ifndef ARCH_HAS_SG_CHAIN
+#ifndef CONFIG_ARCH_HAS_SG_CHAIN
 	BUG();
 #endif
 
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 0a4edfe..d34cf2d 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -31,7 +31,7 @@ enum scsi_timeouts {
  * Like SCSI_MAX_SG_SEGMENTS, but for archs that have sg chaining. This limit
  * is totally arbitrary, a setting of 2048 will get you at least 8mb ios.
  */
-#ifdef ARCH_HAS_SG_CHAIN
+#ifdef CONFIG_ARCH_HAS_SG_CHAIN
 #define SCSI_MAX_SG_CHAIN_SEGMENTS	2048
 #else
 #define SCSI_MAX_SG_CHAIN_SEGMENTS	SCSI_MAX_SG_SEGMENTS
diff --git a/lib/Kconfig b/lib/Kconfig
index 991c98b..32c68d3 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -451,4 +451,11 @@ config UCS2_STRING
 
 source "lib/fonts/Kconfig"
 
+#
+# sg chaining option
+#
+
+config ARCH_HAS_SG_CHAIN
+	def_bool n
+
 endmenu
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 3a8e8e8..4251cbd 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -73,7 +73,7 @@ EXPORT_SYMBOL(sg_nents);
  **/
 struct scatterlist *sg_last(struct scatterlist *sgl, unsigned int nents)
 {
-#ifndef ARCH_HAS_SG_CHAIN
+#ifndef CONFIG_ARCH_HAS_SG_CHAIN
 	struct scatterlist *ret = &sgl[nents - 1];
 #else
 	struct scatterlist *sg, *ret = NULL;
@@ -251,7 +251,7 @@ int __sg_alloc_table(struct sg_table *table, unsigned int nents,
 
 	if (nents == 0)
 		return -EINVAL;
-#ifndef ARCH_HAS_SG_CHAIN
+#ifndef CONFIG_ARCH_HAS_SG_CHAIN
 	if (WARN_ON_ONCE(nents > max_ents))
 		return -EINVAL;
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related

* Re: [PATCH 8/9] powerpc/85xx: add save/restore functions for core registers
From: Scott Wood @ 2014-03-14 23:01 UTC (permalink / raw)
  To: Chenhui Zhao; +Cc: linuxppc-dev, linux-kernel, Jason.Jin, Wang Dongsheng-B40534
In-Reply-To: <20140312094237.GG4706@localhost.localdomain>

On Wed, 2014-03-12 at 17:42 +0800, Chenhui Zhao wrote:
> On Tue, Mar 11, 2014 at 07:45:14PM -0500, Scott Wood wrote:
> > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
> > > From: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > 
> > > Add booke_cpu_state_save() and booke_cpu_state_restore() functions which can be
> > > used to save/restore CPU's registers in the case of deep sleep and hibernation.
> > > 
> > > Supported processors: E6500, E5500, E500MC, E500v2 and E500v1.
> > > 
> > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com>
> > > ---
> > >  arch/powerpc/include/asm/booke_save_regs.h |   96 ++++++++
> > >  arch/powerpc/kernel/Makefile               |    1 +
> > >  arch/powerpc/kernel/booke_save_regs.S      |  361 ++++++++++++++++++++++++++++
> > >  3 files changed, 458 insertions(+), 0 deletions(-)
> > >  create mode 100644 arch/powerpc/include/asm/booke_save_regs.h
> > >  create mode 100644 arch/powerpc/kernel/booke_save_regs.S
> > > 
> > > diff --git a/arch/powerpc/include/asm/booke_save_regs.h b/arch/powerpc/include/asm/booke_save_regs.h
> > > new file mode 100644
> > > index 0000000..87c357a
> > > --- /dev/null
> > > +++ b/arch/powerpc/include/asm/booke_save_regs.h
> > > @@ -0,0 +1,96 @@
> > > +/*
> > > + *  Save/restore e500 series core registers
> > 
> > Filename says booke, comment says e500.
> > 
> > Filename and comment also fail to point out that this is specifically
> > for standby/suspend, not for hibernate which is implemented in
> > swsusp_booke.S/swsusp_asm64.S.
> 
> Sorry for inconsistency. Will changes e500 to booke.
> Hibernation and suspend can share the code.

Maybe they could, but AFAICT this patchset doesn't make that happen --
and I'm not convinced that the churn would be worthwhile.  Note that
swsusp_asm64.S is not just for booke, so most of that file would not be
going away if you did make such a change.

I also don't like the way it looks like booke_save_regs.S is a booke
version of ppc_save_regs.S, even though they serve different purposes
and ppc_save_regs.S is still relevant to booke.

> > > + * Software-Use Registers
> > > + *	SPRG1			0x260		(dw * 76), 64-bit need to save.
> > > + *	SPRG3			0x268		(dw * 77), 32-bit need to save.
> > 
> > What about "CPU and NUMA node for VDSO getcpu" on 64-bit?  Currently
> > SPRG3, but it will need to change for critical interrupt support.
> > 
> > > + * MMU Registers
> > > + *	PID0 - PID2		0x270 ~ 0x280	(dw * 78 ~ dw * 80)
> > 
> > PID1/PID2 are e500v1/v2 only -- and Linux doesn't use them outside of
> > KVM (and you're not in KVM when you're running this code).
> > 
> > Are we ever going to have a non-zero PID at this point?
> 
> I incline to the view that saving all registers regardless of used or
> unused. The good point is that it can be compliant to the future
> changes of the usage of registers.
> 
> What do you think?

I agree to a certain extent, but balance it with the complexity of
dealing with registers that don't exist on all booke chips.  If they
don't really need to be saved, why go through the hassle of conditional
code?

> > > + * Debug Registers
> > > + *	DBCR0 - DBCR2		0x288 ~ 0x298	(dw * 81 ~ dw * 83)
> > > + *	IAC1 - IAC4		0x2a0 ~ 0x2b8	(dw * 84 ~ dw * 87)
> > > + *	DAC1 - DAC2		0x2c0 ~ 0x2c8	(dw * 88 ~ dw * 89)
> > > + *
> > > + */
> > 
> > IAC3-4 are not implemented on e500.
> > 
> > Do we really need to save the debug registers?  We're not going to be in
> > a debugged process when we do suspend.  If the concern is kgdb, it
> > probably needs to be told to get out of the way during suspend for other
> > reasons.
> 
> I think in the ideal case the suspend would not break any context. We
> should try to save/restore all cpu state. Of cause, trade-off is
> unavoidable in practice.
> 
> > 
> > > +#define SR_GPR1			0x000
> > > +#define SR_GPR2			0x008
> > > +#define SR_GPR13		0x010
> > > +#define SR_FPR14		0x0a8
> > > +#define SR_CR			0x138
> > > +#define SR_LR			0x140
> > > +#define SR_MSR			0x148
> > 
> > These are very vague names to be putting in a header file.
> 
> How about BOOKE_xx_OFFSET?

Better, but does it need to be in a public header file at all?
 
> > > +/*
> > > + * hibernation and deepsleep save/restore different number of registers,
> > > + * use these flags to indicate.
> > > + */
> > > +#define HIBERNATION_FLAG	1
> > > +#define DEEPSLEEP_FLAG		2
> > 
> > Again, namespacing -- but why is hibernation using this at all?  What's
> > wrong with the existing hibernation support?
> 
> How about BOOKE_HIBERNATION_FLAG?
> 
> Just wish to share code between hibernation and suspend.

No need until and unless you actually implement the change for
hibernation to use this.  As is, this is dead and untestable code.
 
> > > +booke_cpu_append_save:
> > > +	mfspr	r0, SPRN_PVR
> > > +	rlwinm	r11, r0, 16, 16, 31
> > > +
> > > +	lis	r12, 0
> > > +	ori	r12, r12, PVR_VER_E6500@l
> > > +	cmpw	r11, r12
> > > +	beq	e6500_append_save
> > > +
> > > +	lis	r12, 0
> > > +	ori	r12, r12, PVR_VER_E5500@l
> > > +	cmpw	r11, r12
> > > +	beq	e5500_append_save
> > > +
> > > +	lis	r12, 0
> > > +	ori	r12, r12, PVR_VER_E500MC@l
> > > +	cmpw	r11, r12
> > > +	beq	e500mc_append_save
> > > +
> > > +	lis	r12, 0
> > > +	ori	r12, r12, PVR_VER_E500V2@l
> > > +	cmpw	r11, r12
> > > +	beq	e500v2_append_save
> > > +
> > > +	lis	r12, 0
> > > +	ori	r12, r12, PVR_VER_E500V1@l
> > > +	cmpw	r11, r12
> > > +	beq	e500v1_append_save
> > > +	b	1f
> > > +
> > > +e6500_append_save:
> > > +	do_e6500_sr_interrupt_regs(save)
> > > +	do_e6500_sr_debug_regs(save)
> > > +	b	1f
> > > +
> > > +e5500_append_save:
> > > +	do_e5500_sr_interrupt_regs(save)
> > > +	b	1f
> > > +
> > > +e500mc_append_save:
> > > +	do_e500mc_sr_interrupt_regs(save)
> > > +	b	1f
> > > +
> > > +e500v2_append_save:
> > > +e500v1_append_save:
> > > +	do_e500_sr_interrupt_regs(save)
> > > +1:
> > > +	do_sr_interrupt_regs(save)
> > > +	do_sr_debug_regs(save)
> > > +
> > > +	blr
> > 
> > What is meant by "append" here?
> 
> Means extra registers for specific e500 derivative core.

How is that related to skipping it in the hibernation case?
 
-Scott

^ permalink raw reply

* Re: [PATCH 9/9] powerpc/pm: support deep sleep feature on T1040
From: Scott Wood @ 2014-03-14 23:18 UTC (permalink / raw)
  To: Chenhui Zhao; +Cc: linuxppc-dev, linux-kernel, Jason.Jin
In-Reply-To: <20140312104005.GH4706@localhost.localdomain>

On Wed, 2014-03-12 at 18:40 +0800, Chenhui Zhao wrote:
> On Tue, Mar 11, 2014 at 08:10:24PM -0500, Scott Wood wrote:
> > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
> > > From: Zhao Chenhui <chenhui.zhao@freescale.com>
> > > 
> > > T1040 supports deep sleep feature, which can switch off most parts of
> > > the SoC when it is in deep sleep mode. This way, it becomes more
> > > energy-efficient.
> > > 
> > > The DDR controller will also be powered off in deep sleep. Therefore,
> > > the last stage (the latter part of fsl_dp_enter_low) will run without DDR
> > > access. This piece of code and related TLBs will be prefetched.
> > > 
> > > Due to the different initialization code between 32-bit and 64-bit, they
> > > have seperate resume entry and precedure.
> > > 
> > > The feature supports 32-bit and 64-bit kernel mode.
> > > 
> > > Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > > ---
> > >  arch/powerpc/include/asm/booke_save_regs.h |    3 +
> > >  arch/powerpc/kernel/cpu_setup_fsl_booke.S  |   17 ++
> > >  arch/powerpc/kernel/head_fsl_booke.S       |   30 +++
> > >  arch/powerpc/platforms/85xx/Makefile       |    2 +-
> > >  arch/powerpc/platforms/85xx/deepsleep.c    |  201 +++++++++++++++++++
> > >  arch/powerpc/platforms/85xx/qoriq_pm.c     |   38 ++++
> > >  arch/powerpc/platforms/85xx/sleep.S        |  295 ++++++++++++++++++++++++++++
> > >  arch/powerpc/sysdev/fsl_soc.h              |    7 +
> > >  8 files changed, 592 insertions(+), 1 deletions(-)
> > >  create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c
> > >  create mode 100644 arch/powerpc/platforms/85xx/sleep.S
> > > 
> > > diff --git a/arch/powerpc/include/asm/booke_save_regs.h b/arch/powerpc/include/asm/booke_save_regs.h
> > > index 87c357a..37c1f6c 100644
> > > --- a/arch/powerpc/include/asm/booke_save_regs.h
> > > +++ b/arch/powerpc/include/asm/booke_save_regs.h
> > > @@ -88,6 +88,9 @@
> > >  #define HIBERNATION_FLAG	1
> > >  #define DEEPSLEEP_FLAG		2
> > >  
> > > +#define CPLD_FLAG	1
> > > +#define FPGA_FLAG	2
> > 
> > What is this?
> 
> We have two kind of boards, QDS and RDB.
> They have different register map. Use the flag to indicate the current board is using which kind
> of register map.

CPLD versus FPGA is not a meaningful difference.  We don't care what
technology is used to implement programmable logic -- we care what
programming interface is exposed.  Customers will have their own boards
that will likely not imitate either of these programming interfaces, but
what they do have will still probably be implemented in a CPLD or FPGA.
Likewise, Freescale may have future reference boards whose CPLD/FPGA is
not compatible.

So use better naming, and structure the code so it's easy to plug in
implementations for new or custom boards.
 
> > > diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
> > > index 20204fe..3285752 100644
> > > --- a/arch/powerpc/kernel/head_fsl_booke.S
> > > +++ b/arch/powerpc/kernel/head_fsl_booke.S
> > > @@ -162,6 +162,19 @@ _ENTRY(__early_start)
> > >  #include "fsl_booke_entry_mapping.S"
> > >  #undef ENTRY_MAPPING_BOOT_SETUP
> > >  
> > > +#if defined(CONFIG_SUSPEND) && defined(CONFIG_FSL_CORENET_RCPM)
> > > +	/* if deep_sleep_flag != 0, jump to the deep sleep resume entry */
> > > +	LOAD_REG_ADDR(r4, deep_sleep_flag)
> > > +	lwz	r3, 0(r4)
> > > +	cmpwi	r3, 0
> > > +	beq	11f
> > > +	/* clear deep_sleep_flag */
> > > +	li	r3, 0
> > > +	stw	r3, 0(r4)
> > > +	b	fsl_deepsleep_resume
> > > +11:
> > > +#endif
> > 
> > Why do you come in via the normal kernel entry, versus specifying a
> > direct entry point for deep sleep resume?  How does U-Boot even know
> > what the normal entry is when resuming?
> 
> I wish to return to a specified point (like 64-bit mode), but the code in
> fsl_booke_entry_mapping.S only can run in the first page. Because it
> only setups a temp mapping of 4KB.

Why do you need the entry mapping on 32-bit but not 64-bit?
> 
> > > +#if defined(CONFIG_SUSPEND) && defined(CONFIG_FSL_CORENET_RCPM)
> > > +_ENTRY(__entry_deep_sleep)
> > > +/*
> > > + * Bootloader will jump to here when resuming from deep sleep.
> > > + * After executing the init code in fsl_booke_entry_mapping.S,
> > > + * will jump to the real resume entry.
> > > + */
> > > +	li	r8, 1
> > > +	bl	12f
> > > +12:	mflr	r9
> > > +	addi	r9, r9, (deep_sleep_flag - 12b)
> > > +	stw	r8, 0(r9)
> > > +	b __early_start
> > > +deep_sleep_flag:
> > > +	.long	0
> > > +#endif
> > 
> > It's a bit ambiguous to say "entry_deep_sleep" when it's resuming rather
> > than entering...
> 
> How about __fsl_entry_resume?

fsl_booke_deep_sleep_resume

> > > +#define FSLDELAY(count)		\
> > > +	li	r3, (count)@l;	\
> > > +	slwi	r3, r3, 10;	\
> > > +	mtctr	r3;		\
> > > +101:	nop;			\
> > > +	bdnz	101b;
> > 
> > You don't need a namespace prefix on local macros in a non-header file.
> > 
> > Is the timebase stopped where you're calling this from?
> 
> No. My purpose is to avoid jump in the last stage of entering deep sleep.
> Jump may cause problem at that time.

"bdnz" is a jump.

What problems do you think a jump will cause?

> > You also probably want to do a "sync, readback, data dependency, isync"
> > sequence to make sure that the store has hit CCSR before you begin your
> > delay (or is a delay required at all if you do that?).
> 
> Yes. It is safer with a sync sequence.
> 
> The DDR controller need some time to signal the external DDR modules to
> enter self refresh mode.

Is it documented how much time it requires?

-Scott

^ permalink raw reply

* [PATCH] powerpc/mm: Make sure a local_irq_disable prevent a parallel THP split
From: Aneesh Kumar K.V @ 2014-03-15 10:47 UTC (permalink / raw)
  To: benh, paulus, Rik van Riel
  Cc: linux-mm, linuxppc-dev, linux-kernel, Aneesh Kumar K.V

From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

We have generic code like the one in get_futex_key that assume that
a local_irq_disable prevents a parallel THP split. Support that by
adding a dummy smp call function after setting _PAGE_SPLITTING. Code
paths like get_user_pages_fast still need to check for _PAGE_SPLITTING
after disabling IRQ which indicate that a parallel THP splitting is
ongoing. Now if they don't find _PAGE_SPLITTING set, then we can be
sure that parallel split will now block in pmdp_splitting flush
until we enables IRQ

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/powerpc/mm/pgtable_64.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
index 62bf5e8e78da..f6ce1f111f5b 100644
--- a/arch/powerpc/mm/pgtable_64.c
+++ b/arch/powerpc/mm/pgtable_64.c
@@ -647,6 +647,11 @@ void pmdp_splitting_flush(struct vm_area_struct *vma,
 		if (old & _PAGE_HASHPTE)
 			hpte_do_hugepage_flush(vma->vm_mm, address, pmdp);
 	}
+	/*
+	 * This ensures that generic code that rely on IRQ disabling
+	 * to prevent a parallel THP split work as expected.
+	 */
+	kick_all_cpus_sync();
 }
 
 /*
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH] powerpc/defconfigs: Enable THP in pseries defconfig
From: Aneesh Kumar K.V @ 2014-03-15 11:29 UTC (permalink / raw)
  To: benh, paulus; +Cc: linuxppc-dev, Aneesh Kumar K.V

From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

We also set it to be enabled always. This helps in wider testing

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/powerpc/configs/pseries_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
index e9a8b4e0a0f6..82cf3f8721b9 100644
--- a/arch/powerpc/configs/pseries_defconfig
+++ b/arch/powerpc/configs/pseries_defconfig
@@ -353,3 +353,5 @@ CONFIG_CRYPTO_DEV_NX_ENCRYPT=m
 CONFIG_VIRTUALIZATION=y
 CONFIG_KVM_BOOK3S_64=m
 CONFIG_KVM_BOOK3S_64_HV=y
+CONFIG_TRANSPARENT_HUGEPAGE=y
+CONFIG_TRANSPARENT_HUGEPAGE_ALWAYS=y
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCH] powerpc/mm: Make sure a local_irq_disable prevent a parallel THP split
From: Rik van Riel @ 2014-03-15 19:25 UTC (permalink / raw)
  To: Aneesh Kumar K.V, benh, paulus; +Cc: linux-mm, linuxppc-dev, linux-kernel
In-Reply-To: <1394880478-770-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>

On 03/15/2014 06:47 AM, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> 
> We have generic code like the one in get_futex_key that assume that
> a local_irq_disable prevents a parallel THP split. Support that by
> adding a dummy smp call function after setting _PAGE_SPLITTING. Code
> paths like get_user_pages_fast still need to check for _PAGE_SPLITTING
> after disabling IRQ which indicate that a parallel THP splitting is
> ongoing. Now if they don't find _PAGE_SPLITTING set, then we can be
> sure that parallel split will now block in pmdp_splitting flush
> until we enables IRQ
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Acked-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

^ permalink raw reply

* Re: [PATCH 10/10] Revert "powerpc/watchdog: Don't enable interrupt on PPC64 BookE"
From: Wim Van Sebroeck @ 2014-03-15 19:51 UTC (permalink / raw)
  To: Scott Wood; +Cc: Laurentiu Tudor, linuxppc-dev, Tiejun Chen
In-Reply-To: <1394755249-8856-11-git-send-email-scottwood@freescale.com>

Hi Scott,

> This reverts commit 3978bdb4ed653342b0be66c031bf61b72cc55d60, now that
> critical interrupts are properly supported on ppc64 booke.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> Cc: Laurentiu Tudor <Laurentiu.Tudor@freescale.com>
> Cc: Wim Van Sebroeck <wim@iguana.be>
> ---
>  drivers/watchdog/booke_wdt.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/watchdog/booke_wdt.c b/drivers/watchdog/booke_wdt.c
> index f1b8d55..a8dbceb3 100644
> --- a/drivers/watchdog/booke_wdt.c
> +++ b/drivers/watchdog/booke_wdt.c
> @@ -138,14 +138,6 @@ static void __booke_wdt_enable(void *data)
>  	val &= ~WDTP_MASK;
>  	val |= (TCR_WIE|TCR_WRC(WRC_CHIP)|WDTP(booke_wdt_period));
>  
> -#ifdef CONFIG_PPC_BOOK3E_64
> -	/*
> -	 * Crit ints are currently broken on PPC64 Book-E, so
> -	 * just disable them for now.
> -	 */
> -	val &= ~TCR_WIE;
> -#endif
> -
>  	mtspr(SPRN_TCR, val);
>  }
>  

Patch has been added to linux-watchdog-next.

Kind regards,
Wim.

^ permalink raw reply

* [PATCH] powerpc: Use default set of netfilter modules (CONFIG_NETFILTER_ADVANCED=n)
From: Anton Blanchard @ 2014-03-16  4:04 UTC (permalink / raw)
  To: benh, paulus; +Cc: linuxppc-dev


Our netfilter options are stale and important things like masquerading
are no longer enabled. Instead of trying to keep up with any updates,
set CONFIG_NETFILTER_ADVANCED=n on ppc64* and pseries* defconfigs.
This enables the most common netfilter modules for us.

While here, enable the network bridge module which is heavily used in
KVM setups.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index e015896..f26b267 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -73,74 +73,8 @@ CONFIG_INET_ESP=m
 CONFIG_INET_IPCOMP=m
 # CONFIG_IPV6 is not set
 CONFIG_NETFILTER=y
-CONFIG_NF_CONNTRACK=m
-CONFIG_NF_CONNTRACK_EVENTS=y
-CONFIG_NF_CT_PROTO_SCTP=m
-CONFIG_NF_CONNTRACK_AMANDA=m
-CONFIG_NF_CONNTRACK_FTP=m
-CONFIG_NF_CONNTRACK_H323=m
-CONFIG_NF_CONNTRACK_IRC=m
-CONFIG_NF_CONNTRACK_NETBIOS_NS=m
-CONFIG_NF_CONNTRACK_PPTP=m
-CONFIG_NF_CONNTRACK_SIP=m
-CONFIG_NF_CONNTRACK_TFTP=m
-CONFIG_NF_CT_NETLINK=m
-CONFIG_NETFILTER_XT_TARGET_CLASSIFY=m
-CONFIG_NETFILTER_XT_TARGET_CONNMARK=m
-CONFIG_NETFILTER_XT_TARGET_DSCP=m
-CONFIG_NETFILTER_XT_TARGET_MARK=m
-CONFIG_NETFILTER_XT_TARGET_NFLOG=m
-CONFIG_NETFILTER_XT_TARGET_NFQUEUE=m
-CONFIG_NETFILTER_XT_TARGET_TPROXY=m
-CONFIG_NETFILTER_XT_TARGET_TRACE=m
-CONFIG_NETFILTER_XT_TARGET_TCPMSS=m
-CONFIG_NETFILTER_XT_TARGET_TCPOPTSTRIP=m
-CONFIG_NETFILTER_XT_MATCH_COMMENT=m
-CONFIG_NETFILTER_XT_MATCH_CONNBYTES=m
-CONFIG_NETFILTER_XT_MATCH_CONNLIMIT=m
-CONFIG_NETFILTER_XT_MATCH_CONNMARK=m
-CONFIG_NETFILTER_XT_MATCH_CONNTRACK=m
-CONFIG_NETFILTER_XT_MATCH_DCCP=m
-CONFIG_NETFILTER_XT_MATCH_DSCP=m
-CONFIG_NETFILTER_XT_MATCH_ESP=m
-CONFIG_NETFILTER_XT_MATCH_HASHLIMIT=m
-CONFIG_NETFILTER_XT_MATCH_HELPER=m
-CONFIG_NETFILTER_XT_MATCH_IPRANGE=m
-CONFIG_NETFILTER_XT_MATCH_LENGTH=m
-CONFIG_NETFILTER_XT_MATCH_LIMIT=m
-CONFIG_NETFILTER_XT_MATCH_MAC=m
-CONFIG_NETFILTER_XT_MATCH_MARK=m
-CONFIG_NETFILTER_XT_MATCH_MULTIPORT=m
-CONFIG_NETFILTER_XT_MATCH_OWNER=m
-CONFIG_NETFILTER_XT_MATCH_POLICY=m
-CONFIG_NETFILTER_XT_MATCH_PKTTYPE=m
-CONFIG_NETFILTER_XT_MATCH_QUOTA=m
-CONFIG_NETFILTER_XT_MATCH_RATEEST=m
-CONFIG_NETFILTER_XT_MATCH_REALM=m
-CONFIG_NETFILTER_XT_MATCH_RECENT=m
-CONFIG_NETFILTER_XT_MATCH_SCTP=m
-CONFIG_NETFILTER_XT_MATCH_SOCKET=m
-CONFIG_NETFILTER_XT_MATCH_STATE=m
-CONFIG_NETFILTER_XT_MATCH_STATISTIC=m
-CONFIG_NETFILTER_XT_MATCH_STRING=m
-CONFIG_NETFILTER_XT_MATCH_TCPMSS=m
-CONFIG_NETFILTER_XT_MATCH_U32=m
-CONFIG_NF_CONNTRACK_IPV4=m
-CONFIG_IP_NF_IPTABLES=m
-CONFIG_IP_NF_MATCH_AH=m
-CONFIG_IP_NF_MATCH_ECN=m
-CONFIG_IP_NF_MATCH_TTL=m
-CONFIG_IP_NF_FILTER=m
-CONFIG_IP_NF_TARGET_REJECT=m
-CONFIG_IP_NF_TARGET_ULOG=m
-CONFIG_IP_NF_MANGLE=m
-CONFIG_IP_NF_TARGET_CLUSTERIP=m
-CONFIG_IP_NF_TARGET_ECN=m
-CONFIG_IP_NF_TARGET_TTL=m
-CONFIG_IP_NF_RAW=m
-CONFIG_IP_NF_ARPTABLES=m
-CONFIG_IP_NF_ARPFILTER=m
-CONFIG_IP_NF_ARP_MANGLE=m
+# CONFIG_NETFILTER_ADVANCED is not set
+CONFIG_BRIDGE=m
 CONFIG_BPF_JIT=y
 CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
diff --git a/arch/powerpc/configs/ppc64e_defconfig b/arch/powerpc/configs/ppc64e_defconfig
index f627fda..438e813 100644
--- a/arch/powerpc/configs/ppc64e_defconfig
+++ b/arch/powerpc/configs/ppc64e_defconfig
@@ -48,74 +48,8 @@ CONFIG_INET_ESP=m
 CONFIG_INET_IPCOMP=m
 # CONFIG_IPV6 is not set
 CONFIG_NETFILTER=y
-CONFIG_NF_CONNTRACK=m
-CONFIG_NF_CONNTRACK_EVENTS=y
-CONFIG_NF_CT_PROTO_SCTP=m
-CONFIG_NF_CONNTRACK_AMANDA=m
-CONFIG_NF_CONNTRACK_FTP=m
-CONFIG_NF_CONNTRACK_H323=m
-CONFIG_NF_CONNTRACK_IRC=m
-CONFIG_NF_CONNTRACK_NETBIOS_NS=m
-CONFIG_NF_CONNTRACK_PPTP=m
-CONFIG_NF_CONNTRACK_SIP=m
-CONFIG_NF_CONNTRACK_TFTP=m
-CONFIG_NF_CT_NETLINK=m
-CONFIG_NETFILTER_XT_TARGET_CLASSIFY=m
-CONFIG_NETFILTER_XT_TARGET_CONNMARK=m
-CONFIG_NETFILTER_XT_TARGET_DSCP=m
-CONFIG_NETFILTER_XT_TARGET_MARK=m
-CONFIG_NETFILTER_XT_TARGET_NFLOG=m
-CONFIG_NETFILTER_XT_TARGET_NFQUEUE=m
-CONFIG_NETFILTER_XT_TARGET_TPROXY=m
-CONFIG_NETFILTER_XT_TARGET_TRACE=m
-CONFIG_NETFILTER_XT_TARGET_TCPMSS=m
-CONFIG_NETFILTER_XT_TARGET_TCPOPTSTRIP=m
-CONFIG_NETFILTER_XT_MATCH_COMMENT=m
-CONFIG_NETFILTER_XT_MATCH_CONNBYTES=m
-CONFIG_NETFILTER_XT_MATCH_CONNLIMIT=m
-CONFIG_NETFILTER_XT_MATCH_CONNMARK=m
-CONFIG_NETFILTER_XT_MATCH_CONNTRACK=m
-CONFIG_NETFILTER_XT_MATCH_DCCP=m
-CONFIG_NETFILTER_XT_MATCH_DSCP=m
-CONFIG_NETFILTER_XT_MATCH_ESP=m
-CONFIG_NETFILTER_XT_MATCH_HASHLIMIT=m
-CONFIG_NETFILTER_XT_MATCH_HELPER=m
-CONFIG_NETFILTER_XT_MATCH_IPRANGE=m
-CONFIG_NETFILTER_XT_MATCH_LENGTH=m
-CONFIG_NETFILTER_XT_MATCH_LIMIT=m
-CONFIG_NETFILTER_XT_MATCH_MAC=m
-CONFIG_NETFILTER_XT_MATCH_MARK=m
-CONFIG_NETFILTER_XT_MATCH_MULTIPORT=m
-CONFIG_NETFILTER_XT_MATCH_OWNER=m
-CONFIG_NETFILTER_XT_MATCH_POLICY=m
-CONFIG_NETFILTER_XT_MATCH_PKTTYPE=m
-CONFIG_NETFILTER_XT_MATCH_QUOTA=m
-CONFIG_NETFILTER_XT_MATCH_RATEEST=m
-CONFIG_NETFILTER_XT_MATCH_REALM=m
-CONFIG_NETFILTER_XT_MATCH_RECENT=m
-CONFIG_NETFILTER_XT_MATCH_SCTP=m
-CONFIG_NETFILTER_XT_MATCH_SOCKET=m
-CONFIG_NETFILTER_XT_MATCH_STATE=m
-CONFIG_NETFILTER_XT_MATCH_STATISTIC=m
-CONFIG_NETFILTER_XT_MATCH_STRING=m
-CONFIG_NETFILTER_XT_MATCH_TCPMSS=m
-CONFIG_NETFILTER_XT_MATCH_U32=m
-CONFIG_NF_CONNTRACK_IPV4=m
-CONFIG_IP_NF_IPTABLES=m
-CONFIG_IP_NF_MATCH_AH=m
-CONFIG_IP_NF_MATCH_ECN=m
-CONFIG_IP_NF_MATCH_TTL=m
-CONFIG_IP_NF_FILTER=m
-CONFIG_IP_NF_TARGET_REJECT=m
-CONFIG_IP_NF_TARGET_ULOG=m
-CONFIG_IP_NF_MANGLE=m
-CONFIG_IP_NF_TARGET_CLUSTERIP=m
-CONFIG_IP_NF_TARGET_ECN=m
-CONFIG_IP_NF_TARGET_TTL=m
-CONFIG_IP_NF_RAW=m
-CONFIG_IP_NF_ARPTABLES=m
-CONFIG_IP_NF_ARPFILTER=m
-CONFIG_IP_NF_ARP_MANGLE=m
+# CONFIG_NETFILTER_ADVANCED is not set
+CONFIG_BRIDGE=m
 CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
index e9a8b4e..cb57d99 100644
--- a/arch/powerpc/configs/pseries_defconfig
+++ b/arch/powerpc/configs/pseries_defconfig
@@ -65,57 +65,8 @@ CONFIG_INET_ESP=m
 CONFIG_INET_IPCOMP=m
 # CONFIG_IPV6 is not set
 CONFIG_NETFILTER=y
-CONFIG_NF_CONNTRACK=m
-CONFIG_NF_CONNTRACK_EVENTS=y
-CONFIG_NF_CT_PROTO_UDPLITE=m
-CONFIG_NF_CONNTRACK_FTP=m
-CONFIG_NF_CONNTRACK_IRC=m
-CONFIG_NF_CONNTRACK_TFTP=m
-CONFIG_NF_CT_NETLINK=m
-CONFIG_NETFILTER_XT_TARGET_CLASSIFY=m
-CONFIG_NETFILTER_XT_TARGET_CONNMARK=m
-CONFIG_NETFILTER_XT_TARGET_MARK=m
-CONFIG_NETFILTER_XT_TARGET_NFLOG=m
-CONFIG_NETFILTER_XT_TARGET_NFQUEUE=m
-CONFIG_NETFILTER_XT_TARGET_TCPMSS=m
-CONFIG_NETFILTER_XT_MATCH_COMMENT=m
-CONFIG_NETFILTER_XT_MATCH_CONNBYTES=m
-CONFIG_NETFILTER_XT_MATCH_CONNLIMIT=m
-CONFIG_NETFILTER_XT_MATCH_CONNMARK=m
-CONFIG_NETFILTER_XT_MATCH_CONNTRACK=m
-CONFIG_NETFILTER_XT_MATCH_DCCP=m
-CONFIG_NETFILTER_XT_MATCH_DSCP=m
-CONFIG_NETFILTER_XT_MATCH_ESP=m
-CONFIG_NETFILTER_XT_MATCH_HASHLIMIT=m
-CONFIG_NETFILTER_XT_MATCH_HELPER=m
-CONFIG_NETFILTER_XT_MATCH_IPRANGE=m
-CONFIG_NETFILTER_XT_MATCH_LENGTH=m
-CONFIG_NETFILTER_XT_MATCH_LIMIT=m
-CONFIG_NETFILTER_XT_MATCH_MAC=m
-CONFIG_NETFILTER_XT_MATCH_MARK=m
-CONFIG_NETFILTER_XT_MATCH_MULTIPORT=m
-CONFIG_NETFILTER_XT_MATCH_OWNER=m
-CONFIG_NETFILTER_XT_MATCH_POLICY=m
-CONFIG_NETFILTER_XT_MATCH_PKTTYPE=m
-CONFIG_NETFILTER_XT_MATCH_QUOTA=m
-CONFIG_NETFILTER_XT_MATCH_RATEEST=m
-CONFIG_NETFILTER_XT_MATCH_REALM=m
-CONFIG_NETFILTER_XT_MATCH_RECENT=m
-CONFIG_NETFILTER_XT_MATCH_SCTP=m
-CONFIG_NETFILTER_XT_MATCH_STATE=m
-CONFIG_NETFILTER_XT_MATCH_STATISTIC=m
-CONFIG_NETFILTER_XT_MATCH_STRING=m
-CONFIG_NETFILTER_XT_MATCH_TCPMSS=m
-CONFIG_NETFILTER_XT_MATCH_TIME=m
-CONFIG_NETFILTER_XT_MATCH_U32=m
-CONFIG_NF_CONNTRACK_IPV4=m
-CONFIG_IP_NF_IPTABLES=m
-CONFIG_IP_NF_MATCH_AH=m
-CONFIG_IP_NF_MATCH_ECN=m
-CONFIG_IP_NF_MATCH_TTL=m
-CONFIG_IP_NF_FILTER=m
-CONFIG_IP_NF_TARGET_REJECT=m
-CONFIG_IP_NF_TARGET_ULOG=m
+# CONFIG_NETFILTER_ADVANCED is not set
+CONFIG_BRIDGE=m
 CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
diff --git a/arch/powerpc/configs/pseries_le_defconfig b/arch/powerpc/configs/pseries_le_defconfig
index 62771e0..3c84f9d 100644
--- a/arch/powerpc/configs/pseries_le_defconfig
+++ b/arch/powerpc/configs/pseries_le_defconfig
@@ -67,57 +67,8 @@ CONFIG_INET_ESP=m
 CONFIG_INET_IPCOMP=m
 # CONFIG_IPV6 is not set
 CONFIG_NETFILTER=y
-CONFIG_NF_CONNTRACK=m
-CONFIG_NF_CONNTRACK_EVENTS=y
-CONFIG_NF_CT_PROTO_UDPLITE=m
-CONFIG_NF_CONNTRACK_FTP=m
-CONFIG_NF_CONNTRACK_IRC=m
-CONFIG_NF_CONNTRACK_TFTP=m
-CONFIG_NF_CT_NETLINK=m
-CONFIG_NETFILTER_XT_TARGET_CLASSIFY=m
-CONFIG_NETFILTER_XT_TARGET_CONNMARK=m
-CONFIG_NETFILTER_XT_TARGET_MARK=m
-CONFIG_NETFILTER_XT_TARGET_NFLOG=m
-CONFIG_NETFILTER_XT_TARGET_NFQUEUE=m
-CONFIG_NETFILTER_XT_TARGET_TCPMSS=m
-CONFIG_NETFILTER_XT_MATCH_COMMENT=m
-CONFIG_NETFILTER_XT_MATCH_CONNBYTES=m
-CONFIG_NETFILTER_XT_MATCH_CONNLIMIT=m
-CONFIG_NETFILTER_XT_MATCH_CONNMARK=m
-CONFIG_NETFILTER_XT_MATCH_CONNTRACK=m
-CONFIG_NETFILTER_XT_MATCH_DCCP=m
-CONFIG_NETFILTER_XT_MATCH_DSCP=m
-CONFIG_NETFILTER_XT_MATCH_ESP=m
-CONFIG_NETFILTER_XT_MATCH_HASHLIMIT=m
-CONFIG_NETFILTER_XT_MATCH_HELPER=m
-CONFIG_NETFILTER_XT_MATCH_IPRANGE=m
-CONFIG_NETFILTER_XT_MATCH_LENGTH=m
-CONFIG_NETFILTER_XT_MATCH_LIMIT=m
-CONFIG_NETFILTER_XT_MATCH_MAC=m
-CONFIG_NETFILTER_XT_MATCH_MARK=m
-CONFIG_NETFILTER_XT_MATCH_MULTIPORT=m
-CONFIG_NETFILTER_XT_MATCH_OWNER=m
-CONFIG_NETFILTER_XT_MATCH_POLICY=m
-CONFIG_NETFILTER_XT_MATCH_PKTTYPE=m
-CONFIG_NETFILTER_XT_MATCH_QUOTA=m
-CONFIG_NETFILTER_XT_MATCH_RATEEST=m
-CONFIG_NETFILTER_XT_MATCH_REALM=m
-CONFIG_NETFILTER_XT_MATCH_RECENT=m
-CONFIG_NETFILTER_XT_MATCH_SCTP=m
-CONFIG_NETFILTER_XT_MATCH_STATE=m
-CONFIG_NETFILTER_XT_MATCH_STATISTIC=m
-CONFIG_NETFILTER_XT_MATCH_STRING=m
-CONFIG_NETFILTER_XT_MATCH_TCPMSS=m
-CONFIG_NETFILTER_XT_MATCH_TIME=m
-CONFIG_NETFILTER_XT_MATCH_U32=m
-CONFIG_NF_CONNTRACK_IPV4=m
-CONFIG_IP_NF_IPTABLES=m
-CONFIG_IP_NF_MATCH_AH=m
-CONFIG_IP_NF_MATCH_ECN=m
-CONFIG_IP_NF_MATCH_TTL=m
-CONFIG_IP_NF_FILTER=m
-CONFIG_IP_NF_TARGET_REJECT=m
-CONFIG_IP_NF_TARGET_ULOG=m
+# CONFIG_NETFILTER_ADVANCED is not set
+CONFIG_BRIDGE=m
 CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y

^ permalink raw reply related

* kvmppc_wait_for_nap warnings and KVM host+guest hang on 3.14-rc6*
From: Anton Blanchard @ 2014-03-16  4:44 UTC (permalink / raw)
  To: paulus, benh, mikey, agraf, aneesh.kumar; +Cc: linuxppc-dev


Hi,

I was testing KVM on an upstream kernel (3.14.0-rc6-00145-ga4ecdf8)
with Gregory's emulated MMIO fixes.

I ran with 16 cores and 4 threads (matches the host):

qemu-system-ppc64 -enable-kvm -smp cores=16,threads=4 -m 32G -M pseries -cpu POWER7 -nographic -nodefaults -monitor stdio -serial pty -drive file=XXX -netdev bridge,br=br0,id=net0,helper=/usr/libexec/qemu-bridge-helper -device virtio-net-pci,netdev=net0

Note that in kernel XICS acceleration wasn't enabled (perhaps it should
be the default, it isn't right now):

qemu-system-ppc64: KVM and IRQ_XICS capability must be present for in-kernel XICS

During boot I see processors stuck in the guest:

[   16.145166] Processor 1 is stuck.

And thousands of:

kvmppc_wait_for_nap timeout 0 1
kvmppc_wait_for_nap timeout 0 1
kvmppc_wait_for_nap timeout 0 1
kvmppc_wait_for_nap timeout 0 1

And eventually:

INFO: rcu_sched self-detected stall on CPU { 52}  (t=10170 jiffies g=765 c=764 q=127)
CPU: 52 PID: 11833 Comm: qemu-system-ppc Not tainted 3.14.0-rc6-00145-ga4ecdf8-dirty #10
Call Trace:
[c000000765caad20] [c000000000015f3c] .show_stack+0x7c/0x1f0 (unreliable)
[c000000765caadf0] [c00000000082d83c] .dump_stack+0x88/0xb4
[c000000765caae70] [c0000000000f7cf0] .rcu_check_callbacks+0x5b0/0x950
[c000000765caafa0] [c00000000009c860] .update_process_times+0x50/0xa0
[c000000765cab030] [c000000000103c40] .tick_sched_handle.isra.16+0x20/0xa0
[c000000765cab0b0] [c000000000103d1c] .tick_sched_timer+0x5c/0xa0
[c000000765cab150] [c0000000000ba0f8] .__run_hrtimer+0x98/0x260
[c000000765cab1f0] [c0000000000baef8] .hrtimer_interrupt+0x138/0x320
[c000000765cab300] [c00000000001e430] .timer_interrupt+0x100/0x2f0
[c000000765cab3b0] [c00000000000a8d8] restore_check_irq_replay+0x40/0x5c
--- Exception: 901 at .__kvmppc_vcore_entry+0x140/0x1ac [kvm_hv]
    LR = kvmppc_call_hv_entry+0x8/0xe8
[c000000765cab6a0] [d000000006394f74] .__kvmppc_vcore_entry+0x140/0x1ac [kvm_hv] (unreliable)
[c000000765cab870] [d000000006394148] .kvmppc_vcpu_run_hv+0x8b8/0x1520 [kvm_hv]
[c000000765cab9f0] [d000000005e5c250] .kvmppc_vcpu_run+0x30/0x50 [kvm]
[c000000765caba60] [d000000005e599f4] .kvm_arch_vcpu_ioctl_run+0x54/0x1b0 [kvm]INFO: rcu_sched detected stalls on CPUs/tasks: { 52} (detected by 60, t=10170 jiffies, g=765, c=764, q=127)
Task dump for CPU 52:
qemu-system-ppc R  running task     9184 11833  11817 0x00000084
Call Trace:
[c000000765cab2f0] [c000000765cab3b0] 0xc000000765cab3b0 (unreliable)
[c000000765cab420] [c000000000d518e8] kexec_stack+0x18e8/0x10000
[c000000765cabaf0] [d000000005e54588] .kvm_vcpu_ioctl+0x478/0x740 [kvm]
[c000000765cabcb0] [c0000000001ffcd4] .do_vfs_ioctl+0x4a4/0x760
[c000000765cabd90] [c0000000001fffe8] .SyS_ioctl+0x58/0xb0
[c000000765cabe30] [c00000000000a158] syscall_exit+0x0/0x98

BUG: soft lockup - CPU#52 stuck for 97s! [qemu-system-ppc:11833]
Modules linked in: tun xt_conntrack ipt_MASQUERADE iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_filter ip_tables x_tables bridge stp llc kvm_hv powernv_rng rng_core kvm binfmt_misc nfsd exportfs autofs4
CPU: 52 PID: 11833 Comm: qemu-system-ppc Not tainted 3.14.0-rc6-00145-ga4ecdf8-dirty #10
task: c00000078f7af860 ti: c000000765ca8000 task.ti: c000000765ca8000
NIP: d000000006394f74 LR: c00000000007a1dc CTR: c00000000007a1a0
REGS: c000000765cab420 TRAP: 0901   Not tainted  (3.14.0-rc6-00145-ga4ecdf8-dirty)
MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI>  CR: 24002824  XER: 20000000
CFAR: c00000000007a2b4 SOFTE: 1 
GPR00: c00000000007a1dc c000000765cab6a0 c000000000d518e8 000000aae8fb0387 
GPR04: fffffff3dbd12408 0000000000000000 0000000000000007 9000000000009032 
GPR08: d000000006394f74 c00000078b880000 9000000000001032 c00000000007a1a0 
GPR12: 0000000000000500 c000000001e0d000 
NIP [d000000006394f74] .__kvmppc_vcore_entry+0x140/0x1ac [kvm_hv]
LR [c00000000007a1dc] kvmppc_call_hv_entry+0x8/0xe8
Call Trace:
[c000000765cab6a0] [d000000006394f74] .__kvmppc_vcore_entry+0x140/0x1ac [kvm_hv] (unreliable)
[c000000765cab870] [d000000006394148] .kvmppc_vcpu_run_hv+0x8b8/0x1520 [kvm_hv]
[c000000765cab9f0] [d000000005e5c250] .kvmppc_vcpu_run+0x30/0x50 [kvm]
[c000000765caba60] [d000000005e599f4] .kvm_arch_vcpu_ioctl_run+0x54/0x1b0 [kvm]
[c000000765cabaf0] [d000000005e54588] .kvm_vcpu_ioctl+0x478/0x740 [kvm]
[c000000765cabcb0] [c0000000001ffcd4] .do_vfs_ioctl+0x4a4/0x760
[c000000765cabd90] [c0000000001fffe8] .SyS_ioctl+0x58/0xb0
[c000000765cabe30] [c00000000000a158] syscall_exit+0x0/0x98
Instruction dump:
7d083a14 f90d03c0 60000000 60000000 60000000 60000000 60000000 60000000 
60000000 60000000 60000000 48003af1 <e8410028> e9c100e0 e9e100e8 ea0100f0 

Anton

^ permalink raw reply

* Re: [PATCH 9/9] powerpc/pm: support deep sleep feature on T1040
From: Kevin Hao @ 2014-03-16  4:58 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Chenhui Zhao, Jason.Jin, linux-kernel
In-Reply-To: <1394835987.12479.119.camel@snotra.buserror.net>

[-- Attachment #1: Type: text/plain, Size: 6201 bytes --]

On Fri, Mar 14, 2014 at 05:26:27PM -0500, Scott Wood wrote:
> On Thu, 2014-03-13 at 15:46 +0800, Kevin Hao wrote:
> > On Wed, Mar 12, 2014 at 12:43:05PM -0500, Scott Wood wrote:
> > > > Shouldn't we use "readback, sync" here? The following is quoted form t4240RM:
> > > >   To guarantee that the results of any sequence of writes to configuration
> > > >   registers are in effect, the final configuration register write should be
> > > >   immediately followed by a read of the same register, and that should be
> > > >   followed by a SYNC instruction. Then accesses can safely be made to memory
> > > >   regions affected by the configuration register write.
> > > 
> > > I agree that the sync before the readback is probably not necessary,
> > > since transactions to the same address should already be ordered.
> > > 
> > > A sync after the readback helps if you're trying to order the readback
> > > with subsequent memory accesses, though in that case wouldn't a sync
> > > alone (no readback) be adequate?
> > 
> > No, we don't just want to order the subsequent memory access here.
> > The 'write, readback, sync' is the required sequence if we want to make
> > sure that the writing to CCSR register does really take effect.
> > 
> > >  Though maybe not always -- see the
> > > comment near the end of fsl_elbc_write_buf() in
> > > drivers/mtd/nand_fsl_elbc.c.  I guess the readback does more than just
> > > make sure the device has seen the write, ensuring that the device has
> > > finished the transaction to the point of acting on another one.
> > 
> > Agree.
> > 
> > > 
> > > The data dependency plus isync sequence, which is done by the normal I/O
> > > accessors used from C code, orders the readback versus all future
> > > instructions (not just I/O).  The delay loop is not I/O.
> > 
> > According to the PowerISA, the sequence 'load, date dependency, isync' only
> > order the load accesses. 
> 
> The point is to order the delay loop after the load, not to order
> storage versus storage.

I think the point is to make sure that the writing of the CCSR_DDR_SDRAM_CFG_2
does really take effect before we begin to delay loop. The sequence "write,
readback, sync" will guarantee this according to the manual. If we just want to
order the delay loop after the load, the following sequence should be enough:
	store to CCSR_DDR_SDRAM_CFG_2
	load from CCSR_DDR_SDRAM_CFG_2
	isync or sync
	delay loop

Why do we need the 'date dependency' here? According to the e6500 manual, the
instructions can execute out of order, but complete in order. So I am really
wondering why we need to order the load and the following delay loop if there
is no intention to order the storage access? 

> 
> This is a sequence we're already using on all of our I/O loads
> (excluding accesses like in this patch that don't use the standard
> accessors).  I'm confident that it works even if it's not
> architecturally guaranteed.

This sequence is used to order the load and the followed storage access.
And this is guaranteed by the architecture. But I don't think it is suitable
for a case like this. The following is quoted from PowerISA:
  Because stores cannot be performed “out-of-order”
  (see Book III), if a Store instruction depends on the
  value returned by a preceding Load instruction
  (because the value returned by the Load is used to
  compute either the effective address specified by the
  Store or the value to be stored), the corresponding stor-
  age accesses are performed in program order. The
  same applies if whether the Store instruction is exe-
  cuted depends on a conditional Branch instruction that
  in turn depends on the value returned by a preceding
  Load instruction.
  
  Because an isync instruction prevents the execution of
  instructions following the isync until instructions pre-
  ceding the isync have completed, if an isync follows a
  conditional Branch instruction that depends on the
  value returned by a preceding Load instruction, the
  load on which the Branch depends is performed before
  any loads caused by instructions following the isync.

I think the above description would guarantee that the load will be performed
before any storage access (both load and store) following the isync in the
following scenario:
	lwz	r4, 0(r3)
	twi	0, r4, 0
	isync

>  I'm not sure that there exists a clear
> architectural way of synchronizing non-storage instructions relative to
> storage instructions.

Isn't what the execution synchronization instructions such as sync, isync, mtmsr
do?

> 
> Given that isync is documented as preventing any execution of
> instructions after the isync until all previous instructions complete,
> it doesn't seem to make sense for the architecture to explicitly talk
> about loads (as opposed to any other instruction) following a load,
> dependent conditional branch, isync sequence.

Sorry, I didn't get what you mean.

> 
> > So if we want to order all the storage access as well
> > as execution synchronization, we should choose sync here.
> 
> Do we need execution synchronization or context synchronization?

There is no context-altering instruction here, so I think an execution
synchronizing instruction should be enough here.

> 
> The t4240 RM section that talks about a readback and a sync is in the
> context of subsequent memory operations ("Then accesses can safely be
> made to memory regions affected..."), not arbitrary instructions.

I assume that this sequence also guarantee that the writing does take effect.

>  There
> are also a couple other places in the RM where isync is recommended
> instead (when setting LAWs or CCSRBAR), even though those also only
> involve memory accesses.

No idea why isync is used here.

> 
> In any case, this is not performance critical and thus it's better to
> oversynchronize than undersynchronize.

On the contrary I think that sync is oversynchronize instead of
undersynchronize. It not only provide the execution synchronizing but also
order all the storage accesses. That is why I prefer the sync. :-)

Thanks,
Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH 10/10] Revert "powerpc/watchdog: Don't enable interrupt on PPC64 BookE"
From: Scott Wood @ 2014-03-16  8:07 UTC (permalink / raw)
  To: Wim Van Sebroeck; +Cc: Laurentiu Tudor, linuxppc-dev, Tiejun Chen
In-Reply-To: <20140315195137.GA14681@spo001.leaseweb.com>

On Sat, 2014-03-15 at 20:51 +0100, Wim Van Sebroeck wrote:
> Hi Scott,
> 
> > This reverts commit 3978bdb4ed653342b0be66c031bf61b72cc55d60, now that
> > critical interrupts are properly supported on ppc64 booke.
> > 
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> > Cc: Laurentiu Tudor <Laurentiu.Tudor@freescale.com>
> > Cc: Wim Van Sebroeck <wim@iguana.be>
> > ---
> >  drivers/watchdog/booke_wdt.c | 8 --------
> >  1 file changed, 8 deletions(-)
> > 
> > diff --git a/drivers/watchdog/booke_wdt.c b/drivers/watchdog/booke_wdt.c
> > index f1b8d55..a8dbceb3 100644
> > --- a/drivers/watchdog/booke_wdt.c
> > +++ b/drivers/watchdog/booke_wdt.c
> > @@ -138,14 +138,6 @@ static void __booke_wdt_enable(void *data)
> >  	val &= ~WDTP_MASK;
> >  	val |= (TCR_WIE|TCR_WRC(WRC_CHIP)|WDTP(booke_wdt_period));
> >  
> > -#ifdef CONFIG_PPC_BOOK3E_64
> > -	/*
> > -	 * Crit ints are currently broken on PPC64 Book-E, so
> > -	 * just disable them for now.
> > -	 */
> > -	val &= ~TCR_WIE;
> > -#endif
> > -
> >  	mtspr(SPRN_TCR, val);
> >  }
> >  
> 
> Patch has been added to linux-watchdog-next.

Please unapply it.  It is patch 10/10 and depends on the previous parts
of the patchset to make critical interrupts work properly.

-Scott

^ permalink raw reply

* Re: kvmppc_wait_for_nap warnings and KVM host+guest hang on 3.14-rc6*
From: Paul Mackerras @ 2014-03-16 11:00 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: mikey, linuxppc-dev, agraf, aneesh.kumar
In-Reply-To: <20140316154400.406537e1@kryten>

On Sun, Mar 16, 2014 at 03:44:00PM +1100, Anton Blanchard wrote:
> 
> I was testing KVM on an upstream kernel (3.14.0-rc6-00145-ga4ecdf8)
> with Gregory's emulated MMIO fixes.
> 
> I ran with 16 cores and 4 threads (matches the host):
> 
> qemu-system-ppc64 -enable-kvm -smp cores=16,threads=4 -m 32G -M pseries -cpu POWER7 -nographic -nodefaults -monitor stdio -serial pty -drive file=XXX -netdev bridge,br=br0,id=net0,helper=/usr/libexec/qemu-bridge-helper -device virtio-net-pci,netdev=net0
> 
> Note that in kernel XICS acceleration wasn't enabled (perhaps it should
> be the default, it isn't right now):
> 
> qemu-system-ppc64: KVM and IRQ_XICS capability must be present for in-kernel XICS
> 
> During boot I see processors stuck in the guest:
> 
> [   16.145166] Processor 1 is stuck.
> 
> And thousands of:
> 
> kvmppc_wait_for_nap timeout 0 1
> kvmppc_wait_for_nap timeout 0 1
> kvmppc_wait_for_nap timeout 0 1
> kvmppc_wait_for_nap timeout 0 1

You need the couple of patches I posted recently, which Paolo Bonzini
is hopefully going to get to Linus before 3.14 is released, and which
are in the "next" branch of the kvm tree:

KVM: PPC: Book3S HV: Remove bogus duplicate code
KVM: PPC: Book3S HV: Fix register usage when loading/saving VRSAVE

as well as the series that you have in your "le" branch and Alex has
in his "kvm-ppc-queue" branch:

PPC: KVM: introduce helper to check RESUME_GUEST and related
PPC: KVM: fix VCPU run for HV KVM (v2)
PPC: KVM: fix RESUME_GUEST check before ending CEDE in kvmppc_run_core()
PPC: KVM: fix RESUME_GUEST check before returning from kvmppc_run_core()

Paul.

^ permalink raw reply

* Re: kvmppc_wait_for_nap warnings and KVM host+guest hang on 3.14-rc6*
From: Aneesh Kumar K.V @ 2014-03-16 15:14 UTC (permalink / raw)
  To: Anton Blanchard, paulus, benh, mikey, agraf; +Cc: linuxppc-dev
In-Reply-To: <20140316154400.406537e1@kryten>

Anton Blanchard <anton@samba.org> writes:

> Hi,
>
> I was testing KVM on an upstream kernel (3.14.0-rc6-00145-ga4ecdf8)
> with Gregory's emulated MMIO fixes.
>
> I ran with 16 cores and 4 threads (matches the host):
>
> qemu-system-ppc64 -enable-kvm -smp cores=16,threads=4 -m 32G -M
> pseries -cpu POWER7 -nographic -nodefaults -monitor stdio -serial pty
> -drive file=XXX -netdev
> bridge,br=br0,id=net0,helper=/usr/libexec/qemu-bridge-helper -device
> virtio-net-pci,netdev=net0

>
> Note that in kernel XICS acceleration wasn't enabled (perhaps it should
> be the default, it isn't right now):
>
> qemu-system-ppc64: KVM and IRQ_XICS capability must be present for in-kernel XICS
>
> During boot I see processors stuck in the guest:

The hang during boot is mostly due to missing top 5 patches which you
can find below.

https://github.com/agraf/linux-2.6/commits/kvm-ppc-queue



>
> [   16.145166] Processor 1 is stuck.

I see that on host when i try to kexec without making all the cpu online
(ie not doing --smt=on). Haven't got around to looking at that.

>
> And thousands of:
>
> kvmppc_wait_for_nap timeout 0 1
> kvmppc_wait_for_nap timeout 0 1
> kvmppc_wait_for_nap timeout 0 1
> kvmppc_wait_for_nap timeout 0 1
>

I haven't see that error before.

> And eventually:
>
> INFO: rcu_sched self-detected stall on CPU { 52}  (t=10170 jiffies g=765 c=764 q=127)
> CPU: 52 PID: 11833 Comm: qemu-system-ppc Not tainted 3.14.0-rc6-00145-ga4ecdf8-dirty #10
> Call Trace:
> [c000000765caad20] [c000000000015f3c] .show_stack+0x7c/0x1f0 (unreliable)
> [c000000765caadf0] [c00000000082d83c] .dump_stack+0x88/0xb4
> [c000000765caae70] [c0000000000f7cf0] .rcu_check_callbacks+0x5b0/0x950
> [c000000765caafa0] [c00000000009c860] .update_process_times+0x50/0xa0
> [c000000765cab030] [c000000000103c40] .tick_sched_handle.isra.16+0x20/0xa0
> [c000000765cab0b0] [c000000000103d1c] .tick_sched_timer+0x5c/0xa0
> [c000000765cab150] [c0000000000ba0f8] .__run_hrtimer+0x98/0x260
> [c000000765cab1f0] [c0000000000baef8] .hrtimer_interrupt+0x138/0x320
> [c000000765cab300] [c00000000001e430] .timer_interrupt+0x100/0x2f0
> [c000000765cab3b0] [c00000000000a8d8] restore_check_irq_replay+0x40/0x5c
> --- Exception: 901 at .__kvmppc_vcore_entry+0x140/0x1ac [kvm_hv]
>     LR = kvmppc_call_hv_entry+0x8/0xe8
> [c000000765cab6a0] [d000000006394f74] .__kvmppc_vcore_entry+0x140/0x1ac [kvm_hv] (unreliable)
> [c000000765cab870] [d000000006394148] .kvmppc_vcpu_run_hv+0x8b8/0x1520 [kvm_hv]
> [c000000765cab9f0] [d000000005e5c250] .kvmppc_vcpu_run+0x30/0x50 [kvm]
> [c000000765caba60] [d000000005e599f4] .kvm_arch_vcpu_ioctl_run+0x54/0x1b0 [kvm]INFO: rcu_sched detected stalls on CPUs/tasks: { 52} (detected by 60, t=10170 jiffies, g=765, c=764, q=127)
> Task dump for CPU 52:
> qemu-system-ppc R  running task     9184 11833  11817 0x00000084
> Call Trace:
> [c000000765cab2f0] [c000000765cab3b0] 0xc000000765cab3b0 (unreliable)
> [c000000765cab420] [c000000000d518e8] kexec_stack+0x18e8/0x10000
> [c000000765cabaf0] [d000000005e54588] .kvm_vcpu_ioctl+0x478/0x740 [kvm]
> [c000000765cabcb0] [c0000000001ffcd4] .do_vfs_ioctl+0x4a4/0x760
> [c000000765cabd90] [c0000000001fffe8] .SyS_ioctl+0x58/0xb0
> [c000000765cabe30] [c00000000000a158] syscall_exit+0x0/0x98
>
> BUG: soft lockup - CPU#52 stuck for 97s! [qemu-system-ppc:11833]
> Modules linked in: tun xt_conntrack ipt_MASQUERADE iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_filter ip_tables x_tables bridge stp llc kvm_hv powernv_rng rng_core kvm binfmt_misc nfsd exportfs autofs4
> CPU: 52 PID: 11833 Comm: qemu-system-ppc Not tainted 3.14.0-rc6-00145-ga4ecdf8-dirty #10
> task: c00000078f7af860 ti: c000000765ca8000 task.ti: c000000765ca8000
> NIP: d000000006394f74 LR: c00000000007a1dc CTR: c00000000007a1a0
> REGS: c000000765cab420 TRAP: 0901   Not tainted  (3.14.0-rc6-00145-ga4ecdf8-dirty)
> MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI>  CR: 24002824  XER: 20000000
> CFAR: c00000000007a2b4 SOFTE: 1 
> GPR00: c00000000007a1dc c000000765cab6a0 c000000000d518e8 000000aae8fb0387 
> GPR04: fffffff3dbd12408 0000000000000000 0000000000000007 9000000000009032 
> GPR08: d000000006394f74 c00000078b880000 9000000000001032 c00000000007a1a0 
> GPR12: 0000000000000500 c000000001e0d000 
> NIP [d000000006394f74] .__kvmppc_vcore_entry+0x140/0x1ac [kvm_hv]
> LR [c00000000007a1dc] kvmppc_call_hv_entry+0x8/0xe8
> Call Trace:
> [c000000765cab6a0] [d000000006394f74] .__kvmppc_vcore_entry+0x140/0x1ac [kvm_hv] (unreliable)
> [c000000765cab870] [d000000006394148] .kvmppc_vcpu_run_hv+0x8b8/0x1520 [kvm_hv]
> [c000000765cab9f0] [d000000005e5c250] .kvmppc_vcpu_run+0x30/0x50 [kvm]
> [c000000765caba60] [d000000005e599f4] .kvm_arch_vcpu_ioctl_run+0x54/0x1b0 [kvm]
> [c000000765cabaf0] [d000000005e54588] .kvm_vcpu_ioctl+0x478/0x740 [kvm]
> [c000000765cabcb0] [c0000000001ffcd4] .do_vfs_ioctl+0x4a4/0x760
> [c000000765cabd90] [c0000000001fffe8] .SyS_ioctl+0x58/0xb0
> [c000000765cabe30] [c00000000000a158] syscall_exit+0x0/0x98
> Instruction dump:
> 7d083a14 f90d03c0 60000000 60000000 60000000 60000000 60000000 60000000 
> 60000000 60000000 60000000 48003af1 <e8410028> e9c100e0 e9e100e8 ea0100f0 
>
> Anton

^ permalink raw reply

* Re: [PATCH 10/10] Revert "powerpc/watchdog: Don't enable interrupt on PPC64 BookE"
From: Wim Van Sebroeck @ 2014-03-16 19:56 UTC (permalink / raw)
  To: Scott Wood; +Cc: Laurentiu Tudor, linuxppc-dev, Tiejun Chen
In-Reply-To: <1394957243.12479.155.camel@snotra.buserror.net>

Hi Scott,

> On Sat, 2014-03-15 at 20:51 +0100, Wim Van Sebroeck wrote:
> > Hi Scott,
> > 
> > > This reverts commit 3978bdb4ed653342b0be66c031bf61b72cc55d60, now that
> > > critical interrupts are properly supported on ppc64 booke.
> > > 
> > > Signed-off-by: Scott Wood <scottwood@freescale.com>
> > > Cc: Laurentiu Tudor <Laurentiu.Tudor@freescale.com>
> > > Cc: Wim Van Sebroeck <wim@iguana.be>
> > > ---
> > >  drivers/watchdog/booke_wdt.c | 8 --------
> > >  1 file changed, 8 deletions(-)
> > > 
> > > diff --git a/drivers/watchdog/booke_wdt.c b/drivers/watchdog/booke_wdt.c
> > > index f1b8d55..a8dbceb3 100644
> > > --- a/drivers/watchdog/booke_wdt.c
> > > +++ b/drivers/watchdog/booke_wdt.c
> > > @@ -138,14 +138,6 @@ static void __booke_wdt_enable(void *data)
> > >  	val &= ~WDTP_MASK;
> > >  	val |= (TCR_WIE|TCR_WRC(WRC_CHIP)|WDTP(booke_wdt_period));
> > >  
> > > -#ifdef CONFIG_PPC_BOOK3E_64
> > > -	/*
> > > -	 * Crit ints are currently broken on PPC64 Book-E, so
> > > -	 * just disable them for now.
> > > -	 */
> > > -	val &= ~TCR_WIE;
> > > -#endif
> > > -
> > >  	mtspr(SPRN_TCR, val);
> > >  }
> > >  
> > 
> > Patch has been added to linux-watchdog-next.
> 
> Please unapply it.  It is patch 10/10 and depends on the previous parts
> of the patchset to make critical interrupts work properly.

Unapplied.

Kind regards,
Wim.

^ permalink raw reply

* linux-next: manual merge of the powernv-cpuidle tree with the powerpc tree
From: Stephen Rothwell @ 2014-03-17  8:59 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev
  Cc: Stewart Smith, linux-next, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1751 bytes --]

Hi all,

Today's linux-next merge of the powernv-cpuidle tree got a conflict in
arch/powerpc/include/asm/opal.h between commit c7e64b9ce04a
("powerpc/powernv Platform dump interface") from the powerpc tree and
commit 97eb001f0349 ("powerpc/powernv: Add OPAL call to resync timebase
on wakeup") from the powernv-cpuidle tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc arch/powerpc/include/asm/opal.h
index 2636acfcd340,c71c72e47d47..000000000000
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@@ -159,15 -154,10 +159,16 @@@ extern int opal_enter_rtas(struct rtas_
  #define OPAL_FLASH_VALIDATE			76
  #define OPAL_FLASH_MANAGE			77
  #define OPAL_FLASH_UPDATE			78
+ #define OPAL_RESYNC_TIMEBASE			79
 +#define OPAL_DUMP_INIT				81
 +#define OPAL_DUMP_INFO				82
 +#define OPAL_DUMP_READ				83
 +#define OPAL_DUMP_ACK				84
  #define OPAL_GET_MSG				85
  #define OPAL_CHECK_ASYNC_COMPLETION		86
 +#define OPAL_DUMP_RESEND			91
  #define OPAL_SYNC_HOST_REBOOT			87
 +#define OPAL_DUMP_INFO2				94
  
  #ifndef __ASSEMBLY__
  
@@@ -888,13 -862,11 +889,14 @@@ extern void opal_get_rtc_time(struct rt
  extern unsigned long opal_get_boot_time(void);
  extern void opal_nvram_init(void);
  extern void opal_flash_init(void);
 +extern int opal_elog_init(void);
 +extern void opal_platform_dump_init(void);
  
  extern int opal_machine_check(struct pt_regs *regs);
 +extern bool opal_mce_check_early_recovery(struct pt_regs *regs);
  
  extern void opal_shutdown(void);
+ extern int opal_resync_timebase(void);
  
  extern void opal_lpc_init(void);
  

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* linux-next: manual merge of the powernv-cpuidle tree with the powerpc tree
From: Stephen Rothwell @ 2014-03-17  9:01 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev
  Cc: Stewart Smith, linux-next, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1500 bytes --]

Hi all,

Today's linux-next merge of the powernv-cpuidle tree got a conflict in
arch/powerpc/platforms/powernv/opal-wrappers.S between commit
c7e64b9ce04a ("powerpc/powernv Platform dump interface") from the powerpc
tree and commit 97eb001f0349 ("powerpc/powernv: Add OPAL call to resync
timebase on wakeup") from the powernv-cpuidle tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc arch/powerpc/platforms/powernv/opal-wrappers.S
index 47ec3f738062,aab54b60334f..000000000000
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@@ -131,12 -126,7 +131,13 @@@ OPAL_CALL(opal_write_elog,			OPAL_ELOG_
  OPAL_CALL(opal_validate_flash,			OPAL_FLASH_VALIDATE);
  OPAL_CALL(opal_manage_flash,			OPAL_FLASH_MANAGE);
  OPAL_CALL(opal_update_flash,			OPAL_FLASH_UPDATE);
+ OPAL_CALL(opal_resync_timebase,			OPAL_RESYNC_TIMEBASE);
 +OPAL_CALL(opal_dump_init,			OPAL_DUMP_INIT);
 +OPAL_CALL(opal_dump_info,			OPAL_DUMP_INFO);
 +OPAL_CALL(opal_dump_info2,			OPAL_DUMP_INFO2);
 +OPAL_CALL(opal_dump_read,			OPAL_DUMP_READ);
 +OPAL_CALL(opal_dump_ack,			OPAL_DUMP_ACK);
  OPAL_CALL(opal_get_msg,				OPAL_GET_MSG);
  OPAL_CALL(opal_check_completion,		OPAL_CHECK_ASYNC_COMPLETION);
 +OPAL_CALL(opal_dump_resend_notification,	OPAL_DUMP_RESEND);
  OPAL_CALL(opal_sync_host_reboot,		OPAL_SYNC_HOST_REBOOT);

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2 3/6] powernv, cpufreq:Add per-core locking to serialize frequency transitions
From: Preeti U Murthy @ 2014-03-17  9:04 UTC (permalink / raw)
  To: Gautham R. Shenoy; +Cc: linuxppc-dev, srivatsa.bhat
In-Reply-To: <1394449861-8688-4-git-send-email-ego@linux.vnet.ibm.com>

On 03/10/2014 04:40 PM, Gautham R. Shenoy wrote:
> From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
> 
> On POWER systems, the CPU frequency is controlled at a core-level and
> hence we need to serialize so that only one of the threads in the core
> switches the core's frequency at a time.
> 
> Using a global mutex lock would needlessly serialize _all_ frequency
> transitions in the system (across all cores). So introduce per-core
> locking to enable finer-grained synchronization and thereby enhance
> the speed and responsiveness of the cpufreq driver to varying workload
> demands.
> 
> The design of per-core locking is very simple and straight-forward: we
> first define a Per-CPU lock and use the ones that belongs to the first
> thread sibling of the core.
> 
> cpu_first_thread_sibling() macro is used to find the *common* lock for
> all thread siblings belonging to a core.
> 
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index 4cad727..4c2e8ca 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -24,8 +24,15 @@
>  #include <linux/of.h>
>  #include <asm/cputhreads.h>
>  
> -/* FIXME: Make this per-core */
> -static DEFINE_MUTEX(freq_switch_mutex);
> +/* Per-Core locking for frequency transitions */
> +static DEFINE_PER_CPU(struct mutex, freq_switch_lock);
> +
> +#define lock_core_freq(cpu)				\
> +			mutex_lock(&per_cpu(freq_switch_lock,\
> +				cpu_first_thread_sibling(cpu)));
> +#define unlock_core_freq(cpu)				\
> +			mutex_unlock(&per_cpu(freq_switch_lock,\
> +				cpu_first_thread_sibling(cpu)));
>  
>  #define POWERNV_MAX_PSTATES	256
>  
> @@ -233,7 +240,7 @@ static int powernv_cpufreq_target(struct cpufreq_policy *policy,
>  	freqs.new = powernv_freqs[new_index].frequency;
>  	freqs.cpu = policy->cpu;
>  
> -	mutex_lock(&freq_switch_mutex);
> +	lock_core_freq(policy->cpu);
>  	cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
>  
>  	pr_debug("setting frequency for cpu %d to %d kHz index %d pstate %d",
> @@ -245,7 +252,7 @@ static int powernv_cpufreq_target(struct cpufreq_policy *policy,
>  	rc = powernv_set_freq(policy->cpus, new_index);
>  
>  	cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
> -	mutex_unlock(&freq_switch_mutex);
> +	unlock_core_freq(policy->cpu);
>  
>  	return rc;
>  }
> @@ -262,7 +269,7 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
>  
>  static int __init powernv_cpufreq_init(void)
>  {
> -	int rc = 0;
> +	int cpu, rc = 0;
>  
>  	/* Discover pstates from device tree and init */
>  
> @@ -272,6 +279,10 @@ static int __init powernv_cpufreq_init(void)
>  		pr_info("powernv-cpufreq disabled\n");
>  		return rc;
>  	}
> +	/* Init per-core mutex */
> +	for_each_possible_cpu(cpu) {
> +		mutex_init(&per_cpu(freq_switch_lock, cpu));
> +	}
>  
>  	rc = cpufreq_register_driver(&powernv_cpufreq_driver);
>  	return rc;
> 
Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>

^ permalink raw reply

* Re: [PATCH v2 4/6] powernv:cpufreq: Create pstate_id_to_freq() helper
From: Preeti U Murthy @ 2014-03-17  9:06 UTC (permalink / raw)
  To: Gautham R. Shenoy; +Cc: linuxppc-dev, srivatsa.bhat
In-Reply-To: <1394449861-8688-5-git-send-email-ego@linux.vnet.ibm.com>

On 03/10/2014 04:40 PM, Gautham R. Shenoy wrote:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> 
> Create a helper routine that can return the cpu-frequency for the
> corresponding pstate_id.
> 
> Also, cache the values of the pstate_max, pstate_min and
> pstate_nominal and nr_pstates in a static structure so that they can
> be reused in the future to perform any validations.
> 
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index 4c2e8ca..0ecd163 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -39,6 +39,14 @@ static DEFINE_PER_CPU(struct mutex, freq_switch_lock);
>  static struct cpufreq_frequency_table powernv_freqs[POWERNV_MAX_PSTATES+1];
>  static int powernv_pstate_ids[POWERNV_MAX_PSTATES+1];
>  
> +struct powernv_pstate_info {
> +	int pstate_min_id;
> +	int pstate_max_id;
> +	int pstate_nominal_id;
> +	int nr_pstates;
> +};
> +static struct powernv_pstate_info powernv_pstate_info;
> +
>  /*
>   * Initialize the freq table based on data obtained
>   * from the firmware passed via device-tree
> @@ -112,9 +120,28 @@ static int init_powernv_pstates(void)
>  	for (i = 0; powernv_freqs[i].frequency != CPUFREQ_TABLE_END; i++)
>  		pr_debug("%d: %d\n", i, powernv_freqs[i].frequency);
>  
> +	powernv_pstate_info.pstate_min_id = pstate_min;
> +	powernv_pstate_info.pstate_max_id = pstate_max;
> +	powernv_pstate_info.pstate_nominal_id = pstate_nominal;
> +	powernv_pstate_info.nr_pstates = nr_pstates;
> +
>  	return 0;
>  }
>  
> +/**
> + * Returns the cpu frequency corresponding to the pstate_id.
> + */
> +static unsigned int pstate_id_to_freq(int pstate_id)
> +{
> +	int i;
> +
> +	i = powernv_pstate_info.pstate_max_id - pstate_id;
> +
> +	BUG_ON(i >= powernv_pstate_info.nr_pstates || i < 0);
> +	WARN_ON(powernv_pstate_ids[i] != pstate_id);
> +	return powernv_freqs[i].frequency;
> +}
> +
>  static struct freq_attr *powernv_cpu_freq_attr[] = {
>  	&cpufreq_freq_attr_scaling_available_freqs,
>  	NULL,
> 
Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>

^ permalink raw reply

* Re: [PATCH v2 6/6] powernv:cpufreq: Implement the driver->get() method
From: Preeti U Murthy @ 2014-03-17  9:11 UTC (permalink / raw)
  To: Gautham R. Shenoy; +Cc: linuxppc-dev, srivatsa.bhat
In-Reply-To: <1394449861-8688-7-git-send-email-ego@linux.vnet.ibm.com>

On 03/10/2014 04:41 PM, Gautham R. Shenoy wrote:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> 
> The current frequency of a cpu is reported through the sysfs file
> cpuinfo_cur_freq. This requires the driver to implement a
> "->get(unsigned int cpu)" method which will return the current
> operating frequency.
> 
> Implement a function named powernv_cpufreq_get() which reads the local
> pstate from the PMSR and returns the corresponding frequency.
> 
> Set the powernv_cpufreq_driver.get hook to powernv_cpufreq_get().
> 
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 48 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index 183bbc4..6f3b6e1 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -223,6 +223,53 @@ static inline void set_pmspr(unsigned long sprn, unsigned long val)
>  	BUG();
>  }
>  
> +/*
> + * Computes the current frequency on this cpu
> + * and stores the result in *ret_freq.
> + */
> +static void powernv_read_cpu_freq(void *ret_freq)
> +{
> +	unsigned long pmspr_val;
> +	s8 local_pstate_id;
> +	int *cur_freq, freq, pstate_id;
> +
> +	cur_freq = (int *)ret_freq;
> +	pmspr_val = get_pmspr(SPRN_PMSR);
> +
> +	/* The local pstate id corresponds bits 48..55 in the PMSR.
> +         * Note: Watch out for the sign! */
> +	local_pstate_id = (pmspr_val >> 48) & 0xFF;
> +	pstate_id = local_pstate_id;
> +
> +	freq = pstate_id_to_freq(pstate_id);
> +	pr_debug("cpu %d pmsr %lx pstate_id %d frequency %d \n",
> +		smp_processor_id(), pmspr_val, pstate_id, freq);
> +	*cur_freq = freq;
> +}
> +
> +/*
> + * Returns the cpu frequency as reported by the firmware for 'cpu'.
> + * This value is reported through the sysfs file cpuinfo_cur_freq.
> + */
> +unsigned int powernv_cpufreq_get(unsigned int cpu)
> +{
> +	int ret_freq;
> +	cpumask_var_t sibling_mask;
> +
> +	if (unlikely(!zalloc_cpumask_var(&sibling_mask, GFP_KERNEL))) {
> +		smp_call_function_single(cpu, powernv_read_cpu_freq,
> +					&ret_freq, 1);
> +		return ret_freq;
> +	}
> +
> +	powernv_cpu_to_core_mask(cpu, sibling_mask);
> +	smp_call_function_any(sibling_mask, powernv_read_cpu_freq,
> +			&ret_freq, 1);
> +
> +	free_cpumask_var(sibling_mask);
> +	return ret_freq;
> +}
> +
>  static void set_pstate(void *pstate)
>  {
>  	unsigned long val;
> @@ -309,6 +356,7 @@ static int powernv_cpufreq_target(struct cpufreq_policy *policy,
>  static struct cpufreq_driver powernv_cpufreq_driver = {
>  	.verify		= powernv_cpufreq_verify,
>  	.target		= powernv_cpufreq_target,
> +	.get		= powernv_cpufreq_get,
>  	.init		= powernv_cpufreq_cpu_init,
>  	.exit		= powernv_cpufreq_cpu_exit,
>  	.name		= "powernv-cpufreq",
> 
Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>

^ permalink raw reply

* Re: [PATCH 5/9] powerpc/85xx: disable irq by hardware when suspend for 64-bit
From: Chenhui Zhao @ 2014-03-17  9:37 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, linux-kernel, Jason.Jin
In-Reply-To: <1394836901.12479.125.camel@snotra.buserror.net>

On Fri, Mar 14, 2014 at 05:41:41PM -0500, Scott Wood wrote:
> On Wed, 2014-03-12 at 15:46 +0800, Chenhui Zhao wrote:
> > On Tue, Mar 11, 2014 at 06:51:20PM -0500, Scott Wood wrote:
> > > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
> > > > In 64-bit mode, kernel just clears the irq soft-enable flag
> > > > in struct paca_struct to disable external irqs. But, in
> > > > the case of suspend, irqs should be disabled by hardware.
> > > > Therefore, hook a function to ppc_md.suspend_disable_irqs
> > > > to really disable irqs.
> > > > 
> > > > Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com>
> > > > ---
> > > >  arch/powerpc/platforms/85xx/corenet_generic.c |   12 ++++++++++++
> > > >  1 files changed, 12 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c
> > > > index 3fdf9f3..983d81f 100644
> > > > --- a/arch/powerpc/platforms/85xx/corenet_generic.c
> > > > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c
> > > > @@ -32,6 +32,13 @@
> > > >  #include <sysdev/fsl_pci.h>
> > > >  #include "smp.h"
> > > >  
> > > > +#if defined(CONFIG_PPC64) && defined(CONFIG_SUSPEND)
> > > > +static void fsl_suspend_disable_irqs(void)
> > > > +{
> > > > +	__hard_irq_disable();
> > > > +}
> > > > +#endif
> > > 
> > > Why the underscore version?  Don't you want PACA_IRQ_HARD_DIS to be set?
> > > 
> > > If hard disabling is appropriate here, shouldn't we do it in
> > > generic_suspend_disable_irqs()?
> > > 
> > > Are there any existing platforms that supply a
> > > ppc_md.suspend_disable_irqs()?  I don't see any when grepping.
> > > 
> > > -Scott
> > 
> > Will use hard_irq_disable().
> > 
> > I think this is a general problem for powerpc.
> > Should clear MSR_EE before suspend. I agree to put it
> > in generic_suspend_disable_irqs().
> 
> BTW, make sure you test this patchset with CONFIG_DEBUG_PREEMPT and
> similar debugging options to help ensure that the soft IRQ state is
> being tracked properly.
> 
> -Scott

OK. I'll keep that in mind.

-Chenhui

^ permalink raw reply

* Re: [PATCH 7/9] fsl: add EPU FSM configuration for deep sleep
From: Chenhui Zhao @ 2014-03-17 10:27 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, linux-kernel, Jason.Jin
In-Reply-To: <1394837469.12479.134.camel@snotra.buserror.net>

On Fri, Mar 14, 2014 at 05:51:09PM -0500, Scott Wood wrote:
> On Wed, 2014-03-12 at 16:34 +0800, Chenhui Zhao wrote:
> > On Tue, Mar 11, 2014 at 07:08:43PM -0500, Scott Wood wrote:
> > > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
> > > > From: Hongbo Zhang <hongbo.zhang@freescale.com>
> > > > 
> > > > In the last stage of deep sleep, software will trigger a Finite
> > > > State Machine (FSM) to control the hardware precedure, such as
> > > > board isolation, killing PLLs, removing power, and so on.
> > > > 
> > > > When the system is waked up by an interrupt, the FSM controls the
> > > > hardware to complete the early resume precedure.
> > > > 
> > > > This patch configure the EPU FSM preparing for deep sleep.
> > > > 
> > > > Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
> > > > Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com>
> > > 
> > > Couldn't this be part of qoriq_pm.c?
> > 
> > Put the code in drivers/platform/fsl/ so that LS1 can share these code.
> 
> How can LS1 share it if it's got hardcoded T1040 values?
> 
> > > > diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
> > > > index 09fde58..6539e6d 100644
> > > > --- a/drivers/platform/Kconfig
> > > > +++ b/drivers/platform/Kconfig
> > > > @@ -6,3 +6,7 @@ source "drivers/platform/goldfish/Kconfig"
> > > >  endif
> > > >  
> > > >  source "drivers/platform/chrome/Kconfig"
> > > > +
> > > > +if FSL_SOC
> > > > +source "drivers/platform/fsl/Kconfig"
> > > > +endif
> > > 
> > > Chrome doesn't need an ifdef -- why does this?
> > 
> > Don't wish other platform see these options, and the X86 and GOLDFISH have
> > ifdefs.
> 
> The point is you can implement the dependency inside
> drivers/platform/fsl/Kconfig.

OK.

> 
> > > > diff --git a/drivers/platform/fsl/Makefile b/drivers/platform/fsl/Makefile
> > > > new file mode 100644
> > > > index 0000000..d99ca0e
> > > > --- /dev/null
> > > > +++ b/drivers/platform/fsl/Makefile
> > > > @@ -0,0 +1,5 @@
> > > > +#
> > > > +# Makefile for linux/drivers/platform/fsl
> > > > +# Freescale Specific Power Management Drivers
> > > > +#
> > > > +obj-$(CONFIG_FSL_SLEEP_FSM)	+= sleep_fsm.o
> > > 
> > > Why is this here while the other stuff is in arch/powerpc/sysdev?
> > > 
> > > > +/* Block offsets */
> > > > +#define	RCPM_BLOCK_OFFSET	0x00022000
> > > > +#define	EPU_BLOCK_OFFSET	0x00000000
> > > > +#define	NPC_BLOCK_OFFSET	0x00001000
> > > 
> > > Why don't these block offsets come from the device tree?
> > 
> > Have maped DCSR registers. Don't wish to remap them.
> 
> We don't wish to have hardcoded CCSR/DCSR offsets in the kernel source.
> Sorry.

OK.

>  
> > > > +	/* Configure the EPU Counters */
> > > > +	epu_write(EPCCR15, 0x92840000);
> > > > +	epu_write(EPCCR14, 0x92840000);
> > > > +	epu_write(EPCCR12, 0x92840000);
> > > > +	epu_write(EPCCR11, 0x92840000);
> > > > +	epu_write(EPCCR10, 0x92840000);
> > > > +	epu_write(EPCCR9, 0x92840000);
> > > > +	epu_write(EPCCR8, 0x92840000);
> > > > +	epu_write(EPCCR5, 0x92840000);
> > > > +	epu_write(EPCCR4, 0x92840000);
> > > > +	epu_write(EPCCR2, 0x92840000);
> > > > +
> > > > +	/* Configure the SCUs Inputs */
> > > > +	epu_write(EPSMCR15, 0x76000000);
> > > > +	epu_write(EPSMCR14, 0x00000031);
> > > > +	epu_write(EPSMCR13, 0x00003100);
> > > > +	epu_write(EPSMCR12, 0x7F000000);
> > > > +	epu_write(EPSMCR11, 0x31740000);
> > > > +	epu_write(EPSMCR10, 0x65000030);
> > > > +	epu_write(EPSMCR9, 0x00003000);
> > > > +	epu_write(EPSMCR8, 0x64300000);
> > > > +	epu_write(EPSMCR7, 0x30000000);
> > > > +	epu_write(EPSMCR6, 0x7C000000);
> > > > +	epu_write(EPSMCR5, 0x00002E00);
> > > > +	epu_write(EPSMCR4, 0x002F0000);
> > > > +	epu_write(EPSMCR3, 0x2F000000);
> > > > +	epu_write(EPSMCR2, 0x6C700000);
> > > 
> > > Where do these magic numbers come from?  Which chips are they valid for?
> > 
> > They are for T1040. Can be found in the RCPM chapter of T1040RM.
> 
> Then put in a comment to that effect, including what part of the RCPM
> chapter.
> 
> How do you plan to handle the addition of another SoC with different
> values?
> 
> -Scott

Had thought that using an array to put these values (pairs of offset and value)
and passing the array to the function.

However, luckily T104x and LS1 have same values for these registers
according to the current Reference Manuals.

-Chenhui

^ permalink raw reply

* Re: [PATCH 8/9] powerpc/85xx: add save/restore functions for core registers
From: Chenhui Zhao @ 2014-03-17 10:50 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, linux-kernel, Jason.Jin, Wang Dongsheng-B40534
In-Reply-To: <1394838105.12479.144.camel@snotra.buserror.net>

On Fri, Mar 14, 2014 at 06:01:45PM -0500, Scott Wood wrote:
> On Wed, 2014-03-12 at 17:42 +0800, Chenhui Zhao wrote:
> > On Tue, Mar 11, 2014 at 07:45:14PM -0500, Scott Wood wrote:
> > > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
> > > > From: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > > 
> > > > Add booke_cpu_state_save() and booke_cpu_state_restore() functions which can be
> > > > used to save/restore CPU's registers in the case of deep sleep and hibernation.
> > > > 
> > > > Supported processors: E6500, E5500, E500MC, E500v2 and E500v1.
> > > > 
> > > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > > Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com>
> > > > ---
> > > >  arch/powerpc/include/asm/booke_save_regs.h |   96 ++++++++
> > > >  arch/powerpc/kernel/Makefile               |    1 +
> > > >  arch/powerpc/kernel/booke_save_regs.S      |  361 ++++++++++++++++++++++++++++
> > > >  3 files changed, 458 insertions(+), 0 deletions(-)
> > > >  create mode 100644 arch/powerpc/include/asm/booke_save_regs.h
> > > >  create mode 100644 arch/powerpc/kernel/booke_save_regs.S
> > > > 
> > > > diff --git a/arch/powerpc/include/asm/booke_save_regs.h b/arch/powerpc/include/asm/booke_save_regs.h
> > > > new file mode 100644
> > > > index 0000000..87c357a
> > > > --- /dev/null
> > > > +++ b/arch/powerpc/include/asm/booke_save_regs.h
> > > > @@ -0,0 +1,96 @@
> > > > +/*
> > > > + *  Save/restore e500 series core registers
> > > 
> > > Filename says booke, comment says e500.
> > > 
> > > Filename and comment also fail to point out that this is specifically
> > > for standby/suspend, not for hibernate which is implemented in
> > > swsusp_booke.S/swsusp_asm64.S.
> > 
> > Sorry for inconsistency. Will changes e500 to booke.
> > Hibernation and suspend can share the code.
> 
> Maybe they could, but AFAICT this patchset doesn't make that happen --
> and I'm not convinced that the churn would be worthwhile.  Note that
> swsusp_asm64.S is not just for booke, so most of that file would not be
> going away if you did make such a change.

OK. Let's put Hibernation aside, and change the code just for suspend.

> 
> I also don't like the way it looks like booke_save_regs.S is a booke
> version of ppc_save_regs.S, even though they serve different purposes
> and ppc_save_regs.S is still relevant to booke.
> 
> > > > + * Software-Use Registers
> > > > + *	SPRG1			0x260		(dw * 76), 64-bit need to save.
> > > > + *	SPRG3			0x268		(dw * 77), 32-bit need to save.
> > > 
> > > What about "CPU and NUMA node for VDSO getcpu" on 64-bit?  Currently
> > > SPRG3, but it will need to change for critical interrupt support.
> > > 
> > > > + * MMU Registers
> > > > + *	PID0 - PID2		0x270 ~ 0x280	(dw * 78 ~ dw * 80)
> > > 
> > > PID1/PID2 are e500v1/v2 only -- and Linux doesn't use them outside of
> > > KVM (and you're not in KVM when you're running this code).
> > > 
> > > Are we ever going to have a non-zero PID at this point?
> > 
> > I incline to the view that saving all registers regardless of used or
> > unused. The good point is that it can be compliant to the future
> > changes of the usage of registers.
> > 
> > What do you think?
> 
> I agree to a certain extent, but balance it with the complexity of
> dealing with registers that don't exist on all booke chips.  If they
> don't really need to be saved, why go through the hassle of conditional
> code?

I agree. For these registers, I'll check if they are really needed.

-Chenhui

^ permalink raw reply

* Re: [PATCH 9/9] powerpc/pm: support deep sleep feature on T1040
From: Chenhui Zhao @ 2014-03-17 11:19 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, linux-kernel, Jason.Jin
In-Reply-To: <1394839107.12479.154.camel@snotra.buserror.net>

On Fri, Mar 14, 2014 at 06:18:27PM -0500, Scott Wood wrote:
> On Wed, 2014-03-12 at 18:40 +0800, Chenhui Zhao wrote:
> > On Tue, Mar 11, 2014 at 08:10:24PM -0500, Scott Wood wrote:
> > > On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
> > > > From: Zhao Chenhui <chenhui.zhao@freescale.com>
> > > > 
> > > > T1040 supports deep sleep feature, which can switch off most parts of
> > > > the SoC when it is in deep sleep mode. This way, it becomes more
> > > > energy-efficient.
> > > > 
> > > > The DDR controller will also be powered off in deep sleep. Therefore,
> > > > the last stage (the latter part of fsl_dp_enter_low) will run without DDR
> > > > access. This piece of code and related TLBs will be prefetched.
> > > > 
> > > > Due to the different initialization code between 32-bit and 64-bit, they
> > > > have seperate resume entry and precedure.
> > > > 
> > > > The feature supports 32-bit and 64-bit kernel mode.
> > > > 
> > > > Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > > > ---
> > > >  arch/powerpc/include/asm/booke_save_regs.h |    3 +
> > > >  arch/powerpc/kernel/cpu_setup_fsl_booke.S  |   17 ++
> > > >  arch/powerpc/kernel/head_fsl_booke.S       |   30 +++
> > > >  arch/powerpc/platforms/85xx/Makefile       |    2 +-
> > > >  arch/powerpc/platforms/85xx/deepsleep.c    |  201 +++++++++++++++++++
> > > >  arch/powerpc/platforms/85xx/qoriq_pm.c     |   38 ++++
> > > >  arch/powerpc/platforms/85xx/sleep.S        |  295 ++++++++++++++++++++++++++++
> > > >  arch/powerpc/sysdev/fsl_soc.h              |    7 +
> > > >  8 files changed, 592 insertions(+), 1 deletions(-)
> > > >  create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c
> > > >  create mode 100644 arch/powerpc/platforms/85xx/sleep.S
> > > > 
> > > > diff --git a/arch/powerpc/include/asm/booke_save_regs.h b/arch/powerpc/include/asm/booke_save_regs.h
> > > > index 87c357a..37c1f6c 100644
> > > > --- a/arch/powerpc/include/asm/booke_save_regs.h
> > > > +++ b/arch/powerpc/include/asm/booke_save_regs.h
> > > > @@ -88,6 +88,9 @@
> > > >  #define HIBERNATION_FLAG	1
> > > >  #define DEEPSLEEP_FLAG		2
> > > >  
> > > > +#define CPLD_FLAG	1
> > > > +#define FPGA_FLAG	2
> > > 
> > > What is this?
> > 
> > We have two kind of boards, QDS and RDB.
> > They have different register map. Use the flag to indicate the current board is using which kind
> > of register map.
> 
> CPLD versus FPGA is not a meaningful difference.  We don't care what
> technology is used to implement programmable logic -- we care what
> programming interface is exposed.  Customers will have their own boards
> that will likely not imitate either of these programming interfaces, but
> what they do have will still probably be implemented in a CPLD or FPGA.
> Likewise, Freescale may have future reference boards whose CPLD/FPGA is
> not compatible.

Will use a better name.

> 
> So use better naming, and structure the code so it's easy to plug in
> implementations for new or custom boards.
>  
> > > > diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
> > > > index 20204fe..3285752 100644
> > > > --- a/arch/powerpc/kernel/head_fsl_booke.S
> > > > +++ b/arch/powerpc/kernel/head_fsl_booke.S
> > > > @@ -162,6 +162,19 @@ _ENTRY(__early_start)
> > > >  #include "fsl_booke_entry_mapping.S"
> > > >  #undef ENTRY_MAPPING_BOOT_SETUP
> > > >  
> > > > +#if defined(CONFIG_SUSPEND) && defined(CONFIG_FSL_CORENET_RCPM)
> > > > +	/* if deep_sleep_flag != 0, jump to the deep sleep resume entry */
> > > > +	LOAD_REG_ADDR(r4, deep_sleep_flag)
> > > > +	lwz	r3, 0(r4)
> > > > +	cmpwi	r3, 0
> > > > +	beq	11f
> > > > +	/* clear deep_sleep_flag */
> > > > +	li	r3, 0
> > > > +	stw	r3, 0(r4)
> > > > +	b	fsl_deepsleep_resume
> > > > +11:
> > > > +#endif
> > > 
> > > Why do you come in via the normal kernel entry, versus specifying a
> > > direct entry point for deep sleep resume?  How does U-Boot even know
> > > what the normal entry is when resuming?
> > 
> > I wish to return to a specified point (like 64-bit mode), but the code in
> > fsl_booke_entry_mapping.S only can run in the first page. Because it
> > only setups a temp mapping of 4KB.
> 
> Why do you need the entry mapping on 32-bit but not 64-bit?

fsl_booke_entry_mapping.S is for 32-bit. 64-bit calls
initial_tlb_book3e() in exceptions-64e.S.

> > 
> > > > +#if defined(CONFIG_SUSPEND) && defined(CONFIG_FSL_CORENET_RCPM)
> > > > +_ENTRY(__entry_deep_sleep)
> > > > +/*
> > > > + * Bootloader will jump to here when resuming from deep sleep.
> > > > + * After executing the init code in fsl_booke_entry_mapping.S,
> > > > + * will jump to the real resume entry.
> > > > + */
> > > > +	li	r8, 1
> > > > +	bl	12f
> > > > +12:	mflr	r9
> > > > +	addi	r9, r9, (deep_sleep_flag - 12b)
> > > > +	stw	r8, 0(r9)
> > > > +	b __early_start
> > > > +deep_sleep_flag:
> > > > +	.long	0
> > > > +#endif
> > > 
> > > It's a bit ambiguous to say "entry_deep_sleep" when it's resuming rather
> > > than entering...
> > 
> > How about __fsl_entry_resume?
> 
> fsl_booke_deep_sleep_resume
> 
> > > > +#define FSLDELAY(count)		\
> > > > +	li	r3, (count)@l;	\
> > > > +	slwi	r3, r3, 10;	\
> > > > +	mtctr	r3;		\
> > > > +101:	nop;			\
> > > > +	bdnz	101b;
> > > 
> > > You don't need a namespace prefix on local macros in a non-header file.
> > > 
> > > Is the timebase stopped where you're calling this from?
> > 
> > No. My purpose is to avoid jump in the last stage of entering deep sleep.
> > Jump may cause problem at that time.
> 
> "bdnz" is a jump.
> 
> What problems do you think a jump will cause?

I mean a far jump which can jump to an address which has not been prefetched in
advance. I wish the code is executed in a restricted environment (predictable code
and address).

> 
> > > You also probably want to do a "sync, readback, data dependency, isync"
> > > sequence to make sure that the store has hit CCSR before you begin your
> > > delay (or is a delay required at all if you do that?).
> > 
> > Yes. It is safer with a sync sequence.
> > 
> > The DDR controller need some time to signal the external DDR modules to
> > enter self refresh mode.
> 
> Is it documented how much time it requires?
> 
> -Scott

No.

-Chenhui

^ permalink raw reply

* RE: [PATCH 05/10] powerpc/booke64: Use SPRG7 for VDSO
From: mihai.caraman @ 2014-03-17 14:25 UTC (permalink / raw)
  To: Scott Wood, Benjamin Herrenschmidt
  Cc: kvm-ppc@vger.kernel.org, Tiejun Chen, Paul Mackerras,
	Anton Blanchard, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1394755249-8856-6-git-send-email-scottwood@freescale.com>

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, March 14, 2014 2:01 AM
> To: Benjamin Herrenschmidt
> Cc: linuxppc-dev@lists.ozlabs.org; Kumar Gala; Tiejun Chen; Wood Scott-
> B07421; Caraman Mihai Claudiu-B02008; Anton Blanchard; Paul Mackerras;
> kvm-ppc@vger.kernel.org
> Subject: [PATCH 05/10] powerpc/booke64: Use SPRG7 for VDSO
>=20
> Previously SPRG3 was marked for use by both VDSO and critical
> interrupts (though critical interrupts were not fully implemented).
>=20
> In commit 8b64a9dfb091f1eca8b7e58da82f1e7d1d5fe0ad ("powerpc/booke64:
> Use SPRG0/3 scratch for bolted TLB miss & crit int"), Mihai Caraman
> made an attempt to resolve this conflict by restoring the VDSO value
> early in the critical interrupt, but this has some issues:
>=20

>  - It forces critical exceptions to be a special case handled
>    differently from even machine check and debug level exceptions.

Yes, this was ugly.

> Since we cannot use SPRG4-7 for scratch without corrupting the state of
> a KVM guest, move VDSO to SPRG7 on book3e.

At that time I thought that information exposed through SPRN_USPRG3 is
part of the ABI which can't be changed, and this lead to complicated
gymnastic. But since VSDO's __kernel_getcpu() function hides SPRN_USPRGx
access, I think your unified approach is the right way.

-Mike

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox