* Re: [PATCH 5/6 v8] iommu/fsl: Add addtional attributes specific to the PAMU driver.
From: Joerg Roedel @ 2013-02-27 11:38 UTC (permalink / raw)
To: Varun Sethi; +Cc: scottwood, iommu, linuxppc-dev, linux-kernel, stuart.yoder
In-Reply-To: <1361191939-21260-6-git-send-email-Varun.Sethi@freescale.com>
On Mon, Feb 18, 2013 at 06:22:18PM +0530, Varun Sethi wrote:
> Added the following domain attributes for the FSL PAMU driver:
> 1. Added new iommu stash attribute, which allows setting of the
> LIODN specific stash id parameter through IOMMU API.
> 2. Added an attribute for enabling/disabling DMA to a particular
> memory window.
> 3. Added domain attribute to check for PAMUV1 specific constraints.
>
>
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
> include/linux/iommu.h | 33 +++++++++++++++++++++++++++++++++
> 1 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 529987c..c44e38b 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -40,6 +40,23 @@ struct notifier_block;
> typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
> struct device *, unsigned long, int, void *);
>
> +/* cache stash targets */
> +#define IOMMU_ATTR_CACHE_L1 1
> +#define IOMMU_ATTR_CACHE_L2 2
> +#define IOMMU_ATTR_CACHE_L3 3
> +
> +/* This attribute corresponds to IOMMUs capable of generating
> + * a stash transaction. A stash transaction is typically a
> + * hardware initiated prefetch of data from memory to cache.
> + * This attribute allows configuring stashig specific parameters
> + * in the IOMMU hardware.
> + */
> +
> +struct iommu_stash_attribute {
> + u32 cpu; /* cpu number */
> + u32 cache; /* cache to stash to: L1,L2,L3 */
> +};
Please make the cache-attribute an enum instead of using defines.
Joerg
^ permalink raw reply
* Re: [PATCH 3/6] powerpc/fsl_pci: Added defines for the FSL PCI controller BRR1 register.
From: Joerg Roedel @ 2013-02-27 11:33 UTC (permalink / raw)
To: Varun Sethi; +Cc: scottwood, iommu, linuxppc-dev, linux-kernel, stuart.yoder
In-Reply-To: <1361191939-21260-4-git-send-email-Varun.Sethi@freescale.com>
On Mon, Feb 18, 2013 at 06:22:16PM +0530, Varun Sethi wrote:
> Macros for checking FSL PCI controller version.
>
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
> arch/powerpc/include/asm/pci-bridge.h | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index 025a130..c12ed78 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -14,6 +14,10 @@
>
> struct device_node;
>
> +/* FSL PCI controller BRR1 register */
> +#define PCI_FSL_BRR1 0xbf8
> +#define PCI_FSL_BRR1_VER 0xffff
> +
Please merge this patch with the one where you actually make use of
these defines for the first time.
Joerg
^ permalink raw reply
* Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
From: Joerg Roedel @ 2013-02-27 11:30 UTC (permalink / raw)
To: Varun Sethi; +Cc: scottwood, iommu, linuxppc-dev, linux-kernel, stuart.yoder
In-Reply-To: <1361191939-21260-2-git-send-email-Varun.Sethi@freescale.com>
On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
> Add a new field in the device (powerpc) archdata structure for storing iommu domain
> information pointer. This pointer is stored when the device is attached to a particular
> domain.
>
>
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
> - no change.
> arch/powerpc/include/asm/device.h | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h
> index 77e97dd..6dc79fe 100644
> --- a/arch/powerpc/include/asm/device.h
> +++ b/arch/powerpc/include/asm/device.h
> @@ -28,6 +28,10 @@ struct dev_archdata {
> void *iommu_table_base;
> } dma_data;
>
> + /* IOMMU domain information pointer. This would be set
> + * when this device is attached to an iommu_domain.
> + */
> + void *iommu_domain;
Please Cc the PowerPC Maintainers on this, so that they can have a look
at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.
Joerg
^ permalink raw reply
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Michel Lespinasse @ 2013-02-27 11:11 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Lai Jiangshan, linux-doc, peterz, fweisbec, linux-kernel, mingo,
linux-arch, linux, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, rostedt, rjw, namhyung, tglx, linux-arm-kernel,
netdev, oleg, vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <512D0D67.9010609@linux.vnet.ibm.com>
Hi Srivatsa,
I think there is some elegance in Lai's proposal of using a local
trylock for the reader uncontended case and global rwlock to deal with
the contended case without deadlocks. He apparently didn't realize
initially that nested read locks are common, and he seems to have
confused you because of that, but I think his proposal could be
changed easily to account for that and result in short, easily
understood code. What about the following:
- local_refcnt is a local lock count; it indicates how many recursive
locks are taken using the local lglock
- lglock is used by readers for local locking; it must be acquired
before local_refcnt becomes nonzero and released after local_refcnt
goes back to zero.
- fallback_rwlock is used by readers for global locking; it is
acquired when fallback_reader_refcnt is zero and the trylock fails on
lglock
+void lg_rwlock_local_read_lock(struct lgrwlock *lgrw)
+{
+ preempt_disable();
+
+ if (__this_cpu_read(*lgrw->local_refcnt) ||
+ arch_spin_trylock(this_cpu_ptr(lgrw->lglock->lock))) {
+ __this_cpu_inc(*lgrw->local_refcnt);
+
rwlock_acquire_read(&lgrw->fallback_rwlock->lock_dep_map, 0, 0,
_RET_IP_);
+ } else {
+ read_lock(&lgrw->fallback_rwlock);
+ }
+}
+EXPORT_SYMBOL(lg_rwlock_local_read_lock);
+
+void lg_rwlock_local_read_unlock(struct lgrwlock *lgrw)
+{
+ if (likely(__this_cpu_read(*lgrw->local_refcnt))) {
+ rwlock_release(&lgrw->fallback_rwlock->lock_dep_map,
1, _RET_IP_);
+ if (!__this_cpu_dec_return(*lgrw->local_refcnt))
+ arch_spin_unlock(this_cpu_ptr(lgrw->lglock->lock));
+ } else {
+ read_unlock(&lgrw->fallback_rwlock);
+ }
+
+ preempt_enable();
+}
+EXPORT_SYMBOL(lg_rwlock_local_read_unlock);
+
+void lg_rwlock_global_write_lock(struct lgrwlock *lgrw)
+{
+ int i;
+
+ preempt_disable();
+
+ for_each_possible_cpu(i)
+ arch_spin_lock(per_cpu_ptr(lgrw->lglock->lock, i));
+ write_lock(&lgrw->fallback_rwlock);
+}
+EXPORT_SYMBOL(lg_rwlock_global_write_lock);
+
+void lg_rwlock_global_write_unlock(struct lgrwlock *lgrw)
+{
+ int i;
+
+ write_unlock(&lgrw->fallback_rwlock);
+ for_each_possible_cpu(i)
+ arch_spin_unlock(per_cpu_ptr(lgrw->lglock->lock, i));
+
+ preempt_enable();
+}
+EXPORT_SYMBOL(lg_rwlock_global_write_unlock);
This is to me relatively easier to understand than Srivatsa's
proposal. Now I'm not sure who wins efficiency wise, but I think it
should be relatively close as readers at least don't touch shared
state in the uncontended case (even with some recursion going on).
There is an interesting case where lg_rwlock_local_read_lock could be
interrupted after getting the local lglock but before incrementing
local_refcnt to 1; if that happens any nested readers within that
interrupt will have to take the global rwlock read side. I think this
is perfectly acceptable as this should not be a common case though
(and thus the global rwlock cache line probably wouldn't even bounce
between cpus then).
--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
^ permalink raw reply
* Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.
From: Joerg Roedel @ 2013-02-27 10:45 UTC (permalink / raw)
To: Sethi Varun-B16395
Cc: Wood Scott-B07421, Stuart Yoder, linux-kernel@vger.kernel.org,
Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D3BD338@039-SN2MPN1-013.039d.mgd.msft.net>
On Tue, Feb 26, 2013 at 06:16:10AM +0000, Sethi Varun-B16395 wrote:
> This patch is not present in Joerg's tree and the add_device API in
> the PAMU driver requires this patch.
Will this patch be part of v3.9-rc1?
Joerg
^ permalink raw reply
* RE: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.
From: Sethi Varun-B16395 @ 2013-02-27 10:56 UTC (permalink / raw)
To: Joerg Roedel
Cc: Wood Scott-B07421, Stuart Yoder, linux-kernel@vger.kernel.org,
Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130227104505.GE26252@8bytes.org>
This patch is present in the "next branch" of linux ppc tree maintained by =
Kumar Gala.
Following is the commit id:
52c5affc545053d37c0b05224bbf70f5336caa20
I am not sure if this would be part of 3.9-rc1.
Regards
varun
> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Wednesday, February 27, 2013 4:15 PM
> To: Sethi Varun-B16395
> Cc: Stuart Yoder; iommu@lists.linux-foundation.org; linuxppc-
> dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Wood Scott-B07421;
> Yoder Stuart-B08248
> Subject: Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device
> information corresponding to the pci controller.
>=20
> On Tue, Feb 26, 2013 at 06:16:10AM +0000, Sethi Varun-B16395 wrote:
> > This patch is not present in Joerg's tree and the add_device API in
> > the PAMU driver requires this patch.
>=20
> Will this patch be part of v3.9-rc1?
>=20
>=20
> Joerg
>=20
>=20
^ permalink raw reply
* [v3][PATCH 6/6] powerpc/kgdb: remove copying the thread_info
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
Currently BookE and Book3E always copy the thread_info from
the kernel stack when we enter the debug exception, so we can
remove these action here to avoid copying again.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/kgdb.c | 28 ----------------------------
1 file changed, 28 deletions(-)
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 1a57307..e954888 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -153,39 +153,11 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
static int kgdb_singlestep(struct pt_regs *regs)
{
- struct thread_info *thread_info, *exception_thread_info;
- struct thread_info *backup_current_thread_info;
-
if (user_mode(regs))
return 0;
- backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
- /*
- * On Book E and perhaps other processors, singlestep is handled on
- * the critical exception stack. This causes current_thread_info()
- * to fail, since it it locates the thread_info by masking off
- * the low bits of the current stack pointer. We work around
- * this issue by copying the thread_info from the kernel stack
- * before calling kgdb_handle_exception, and copying it back
- * afterwards. On most processors the copy is avoided since
- * exception_thread_info == thread_info.
- */
- thread_info = (struct thread_info *)(regs->gpr[1] & ~(THREAD_SIZE-1));
- exception_thread_info = current_thread_info();
-
- if (thread_info != exception_thread_info) {
- /* Save the original current_thread_info. */
- memcpy(backup_current_thread_info, exception_thread_info, sizeof *thread_info);
- memcpy(exception_thread_info, thread_info, sizeof *thread_info);
- }
-
kgdb_handle_exception(0, SIGTRAP, 0, regs);
- if (thread_info != exception_thread_info)
- /* Restore current_thread_info lastly. */
- memcpy(exception_thread_info, backup_current_thread_info, sizeof *thread_info);
-
- kfree(backup_current_thread_info);
return 1;
}
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 5/6] kgdb/kgdbts: support ppc64
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
We can't look up the address of the entry point of the function simply
via that function symbol for all architectures.
For PPC64 ABI, actually there is a function descriptors structure.
A function descriptor is a three doubleword data structure that contains
the following values:
* The first doubleword contains the address of the entry point of
the function.
* The second doubleword contains the TOC base address for
the function.
* The third doubleword contains the environment pointer for
languages such as Pascal and PL/1.
So we should call a wapperred dereference_function_descriptor() to get
the address of the entry point of the function.
Note this is also safe for other architecture after refer to
"include/asm-generic/sections.h" since:
dereference_function_descriptor(p) always is (p) if without arched definition.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
drivers/misc/kgdbts.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index 3aa9a96..4799e1f 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -103,6 +103,7 @@
#include <linux/delay.h>
#include <linux/kthread.h>
#include <linux/module.h>
+#include <asm/sections.h>
#define v1printk(a...) do { \
if (verbose) \
@@ -222,6 +223,7 @@ static unsigned long lookup_addr(char *arg)
addr = (unsigned long)do_fork;
else if (!strcmp(arg, "hw_break_val"))
addr = (unsigned long)&hw_break_val;
+ addr = (unsigned long )dereference_function_descriptor((void *)addr);
return addr;
}
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 4/6] powerpc/book3e: support kgdb for kernel space
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
Currently we need to skip this for supporting KGDB.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/exceptions-64e.S | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 7df9a1f..800e2a3 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -598,11 +598,14 @@ kernel_dbg_exc:
rfdi
/* Normal debug exception */
+1:
+#ifndef CONFIG_KGDB
/* XXX We only handle coming from userspace for now since we can't
* quite save properly an interrupted kernel state yet
*/
-1: andi. r14,r11,MSR_PR; /* check for userspace again */
+ andi. r14,r11,MSR_PR; /* check for userspace again */
beq kernel_dbg_exc; /* if from kernel mode */
+#endif
/* Now we mash up things to make it look like we are coming on a
* normal exception
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 3/6] book3e/kgdb: update thread's dbcr0
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
gdb always need to generate a single step properly to invoke
a kgdb state. But with lazy interrupt, book3e can't always
trigger a debug exception with a single step since the current
is blocked for handling those pending exception, then we miss
that expected dbcr configuration at last to generate a debug
exception.
So here we also update thread's dbcr0 to make sure the current
can go back with that missed dbcr0 configuration.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/kgdb.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 5ca82cd..1a57307 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -410,7 +410,7 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
struct pt_regs *linux_regs)
{
char *ptr = &remcom_in_buffer[1];
- unsigned long addr;
+ unsigned long addr, dbcr0;
switch (remcom_in_buffer[0]) {
/*
@@ -427,8 +427,15 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
/* set the trace bit if we're stepping */
if (remcom_in_buffer[0] == 's') {
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- mtspr(SPRN_DBCR0,
- mfspr(SPRN_DBCR0) | DBCR0_IC | DBCR0_IDM);
+ dbcr0 = mfspr(SPRN_DBCR0) | DBCR0_IC | DBCR0_IDM;
+ mtspr(SPRN_DBCR0, dbcr0);
+#ifdef CONFIG_PPC_BOOK3E_64
+ /* With lazy interrut we have to update thread dbcr0 here
+ * to make sure we can set debug properly at last to invoke
+ * kgdb again to work well.
+ */
+ current->thread.dbcr0 = dbcr0;
+#endif
linux_regs->msr |= MSR_DE;
#else
linux_regs->msr |= MSR_SE;
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 2/6] powerpc/book3e: store critical/machine/debug exception thread info
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
We need to store thread info to these exception thread info like something
we already did for PPC32.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/exceptions-64e.S | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 7fd6af0..7df9a1f 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -67,6 +67,18 @@
std r10,PACA_##level##_STACK(r13);
#endif
+/* Store something to exception thread info */
+#define BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(type) \
+ ld r14,PACAKSAVE(r13); \
+ CURRENT_THREAD_INFO(r14, r14); \
+ CURRENT_THREAD_INFO(r15, r1); \
+ ld r10,TI_FLAGS(r14); \
+ std r10,TI_FLAGS(r15); \
+ ld r10,TI_PREEMPT(r14); \
+ std r10,TI_PREEMPT(r15); \
+ ld r10,TI_TASK(r14); \
+ std r10,TI_TASK(r15);
+
/* Exception prolog code for all exceptions */
#define EXCEPTION_PROLOG(n, intnum, type, addition) \
mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
@@ -104,6 +116,7 @@
BOOK3E_LOAD_EXC_LEVEL_STACK(CRIT); \
ld r1,PACA_CRIT_STACK(r13); \
subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+ BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(CRIT); \
1:
#define SPRN_CRIT_SRR0 SPRN_CSRR0
#define SPRN_CRIT_SRR1 SPRN_CSRR1
@@ -114,6 +127,7 @@
BOOK3E_LOAD_EXC_LEVEL_STACK(DBG); \
ld r1,PACA_DBG_STACK(r13); \
subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+ BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(DBG); \
1:
#define SPRN_DBG_SRR0 SPRN_DSRR0
#define SPRN_DBG_SRR1 SPRN_DSRR1
@@ -124,6 +138,7 @@
BOOK3E_LOAD_EXC_LEVEL_STACK(MC); \
ld r1,PACA_MC_STACK(r13); \
subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+ BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(MC); \
1:
#define SPRN_MC_SRR0 SPRN_MCSRR0
#define SPRN_MC_SRR1 SPRN_MCSRR1
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 1/6] powerpc/book3e: load critical/machine/debug exception stack
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
We always alloc critical/machine/debug check exceptions. This is
different from the normal exception. So we should load these exception
stack properly like we did for booke.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/exceptions-64e.S | 49 +++++++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 1e7782b..7fd6af0 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -36,6 +36,37 @@
*/
#define SPECIAL_EXC_FRAME_SIZE INT_FRAME_SIZE
+/* only on book3e */
+#define DBG_STACK_BASE dbgirq_ctx
+#define MC_STACK_BASE mcheckirq_ctx
+#define CRIT_STACK_BASE critirq_ctx
+
+#ifdef CONFIG_RELOCATABLE
+#define LOAD_STACK_BASE(reg, level) \
+ tovirt(r2,r2); \
+ LOAD_REG_ADDR(reg, level##_STACK_BASE);
+#else
+#define LOAD_STACK_BASE(reg, level) \
+ LOAD_REG_IMMEDIATE(reg, level##_STACK_BASE);
+#endif
+
+#ifdef CONFIG_SMP
+#define BOOK3E_LOAD_EXC_LEVEL_STACK(level) \
+ mfspr r14,SPRN_PIR; \
+ slwi r14,r14,3; \
+ LOAD_STACK_BASE(r10, level); \
+ add r10,r10,r14; \
+ ld r10,0(r10); \
+ addi r10,r10,THREAD_SIZE; \
+ std r10,PACA_##level##_STACK(r13);
+#else
+#define BOOK3E_LOAD_EXC_LEVEL_STACK(level) \
+ LOAD_STACK_BASE(r10, level); \
+ ld r10,0(r10); \
+ addi r10,r10,THREAD_SIZE; \
+ std r10,PACA_##level##_STACK(r13);
+#endif
+
/* Exception prolog code for all exceptions */
#define EXCEPTION_PROLOG(n, intnum, type, addition) \
mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
@@ -68,20 +99,32 @@
#define SPRN_GDBELL_SRR1 SPRN_GSRR1
#define CRIT_SET_KSTACK \
+ andi. r10,r11,MSR_PR; \
+ bne 1f; \
+ BOOK3E_LOAD_EXC_LEVEL_STACK(CRIT); \
ld r1,PACA_CRIT_STACK(r13); \
- subi r1,r1,SPECIAL_EXC_FRAME_SIZE;
+ subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+1:
#define SPRN_CRIT_SRR0 SPRN_CSRR0
#define SPRN_CRIT_SRR1 SPRN_CSRR1
#define DBG_SET_KSTACK \
+ andi. r10,r11,MSR_PR; \
+ bne 1f; \
+ BOOK3E_LOAD_EXC_LEVEL_STACK(DBG); \
ld r1,PACA_DBG_STACK(r13); \
- subi r1,r1,SPECIAL_EXC_FRAME_SIZE;
+ subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+1:
#define SPRN_DBG_SRR0 SPRN_DSRR0
#define SPRN_DBG_SRR1 SPRN_DSRR1
#define MC_SET_KSTACK \
+ andi. r10,r11,MSR_PR; \
+ bne 1f; \
+ BOOK3E_LOAD_EXC_LEVEL_STACK(MC); \
ld r1,PACA_MC_STACK(r13); \
- subi r1,r1,SPECIAL_EXC_FRAME_SIZE;
+ subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+1:
#define SPRN_MC_SRR0 SPRN_MCSRR0
#define SPRN_MC_SRR1 SPRN_MCSRR1
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 0/6] powerpc/book3e: make kgdb to work well
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
This patchset is used to support kgdb/gdb on book3e.
Validated on p4080ds and p5040ds with test single step and breakpoint
v3:
* make work when enable CONFIG_RELOCATABLE
* fix one typo in patch,
"powerpc/book3e: store critical/machine/debug exception thread info":
ld r1,PACAKSAVE(r13);
-> ld r14,PACAKSAVE(r13);
* remove copying the thread_info since booke and book3e always copy
the thead_info now when we enter the debug exception, and so drop
the v2 patch, "book3e/kgdb: Fix a single stgep case of lazy IRQ"
v2:
* Make sure we cover CONFIG_PPC_BOOK3E_64 safely
* Use LOAD_REG_IMMEDIATE() to load properly
the value of the constant expression in load debug exception stack
* Copy thread infor form the kernel stack coming from usr
* Rebase latest powerpc git tree
v1:
* Copy thread info only when we are from !user mode since we'll get kernel stack
coming from usr directly.
* remove save/restore EX_R14/EX_R15 since DBG_EXCEPTION_PROLOG already covered
this.
* use CURRENT_THREAD_INFO() conveniently to get thread.
* fix some typos
* add a patch to make sure gdb can generate a single step properly to invoke a
kgdb state.
* add a patch to if we need to replay an interrupt, we shouldn't restore that
previous backup thread info to make sure we can replay an interrupt lately
with a proper thread info.
* rebase latest powerpc git tree
v0:
This patchset is used to support kgdb for book3e.
------
Tiejun Chen (6):
powerpc/book3e: load critical/machine/debug exception stack
powerpc/book3e: store critical/machine/debug exception thread info
book3e/kgdb: update thread's dbcr0
powerpc/book3e: support kgdb for kernel space
kgdb/kgdbts: support ppc64
powerpc/kgdb: remove copying the thread_info
arch/powerpc/kernel/exceptions-64e.S | 69 ++++++++++++++++++++++++++++++++--
arch/powerpc/kernel/kgdb.c | 41 +++++---------------
drivers/misc/kgdbts.c | 2 +
3 files changed, 77 insertions(+), 35 deletions(-)
Tiejun
^ permalink raw reply
* Re: [v3][PATCH 0/6] powerpc/book3e: make kgdb to work well
From: tiejun.chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: Tiejun Chen; +Cc: linuxppc-dev
In-Reply-To: <1361934261-31840-1-git-send-email-tiejun.chen@windriver.com>
On 02/27/2013 11:04 AM, Tiejun Chen wrote:
> This patchset is used to support kgdb/gdb on book3e.
>
> Validated on p4080ds and p5040ds with test single step and breakpoint
Please ignore this thread since looks I'm missing to CC Jason :(
Tiejun
>
> v3:
>
> * make work when enable CONFIG_RELOCATABLE
> * fix one typo in patch,
> "powerpc/book3e: store critical/machine/debug exception thread info":
> ld r1,PACAKSAVE(r13);
> -> ld r14,PACAKSAVE(r13);
> * remove copying the thread_info since booke and book3e always copy
> the thead_info now when we enter the debug exception, and so drop
> the v2 patch, "book3e/kgdb: Fix a single stgep case of lazy IRQ"
>
> v2:
>
> * Make sure we cover CONFIG_PPC_BOOK3E_64 safely
> * Use LOAD_REG_IMMEDIATE() to load properly
> the value of the constant expression in load debug exception stack
> * Copy thread infor form the kernel stack coming from usr
> * Rebase latest powerpc git tree
>
> v1:
> * Copy thread info only when we are from !user mode since we'll get kernel stack
> coming from usr directly.
> * remove save/restore EX_R14/EX_R15 since DBG_EXCEPTION_PROLOG already covered
> this.
> * use CURRENT_THREAD_INFO() conveniently to get thread.
> * fix some typos
> * add a patch to make sure gdb can generate a single step properly to invoke a
> kgdb state.
> * add a patch to if we need to replay an interrupt, we shouldn't restore that
> previous backup thread info to make sure we can replay an interrupt lately
> with a proper thread info.
> * rebase latest powerpc git tree
>
> v0:
> This patchset is used to support kgdb for book3e.
>
> ------
> Tiejun Chen (6):
> powerpc/book3e: load critical/machine/debug exception stack
> powerpc/book3e: store critical/machine/debug exception thread info
> book3e/kgdb: update thread's dbcr0
> powerpc/book3e: support kgdb for kernel space
> kgdb/kgdbts: support ppc64
> powerpc/kgdb: remove copying the thread_info
>
> arch/powerpc/kernel/exceptions-64e.S | 69 ++++++++++++++++++++++++++++++++--
> arch/powerpc/kernel/kgdb.c | 41 +++++---------------
> drivers/misc/kgdbts.c | 2 +
> 3 files changed, 77 insertions(+), 35 deletions(-)
>
> Tiejun
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
^ permalink raw reply
* [v3][PATCH 6/6] powerpc/kgdb: remove copying the thread_info
From: Tiejun Chen @ 2013-02-27 3:04 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev
In-Reply-To: <1361934261-31840-1-git-send-email-tiejun.chen@windriver.com>
Currently BookE and Book3E always copy the thread_info from
the kernel stack when we enter the debug exception, so we can
remove these action here to avoid copying again.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/kgdb.c | 28 ----------------------------
1 file changed, 28 deletions(-)
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 1a57307..e954888 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -153,39 +153,11 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
static int kgdb_singlestep(struct pt_regs *regs)
{
- struct thread_info *thread_info, *exception_thread_info;
- struct thread_info *backup_current_thread_info;
-
if (user_mode(regs))
return 0;
- backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
- /*
- * On Book E and perhaps other processors, singlestep is handled on
- * the critical exception stack. This causes current_thread_info()
- * to fail, since it it locates the thread_info by masking off
- * the low bits of the current stack pointer. We work around
- * this issue by copying the thread_info from the kernel stack
- * before calling kgdb_handle_exception, and copying it back
- * afterwards. On most processors the copy is avoided since
- * exception_thread_info == thread_info.
- */
- thread_info = (struct thread_info *)(regs->gpr[1] & ~(THREAD_SIZE-1));
- exception_thread_info = current_thread_info();
-
- if (thread_info != exception_thread_info) {
- /* Save the original current_thread_info. */
- memcpy(backup_current_thread_info, exception_thread_info, sizeof *thread_info);
- memcpy(exception_thread_info, thread_info, sizeof *thread_info);
- }
-
kgdb_handle_exception(0, SIGTRAP, 0, regs);
- if (thread_info != exception_thread_info)
- /* Restore current_thread_info lastly. */
- memcpy(exception_thread_info, backup_current_thread_info, sizeof *thread_info);
-
- kfree(backup_current_thread_info);
return 1;
}
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 5/6] kgdb/kgdbts: support ppc64
From: Tiejun Chen @ 2013-02-27 3:04 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev
In-Reply-To: <1361934261-31840-1-git-send-email-tiejun.chen@windriver.com>
We can't look up the address of the entry point of the function simply
via that function symbol for all architectures.
For PPC64 ABI, actually there is a function descriptors structure.
A function descriptor is a three doubleword data structure that contains
the following values:
* The first doubleword contains the address of the entry point of
the function.
* The second doubleword contains the TOC base address for
the function.
* The third doubleword contains the environment pointer for
languages such as Pascal and PL/1.
So we should call a wapperred dereference_function_descriptor() to get
the address of the entry point of the function.
Note this is also safe for other architecture after refer to
"include/asm-generic/sections.h" since:
dereference_function_descriptor(p) always is (p) if without arched definition.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
drivers/misc/kgdbts.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index 3aa9a96..4799e1f 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -103,6 +103,7 @@
#include <linux/delay.h>
#include <linux/kthread.h>
#include <linux/module.h>
+#include <asm/sections.h>
#define v1printk(a...) do { \
if (verbose) \
@@ -222,6 +223,7 @@ static unsigned long lookup_addr(char *arg)
addr = (unsigned long)do_fork;
else if (!strcmp(arg, "hw_break_val"))
addr = (unsigned long)&hw_break_val;
+ addr = (unsigned long )dereference_function_descriptor((void *)addr);
return addr;
}
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 4/6] powerpc/book3e: support kgdb for kernel space
From: Tiejun Chen @ 2013-02-27 3:04 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev
In-Reply-To: <1361934261-31840-1-git-send-email-tiejun.chen@windriver.com>
Currently we need to skip this for supporting KGDB.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/exceptions-64e.S | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 7df9a1f..800e2a3 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -598,11 +598,14 @@ kernel_dbg_exc:
rfdi
/* Normal debug exception */
+1:
+#ifndef CONFIG_KGDB
/* XXX We only handle coming from userspace for now since we can't
* quite save properly an interrupted kernel state yet
*/
-1: andi. r14,r11,MSR_PR; /* check for userspace again */
+ andi. r14,r11,MSR_PR; /* check for userspace again */
beq kernel_dbg_exc; /* if from kernel mode */
+#endif
/* Now we mash up things to make it look like we are coming on a
* normal exception
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 3/6] book3e/kgdb: update thread's dbcr0
From: Tiejun Chen @ 2013-02-27 3:04 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev
In-Reply-To: <1361934261-31840-1-git-send-email-tiejun.chen@windriver.com>
gdb always need to generate a single step properly to invoke
a kgdb state. But with lazy interrupt, book3e can't always
trigger a debug exception with a single step since the current
is blocked for handling those pending exception, then we miss
that expected dbcr configuration at last to generate a debug
exception.
So here we also update thread's dbcr0 to make sure the current
can go back with that missed dbcr0 configuration.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/kgdb.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 5ca82cd..1a57307 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -410,7 +410,7 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
struct pt_regs *linux_regs)
{
char *ptr = &remcom_in_buffer[1];
- unsigned long addr;
+ unsigned long addr, dbcr0;
switch (remcom_in_buffer[0]) {
/*
@@ -427,8 +427,15 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
/* set the trace bit if we're stepping */
if (remcom_in_buffer[0] == 's') {
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- mtspr(SPRN_DBCR0,
- mfspr(SPRN_DBCR0) | DBCR0_IC | DBCR0_IDM);
+ dbcr0 = mfspr(SPRN_DBCR0) | DBCR0_IC | DBCR0_IDM;
+ mtspr(SPRN_DBCR0, dbcr0);
+#ifdef CONFIG_PPC_BOOK3E_64
+ /* With lazy interrut we have to update thread dbcr0 here
+ * to make sure we can set debug properly at last to invoke
+ * kgdb again to work well.
+ */
+ current->thread.dbcr0 = dbcr0;
+#endif
linux_regs->msr |= MSR_DE;
#else
linux_regs->msr |= MSR_SE;
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 2/6] powerpc/book3e: store critical/machine/debug exception thread info
From: Tiejun Chen @ 2013-02-27 3:04 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev
In-Reply-To: <1361934261-31840-1-git-send-email-tiejun.chen@windriver.com>
We need to store thread info to these exception thread info like something
we already did for PPC32.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/exceptions-64e.S | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 7fd6af0..7df9a1f 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -67,6 +67,18 @@
std r10,PACA_##level##_STACK(r13);
#endif
+/* Store something to exception thread info */
+#define BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(type) \
+ ld r14,PACAKSAVE(r13); \
+ CURRENT_THREAD_INFO(r14, r14); \
+ CURRENT_THREAD_INFO(r15, r1); \
+ ld r10,TI_FLAGS(r14); \
+ std r10,TI_FLAGS(r15); \
+ ld r10,TI_PREEMPT(r14); \
+ std r10,TI_PREEMPT(r15); \
+ ld r10,TI_TASK(r14); \
+ std r10,TI_TASK(r15);
+
/* Exception prolog code for all exceptions */
#define EXCEPTION_PROLOG(n, intnum, type, addition) \
mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
@@ -104,6 +116,7 @@
BOOK3E_LOAD_EXC_LEVEL_STACK(CRIT); \
ld r1,PACA_CRIT_STACK(r13); \
subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+ BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(CRIT); \
1:
#define SPRN_CRIT_SRR0 SPRN_CSRR0
#define SPRN_CRIT_SRR1 SPRN_CSRR1
@@ -114,6 +127,7 @@
BOOK3E_LOAD_EXC_LEVEL_STACK(DBG); \
ld r1,PACA_DBG_STACK(r13); \
subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+ BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(DBG); \
1:
#define SPRN_DBG_SRR0 SPRN_DSRR0
#define SPRN_DBG_SRR1 SPRN_DSRR1
@@ -124,6 +138,7 @@
BOOK3E_LOAD_EXC_LEVEL_STACK(MC); \
ld r1,PACA_MC_STACK(r13); \
subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+ BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(MC); \
1:
#define SPRN_MC_SRR0 SPRN_MCSRR0
#define SPRN_MC_SRR1 SPRN_MCSRR1
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 1/6] powerpc/book3e: load critical/machine/debug exception stack
From: Tiejun Chen @ 2013-02-27 3:04 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev
In-Reply-To: <1361934261-31840-1-git-send-email-tiejun.chen@windriver.com>
We always alloc critical/machine/debug check exceptions. This is
different from the normal exception. So we should load these exception
stack properly like we did for booke.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/exceptions-64e.S | 49 +++++++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 1e7782b..7fd6af0 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -36,6 +36,37 @@
*/
#define SPECIAL_EXC_FRAME_SIZE INT_FRAME_SIZE
+/* only on book3e */
+#define DBG_STACK_BASE dbgirq_ctx
+#define MC_STACK_BASE mcheckirq_ctx
+#define CRIT_STACK_BASE critirq_ctx
+
+#ifdef CONFIG_RELOCATABLE
+#define LOAD_STACK_BASE(reg, level) \
+ tovirt(r2,r2); \
+ LOAD_REG_ADDR(reg, level##_STACK_BASE);
+#else
+#define LOAD_STACK_BASE(reg, level) \
+ LOAD_REG_IMMEDIATE(reg, level##_STACK_BASE);
+#endif
+
+#ifdef CONFIG_SMP
+#define BOOK3E_LOAD_EXC_LEVEL_STACK(level) \
+ mfspr r14,SPRN_PIR; \
+ slwi r14,r14,3; \
+ LOAD_STACK_BASE(r10, level); \
+ add r10,r10,r14; \
+ ld r10,0(r10); \
+ addi r10,r10,THREAD_SIZE; \
+ std r10,PACA_##level##_STACK(r13);
+#else
+#define BOOK3E_LOAD_EXC_LEVEL_STACK(level) \
+ LOAD_STACK_BASE(r10, level); \
+ ld r10,0(r10); \
+ addi r10,r10,THREAD_SIZE; \
+ std r10,PACA_##level##_STACK(r13);
+#endif
+
/* Exception prolog code for all exceptions */
#define EXCEPTION_PROLOG(n, intnum, type, addition) \
mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
@@ -68,20 +99,32 @@
#define SPRN_GDBELL_SRR1 SPRN_GSRR1
#define CRIT_SET_KSTACK \
+ andi. r10,r11,MSR_PR; \
+ bne 1f; \
+ BOOK3E_LOAD_EXC_LEVEL_STACK(CRIT); \
ld r1,PACA_CRIT_STACK(r13); \
- subi r1,r1,SPECIAL_EXC_FRAME_SIZE;
+ subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+1:
#define SPRN_CRIT_SRR0 SPRN_CSRR0
#define SPRN_CRIT_SRR1 SPRN_CSRR1
#define DBG_SET_KSTACK \
+ andi. r10,r11,MSR_PR; \
+ bne 1f; \
+ BOOK3E_LOAD_EXC_LEVEL_STACK(DBG); \
ld r1,PACA_DBG_STACK(r13); \
- subi r1,r1,SPECIAL_EXC_FRAME_SIZE;
+ subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+1:
#define SPRN_DBG_SRR0 SPRN_DSRR0
#define SPRN_DBG_SRR1 SPRN_DSRR1
#define MC_SET_KSTACK \
+ andi. r10,r11,MSR_PR; \
+ bne 1f; \
+ BOOK3E_LOAD_EXC_LEVEL_STACK(MC); \
ld r1,PACA_MC_STACK(r13); \
- subi r1,r1,SPECIAL_EXC_FRAME_SIZE;
+ subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+1:
#define SPRN_MC_SRR0 SPRN_MCSRR0
#define SPRN_MC_SRR1 SPRN_MCSRR1
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 0/6] powerpc/book3e: make kgdb to work well
From: Tiejun Chen @ 2013-02-27 3:04 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev
This patchset is used to support kgdb/gdb on book3e.
Validated on p4080ds and p5040ds with test single step and breakpoint
v3:
* make work when enable CONFIG_RELOCATABLE
* fix one typo in patch,
"powerpc/book3e: store critical/machine/debug exception thread info":
ld r1,PACAKSAVE(r13);
-> ld r14,PACAKSAVE(r13);
* remove copying the thread_info since booke and book3e always copy
the thead_info now when we enter the debug exception, and so drop
the v2 patch, "book3e/kgdb: Fix a single stgep case of lazy IRQ"
v2:
* Make sure we cover CONFIG_PPC_BOOK3E_64 safely
* Use LOAD_REG_IMMEDIATE() to load properly
the value of the constant expression in load debug exception stack
* Copy thread infor form the kernel stack coming from usr
* Rebase latest powerpc git tree
v1:
* Copy thread info only when we are from !user mode since we'll get kernel stack
coming from usr directly.
* remove save/restore EX_R14/EX_R15 since DBG_EXCEPTION_PROLOG already covered
this.
* use CURRENT_THREAD_INFO() conveniently to get thread.
* fix some typos
* add a patch to make sure gdb can generate a single step properly to invoke a
kgdb state.
* add a patch to if we need to replay an interrupt, we shouldn't restore that
previous backup thread info to make sure we can replay an interrupt lately
with a proper thread info.
* rebase latest powerpc git tree
v0:
This patchset is used to support kgdb for book3e.
------
Tiejun Chen (6):
powerpc/book3e: load critical/machine/debug exception stack
powerpc/book3e: store critical/machine/debug exception thread info
book3e/kgdb: update thread's dbcr0
powerpc/book3e: support kgdb for kernel space
kgdb/kgdbts: support ppc64
powerpc/kgdb: remove copying the thread_info
arch/powerpc/kernel/exceptions-64e.S | 69 ++++++++++++++++++++++++++++++++--
arch/powerpc/kernel/kgdb.c | 41 +++++---------------
drivers/misc/kgdbts.c | 2 +
3 files changed, 77 insertions(+), 35 deletions(-)
Tiejun
^ permalink raw reply
* Re: [PATCH] powerpc: kernel/kgdb.c: fix memory leakage
From: tiejun.chen @ 2013-02-27 2:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Stephen Rothwell, Michael Neuling, Cong Ding, linux-kernel,
Paul Mackerras, Jason Wessel, linuxppc-dev
In-Reply-To: <1360291273.2650.52.camel@pasglop>
On 02/08/2013 10:41 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2013-01-31 at 20:04 -0600, Jason Wessel wrote:
>
>>> diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
>>> index 8747447..5ca82cd 100644
>>> --- a/arch/powerpc/kernel/kgdb.c
>>> +++ b/arch/powerpc/kernel/kgdb.c
>>> @@ -154,12 +154,12 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
>>> static int kgdb_singlestep(struct pt_regs *regs)
>>> {
>>> struct thread_info *thread_info, *exception_thread_info;
>>> - struct thread_info *backup_current_thread_info = \
>>> - (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
>>> + struct thread_info *backup_current_thread_info;
>>
>>
>>
>> Woh... This is definitely wrong. You have found a problem for sure,
>> but this is not the right way to fix it.
>>
>> It is not a good idea to kmalloc while single stepping because you can
>> hang the kernel if you single step any operation in kmalloc().
>
> Ok. I applied the fix because it was obviously correct vs. the previous
> version of the code (ie. the kmalloc was already there)
>
>> I am in the process of going through all the kgdb mails from the last
>> few months while I had been away from the project, so I didn't catch
>> this one and I see it has upstream commit (fefd9e6f8). I'll submit
>> another patch to fix this the right way and use a static variable.
>> This is ok to use a static variable here because this is not something
>> we can recursively call at a single CPU level.
>
> But a static will be shared by all CPUs or do you mean a per-cpu one ?
>
>> If Ben prefers we not burn the memory unless kgdb is active we can
>> kmalloc / kfree the space we need at the time that kgdb is
>> initialized. Else we can go with this patch you see below. We'll see
>> what Ben desires.
>
> What about the stack ? thread info isn't very big...
Actually booke already copy the thread_info when we enter the debug exception,
and I will send a patch to do the same thing for book3e, so I also remove these
copying action here then we're happy without these nasty stuff :)
Tiejun
>
> Cheers,
> Ben.
>
>> -----
>> diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
>> index a7bc752..bb12c8b 100644
>> --- a/arch/powerpc/kernel/kgdb.c
>> +++ b/arch/powerpc/kernel/kgdb.c
>> @@ -151,15 +151,16 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
>> return 1;
>> }
>>
>> +static struct thread_info kgdb_backup_thread_info[NR_CPUS];
>> +
>> static int kgdb_singlestep(struct pt_regs *regs)
>> {
>> struct thread_info *thread_info, *exception_thread_info;
>> - struct thread_info *backup_current_thread_info;
>> + int cpu = raw_smp_processor_id();
>>
>> if (user_mode(regs))
>> return 0;
>>
>> - backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
>> /*
>> * On Book E and perhaps other processors, singlestep is handled on
>> * the critical exception stack. This causes current_thread_info()
>> @@ -175,7 +176,7 @@ static int kgdb_singlestep(struct pt_regs *regs)
>>
>> if (thread_info != exception_thread_info) {
>> /* Save the original current_thread_info. */
>> - memcpy(backup_current_thread_info, exception_thread_info, sizeof *thread_info);
>> + memcpy(&kgdb_backup_thread_info[cpu], exception_thread_info, sizeof *thread_info);
>> memcpy(exception_thread_info, thread_info, sizeof *thread_info);
>> }
>>
>> @@ -183,9 +184,8 @@ static int kgdb_singlestep(struct pt_regs *regs)
>>
>> if (thread_info != exception_thread_info)
>> /* Restore current_thread_info lastly. */
>> - memcpy(exception_thread_info, backup_current_thread_info, sizeof *thread_info);
>> + memcpy(exception_thread_info, &kgdb_backup_thread_info[cpu], sizeof *thread_info);
>>
>> - kfree(backup_current_thread_info);
>> return 1;
>> }
>>
>>
>> -----
>>
>>
>> Thanks,
>> Jason.
>>
>>
>>>
>>> if (user_mode(regs))
>>> return 0;
>>>
>>> + backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
>>> /*
>>> * On Book E and perhaps other processors, singlestep is handled on
>>> * the critical exception stack. This causes current_thread_info()
>>> @@ -185,6 +185,7 @@ static int kgdb_singlestep(struct pt_regs *regs)
>>> /* Restore current_thread_info lastly. */
>>> memcpy(exception_thread_info, backup_current_thread_info, sizeof *thread_info);
>>>
>>> + kfree(backup_current_thread_info);
>>> return 1;
>>> }
>>>
>>>
>
>
>
>
^ permalink raw reply
* Re: [PATCH 5/6][v4]: perf: Create a sysfs entry for Power event format
From: Michael Ellerman @ 2013-02-27 1:27 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: Andi Kleen, Peter Zijlstra, robert.richter, Anton Blanchard,
linux-kernel, Stephane Eranian, linuxppc-dev, Ingo Molnar,
Paul Mackerras, Arnaldo Carvalho de Melo, Jiri Olsa
In-Reply-To: <20130123062613.GF13720@us.ibm.com>
On Tue, Jan 22, 2013 at 10:26:13PM -0800, Sukadev Bhattiprolu wrote:
>
> [PATCH 5/6][v4]: perf: Create a sysfs entry for Power event format
>
> Create a sysfs entry, '/sys/bus/event_source/devices/cpu/format/event'
> which describes the format of a POWER cpu.
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index fa476d5..4ae044b 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1315,6 +1315,18 @@ ssize_t power_events_sysfs_show(struct device *dev,
> return sprintf(page, "event=0x%02llx\n", pmu_attr->id);
> }
>
> +PMU_FORMAT_ATTR(event, "config:0-20");
Just noticed, it's 20 bits, which is 0-19.
cheers
^ permalink raw reply
* Re: [PATCH 5/6][v4]: perf: Create a sysfs entry for Power event format
From: Michael Ellerman @ 2013-02-27 1:17 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: Andi Kleen, Peter Zijlstra, robert.richter, Anton Blanchard,
linux-kernel, Stephane Eranian, linuxppc-dev, Ingo Molnar,
Paul Mackerras, Arnaldo Carvalho de Melo, Jiri Olsa
In-Reply-To: <20130226200343.GA21543@us.ibm.com>
On Tue, Feb 26, 2013 at 12:03:43PM -0800, Sukadev Bhattiprolu wrote:
> Michael Ellerman [michael@ellerman.id.au] wrote:
> | On Tue, Jan 22, 2013 at 10:26:13PM -0800, Sukadev Bhattiprolu wrote:
> | >
> | > [PATCH 5/6][v4]: perf: Create a sysfs entry for Power event format
> | >
> | > Create a sysfs entry, '/sys/bus/event_source/devices/cpu/format/event'
> | > which describes the format of a POWER cpu.
> |
> | Did this patch go upstream? I don't see it.
>
> Hmm, patches 1..4,6 are in linux-tip and Arnaldo's trees but patch 5 is
> in neither.
I suspect Arnaldo was either waiting for an ACK from Ben, or was
expecting Ben to take it?
> | > The format of the event is the same for all POWER cpus at least in
> | > (Power6, Power7), so bulk of this change is common in the code common
> | > to POWER cpus.
> |
> | No. The event format is different on most POWER cpus, in particular it
> | is different on Power6 and Power7, and will be different again on
> | Power8.
>
> Sigh. The port of this patchset to Power6 has not started yet.
It should hardly require a port, it's about five lines. But it doesn't
matter for now.
> But this patchset does work on Power7 correct ?
Yes, and without it the rest of the series is essentially useless. So I
guess it's better than nothing and we should get it in, we can fix it up
later to work across different chips.
Ben can you grab it, it's all arch/powerpc.
cheers
^ permalink raw reply
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Lai Jiangshan @ 2013-02-27 0:33 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: linux-doc, peterz, fweisbec, linux-kernel, Michel Lespinasse,
mingo, linux-arch, linux, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, rostedt, rjw, namhyung, tglx, linux-arm-kernel,
netdev, oleg, vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <512D0D67.9010609@linux.vnet.ibm.com>
On Wed, Feb 27, 2013 at 3:30 AM, Srivatsa S. Bhat
<srivatsa.bhat@linux.vnet.ibm.com> wrote:
> On 02/26/2013 09:55 PM, Lai Jiangshan wrote:
>> On Tue, Feb 26, 2013 at 10:22 PM, Srivatsa S. Bhat
>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>
>>> Hi Lai,
>>>
>>> I'm really not convinced that piggy-backing on lglocks would help
>>> us in any way. But still, let me try to address some of the points
>>> you raised...
>>>
>>> On 02/26/2013 06:29 PM, Lai Jiangshan wrote:
>>>> On Tue, Feb 26, 2013 at 5:02 PM, Srivatsa S. Bhat
>>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>> On 02/26/2013 05:47 AM, Lai Jiangshan wrote:
>>>>>> On Tue, Feb 26, 2013 at 3:26 AM, Srivatsa S. Bhat
>>>>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>>>> Hi Lai,
>>>>>>>
>>>>>>> On 02/25/2013 09:23 PM, Lai Jiangshan wrote:
>>>>>>>> Hi, Srivatsa,
>>>>>>>>
>>>>>>>> The target of the whole patchset is nice for me.
>>>>>>>
>>>>>>> Cool! Thanks :-)
>>>>>>>
>>>>> [...]
>>>>>
>>>>> Unfortunately, I see quite a few issues with the code above. IIUC, the
>>>>> writer and the reader both increment the same counters. So how will the
>>>>> unlock() code in the reader path know when to unlock which of the locks?
>>>>
>>>> The same as your code, the reader(which nested in write C.S.) just dec
>>>> the counters.
>>>
>>> And that works fine in my case because the writer and the reader update
>>> _two_ _different_ counters.
>>
>> I can't find any magic in your code, they are the same counter.
>>
>> /*
>> * It is desirable to allow the writer to acquire the percpu-rwlock
>> * for read (if necessary), without deadlocking or getting complaints
>> * from lockdep. To achieve that, just increment the reader_refcnt of
>> * this CPU - that way, any attempt by the writer to acquire the
>> * percpu-rwlock for read, will get treated as a case of nested percpu
>> * reader, which is safe, from a locking perspective.
>> */
>> this_cpu_inc(pcpu_rwlock->rw_state->reader_refcnt);
>>
>
> Whoa! Hold on, were you really referring to _this_ increment when you said
> that, in your patch you would increment the refcnt at the writer? Then I guess
> there is a major disconnect in our conversations. (I had assumed that you were
> referring to the update of writer_signal, and were just trying to have a single
> refcnt instead of reader_refcnt and writer_signal).
https://github.com/laijs/linux/commit/53e5053d5b724bea7c538b11743d0f420d98f38d
Sorry the name "fallback_reader_refcnt" misled you.
>
> So, please let me clarify things a bit here. Forget about the above increment
> of reader_refcnt at the writer side. Its almost utterly insignificant for our
> current discussion. We can simply replace it with a check as shown below, at
> the reader side:
>
> void percpu_read_lock_irqsafe()
> {
> if (current == active_writer)
> return;
>
> /* Rest of the code */
> }
>
> Now, assuming that, in your patch, you were trying to use the per-cpu refcnt
> to allow the writer to safely take the reader path, you can simply get rid of
> that percpu-refcnt, as demonstrated above.
>
> So that would reduce your code to the following (after simplification):
>
> lg_rwlock_local_read_lock()
> {
> if (current == active_writer)
> return;
> if (arch_spin_trylock(per-cpu-spinlock))
> return;
> read_lock(global-rwlock);
> }
>
> Now, let us assume that hotplug is not happening, meaning, nobody is running
> the writer side code. Now let's see what happens at the reader side in your
> patch. As I mentioned earlier, the readers are _very_ frequent and can be in
> very hot paths. And they also happen to be nested quite often.
>
> So, a non-nested reader acquires the per-cpu spinlock. Every subsequent nested
> reader on that CPU has to acquire the global rwlock for read. Right there you
> have 2 significant performance issues -
> 1. Acquiring the (spin) lock is costly
> 2. Acquiring the global rwlock causes cacheline bouncing, which hurts
> performance.
>
> And why do we care so much about performance here? Because, the existing
> kernel does an efficient preempt_disable() here - which is an optimized
> per-cpu counter increment. Replacing that with such heavy primitives on the
> reader side can be very bad.
>
> Now, how does my patchset tackle this? My scheme just requires an increment
> of a per-cpu refcnt (reader_refcnt) and memory barrier. Which is acceptable
> from a performance-perspective, because IMHO its not horrendously worse than
> a preempt_disable().
>
>>
>>> If both of them update the same counter, there
>>> will be a semantic clash - an increment of the counter can either mean that
>>> a new writer became active, or it can also indicate a nested reader. A decrement
>>> can also similarly have 2 meanings. And thus it will be difficult to decide
>>> the right action to take, based on the value of the counter.
>>>
>>>>
>>>>> (The counter-dropping-to-zero logic is not safe, since it can be updated
>>>>> due to different reasons). And now that I look at it again, in the absence
>>>>> of the writer, the reader is allowed to be recursive at the heavy cost of
>>>>> taking the global rwlock for read, every 2nd time you nest (because the
>>>>> spinlock is non-recursive).
>>>>
>>>> (I did not understand your comments of this part)
>>>> nested reader is considered seldom.
>>>
>>> No, nested readers can be _quite_ frequent. Because, potentially all users
>>> of preempt_disable() are readers - and its well-known how frequently we
>>> nest preempt_disable(). As a simple example, any atomic reader who calls
>>> smp_call_function() will become a nested reader, because smp_call_function()
>>> itself is a reader. So reader nesting is expected to be quite frequent.
>>>
>>>> But if N(>=2) nested readers happen,
>>>> the overhead is:
>>>> 1 spin_try_lock() + 1 read_lock() + (N-1) __this_cpu_inc()
>>>>
>>>
>>> In my patch, its just this_cpu_inc(). Note that these are _very_ hot paths.
>>> So every bit of optimization that you can add is worthwhile.
>>>
>>> And your read_lock() is a _global_ lock - thus, it can lead to a lot of
>>> cache-line bouncing. That's *exactly* why I have used per-cpu refcounts in
>>> my synchronization scheme, to avoid taking the global rwlock as much as possible.
>>>
>>> Another important point to note is that, the overhead we are talking about
>>> here, exists even when _not_ performing hotplug. And its the replacement to
>>> the super-fast preempt_disable(). So its extremely important to consciously
>>> minimize this overhead - else we'll end up slowing down the system significantly.
>>>
>>
>> All I was considered is "nested reader is seldom", so I always
>> fallback to rwlock when nested.
>> If you like, I can add 6 lines of code, the overhead is
>> 1 spin_try_lock()(fast path) + N __this_cpu_inc()
>>
>
> I'm assuming that calculation is no longer valid, considering that
> we just discussed how the per-cpu refcnt that you were using is quite
> unnecessary and can be removed.
>
> IIUC, the overhead with your code, as per above discussion would be:
> 1 spin_try_lock() [non-nested] + N read_lock(global_rwlock).
https://github.com/laijs/linux/commit/46334544bb7961550b7065e015da76f6dab21f16
Again, I'm so sorry the name "fallback_reader_refcnt" misled you.
>
> Note that I'm referring to the scenario when hotplug is _not_ happening
> (ie., nobody is running writer side code).
>
>> The overhead of your code is
>> 2 smp_mb() + N __this_cpu_inc()
>>
>
> Right. And as you can see, this is much much better than the overhead
> shown above.
I will write a test to compare it to "1 spin_try_lock()(fast path) +
N __this_cpu_inc()"
>
>> I don't see how much different.
>>
>>>>> Also, this lg_rwlock implementation uses 3
>>>>> different data-structures - a per-cpu spinlock, a global rwlock and
>>>>> a per-cpu refcnt, and its not immediately apparent why you need those many
>>>>> or even those many varieties.
>>>>
>>>> data-structures is the same as yours.
>>>> fallback_reader_refcnt <--> reader_refcnt
>>>
>>> This has semantic problems, as noted above.
>>>
>>>> per-cpu spinlock <--> write_signal
>>>
>>> Acquire/release of (spin) lock is costlier than inc/dec of a counter, IIUC.
>>>
>>>> fallback_rwlock <---> global_rwlock
>>>>
>>>>> Also I see that this doesn't handle the
>>>>> case of interrupt-handlers also being readers.
>>>>
>>>> handled. nested reader will see the ref or take the fallback_rwlock
>>>>
>>
>> Sorry, _reentrance_ read_lock() will see the ref or take the fallback_rwlock
>>
>>>
>>> I'm not referring to simple nested readers here, but interrupt handlers who
>>> can act as readers. For starters, the arch_spin_trylock() is not safe when
>>> interrupt handlers can also run the same code, right? You'll need to save
>>> and restore interrupts at critical points in the code. Also, the __foo()
>>> variants used to read/update the counters are not interrupt-safe.
>>
>> I must missed something.
>>
>> Could you elaborate more why arch_spin_trylock() is not safe when
>> interrupt handlers can also run the same code?
>>
>> Could you elaborate more why __this_cpu_op variants is not
>> interrupt-safe since they are always called paired.
>>
>
> Take a look at include/linux/percpu.h. You'll note that __this_cpu_*
> operations map to __this_cpu_generic_to_op(), which doesn't disable interrupts
> while doing the update. Hence you can get inconsistent results if an interrupt
> hits the CPU at that time and the interrupt handler tries to do the same thing.
> In contrast, if you use this_cpu_inc() for example, interrupts are explicitly
> disabled during the update and hence you won't get inconsistent results.
xx_lock()/xx_unlock() are must called paired, if interrupts happens
the value of the data is recovered after the interrupts return.
the same reason, preempt_disable() itself it is not irqsafe,
but preempt_disable()/preempt_enable() are called paired, so they are
all irqsafe.
>
>>
>>> And,
>>> the unlock() code in the reader path is again going to be confused about
>>> what to do when interrupt handlers interrupt regular readers, due to the
>>> messed up refcount.
>>
>> I still can't understand.
>>
>
> The primary reason _I_ was using the refcnt vs the reason _you_ were using the
> refcnt, appears to be very different. Maybe that's why the above statement
> didn't make sense. In your case, IIUC, you can simply get rid of the refcnt
> and replace it with the simple check I mentioned above. Whereas, I use
> refcnts to keep the reader-side synchronization fast (and for reader-writer
> communication).
>
>
> Regards,
> Srivatsa S. Bhat
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox