LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] powerpc: enable relocatable support for 6xx
From: Kevin Hao @ 2013-06-19  9:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc
In-Reply-To: <1371633604-4491-1-git-send-email-haokexin@gmail.com>

This is based on the codes in head_44x.S. With this patch the kernel
can only boot from 0 with CONFIG_RELOCATABLE enabled. We will add the
support to boot from a non 0 address in the following patches.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/Kconfig                   |   2 +-
 arch/powerpc/include/asm/page.h        |   2 +-
 arch/powerpc/kernel/head_32.S          | 103 +++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/prom_init_check.sh |   2 +-
 4 files changed, 106 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c33e3ad..8fe2792 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -866,7 +866,7 @@ config DYNAMIC_MEMSTART
 
 config RELOCATABLE
 	bool "Build a relocatable kernel"
-	depends on ADVANCED_OPTIONS && FLATMEM && 44x
+	depends on ADVANCED_OPTIONS && FLATMEM && (44x || 6xx)
 	select NONSTATIC_KERNEL
 	help
 	  This builds a kernel image that is capable of running at the
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 988c812..7145c14 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -207,7 +207,7 @@ extern long long virt_phys_offset;
  * On non-Book-E PPC64 PAGE_OFFSET and MEMORY_START are constants so use
  * the other definitions for __va & __pa.
  */
-#ifdef CONFIG_BOOKE
+#if defined(CONFIG_BOOKE) || defined(CONFIG_6xx)
 #define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) + VIRT_PHYS_OFFSET))
 #define __pa(x) ((unsigned long)(x) - VIRT_PHYS_OFFSET)
 #else
diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index dc0488b..eb47b13 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -73,6 +73,8 @@ _ENTRY(_start);
 	nop	/* used by __secondary_hold on prep (mtx) and chrp smp */
 	nop
 
+	bl	perform_relocation
+
 /* PMAC
  * Enter here with the kernel text, data and bss loaded starting at
  * 0, running with virtual == physical mapping.
@@ -149,6 +151,8 @@ __start:
  */
 	bl	early_init
 
+	bl	after_relocation_init
+
 /* Switch MMU off, clear BATs and flush TLB. At this point, r3 contains
  * the physical address we are running at, returned by early_init()
  */
@@ -180,6 +184,7 @@ __after_mmu_off:
 #endif /* CONFIG_6xx */
 
 
+#ifndef CONFIG_RELOCATABLE
 /*
  * We need to run with _start at physical address 0.
  * On CHRP, we are loaded at 0x10000 since OF on CHRP uses
@@ -193,6 +198,8 @@ __after_mmu_off:
 	lis	r5,PHYSICAL_START@h
 	cmplw	0,r4,r5			/* already running at PHYSICAL_START? */
 	bne	relocate_kernel
+#endif
+
 /*
  * we now have the 1st 16M of ram mapped with the bats.
  * prep needs the mmu to be turned on here, but pmac already has it on.
@@ -1263,6 +1270,102 @@ m8260_gorom:
 	blr
 #endif
 
+perform_relocation:
+#ifdef CONFIG_RELOCATABLE
+/*
+ * Relocate ourselves to the current runtime address.
+ * This is called only by the Boot CPU.
+ * r21 will be loaded with the physical runtime address of _stext
+ * Save the r3~r7 since these registers may contain the values needed
+ * by the following boot code.
+ */
+	mr	r22,r3
+	mr	r23,r4
+	mr	r24,r5
+	mr	r25,r6
+	mr	r26,r7
+
+	mflr	r20
+	bl	0f				/* Get our runtime address */
+0:	mflr	r21				/* Make it accessible */
+	addis	r21,r21,(_stext - 0b)@ha
+	addi	r21,r21,(_stext - 0b)@l 	/*Get our current runtime base*/
+
+	/*
+	 * We have the runtime address of our base.
+	 * We calculate our shift of offset from a 256M page.
+	 * We could map the 256M page we belong to at PAGE_OFFSET and
+	 * get going from there.
+	 */
+	lis	r4,KERNELBASE@h
+	ori	r4,r4,KERNELBASE@l
+	rlwinm	r6,r21,0,4,31			/* r6 = PHYS_START % 256M */
+	rlwinm	r5,r4,0,4,31			/* r5 = KERNELBASE % 256M */
+	subf	r3,r5,r6			/* r3 = r6 - r5 */
+	add	r3,r4,r3			/* Required Virutal Address */
+
+	bl	relocate
+	mtlr	r20
+	mr	r3,r22
+	mr	r4,r23
+	mr	r5,r24
+	mr	r6,r25
+	mr	r7,r26
+#endif
+	blr
+
+after_relocation_init:
+#ifdef CONFIG_RELOCATABLE
+	/*
+	 * Relocatable kernel support based on processing of dynamic
+	 * relocation entries.
+	 *
+	 * r21 will contain the current offset of _stext
+	 */
+	lis	r3,kernstart_addr@ha
+	la	r3,kernstart_addr@l(r3)
+
+	/* Store kernstart_addr */
+	tophys(r3,r3)
+	stw	r21,0(r3)
+
+	/*
+	 * Compute the virt_phys_offset :
+	 * virt_phys_offset = stext.run - kernstart_addr
+	 *
+	 * stext.run = (KERNELBASE & ~0xfffffff) + (kernstart_addr & 0xfffffff)
+	 * When we relocate, we have :
+	 *
+	 *	(kernstart_addr & 0xfffffff) = (stext.run & 0xfffffff)
+	 *
+	 * hence:
+	 *  virt_phys_offset = (KERNELBASE & ~0xfffffff)
+	 *                            - (kernstart_addr & ~0xfffffff)
+	 *
+	 */
+
+	/* KERNELBASE&~0xfffffff => (r4,r5) */
+	lis	r5,KERNELBASE@h
+	rlwinm	r5,r5,0,0,3	/* Align to 256M, lower 32bit */
+
+	/* kernelstart_addr & ~0xfffffff => (r6,r7) */
+	rlwinm	r7,r21,0,0,3	/* Align to 256M, lower 32bit */
+
+	/*
+	 * 64bit subtraction.
+	 */
+	subf	r5,r7,r5
+
+	/* Store virt_phys_offset */
+	lis	r3,virt_phys_offset@h
+	ori	r3,r3,virt_phys_offset@l
+
+	tophys(r3,r3)
+	li	r4,0
+	stw	r4,0(r3)	/* Higher 32bit */
+	stw	r5,4(r3)	/* Lower 32bit */
+#endif
+	blr
 
 /*
  * We put a few things here that have to be page-aligned.
diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
index 3765da6..a24c208 100644
--- a/arch/powerpc/kernel/prom_init_check.sh
+++ b/arch/powerpc/kernel/prom_init_check.sh
@@ -22,7 +22,7 @@ __secondary_hold_acknowledge __secondary_hold_spinloop __start
 strcmp strcpy strlcpy strlen strncmp strstr logo_linux_clut224
 reloc_got2 kernstart_addr memstart_addr linux_banner _stext
 opal_query_takeover opal_do_takeover opal_enter_rtas opal_secondary_entry
-boot_command_line __prom_init_toc_start __prom_init_toc_end"
+boot_command_line __prom_init_toc_start __prom_init_toc_end virt_phys_offset"
 
 NM="$1"
 OBJ="$2"
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH 2/4] powerpc: move the exception trampoline helper functions to a separate file
From: Kevin Hao @ 2013-06-19  9:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc
In-Reply-To: <1371633604-4491-1-git-send-email-haokexin@gmail.com>

For some platform such as 6xx we need to setup the exception
trampoline for a relocatable kernel even through the CONFIG_CRASH_DUMP
is disabled. So move these functions to a separate file so they can
be used by non dump kernel.  This patch doesn't introduce any function
change.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/include/asm/exception_trampoline.h | 35 +++++++++++++
 arch/powerpc/include/asm/kdump.h                | 32 -----------
 arch/powerpc/kernel/Makefile                    |  2 +-
 arch/powerpc/kernel/crash_dump.c                | 41 ---------------
 arch/powerpc/kernel/exception_trampoline.c      | 70 +++++++++++++++++++++++++
 arch/powerpc/kernel/prom.c                      |  2 +-
 arch/powerpc/kernel/setup_32.c                  |  1 +
 arch/powerpc/kernel/setup_64.c                  |  2 +-
 8 files changed, 109 insertions(+), 76 deletions(-)
 create mode 100644 arch/powerpc/include/asm/exception_trampoline.h
 create mode 100644 arch/powerpc/kernel/exception_trampoline.c

diff --git a/arch/powerpc/include/asm/exception_trampoline.h b/arch/powerpc/include/asm/exception_trampoline.h
new file mode 100644
index 0000000..707ad6c
--- /dev/null
+++ b/arch/powerpc/include/asm/exception_trampoline.h
@@ -0,0 +1,35 @@
+#ifndef _EXCEPTION_TRAMPOLINE_H
+#define _EXCEPTION_TRAMPOLINE_H
+
+/* How many bytes to reserve at zero for kdump. The reserve limit should
+ * be greater or equal to the trampoline's end address.
+ * Reserve to the end of the FWNMI area, see head_64.S */
+#define KDUMP_RESERVE_LIMIT	0x10000 /* 64K */
+
+/*
+ * On PPC64 translation is disabled during trampoline setup, so we use
+ * physical addresses. Though on PPC32 translation is already enabled,
+ * so we can't do the same. Luckily create_trampoline() creates relative
+ * branches, so we can just add the PAGE_OFFSET and don't worry about it.
+ */
+#ifdef __powerpc64__
+#define KDUMP_TRAMPOLINE_START	0x0100
+#define KDUMP_TRAMPOLINE_END	0x3000
+#else
+#define KDUMP_TRAMPOLINE_START	(0x0100 + PAGE_OFFSET)
+#define KDUMP_TRAMPOLINE_END	(0x3000 + PAGE_OFFSET)
+#endif /* __powerpc64__ */
+
+#ifndef __ASSEMBLY__
+
+#if defined(CONFIG_CRASH_DUMP) && !defined(CONFIG_NONSTATIC_KERNEL)
+extern void reserve_kdump_trampoline(void);
+extern void setup_kdump_trampoline(void);
+#else
+/* !CRASH_DUMP || !NONSTATIC_KERNEL */
+static inline void reserve_kdump_trampoline(void) { ; }
+static inline void setup_kdump_trampoline(void) { ; }
+#endif
+
+#endif /* __ASSEMBLY__ */
+#endif /* _EXCEPTION_TRAMPOLINE_H */
diff --git a/arch/powerpc/include/asm/kdump.h b/arch/powerpc/include/asm/kdump.h
index c977620..7ac3553 100644
--- a/arch/powerpc/include/asm/kdump.h
+++ b/arch/powerpc/include/asm/kdump.h
@@ -5,42 +5,10 @@
 
 #define KDUMP_KERNELBASE	0x2000000
 
-/* How many bytes to reserve at zero for kdump. The reserve limit should
- * be greater or equal to the trampoline's end address.
- * Reserve to the end of the FWNMI area, see head_64.S */
-#define KDUMP_RESERVE_LIMIT	0x10000 /* 64K */
-
 #ifdef CONFIG_CRASH_DUMP
 
-/*
- * On PPC64 translation is disabled during trampoline setup, so we use
- * physical addresses. Though on PPC32 translation is already enabled,
- * so we can't do the same. Luckily create_trampoline() creates relative
- * branches, so we can just add the PAGE_OFFSET and don't worry about it.
- */
-#ifdef __powerpc64__
-#define KDUMP_TRAMPOLINE_START	0x0100
-#define KDUMP_TRAMPOLINE_END	0x3000
-#else
-#define KDUMP_TRAMPOLINE_START	(0x0100 + PAGE_OFFSET)
-#define KDUMP_TRAMPOLINE_END	(0x3000 + PAGE_OFFSET)
-#endif /* __powerpc64__ */
-
 #define KDUMP_MIN_TCE_ENTRIES	2048
 
 #endif /* CONFIG_CRASH_DUMP */
 
-#ifndef __ASSEMBLY__
-
-#if defined(CONFIG_CRASH_DUMP) && !defined(CONFIG_NONSTATIC_KERNEL)
-extern void reserve_kdump_trampoline(void);
-extern void setup_kdump_trampoline(void);
-#else
-/* !CRASH_DUMP || !NONSTATIC_KERNEL */
-static inline void reserve_kdump_trampoline(void) { ; }
-static inline void setup_kdump_trampoline(void) { ; }
-#endif
-
-#endif /* __ASSEMBLY__ */
-
 #endif /* __PPC64_KDUMP_H */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index f960a79..c73a0e3 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -59,7 +59,7 @@ obj-$(CONFIG_LPARCFG)		+= lparcfg.o
 obj-$(CONFIG_IBMVIO)		+= vio.o
 obj-$(CONFIG_IBMEBUS)           += ibmebus.o
 obj-$(CONFIG_GENERIC_TBSYNC)	+= smp-tbsync.o
-obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
+obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o exception_trampoline.o
 obj-$(CONFIG_FA_DUMP)		+= fadump.o
 ifeq ($(CONFIG_PPC32),y)
 obj-$(CONFIG_E500)		+= idle_e500.o
diff --git a/arch/powerpc/kernel/crash_dump.c b/arch/powerpc/kernel/crash_dump.c
index 9ec3fe1..56dab87 100644
--- a/arch/powerpc/kernel/crash_dump.c
+++ b/arch/powerpc/kernel/crash_dump.c
@@ -28,47 +28,6 @@
 #define DBG(fmt...)
 #endif
 
-#ifndef CONFIG_NONSTATIC_KERNEL
-void __init reserve_kdump_trampoline(void)
-{
-	memblock_reserve(0, KDUMP_RESERVE_LIMIT);
-}
-
-static void __init create_trampoline(unsigned long addr)
-{
-	unsigned int *p = (unsigned int *)addr;
-
-	/* The maximum range of a single instruction branch, is the current
-	 * instruction's address + (32 MB - 4) bytes. For the trampoline we
-	 * need to branch to current address + 32 MB. So we insert a nop at
-	 * the trampoline address, then the next instruction (+ 4 bytes)
-	 * does a branch to (32 MB - 4). The net effect is that when we
-	 * branch to "addr" we jump to ("addr" + 32 MB). Although it requires
-	 * two instructions it doesn't require any registers.
-	 */
-	patch_instruction(p, PPC_INST_NOP);
-	patch_branch(++p, addr + PHYSICAL_START, 0);
-}
-
-void __init setup_kdump_trampoline(void)
-{
-	unsigned long i;
-
-	DBG(" -> setup_kdump_trampoline()\n");
-
-	for (i = KDUMP_TRAMPOLINE_START; i < KDUMP_TRAMPOLINE_END; i += 8) {
-		create_trampoline(i);
-	}
-
-#ifdef CONFIG_PPC_PSERIES
-	create_trampoline(__pa(system_reset_fwnmi) - PHYSICAL_START);
-	create_trampoline(__pa(machine_check_fwnmi) - PHYSICAL_START);
-#endif /* CONFIG_PPC_PSERIES */
-
-	DBG(" <- setup_kdump_trampoline()\n");
-}
-#endif /* CONFIG_NONSTATIC_KERNEL */
-
 static int __init parse_savemaxmem(char *p)
 {
 	if (p)
diff --git a/arch/powerpc/kernel/exception_trampoline.c b/arch/powerpc/kernel/exception_trampoline.c
new file mode 100644
index 0000000..71f4b72
--- /dev/null
+++ b/arch/powerpc/kernel/exception_trampoline.c
@@ -0,0 +1,70 @@
+/*
+ * Routines for doing kexec-based kdump.
+ *
+ * Copyright (C) 2005, IBM Corp.
+ *
+ * Created by: Michael Ellerman
+ *
+ * This source code is licensed under the GNU General Public License,
+ * Version 2.  See the file COPYING for more details.
+ */
+
+#undef DEBUG
+
+#include <linux/crash_dump.h>
+#include <linux/bootmem.h>
+#include <linux/memblock.h>
+#include <asm/code-patching.h>
+#include <asm/kdump.h>
+#include <asm/prom.h>
+#include <asm/firmware.h>
+#include <asm/uaccess.h>
+#include <asm/rtas.h>
+
+#ifdef DEBUG
+#include <asm/udbg.h>
+#define DBG(fmt...) udbg_printf(fmt)
+#else
+#define DBG(fmt...)
+#endif
+
+#ifndef CONFIG_NONSTATIC_KERNEL
+void __init reserve_kdump_trampoline(void)
+{
+	memblock_reserve(0, KDUMP_RESERVE_LIMIT);
+}
+
+static void __init create_trampoline(unsigned long addr)
+{
+	unsigned int *p = (unsigned int *)addr;
+
+	/* The maximum range of a single instruction branch, is the current
+	 * instruction's address + (32 MB - 4) bytes. For the trampoline we
+	 * need to branch to current address + 32 MB. So we insert a nop at
+	 * the trampoline address, then the next instruction (+ 4 bytes)
+	 * does a branch to (32 MB - 4). The net effect is that when we
+	 * branch to "addr" we jump to ("addr" + 32 MB). Although it requires
+	 * two instructions it doesn't require any registers.
+	 */
+	patch_instruction(p, PPC_INST_NOP);
+	patch_branch(++p, addr + PHYSICAL_START, 0);
+}
+
+void __init setup_kdump_trampoline(void)
+{
+	unsigned long i;
+
+	DBG(" -> setup_kdump_trampoline()\n");
+
+	for (i = KDUMP_TRAMPOLINE_START; i < KDUMP_TRAMPOLINE_END; i += 8) {
+		create_trampoline(i);
+	}
+
+#ifdef CONFIG_PPC_PSERIES
+	create_trampoline(__pa(system_reset_fwnmi) - PHYSICAL_START);
+	create_trampoline(__pa(machine_check_fwnmi) - PHYSICAL_START);
+#endif /* CONFIG_PPC_PSERIES */
+
+	DBG(" <- setup_kdump_trampoline()\n");
+}
+#endif /* CONFIG_NONSTATIC_KERNEL */
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 8b6f7a9..4a13ac5 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -40,7 +40,7 @@
 #include <asm/processor.h>
 #include <asm/irq.h>
 #include <asm/io.h>
-#include <asm/kdump.h>
+#include <asm/exception_trampoline.h>
 #include <asm/smp.h>
 #include <asm/mmu.h>
 #include <asm/paca.h>
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index a8f54ec..9f3aa43 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -38,6 +38,7 @@
 #include <asm/serial.h>
 #include <asm/udbg.h>
 #include <asm/mmu_context.h>
+#include <asm/exception_trampoline.h>
 
 #include "setup.h"
 
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index e379d3f..06b8562 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -38,7 +38,7 @@
 #include <linux/hugetlb.h>
 
 #include <asm/io.h>
-#include <asm/kdump.h>
+#include <asm/exception_trampoline.h>
 #include <asm/prom.h>
 #include <asm/processor.h>
 #include <asm/pgtable.h>
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH 3/4] powerpc: s/kdump/exception/ for the exception trampoline functions
From: Kevin Hao @ 2013-06-19  9:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc
In-Reply-To: <1371633604-4491-1-git-send-email-haokexin@gmail.com>

These functions are not just kdump specific. Replace the 'kdump' with
the 'exception' to make them more general.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/include/asm/exception_trampoline.h | 20 ++++++++++----------
 arch/powerpc/kernel/exception_trampoline.c      | 14 +++++++-------
 arch/powerpc/kernel/prom.c                      |  2 +-
 arch/powerpc/kernel/setup_32.c                  |  2 +-
 arch/powerpc/kernel/setup_64.c                  |  2 +-
 5 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/include/asm/exception_trampoline.h b/arch/powerpc/include/asm/exception_trampoline.h
index 707ad6c..88281c9 100644
--- a/arch/powerpc/include/asm/exception_trampoline.h
+++ b/arch/powerpc/include/asm/exception_trampoline.h
@@ -1,10 +1,10 @@
 #ifndef _EXCEPTION_TRAMPOLINE_H
 #define _EXCEPTION_TRAMPOLINE_H
 
-/* How many bytes to reserve at zero for kdump. The reserve limit should
+/* How many bytes to reserve at zero for exception. The reserve limit should
  * be greater or equal to the trampoline's end address.
  * Reserve to the end of the FWNMI area, see head_64.S */
-#define KDUMP_RESERVE_LIMIT	0x10000 /* 64K */
+#define EXCEPTION_RESERVE_LIMIT	0x10000 /* 64K */
 
 /*
  * On PPC64 translation is disabled during trampoline setup, so we use
@@ -13,22 +13,22 @@
  * branches, so we can just add the PAGE_OFFSET and don't worry about it.
  */
 #ifdef __powerpc64__
-#define KDUMP_TRAMPOLINE_START	0x0100
-#define KDUMP_TRAMPOLINE_END	0x3000
+#define EXCEPTION_TRAMPOLINE_START	0x0100
+#define EXCEPTION_TRAMPOLINE_END	0x3000
 #else
-#define KDUMP_TRAMPOLINE_START	(0x0100 + PAGE_OFFSET)
-#define KDUMP_TRAMPOLINE_END	(0x3000 + PAGE_OFFSET)
+#define EXCEPTION_TRAMPOLINE_START	(0x0100 + PAGE_OFFSET)
+#define EXCEPTION_TRAMPOLINE_END	(0x3000 + PAGE_OFFSET)
 #endif /* __powerpc64__ */
 
 #ifndef __ASSEMBLY__
 
 #if defined(CONFIG_CRASH_DUMP) && !defined(CONFIG_NONSTATIC_KERNEL)
-extern void reserve_kdump_trampoline(void);
-extern void setup_kdump_trampoline(void);
+extern void reserve_exception_trampoline(void);
+extern void setup_exception_trampoline(void);
 #else
 /* !CRASH_DUMP || !NONSTATIC_KERNEL */
-static inline void reserve_kdump_trampoline(void) { ; }
-static inline void setup_kdump_trampoline(void) { ; }
+static inline void reserve_exception_trampoline(void) { ; }
+static inline void setup_exception_trampoline(void) { ; }
 #endif
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/kernel/exception_trampoline.c b/arch/powerpc/kernel/exception_trampoline.c
index 71f4b72..b725116 100644
--- a/arch/powerpc/kernel/exception_trampoline.c
+++ b/arch/powerpc/kernel/exception_trampoline.c
@@ -15,7 +15,7 @@
 #include <linux/bootmem.h>
 #include <linux/memblock.h>
 #include <asm/code-patching.h>
-#include <asm/kdump.h>
+#include <asm/exception_trampoline.h>
 #include <asm/prom.h>
 #include <asm/firmware.h>
 #include <asm/uaccess.h>
@@ -29,9 +29,9 @@
 #endif
 
 #ifndef CONFIG_NONSTATIC_KERNEL
-void __init reserve_kdump_trampoline(void)
+void __init reserve_exception_trampoline(void)
 {
-	memblock_reserve(0, KDUMP_RESERVE_LIMIT);
+	memblock_reserve(0, EXCEPTION_RESERVE_LIMIT);
 }
 
 static void __init create_trampoline(unsigned long addr)
@@ -50,13 +50,13 @@ static void __init create_trampoline(unsigned long addr)
 	patch_branch(++p, addr + PHYSICAL_START, 0);
 }
 
-void __init setup_kdump_trampoline(void)
+void __init setup_exception_trampoline(void)
 {
 	unsigned long i;
 
-	DBG(" -> setup_kdump_trampoline()\n");
+	DBG(" -> setup_exception_trampoline()\n");
 
-	for (i = KDUMP_TRAMPOLINE_START; i < KDUMP_TRAMPOLINE_END; i += 8) {
+	for (i = EXCEPTION_TRAMPOLINE_START; i < EXCEPTION_TRAMPOLINE_END; i += 8) {
 		create_trampoline(i);
 	}
 
@@ -65,6 +65,6 @@ void __init setup_kdump_trampoline(void)
 	create_trampoline(__pa(machine_check_fwnmi) - PHYSICAL_START);
 #endif /* CONFIG_PPC_PSERIES */
 
-	DBG(" <- setup_kdump_trampoline()\n");
+	DBG(" <- setup_exception_trampoline()\n");
 }
 #endif /* CONFIG_NONSTATIC_KERNEL */
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4a13ac5..57214ee 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -669,7 +669,7 @@ void __init early_init_devtree(void *params)
 	/* If relocatable, reserve first 32k for interrupt vectors etc. */
 	if (PHYSICAL_START > MEMORY_START)
 		memblock_reserve(MEMORY_START, 0x8000);
-	reserve_kdump_trampoline();
+	reserve_exception_trampoline();
 #ifdef CONFIG_FA_DUMP
 	/*
 	 * If we fail to reserve memory for firmware-assisted dump then
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 9f3aa43..e040bb2 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -133,7 +133,7 @@ notrace void __init machine_init(u64 dt_ptr)
 
 	probe_machine();
 
-	setup_kdump_trampoline();
+	setup_exception_trampoline();
 
 #ifdef CONFIG_6xx
 	if (cpu_has_feature(CPU_FTR_CAN_DOZE) ||
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 06b8562..b077847 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -222,7 +222,7 @@ void __init early_setup(unsigned long dt_ptr)
 	/* Probe the machine type */
 	probe_machine();
 
-	setup_kdump_trampoline();
+	setup_exception_trampoline();
 
 	DBG("Found, Initializing memory management...\n");
 
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH 4/4] powerpc: make the kernel bootable from non 0 address for 6xx
From: Kevin Hao @ 2013-06-19  9:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc
In-Reply-To: <1371633604-4491-1-git-send-email-haokexin@gmail.com>

Add the support to boot the kernel from a non 0 address for 6xx.
Setup the exception trampoline if the physical start address is
not 0.

For a kdump kernel, enable the relocatable support implicitly.
Since the memstart_adddr of the kdump is not 0, we definitely
should regard this when setting up the BAT map.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/Kconfig                            |  2 +-
 arch/powerpc/include/asm/exception_trampoline.h |  4 ++--
 arch/powerpc/kernel/Makefile                    |  3 ++-
 arch/powerpc/kernel/exception_trampoline.c      | 18 +++++++++++++++---
 arch/powerpc/mm/ppc_mmu_32.c                    |  7 +------
 5 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8fe2792..6e03028 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -382,7 +382,7 @@ config KEXEC
 config CRASH_DUMP
 	bool "Build a kdump crash kernel"
 	depends on PPC64 || 6xx || FSL_BOOKE || (44x && !SMP)
-	select RELOCATABLE if PPC64 || 44x
+	select RELOCATABLE if PPC64 || 44x || 6xx
 	select DYNAMIC_MEMSTART if FSL_BOOKE
 	help
 	  Build a kernel suitable for use as a kdump capture kernel.
diff --git a/arch/powerpc/include/asm/exception_trampoline.h b/arch/powerpc/include/asm/exception_trampoline.h
index 88281c9..df4af6a 100644
--- a/arch/powerpc/include/asm/exception_trampoline.h
+++ b/arch/powerpc/include/asm/exception_trampoline.h
@@ -22,11 +22,11 @@
 
 #ifndef __ASSEMBLY__
 
-#if defined(CONFIG_CRASH_DUMP) && !defined(CONFIG_NONSTATIC_KERNEL)
+#if  defined(CONFIG_RELOCATABLE) && defined(CONFIG_6xx)
 extern void reserve_exception_trampoline(void);
 extern void setup_exception_trampoline(void);
 #else
-/* !CRASH_DUMP || !NONSTATIC_KERNEL */
+/* !CONFIG_RELOCATABLE || !CONFIG_6xx */
 static inline void reserve_exception_trampoline(void) { ; }
 static inline void setup_exception_trampoline(void) { ; }
 #endif
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index c73a0e3..c722156 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -59,7 +59,8 @@ obj-$(CONFIG_LPARCFG)		+= lparcfg.o
 obj-$(CONFIG_IBMVIO)		+= vio.o
 obj-$(CONFIG_IBMEBUS)           += ibmebus.o
 obj-$(CONFIG_GENERIC_TBSYNC)	+= smp-tbsync.o
-obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o exception_trampoline.o
+obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
+obj-$(CONFIG_RELOCATABLE)	+= exception_trampoline.o
 obj-$(CONFIG_FA_DUMP)		+= fadump.o
 ifeq ($(CONFIG_PPC32),y)
 obj-$(CONFIG_E500)		+= idle_e500.o
diff --git a/arch/powerpc/kernel/exception_trampoline.c b/arch/powerpc/kernel/exception_trampoline.c
index b725116..68aed9e 100644
--- a/arch/powerpc/kernel/exception_trampoline.c
+++ b/arch/powerpc/kernel/exception_trampoline.c
@@ -28,10 +28,16 @@
 #define DBG(fmt...)
 #endif
 
-#ifndef CONFIG_NONSTATIC_KERNEL
+#ifdef CONFIG_6xx
 void __init reserve_exception_trampoline(void)
 {
-	memblock_reserve(0, EXCEPTION_RESERVE_LIMIT);
+	/*
+	 * We don't need to reserve this memory region for a kdump kernel
+	 * since this is not included in the memory regions of kdump kernel.
+	 */
+
+	if (!memstart_addr && PHYSICAL_START)
+		memblock_reserve(0, EXCEPTION_RESERVE_LIMIT);
 }
 
 static void __init create_trampoline(unsigned long addr)
@@ -54,6 +60,12 @@ void __init setup_exception_trampoline(void)
 {
 	unsigned long i;
 
+	if (!PHYSICAL_START)
+		return;
+
+	if (PHYSICAL_START > 0x2000000)
+		panic("Don't support to load a kernel above 32M address");
+
 	DBG(" -> setup_exception_trampoline()\n");
 
 	for (i = EXCEPTION_TRAMPOLINE_START; i < EXCEPTION_TRAMPOLINE_END; i += 8) {
@@ -67,4 +79,4 @@ void __init setup_exception_trampoline(void)
 
 	DBG(" <- setup_exception_trampoline()\n");
 }
-#endif /* CONFIG_NONSTATIC_KERNEL */
+#endif /* CONFIG_6xx */
diff --git a/arch/powerpc/mm/ppc_mmu_32.c b/arch/powerpc/mm/ppc_mmu_32.c
index 11571e1..99ce477 100644
--- a/arch/powerpc/mm/ppc_mmu_32.c
+++ b/arch/powerpc/mm/ppc_mmu_32.c
@@ -86,7 +86,7 @@ unsigned long __init mmu_mapin_ram(unsigned long top)
 
 	/* Make sure we don't map a block larger than the
 	   smallest alignment of the physical address. */
-	tot = top;
+	tot = top + memstart_addr;
 	for (bl = 128<<10; bl < max_size; bl <<= 1) {
 		if (bl * 2 > tot)
 			break;
@@ -275,11 +275,6 @@ void __init MMU_init_hw(void)
 void setup_initial_memory_limit(phys_addr_t first_memblock_base,
 				phys_addr_t first_memblock_size)
 {
-	/* We don't currently support the first MEMBLOCK not mapping 0
-	 * physical on those processors
-	 */
-	BUG_ON(first_memblock_base != 0);
-
 	/* 601 can only access 16MB at the moment */
 	if (PVR_VER(mfspr(SPRN_PVR)) == 1)
 		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x01000000));
-- 
1.8.1.4

^ permalink raw reply related

* Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
From: Alexander Graf @ 2013-06-19  9:58 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: kvm@vger.kernel.org mailing list, Alexey Kardashevskiy,
	Joerg Roedel, Rusty Russell, open list, kvm-ppc, Alex Williamson,
	Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1371617970.21896.232.camel@pasglop>


On 19.06.2013, at 06:59, Benjamin Herrenschmidt wrote:

> On Wed, 2013-06-19 at 13:05 +0930, Rusty Russell wrote:
>> symbol_get() won't try to load a module; it'll just fail.  This is =
what
>> you want, since they must have vfio in the kernel to get a valid =
fd...
>=20
> Ok, cool. I suppose what we want here Alexey is slightly higher level,
> something like:
>=20
> 	vfio_validate_iommu_id(file, iommu_id)
>=20
> Which verifies that the file that was passed in is allowed to use
> that iommu_id.
>=20
> That's a simple and flexible interface (ie, it will work even if we
> support multiple iommu IDs in the future for a vfio, for example
> for DDW windows etc...), the logic to know about the ID remains
> in qemu, this is strictly a validation call.
>=20
> That way we also don't have to expose the containing vfio struct =
etc...
> just that simple function.
>=20
> Alex, any objection ?

Which Alex? :)

I think validate works, it keeps iteration logic out of the kernel which =
is a good thing. There still needs to be an interface for getting the =
iommu id in VFIO, but I suppose that one's for the other Alex and J=F6rg =
to comment on.

>=20
> Do we need to make it a get/put interface instead ?
>=20
> 	vfio_validate_and_use_iommu(file, iommu_id);
>=20
> 	vfio_release_iommu(file, iommu_id);
>=20
> To ensure that the resource remains owned by the process until KVM
> is closed as well ?
>=20
> Or do we want to register with VFIO with a callback so that VFIO can
> call us if it needs us to give it up ?

Can't we just register a handler on the fd and get notified when it =
closes? Can you kill VFIO access without closing the fd?


Alex

^ permalink raw reply

* Re: [PATCH] powerpc/THP: Wait for all hash_page calls to finish before invalidating HPTE entries
From: Benjamin Herrenschmidt @ 2013-06-19 10:18 UTC (permalink / raw)
  To: Michael Neuling; +Cc: linuxppc-dev, paulus, Aneesh Kumar K.V
In-Reply-To: <9335.1371626096@ale.ozlabs.ibm.com>

On Wed, 2013-06-19 at 17:14 +1000, Michael Neuling wrote:

> > This is a bug fix. The details can be found at 
> 
> Can you make this more obvious in the changelog (as well as making it 80
> col).  I don't see 'bug' mentioned anywhere.  'Fix' is mentioned
> somewhere in the middle of the changelog.
> 
> > 
> > http://article.gmane.org/gmane.linux.ports.ppc.embedded/60266
> 
> OK, but V10 THP is not in yet, right?  So why not roll it into that
> series rather than pushing broken stuff and fixing it?

Right, this is expected. This fix is a result of me asking him to do it
that way on ST and should eventually be folded in the series (or
re-ordered to be in there before the patch that enables THP).

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 05/31] powerpc/eeh: Trace PCI bus from PE
From: Gavin Shan @ 2013-06-19 10:20 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1371544435-4943-6-git-send-email-shangw@linux.vnet.ibm.com>

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

On Tue, Jun 18, 2013 at 04:33:29PM +0800, Gavin Shan wrote:
>There're several types of PEs can be supported for now: PHB, Bus
>and Device dependent PE. For PCI bus dependent PE, tracing the
>corresponding PCI bus from PE (struct eeh_pe) would make the code
>more efficient. The patch also enables the retrieval of PCI bus based
>on the PCI bus dependent PE.
>
>Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>---

Please use the updated [5/31] in the attachment.

Thanks,
Gavin


[-- Attachment #2: 0005-powerpc-eeh-Trace-PCI-bus-from-PE.patch --]
[-- Type: text/x-diff, Size: 2585 bytes --]

>From ebfc469dc8fa0748571c5fc9766b5606ce5d8620 Mon Sep 17 00:00:00 2001
From: Gavin Shan <shangw@linux.vnet.ibm.com>
Date: Wed, 19 Jun 2013 14:11:43 +0800
Subject: [PATCH 05/31] powerpc/eeh: Trace PCI bus from PE

There're several types of PEs can be supported for now: PHB, Bus
and Device dependent PE. For PCI bus dependent PE, tracing the
corresponding PCI bus from PE (struct eeh_pe) would make the code
more efficient. The patch also enables the retrieval of PCI bus based
on the PCI bus dependent PE.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h |    1 +
 arch/powerpc/kernel/eeh_pe.c   |   17 +++++++++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index acdfcaa..f3b49d6 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -59,6 +59,7 @@ struct eeh_pe {
 	int config_addr;		/* Traditional PCI address	*/
 	int addr;			/* PE configuration address	*/
 	struct pci_controller *phb;	/* Associated PHB		*/
+	struct pci_bus *bus;		/* Top PCI bus for bus PE	*/
 	int check_count;		/* Times of ignored error	*/
 	int freeze_count;		/* Times of froze up		*/
 	int false_positives;		/* Times of reported #ff's	*/
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 3d2dcf5..c963667 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -365,6 +365,17 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
 	pe->config_addr	= edev->config_addr;
 
 	/*
+	 * While doing PE reset, we probably hot-reset the
+	 * upstream bridge. However, the PCI devices including
+	 * the associated EEH devices might be removed when EEH
+	 * core is doing recovery. So that won't safe to retrieve
+	 * the bridge through downstream EEH device. We have to
+	 * trace the parent PCI bus, then the upstream bridge.
+	 */
+	if (eeh_probe_mode_dev())
+		pe->bus = eeh_dev_to_pci_dev(edev)->bus;
+
+	/*
 	 * Put the new EEH PE into hierarchy tree. If the parent
 	 * can't be found, the newly created PE will be attached
 	 * to PHB directly. Otherwise, we have to associate the
@@ -641,12 +652,18 @@ struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe)
 		bus = pe->phb->bus;
 	} else if (pe->type & EEH_PE_BUS ||
 		   pe->type & EEH_PE_DEVICE) {
+		if (pe->bus) {
+			bus = pe->bus;
+			goto out;
+		}
+
 		edev = list_first_entry(&pe->edevs, struct eeh_dev, list);
 		pdev = eeh_dev_to_pci_dev(edev);
 		if (pdev)
 			bus = pdev->bus;
 	}
 
+out:
 	eeh_unlock();
 
 	return bus;
-- 
1.7.5.4


^ permalink raw reply related

* Re: [PATCH] powerpc/kvm: Handle transparent hugepage in KVM
From: Aneesh Kumar K.V @ 2013-06-19 12:30 UTC (permalink / raw)
  To: Michael Neuling; +Cc: paulus, linuxppc-dev
In-Reply-To: <9103.1371625902@ale.ozlabs.ibm.com>

Michael Neuling <mikey@neuling.org> writes:

> Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>> 
>> We can find pte that are splitting while walking page tables. Return
>> None pte in that case.
>
> Can you expand on this more please.  There are a lot of details below
> like removing a ldarx/stdcx loop that should be better described here.
>
>> 
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/kvm_book3s_64.h | 51 ++++++++++++++++++--------------
>>  arch/powerpc/kvm/book3s_64_mmu_hv.c      |  7 +++--
>>  arch/powerpc/kvm/book3s_hv_rm_mmu.c      |  4 +--
>>  3 files changed, 34 insertions(+), 28 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
>> index 9c1ff33..ce20f7e 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
>> @@ -162,33 +162,40 @@ static inline int hpte_cache_flags_ok(unsigned long ptel, unsigned long io_type)
>>   * Lock and read a linux PTE.  If it's present and writable, atomically
>>   * set dirty and referenced bits and return the PTE, otherwise return 0.
>
> This is comment still valid now the ldarx/stdcx is gone?  

In a way yes. Instead of lock and read as it was before, it is now done
via cmpxchg which still use ldarx/stdcx


>
>>   */
>> -static inline pte_t kvmppc_read_update_linux_pte(pte_t *p, int writing)
>> +static inline pte_t kvmppc_read_update_linux_pte(pte_t *ptep, int writing,
>> +						 unsigned int hugepage)
>>  {
>> -	pte_t pte, tmp;
>> -
>> -	/* wait until _PAGE_BUSY is clear then set it atomically */
>> -	__asm__ __volatile__ (
>> -		"1:	ldarx	%0,0,%3\n"
>> -		"	andi.	%1,%0,%4\n"
>> -		"	bne-	1b\n"
>> -		"	ori	%1,%0,%4\n"
>> -		"	stdcx.	%1,0,%3\n"
>> -		"	bne-	1b"
>> -		: "=&r" (pte), "=&r" (tmp), "=m" (*p)
>> -		: "r" (p), "i" (_PAGE_BUSY)
>> -		: "cc");
>> -
>> -	if (pte_present(pte)) {
>> -		pte = pte_mkyoung(pte);
>> -		if (writing && pte_write(pte))
>> -			pte = pte_mkdirty(pte);
>> -	}
>> +	pte_t old_pte, new_pte = __pte(0);
>> +repeat:
>> +	do {
>> +		old_pte = pte_val(*ptep);
>> +		/*
>> +		 * wait until _PAGE_BUSY is clear then set it atomically
>> +		 */
>> +		if (unlikely(old_pte & _PAGE_BUSY))
>> +			goto repeat;
>
> continue here?  Please don't create looping primitives.

No that would be wrong. (I did that in an earlier version :).We really
don't want the below cmpxchg to run if we find _PAGE_BUSY.

>   
>> +
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> +		/* If hugepage and is trans splitting return None */
>> +		if (unlikely(hugepage &&
>> +			     pmd_trans_splitting(pte_pmd(old_pte))))
>
> Comment looks much like the code... seems redundant.
>
>> +			return __pte(0);
>> +#endif
>>  
>> -	*p = pte;	/* clears _PAGE_BUSY */
>> +		/* If pte is not present return None */
>> +		if (unlikely(!(old_pte & _PAGE_PRESENT)))
>> +			return __pte(0);
>>  
>> -	return pte;
>> +		new_pte = pte_mkyoung(old_pte);
>> +		if (writing && pte_write(old_pte))
>> +			new_pte = pte_mkdirty(new_pte);
>> +
>> +	} while (old_pte != __cmpxchg_u64((unsigned long *)ptep,
>> +					  old_pte, new_pte));
>> +	return new_pte;
>>  }
>>  
>> +
>
> Whitespace
>
>>  /* Return HPTE cache control bits corresponding to Linux pte bits */
>>  static inline unsigned long hpte_cache_bits(unsigned long pte_val)
>>  {
>> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
>> index 5880dfb..e1a9415 100644
>> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
>> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
>> @@ -675,6 +675,7 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
>>  		}
>>  		/* if the guest wants write access, see if that is OK */
>>  		if (!writing && hpte_is_writable(r)) {
>> +			unsigned int shift;
>>  			pte_t *ptep, pte;
>>  
>>  			/*
>> @@ -683,9 +684,9 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
>>  			 */
>>  			rcu_read_lock_sched();
>>  			ptep = find_linux_pte_or_hugepte(current->mm->pgd,
>> -							 hva, NULL);
>> -			if (ptep && pte_present(*ptep)) {
>> -				pte = kvmppc_read_update_linux_pte(ptep, 1);
>> +							 hva, &shift);
>> +			if (ptep) {
>> +				pte = kvmppc_read_update_linux_pte(ptep, 1, shift);
>>  				if (pte_write(pte))
>>  					write_ok = 1;
>>  			}
>> diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
>> index dcf892d..39ae723 100644
>> --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
>> +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
>> @@ -150,9 +150,7 @@ static pte_t lookup_linux_pte(pgd_t *pgdir, unsigned long hva,
>>  		*pte_sizep = PAGE_SIZE;
>>  	if (ps > *pte_sizep)
>>  		return __pte(0);
>> -	if (!pte_present(*ptep))
>> -		return __pte(0);
>> -	return kvmppc_read_update_linux_pte(ptep, writing);
>> +	return kvmppc_read_update_linux_pte(ptep, writing, shift);
>
> 'shift' goes into the new 'hugepage' parameter?  Doesn't seem logical?
> Can we harmonise the name to make it less confusing?
>

it is actually the shift bits represending hugepage size. We set it to 0
if we don't find hugepage in find_linux_pte_or_hugepte. May be something
like hugepage_shift is better ?

-aneesh

^ permalink raw reply

* DEBUG_PAGEALLOC on PPC not working (kernels 2.6-25, 3.0-34)
From: saikia.partha @ 2013-06-19 12:58 UTC (permalink / raw)
  To: linuxppc-dev

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

Hi,

We have two Freescale PowerPC boards on which we're trying to enable
DEBUG_PAGEALLOC with the hope that we'll see an exception whenever some
code tries to modify a page that's been already freed. To test it, we wrote
this sample code -

===
#define BUF_SIZE    4096
void
pagealloc_test(void)
{
    char *buf = kmalloc(BUF_SIZE, GFP_KERNEL);

    if (!buf) {
        printk("%s[%d] - alloc failed!\n", __func__, __LINE__);
        return;
    }
    printk("%s[%d] - alloc'd\n", __func__, __LINE__);
    memset(&buf[0], 0, BUF_SIZE);
    printk("%s[%d] - memset'd\n", __func__, __LINE__);
    kfree(buf);
    printk("%s[%d] - free'd\n", __func__, __LINE__);
    memset(&buf[0], 1, BUF_SIZE);
    printk("%s[%d] - memset'd after free!\n", __func__, __LINE__);
}
===

Here, the last memset() should generate an exception if PAGEALLOC code
correctly unmapped the page during kfree(). However, kernel is happily
running after the memset post-free. Any clue?
Also, the 2nd board has Book-E which has a different MMU architecture.
Here, I saw that the function __change_page_attr() always returns as the
condition v_mapped_by_tlbcam() is true. How do we remove the pte entry from
the TLB in this case?  Has anyone tried DEBUG_PAGEALLOC on Book-E?

Please find the board details below -
Board 1: CPU - e300c4, kernel 2.6-25
====================================
CONFIG_6xx=y
CONFIG_PPC_FPU=y
CONFIG_PPC_STD_MMU=y
CONFIG_PPC_STD_MMU_32=y
CONFIG_PPC32=y
CONFIG_WORD_SIZE=32
CONFIG_PPC_MERGE=y
CONFIG_MMU=y
CONFIG_IRQ_PER_CPU=y
CONFIG_PPC=y

Board 2: CPU - e500v2, kernel 3.0-34
====================================
# CONFIG_PPC_BOOK3S_32 is not set
CONFIG_PPC_85xx=y
CONFIG_E500=y
# CONFIG_PPC_E500MC is not set
CONFIG_FSL_EMB_PERFMON=y
CONFIG_BOOKE=y
CONFIG_FSL_BOOKE=y
CONFIG_PPC_FSL_BOOK3E=y
# CONFIG_PHYS_64BIT is not set
CONFIG_SPE=y
CONFIG_PPC_MMU_NOHASH=y
CONFIG_PPC_MMU_NOHASH_32=y
CONFIG_PPC_BOOK3E_MMU=y
# CONFIG_PPC_MM_SLICES is not set
CONFIG_SMP=y
CONFIG_NR_CPUS=2
CONFIG_PPC32=y
CONFIG_32BIT=y
CONFIG_WORD_SIZE=32
# CONFIG_ARCH_PHYS_ADDR_T_64BIT is not set
# CONFIG_ARCH_DMA_ADDR_T_64BIT is not set
CONFIG_MMU=y
CONFIG_NR_IRQS=512
CONFIG_PPC=y

Regards,
Partha

[-- Attachment #2: Type: text/html, Size: 2558 bytes --]

^ permalink raw reply

* DEBUG_PAGEALLOC on PPC not working (kernels 2.6-25, 3.0-34)
From: saikia.partha @ 2013-06-19 13:09 UTC (permalink / raw)
  To: linuxppc-dev

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

Hi,

We have two Freescale PowerPC boards on which we're trying to enable
DEBUG_PAGEALLOC with the hope that we'll see an exception whenever some
code tries to modify a page that's been already freed. To test it, we wrote
this sample code -

===
#define BUF_SIZE    4096
void
pagealloc_test(void)
{
    char *buf = kmalloc(BUF_SIZE, GFP_KERNEL);

    if (!buf) {
        printk("%s[%d] - alloc failed!\n", __func__, __LINE__);
        return;
    }
    printk("%s[%d] - alloc'd\n", __func__, __LINE__);
    memset(&buf[0], 0, BUF_SIZE);
    printk("%s[%d] - memset'd\n", __func__, __LINE__);
    kfree(buf);
    printk("%s[%d] - free'd\n", __func__, __LINE__);
    memset(&buf[0], 1, BUF_SIZE);
    printk("%s[%d] - memset'd after free!\n", __func__, __LINE__);
}
===

Here, the last memset() should generate an exception if PAGEALLOC code
correctly unmapped the page during kfree(). However, kernel is happily
running after the memset post-free. Any clue?
Also, the 2nd board has Book-E which has a different MMU architecture.
Here, I saw that the function __change_page_attr() always returns as the
condition v_mapped_by_tlbcam() is true. How do we remove the pte entry from
the TLB in this case?  Has anyone tried DEBUG_PAGEALLOC on Book-E?

Please find the board details below -
Board 1: CPU - e300c4, kernel 2.6-25
==============================
======
CONFIG_6xx=y
CONFIG_PPC_FPU=y
CONFIG_PPC_STD_MMU=y
CONFIG_PPC_STD_MMU_32=y
CONFIG_PPC32=y
CONFIG_WORD_SIZE=32
CONFIG_PPC_MERGE=y
CONFIG_MMU=y
CONFIG_IRQ_PER_CPU=y
CONFIG_PPC=y

Board 2: CPU - e500v2, kernel 3.0-34
====================================
# CONFIG_PPC_BOOK3S_32 is not set
CONFIG_PPC_85xx=y
CONFIG_E500=y
# CONFIG_PPC_E500MC is not set
CONFIG_FSL_EMB_PERFMON=y
CONFIG_BOOKE=y
CONFIG_FSL_BOOKE=y
CONFIG_PPC_FSL_BOOK3E=y
# CONFIG_PHYS_64BIT is not set
CONFIG_SPE=y
CONFIG_PPC_MMU_NOHASH=y
CONFIG_PPC_MMU_NOHASH_32=y
CONFIG_PPC_BOOK3E_MMU=y
# CONFIG_PPC_MM_SLICES is not set
CONFIG_SMP=y
CONFIG_NR_CPUS=2
CONFIG_PPC32=y
CONFIG_32BIT=y
CONFIG_WORD_SIZE=32
# CONFIG_ARCH_PHYS_ADDR_T_64BIT is not set
# CONFIG_ARCH_DMA_ADDR_T_64BIT is not set
CONFIG_MMU=y
CONFIG_NR_IRQS=512
CONFIG_PPC=y

Regards,
Partha

[-- Attachment #2: Type: text/html, Size: 2605 bytes --]

^ permalink raw reply

* Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
From: Benjamin Herrenschmidt @ 2013-06-19 14:50 UTC (permalink / raw)
  To: Alexander Graf
  Cc: kvm@vger.kernel.org mailing list, Alexey Kardashevskiy,
	Joerg Roedel, Rusty Russell, open list, kvm-ppc, Alex Williamson,
	Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <E63966E3-336F-4CB8-96EC-24D2C1644E62@suse.de>

On Wed, 2013-06-19 at 11:58 +0200, Alexander Graf wrote:

> > Alex, any objection ?
> 
> Which Alex? :)

Heh, mostly Williamson in this specific case but your input is still
welcome :-)

> I think validate works, it keeps iteration logic out of the kernel
> which is a good thing. There still needs to be an interface for
> getting the iommu id in VFIO, but I suppose that one's for the other
> Alex and Jörg to comment on.

I think getting the iommu fd is already covered by separate patches from
Alexey.

> > 
> > Do we need to make it a get/put interface instead ?
> > 
> > 	vfio_validate_and_use_iommu(file, iommu_id);
> > 
> > 	vfio_release_iommu(file, iommu_id);
> > 
> > To ensure that the resource remains owned by the process until KVM
> > is closed as well ?
> > 
> > Or do we want to register with VFIO with a callback so that VFIO can
> > call us if it needs us to give it up ?
> 
> Can't we just register a handler on the fd and get notified when it
> closes? Can you kill VFIO access without closing the fd?

That sounds actually harder :-)

The question is basically: When we validate that relationship between a
specific VFIO struct file with an iommu, what is the lifetime of that
and how do we handle this lifetime properly.

There's two ways for that sort of situation: The notification model
where we get notified when the relationship is broken, and the refcount
model where we become a "user" and thus delay the breaking of the
relationship until we have been disposed of as well.

In this specific case, it's hard to tell what is the right model from my
perspective, which is why I would welcome Alex (W.) input.

In the end, the solution will end up being in the form of APIs exposed
by VFIO for use by KVM (via that symbol lookup mechanism) so Alex (W),
as owner of VFIO at this stage, what do you want those to look
like ? :-)

Cheers,
Ben.
 
> 
> Alex
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Pull request: scottwood/linux.git for-3.10
From: Kumar Gala @ 2013-06-19 15:06 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20130618201452.GA5429@home.buserror.net>


On Jun 18, 2013, at 3:14 PM, Scott Wood wrote:

> This fixes a regression that causes 83xx to oops on boot if a
> non-express PCI bus is present.
>=20
> The following changes since commit =
17858ca65eef148d335ffd4cfc09228a1c1cbfb5:
>=20
>  Merge tag 'please-pull-fixia64' of =
git://git.kernel.org/pub/scm/linux/kernel/git/aegl/linux (2013-06-18 =
06:29:19 -1000)
>=20
> are available in the git repository at:
>=20
>=20
>  git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git =
for-3.10
>=20
> for you to fetch changes up to =
2383ea94854bcf5a0df3c6803b980868cef95418:
>=20
>  powerpc/pci: Fix setup of Freescale PCI / PCIe controllers =
(2013-06-18 14:44:57 -0500)
>=20
> ----------------------------------------------------------------
> Rojhalat Ibrahim (1):
>      powerpc/pci: Fix setup of Freescale PCI / PCIe controllers
>=20
> arch/powerpc/sysdev/fsl_pci.c |   24 +++++++++---------------
> 1 file changed, 9 insertions(+), 15 deletions(-)

What about Rohit's patch: powerpc/pci: Fix setup of Freescale PCI / PCIe =
controllers?  Seems like also a fix for 3.10

- k=

^ permalink raw reply

* Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
From: Alex Williamson @ 2013-06-19 15:49 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: kvm@vger.kernel.org mailing list, Alexey Kardashevskiy,
	Joerg Roedel, Rusty Russell, Alexander Graf, kvm-ppc, open list,
	Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1371653443.21896.291.camel@pasglop>

On Thu, 2013-06-20 at 00:50 +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2013-06-19 at 11:58 +0200, Alexander Graf wrote:
> 
> > > Alex, any objection ?
> > 
> > Which Alex? :)
> 
> Heh, mostly Williamson in this specific case but your input is still
> welcome :-)
> 
> > I think validate works, it keeps iteration logic out of the kernel
> > which is a good thing. There still needs to be an interface for
> > getting the iommu id in VFIO, but I suppose that one's for the other
> > Alex and Jörg to comment on.
> 
> I think getting the iommu fd is already covered by separate patches from
> Alexey.
> 
> > > 
> > > Do we need to make it a get/put interface instead ?
> > > 
> > > 	vfio_validate_and_use_iommu(file, iommu_id);
> > > 
> > > 	vfio_release_iommu(file, iommu_id);
> > > 
> > > To ensure that the resource remains owned by the process until KVM
> > > is closed as well ?
> > > 
> > > Or do we want to register with VFIO with a callback so that VFIO can
> > > call us if it needs us to give it up ?
> > 
> > Can't we just register a handler on the fd and get notified when it
> > closes? Can you kill VFIO access without closing the fd?
> 
> That sounds actually harder :-)
> 
> The question is basically: When we validate that relationship between a
> specific VFIO struct file with an iommu, what is the lifetime of that
> and how do we handle this lifetime properly.
> 
> There's two ways for that sort of situation: The notification model
> where we get notified when the relationship is broken, and the refcount
> model where we become a "user" and thus delay the breaking of the
> relationship until we have been disposed of as well.
> 
> In this specific case, it's hard to tell what is the right model from my
> perspective, which is why I would welcome Alex (W.) input.
> 
> In the end, the solution will end up being in the form of APIs exposed
> by VFIO for use by KVM (via that symbol lookup mechanism) so Alex (W),
> as owner of VFIO at this stage, what do you want those to look
> like ? :-)

My first thought is that we should use the same reference counting as we
have for vfio devices (group->container_users).  An interface for that
might look like:

int vfio_group_add_external_user(struct file *filep)
{
	struct vfio_group *group = filep->private_data;

	if (filep->f_op != &vfio_group_fops)
		return -EINVAL;


	if (!atomic_inc_not_zero(&group->container_users))
		return -EINVAL;

	return 0;
}

void vfio_group_del_external_user(struct file *filep)
{
	struct vfio_group *group = filep->private_data;

	BUG_ON(filep->f_op != &vfio_group_fops);

	vfio_group_try_dissolve_container(group);
}

int vfio_group_iommu_id_from_file(struct file *filep)
{
	struct vfio_group *group = filep->private_data;

	BUG_ON(filep->f_op != &vfio_group_fops);

	return iommu_group_id(group->iommu_group);
}

Would that work?  Thanks,

Alex

^ permalink raw reply

* Re: [PATCH 1/2] perf tools: fix a typo of a Power7 event name
From: Sukadev Bhattiprolu @ 2013-06-19 16:13 UTC (permalink / raw)
  To: Runzhen Wang
  Cc: acme, icycoder, linux-kernel, xiaoguangrong, paulus, linuxppc-dev
In-Reply-To: <1371633326-7696-1-git-send-email-runzhen@linux.vnet.ibm.com>

Runzhen Wang [runzhen@linux.vnet.ibm.com] wrote:
| In the Power7 PMU guide:
| https://www.power.org/documentation/commonly-used-metrics-for-performance-analysis/
| PM_BRU_MPRED is referred to as PM_BR_MPRED.
| 
| This patch fix the typo by changing the name of the event in kernel and
| documentation accordingly.
| 
| Signed-off-by: Runzhen Wang <runzhen@linux.vnet.ibm.com>

Reviewed-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>

^ permalink raw reply

* Re: [PATCH 2/2] perf tools: Make Power7 events available for perf
From: Sukadev Bhattiprolu @ 2013-06-19 16:14 UTC (permalink / raw)
  To: Runzhen Wang
  Cc: acme, icycoder, linux-kernel, xiaoguangrong, paulus, linuxppc-dev
In-Reply-To: <1371633326-7696-2-git-send-email-runzhen@linux.vnet.ibm.com>

Runzhen Wang [runzhen@linux.vnet.ibm.com] wrote:
| Power7 supports over 530 different perf events but only a small
| subset of these can be specified by name, for the remaining
| events, we must specify them by their raw code:
| 
|         perf stat -e r2003c <application>
| 
| This patch makes all the POWER7 events available in sysfs.
| So we can instead specify these as:
| 
|         perf stat -e 'cpu/PM_CMPLU_STALL_DFU/' <application>
| 
| where PM_CMPLU_STALL_DFU is the r2003c in previous example.
| 
| Before this patch is applied, the size of power7-pmu.o is:
| 
| $ size arch/powerpc/perf/power7-pmu.o
|    text	   data	    bss	    dec	    hex	filename
|    3073	   2720	      0	   5793	   16a1	arch/powerpc/perf/power7-pmu.o
| 
| and after the patch is applied, it is:
| 
| $ size arch/powerpc/perf/power7-pmu.o
|    text	   data	    bss	    dec	    hex	filename
|   14451	  31112	      0	  45563	   b1fb	arch/powerpc/perf/power7-pmu.o
| 
| Signed-off-by: Runzhen Wang <runzhen@linux.vnet.ibm.com>

Reviewed-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>

^ permalink raw reply

* Re: Pull request: scottwood/linux.git for-3.10
From: Scott Wood @ 2013-06-19 16:50 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <AEF681B9-AB7E-4C3D-99D6-BD29EF038A50@kernel.crashing.org>

On 06/19/2013 10:06:38 AM, Kumar Gala wrote:
>=20
> On Jun 18, 2013, at 3:14 PM, Scott Wood wrote:
>=20
> > This fixes a regression that causes 83xx to oops on boot if a
> > non-express PCI bus is present.
> >
> > The following changes since commit =20
> 17858ca65eef148d335ffd4cfc09228a1c1cbfb5:
> >
> >  Merge tag 'please-pull-fixia64' of =20
> git://git.kernel.org/pub/scm/linux/kernel/git/aegl/linux (2013-06-18 =20
> 06:29:19 -1000)
> >
> > are available in the git repository at:
> >
> >
> >  git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git =20
> for-3.10
> >
> > for you to fetch changes up to =20
> 2383ea94854bcf5a0df3c6803b980868cef95418:
> >
> >  powerpc/pci: Fix setup of Freescale PCI / PCIe controllers =20
> (2013-06-18 14:44:57 -0500)
> >
> > ----------------------------------------------------------------
> > Rojhalat Ibrahim (1):
> >      powerpc/pci: Fix setup of Freescale PCI / PCIe controllers
> >
> > arch/powerpc/sysdev/fsl_pci.c |   24 +++++++++---------------
> > 1 file changed, 9 insertions(+), 15 deletions(-)
>=20
> What about Rohit's patch: powerpc/pci: Fix setup of Freescale PCI / =20
> PCIe controllers?  Seems like also a fix for 3.10

That's the patch I'm asking you to pull. :-P

-Scott=

^ permalink raw reply

* RE: [PATCH v4 0/8] Nvram-to-pstore
From: Luck, Tony @ 2013-06-19 18:46 UTC (permalink / raw)
  To: Michael Neuling, Aruna Balakrishnaiah
  Cc: jkenisto@linux.vnet.ibm.com, keescook@chromium.org,
	mahesh@linux.vnet.ibm.com, cbouatmailru@gmail.com,
	linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	paulus@samba.org, anton@samba.org, ccross@android.com
In-Reply-To: <5493.1371623906@ale.ozlabs.ibm.com>

> You need to mount pstore to access the files.
>=20
> # mkdir /dev/pstore
> # mount -t pstore - /dev/pstore
>=20
> to unmount
>=20
> # umount /dev/pstore
>=20
> References: http://lwn.net/Articles/421297/

Note that /dev/pstore has fallen out of fashion as the mount point ... we n=
ow (since 3.9)
suggest /sys/fs/pstore

> Documentation/ABI/testing/pstore

This file was updated with the new location.

-Tony
=20

^ permalink raw reply

* Re: DEBUG_PAGEALLOC on PPC not working (kernels 2.6-25, 3.0-34)
From: Scott Wood @ 2013-06-19 21:00 UTC (permalink / raw)
  To: saikia.partha; +Cc: linuxppc-dev
In-Reply-To: <CABeCxOKXpgVaoigGBdCVo3tYgbfJnesNPOuOyTc0EKv_CTKoyg@mail.gmail.com>

On 06/19/2013 08:09:44 AM, saikia.partha wrote:
> Hi,
>=20
> We have two Freescale PowerPC boards on which we're trying to enable
> DEBUG_PAGEALLOC with the hope that we'll see an exception whenever =20
> some
> code tries to modify a page that's been already freed. To test it, we =20
> wrote
> this sample code -
>=20
> =3D=3D=3D
> #define BUF_SIZE    4096
> void
> pagealloc_test(void)
> {
>     char *buf =3D kmalloc(BUF_SIZE, GFP_KERNEL);
>=20
>     if (!buf) {
>         printk("%s[%d] - alloc failed!\n", __func__, __LINE__);
>         return;
>     }
>     printk("%s[%d] - alloc'd\n", __func__, __LINE__);
>     memset(&buf[0], 0, BUF_SIZE);
>     printk("%s[%d] - memset'd\n", __func__, __LINE__);
>     kfree(buf);
>     printk("%s[%d] - free'd\n", __func__, __LINE__);
>     memset(&buf[0], 1, BUF_SIZE);
>     printk("%s[%d] - memset'd after free!\n", __func__, __LINE__);
> }
> =3D=3D=3D
>=20
> Here, the last memset() should generate an exception if PAGEALLOC code
> correctly unmapped the page during kfree(). However, kernel is happily
> running after the memset post-free. Any clue?
> Also, the 2nd board has Book-E which has a different MMU architecture.
> Here, I saw that the function __change_page_attr() always returns as =20
> the
> condition v_mapped_by_tlbcam() is true. How do we remove the pte =20
> entry from
> the TLB in this case?  Has anyone tried DEBUG_PAGEALLOC on Book-E?

This is not supported on booke -- the tlbcam mapping is required for =20
all lowmem.

> Please find the board details below -
> Board 1: CPU - e300c4, kernel 2.6-25

For e300, I think I had it working at one point a few years ago (see =20
commit bde6c6e16aa489ea76c762fb7ffb0abb48660dd8).

The reason we can do it on e300 and not on booke is because e300 takes =20
exceptions in real mode.  On e500 the MMU is always enabled, so we need =20
bolted TLB1 entries that cover at least all exception code (up to the =20
point where a TLB miss could safely be taken) and all page tables (in =20
practice, we just bolt all lowmem) and other data that can be =20
referenced from said exception code.  There are not enough TLB1 entries =20
to do this on a fine-grained basis.

-Scott=

^ permalink raw reply

* Re: [PATCH] powerpc/kvm: Handle transparent hugepage in KVM
From: Michael Neuling @ 2013-06-19 23:59 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: paulus, linuxppc-dev
In-Reply-To: <87k3lqxoy4.fsf@linux.vnet.ibm.com>

> >> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> >> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> >> @@ -162,33 +162,40 @@ static inline int hpte_cache_flags_ok(unsigned long ptel, unsigned long io_type)
> >>   * Lock and read a linux PTE.  If it's present and writable, atomically
> >>   * set dirty and referenced bits and return the PTE, otherwise return 0.
> >
> > This is comment still valid now the ldarx/stdcx is gone?  
> 
> In a way yes. Instead of lock and read as it was before, it is now done
> via cmpxchg which still use ldarx/stdcx

OK, maybe you can update to reflect that.

> >> +	pte_t old_pte, new_pte = __pte(0);
> >> +repeat:
> >> +	do {
> >> +		old_pte = pte_val(*ptep);
> >> +		/*
> >> +		 * wait until _PAGE_BUSY is clear then set it atomically
> >> +		 */
> >> +		if (unlikely(old_pte & _PAGE_BUSY))
> >> +			goto repeat;
> >
> > continue here?  Please don't create looping primitives.
> 
> No that would be wrong. (I did that in an earlier version :).We really
> don't want the below cmpxchg to run if we find _PAGE_BUSY.

How about something like this then?

while (1) {
      if (unlikely(old_pte & _PAGE_BUSY))
	    continue;
.....
      if cmpxchg(foo)
      	 break;
}


> 
> >   
> >> +
> >> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> >> +		/* If hugepage and is trans splitting return None */
> >> +		if (unlikely(hugepage &&
> >> +			     pmd_trans_splitting(pte_pmd(old_pte))))
> >
> > Comment looks much like the code... seems redundant.
> >
> >> +			return __pte(0);
> >> +#endif
> >>  
> >> -	*p = pte;	/* clears _PAGE_BUSY */
> >> +		/* If pte is not present return None */
> >> +		if (unlikely(!(old_pte & _PAGE_PRESENT)))
> >> +			return __pte(0);
> >>  
> >> -	return pte;
> >> +		new_pte = pte_mkyoung(old_pte);
> >> +		if (writing && pte_write(old_pte))
> >> +			new_pte = pte_mkdirty(new_pte);
> >> +
> >> +	} while (old_pte != __cmpxchg_u64((unsigned long *)ptep,
> >> +					  old_pte, new_pte));
> >> +	return new_pte;
> >>  }
> >>  
> >> +
> >
> > Whitespace



> >> diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> >> index dcf892d..39ae723 100644
> >> --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> >> +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> >> @@ -150,9 +150,7 @@ static pte_t lookup_linux_pte(pgd_t *pgdir, unsigned long hva,
> >>  		*pte_sizep = PAGE_SIZE;
> >>  	if (ps > *pte_sizep)
> >>  		return __pte(0);
> >> -	if (!pte_present(*ptep))
> >> -		return __pte(0);
> >> -	return kvmppc_read_update_linux_pte(ptep, writing);
> >> +	return kvmppc_read_update_linux_pte(ptep, writing, shift);
> >
> > 'shift' goes into the new 'hugepage' parameter?  Doesn't seem logical?
> > Can we harmonise the name to make it less confusing?
> >
> 
> it is actually the shift bits represending hugepage size. We set it to 0
> if we don't find hugepage in find_linux_pte_or_hugepte. May be something
> like hugepage_shift is better ?

Sure.

Mikey

^ permalink raw reply

* Re: [PATCH 1/2] perf tools: fix a typo of a Power7 event name
From: Michael Ellerman @ 2013-06-20  1:21 UTC (permalink / raw)
  To: Runzhen Wang
  Cc: icycoder, linux-kernel, xiaoguangrong, acme, paulus, sukadev,
	linuxppc-dev
In-Reply-To: <1371633326-7696-1-git-send-email-runzhen@linux.vnet.ibm.com>

On Wed, 2013-06-19 at 17:15 +0800, Runzhen Wang wrote:
> In the Power7 PMU guide:
> https://www.power.org/documentation/commonly-used-metrics-for-performance-analysis/
> PM_BRU_MPRED is referred to as PM_BR_MPRED.
> 
> This patch fix the typo by changing the name of the event in kernel and
> documentation accordingly.
> 
> Signed-off-by: Runzhen Wang <runzhen@linux.vnet.ibm.com>
> ---
>  .../testing/sysfs-bus-event_source-devices-events  |    2 +-
>  arch/powerpc/perf/power7-pmu.c                     |   12 ++++++------
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> index 8b25ffb..3c1cc24 100644
> --- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> +++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> @@ -29,7 +29,7 @@ Description:	Generic performance monitoring events
>  
>  What: 		/sys/devices/cpu/events/PM_1PLUS_PPC_CMPL
>  		/sys/devices/cpu/events/PM_BRU_FIN
> -		/sys/devices/cpu/events/PM_BRU_MPRED
> +		/sys/devices/cpu/events/PM_BR_MPRED

So you're changing the ABI.

I think that's probably OK, but you at least need to mention _why_ you
think it's OK.

cheers

^ permalink raw reply

* Re: [PATCH 2/2] perf tools: Make Power7 events available for perf
From: Michael Ellerman @ 2013-06-20  1:41 UTC (permalink / raw)
  To: Runzhen Wang
  Cc: icycoder, linux-kernel, xiaoguangrong, acme, paulus, sukadev,
	linuxppc-dev
In-Reply-To: <1371633326-7696-2-git-send-email-runzhen@linux.vnet.ibm.com>

On Wed, 2013-06-19 at 17:15 +0800, Runzhen Wang wrote:
> Power7 supports over 530 different perf events but only a small
> subset of these can be specified by name, for the remaining
> events, we must specify them by their raw code:
> 
>         perf stat -e r2003c <application>
> 
> This patch makes all the POWER7 events available in sysfs.
> So we can instead specify these as:
> 
>         perf stat -e 'cpu/PM_CMPLU_STALL_DFU/' <application>
> 
> where PM_CMPLU_STALL_DFU is the r2003c in previous example.
> 
> Before this patch is applied, the size of power7-pmu.o is:
> 
> $ size arch/powerpc/perf/power7-pmu.o
>    text	   data	    bss	    dec	    hex	filename
>    3073	   2720	      0	   5793	   16a1	arch/powerpc/perf/power7-pmu.o
> 
> and after the patch is applied, it is:
> 
> $ size arch/powerpc/perf/power7-pmu.o
>    text	   data	    bss	    dec	    hex	filename
>   14451	  31112	      0	  45563	   b1fb	arch/powerpc/perf/power7-pmu.o

OK so that's ~38K. Which is not terrible.

Can you measure the runtime overhead as well. I suspect it will be more.


You'll notice below that each event name is repeated four times, which
for 530 events is a bit ugly.

I think we should be able to do something better using the C
preprocessor, this is exactly the sort of thing it's good at.

What I mean is something like we do with arch/powerpc/include/asm/systbl.h,
where we define the list of syscalls once, and then include it in
multiple places, using different macro definitions to get different
outputs.

So perhaps you'd define the list of events like:

#define EVENT(PM_CMPLU_STALL_FXU, 0x20014)
#define EVENT(PM_CMPLU_STALL_DIV, 0x40014)

etc.

Then you do something approximately like:

#define EVENT(_name, _code)	POWER_EVENT_ATTR(_name, _code)
#include "event-list.h"
#undef EVENT

#define EVENT(_name, _code)	POWER_EVENT_PTR(_name)

static struct attribute *power7_events_attr[] = {
#include "event-list.h"
};


You will obviously need to rework the POWER_EVENT macros to make that
work, but it should be possible.

The end result will be we have a single list of the events which we can
check for accuracy once. And we can be sure that there are no mixups
between events.


> diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
> index d1821b8..55e2404 100644
> --- a/arch/powerpc/perf/power7-pmu.c
> +++ b/arch/powerpc/perf/power7-pmu.c
> @@ -53,37 +53,544 @@
>  /*
>   * Power7 event codes.
>   */

...

> +#define PME_PM_MRK_DERAT_MISS_64K                 0x2d05c
> +#define PME_PM_INST_PTEG_FROM_DL2L3_MOD           0x4e054
> +#define PME_PM_L2_ST_MISS                         0x26082
> +#define PME_PM_MRK_PTEG_FROM_L21_SHR              0x4d056
> +#undef  LWSYNC
    ^
What is this doing here?

That is not your macro to undefine. Please be more careful.

> +#define PME_PM_LWSYNC                             0xd094



cheers

^ permalink raw reply

* Re: [PATCH 1/2] perf tools: fix a typo of a Power7 event name
From: Sukadev Bhattiprolu @ 2013-06-20  2:28 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: icycoder, linux-kernel, xiaoguangrong, acme, paulus, Runzhen Wang,
	linuxppc-dev
In-Reply-To: <1371691282.21846.1.camel@concordia>

Michael Ellerman [michael@ellerman.id.au] wrote:
| On Wed, 2013-06-19 at 17:15 +0800, Runzhen Wang wrote:
| > In the Power7 PMU guide:
| > https://www.power.org/documentation/commonly-used-metrics-for-performance-analysis/
| > PM_BRU_MPRED is referred to as PM_BR_MPRED.
| > 
| > This patch fix the typo by changing the name of the event in kernel and
| > documentation accordingly.
| > 
| > Signed-off-by: Runzhen Wang <runzhen@linux.vnet.ibm.com>
| > ---
| >  .../testing/sysfs-bus-event_source-devices-events  |    2 +-
| >  arch/powerpc/perf/power7-pmu.c                     |   12 ++++++------
| >  2 files changed, 7 insertions(+), 7 deletions(-)
| > 
| > diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
| > index 8b25ffb..3c1cc24 100644
| > --- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
| > +++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
| > @@ -29,7 +29,7 @@ Description:	Generic performance monitoring events
| >  
| >  What: 		/sys/devices/cpu/events/PM_1PLUS_PPC_CMPL
| >  		/sys/devices/cpu/events/PM_BRU_FIN
| > -		/sys/devices/cpu/events/PM_BRU_MPRED
| > +		/sys/devices/cpu/events/PM_BR_MPRED
| 
| So you're changing the ABI.
| 
| I think that's probably OK, but you at least need to mention _why_ you
| think it's OK.

Some of the reasons I think its ok to change:

- It is relatively new interface, specific to the Power7 platform.

- No tools that we know of actually use this interface at this point (none
  are listed near the interface).

- Users of this interface (eg oprofile users migrating to perf) would be
  more used to the "PM_BR_MPRED" rather than "PM_BRU_MPRED".

- These are in the ABI/testing at this point rather than ABI/stable, so
  hoping we have some wiggle room.

But of course this is not a "grave error" or "security hole" which are
the justifications listed in the Documentation/ABI/README.  If these
are not good enough reasons, we can leave the misnomer around and
create another file in sysfs with the correct name PM_BR_MPRED.

Sukadev

^ permalink raw reply

* [PATCH -V2] powerpc/kvm: Handle transparent hugepage in KVM
From: Aneesh Kumar K.V @ 2013-06-20  4:41 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 can find pte that are splitting while walking page tables. Return
None pte in that case.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/kvm_book3s_64.h | 57 ++++++++++++++++++--------------
 arch/powerpc/kvm/book3s_64_mmu_hv.c      |  8 +++--
 arch/powerpc/kvm/book3s_hv_rm_mmu.c      | 12 +++----
 3 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index 9c1ff33..937c51b 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -159,36 +159,45 @@ static inline int hpte_cache_flags_ok(unsigned long ptel, unsigned long io_type)
 }
 
 /*
- * Lock and read a linux PTE.  If it's present and writable, atomically
- * set dirty and referenced bits and return the PTE, otherwise return 0.
+ * If it's present and writable, atomically set dirty and referenced bits and
+ * return the PTE, otherwise return 0. If we find a transparent hugepage
+ * and if it is marked splitting we return 0;
  */
-static inline pte_t kvmppc_read_update_linux_pte(pte_t *p, int writing)
+static inline pte_t kvmppc_read_update_linux_pte(pte_t *ptep, int writing,
+						 unsigned int hugepage)
 {
-	pte_t pte, tmp;
-
-	/* wait until _PAGE_BUSY is clear then set it atomically */
-	__asm__ __volatile__ (
-		"1:	ldarx	%0,0,%3\n"
-		"	andi.	%1,%0,%4\n"
-		"	bne-	1b\n"
-		"	ori	%1,%0,%4\n"
-		"	stdcx.	%1,0,%3\n"
-		"	bne-	1b"
-		: "=&r" (pte), "=&r" (tmp), "=m" (*p)
-		: "r" (p), "i" (_PAGE_BUSY)
-		: "cc");
-
-	if (pte_present(pte)) {
-		pte = pte_mkyoung(pte);
-		if (writing && pte_write(pte))
-			pte = pte_mkdirty(pte);
-	}
+	pte_t old_pte, new_pte = __pte(0);
+
+	while (1) {
+		old_pte = pte_val(*ptep);
+		/*
+		 * wait until _PAGE_BUSY is clear then set it atomically
+		 */
+		if (unlikely(old_pte & _PAGE_BUSY))
+			continue;
+
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+		/* If hugepage and is trans splitting return None */
+		if (unlikely(hugepage &&
+			     pmd_trans_splitting(pte_pmd(old_pte))))
+			return __pte(0);
+#endif
+		/* If pte is not present return None */
+		if (unlikely(!(old_pte & _PAGE_PRESENT)))
+			return __pte(0);
 
-	*p = pte;	/* clears _PAGE_BUSY */
+		new_pte = pte_mkyoung(old_pte);
+		if (writing && pte_write(old_pte))
+			new_pte = pte_mkdirty(new_pte);
 
-	return pte;
+		if (old_pte == __cmpxchg_u64((unsigned long *)ptep, old_pte,
+					     new_pte))
+			break;
+	}
+	return new_pte;
 }
 
+
 /* Return HPTE cache control bits corresponding to Linux pte bits */
 static inline unsigned long hpte_cache_bits(unsigned long pte_val)
 {
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 5880dfb..710d313 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -675,6 +675,7 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
 		}
 		/* if the guest wants write access, see if that is OK */
 		if (!writing && hpte_is_writable(r)) {
+			unsigned int hugepage_shift;
 			pte_t *ptep, pte;
 
 			/*
@@ -683,9 +684,10 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			 */
 			rcu_read_lock_sched();
 			ptep = find_linux_pte_or_hugepte(current->mm->pgd,
-							 hva, NULL);
-			if (ptep && pte_present(*ptep)) {
-				pte = kvmppc_read_update_linux_pte(ptep, 1);
+							 hva, &hugepage_shift);
+			if (ptep) {
+				pte = kvmppc_read_update_linux_pte(ptep, 1,
+							   hugepage_shift);
 				if (pte_write(pte))
 					write_ok = 1;
 			}
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index dcf892d..fc25689 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -139,20 +139,18 @@ static pte_t lookup_linux_pte(pgd_t *pgdir, unsigned long hva,
 {
 	pte_t *ptep;
 	unsigned long ps = *pte_sizep;
-	unsigned int shift;
+	unsigned int hugepage_shift;
 
-	ptep = find_linux_pte_or_hugepte(pgdir, hva, &shift);
+	ptep = find_linux_pte_or_hugepte(pgdir, hva, &hugepage_shift);
 	if (!ptep)
 		return __pte(0);
-	if (shift)
-		*pte_sizep = 1ul << shift;
+	if (hugepage_shift)
+		*pte_sizep = 1ul << hugepage_shift;
 	else
 		*pte_sizep = PAGE_SIZE;
 	if (ps > *pte_sizep)
 		return __pte(0);
-	if (!pte_present(*ptep))
-		return __pte(0);
-	return kvmppc_read_update_linux_pte(ptep, writing);
+	return kvmppc_read_update_linux_pte(ptep, writing, hugepage_shift);
 }
 
 static inline void unlock_hpte(unsigned long *hpte, unsigned long hpte_v)
-- 
1.8.1.2

^ permalink raw reply related

* Re: [PATCH -V2] powerpc/kvm: Handle transparent hugepage in KVM
From: Benjamin Herrenschmidt @ 2013-06-20  4:56 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: linuxppc-dev, paulus
In-Reply-To: <1371703304-31687-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>

On Thu, 2013-06-20 at 10:11 +0530, Aneesh Kumar K.V wrote:
> +       while (1) {
> +               old_pte = pte_val(*ptep);
> +               /*
> +                * wait until _PAGE_BUSY is clear then set it
> atomically
> +                */
> +               if (unlikely(old_pte & _PAGE_BUSY))
> +                       continue;
> +

Add a cpu_relax...

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
From: Alexey Kardashevskiy @ 2013-06-20  4:58 UTC (permalink / raw)
  To: Alex Williamson
  Cc: kvm@vger.kernel.org mailing list, Joerg Roedel, Rusty Russell,
	Alexander Graf, kvm-ppc, open list, Paul Mackerras, linuxppc-dev,
	David Gibson
In-Reply-To: <1371656989.22659.98.camel@ul30vt.home>

On 06/20/2013 01:49 AM, Alex Williamson wrote:
> On Thu, 2013-06-20 at 00:50 +1000, Benjamin Herrenschmidt wrote:
>> On Wed, 2013-06-19 at 11:58 +0200, Alexander Graf wrote:
>>
>>>> Alex, any objection ?
>>>
>>> Which Alex? :)
>>
>> Heh, mostly Williamson in this specific case but your input is still
>> welcome :-)
>>
>>> I think validate works, it keeps iteration logic out of the kernel
>>> which is a good thing. There still needs to be an interface for
>>> getting the iommu id in VFIO, but I suppose that one's for the other
>>> Alex and Jörg to comment on.
>>
>> I think getting the iommu fd is already covered by separate patches from
>> Alexey.
>>
>>>>
>>>> Do we need to make it a get/put interface instead ?
>>>>
>>>> 	vfio_validate_and_use_iommu(file, iommu_id);
>>>>
>>>> 	vfio_release_iommu(file, iommu_id);
>>>>
>>>> To ensure that the resource remains owned by the process until KVM
>>>> is closed as well ?
>>>>
>>>> Or do we want to register with VFIO with a callback so that VFIO can
>>>> call us if it needs us to give it up ?
>>>
>>> Can't we just register a handler on the fd and get notified when it
>>> closes? Can you kill VFIO access without closing the fd?
>>
>> That sounds actually harder :-)
>>
>> The question is basically: When we validate that relationship between a
>> specific VFIO struct file with an iommu, what is the lifetime of that
>> and how do we handle this lifetime properly.
>>
>> There's two ways for that sort of situation: The notification model
>> where we get notified when the relationship is broken, and the refcount
>> model where we become a "user" and thus delay the breaking of the
>> relationship until we have been disposed of as well.
>>
>> In this specific case, it's hard to tell what is the right model from my
>> perspective, which is why I would welcome Alex (W.) input.
>>
>> In the end, the solution will end up being in the form of APIs exposed
>> by VFIO for use by KVM (via that symbol lookup mechanism) so Alex (W),
>> as owner of VFIO at this stage, what do you want those to look
>> like ? :-)
> 
> My first thought is that we should use the same reference counting as we
> have for vfio devices (group->container_users).  An interface for that
> might look like:
> 
> int vfio_group_add_external_user(struct file *filep)
> {
> 	struct vfio_group *group = filep->private_data;
> 
> 	if (filep->f_op != &vfio_group_fops)
> 		return -EINVAL;
> 
> 
> 	if (!atomic_inc_not_zero(&group->container_users))
> 		return -EINVAL;
> 
> 	return 0;
> }
> 
> void vfio_group_del_external_user(struct file *filep)
> {
> 	struct vfio_group *group = filep->private_data;
> 
> 	BUG_ON(filep->f_op != &vfio_group_fops);
> 
> 	vfio_group_try_dissolve_container(group);
> }
> 
> int vfio_group_iommu_id_from_file(struct file *filep)
> {
> 	struct vfio_group *group = filep->private_data;
> 
> 	BUG_ON(filep->f_op != &vfio_group_fops);
> 
> 	return iommu_group_id(group->iommu_group);
> }
> 
> Would that work?  Thanks,


Just out of curiosity - would not get_file() and fput_atomic() on a group's
file* do the right job instead of vfio_group_add_external_user() and
vfio_group_del_external_user()?



-- 
Alexey

^ 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