* RE: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
From: Sethi Varun-B16395 @ 2013-02-27 12:04 UTC (permalink / raw)
To: Kumar Gala, Benjamin Herrenschmidt
Cc: Wood Scott-B07421, Joerg Roedel, linux-kernel@vger.kernel.org,
Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130227113045.GH26252@8bytes.org>
Hi Kumar,Ben,
I am implementing the Freescale PAMU (IOMMU) driver using the Linux IOMMU A=
PI. In this particular patch, I have added a new field to dev_archdata stru=
cture to store the dma domain information.
This field is updated whenever we attach a device to an iommu domain.
Regards
Varun
> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Wednesday, February 27, 2013 5:01 PM
> To: Sethi Varun-B16395
> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
> pointer in archdata.
>=20
> 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;
>=20
> 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.
>=20
>=20
> Joerg
>=20
>=20
^ permalink raw reply
* RE: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Sethi Varun-B16395 @ 2013-02-27 11:56 UTC (permalink / raw)
To: Stuart Yoder
Cc: Wood Scott-B07421, Joerg Roedel, linux-kernel@vger.kernel.org,
Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CALRxmdBBO2AoAnKSTNNTYhHZ9fitJKWWeZMUj1ObtFhsZS5iSw@mail.gmail.com>
> -----Original Message-----
> From: Stuart Yoder [mailto:b08248@gmail.com]
> Sent: Wednesday, February 27, 2013 4:03 AM
> To: Sethi Varun-B16395
> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; Wood Scott-B07421; Joerg Roedel; Yoder
> Stuart-B08248
> Subject: Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU
> API implementation.
>=20
> Have not got through the entire file, but have a few comments...
>=20
> +/*
> + * Set the PAACE type as primary and set the coherency required domain
> + * attribute
> + */
> +static void pamu_setup_default_xfer_to_host_ppaace(struct paace
> +*ppaace) {
> + set_bf(ppaace->addr_bitfields, PAACE_AF_PT, PAACE_PT_PRIMARY);
> +
> + set_bf(ppaace->domain_attr.to_host.coherency_required,
> PAACE_DA_HOST_CR,
> + PAACE_M_COHERENCE_REQ);
> +}
> +
> +/*
> + * Set the PAACE type as secondary and set the coherency required
> +domain
> + * attribute.
> + */
> +static void pamu_setup_default_xfer_to_host_spaace(struct paace
> +*spaace) {
> + set_bf(spaace->addr_bitfields, PAACE_AF_PT, PAACE_PT_SECONDARY);
> + set_bf(spaace->domain_attr.to_host.coherency_required,
> PAACE_DA_HOST_CR,
> + PAACE_M_COHERENCE_REQ);
> +}
>=20
> Can we change the names of the above functions... I know there is some
> history
> with the name, but "xfer_to_host" is confusing.
>=20
> Maybe just call them:
>=20
> pamu_init_paace()
> pamu_init_spaace()
>=20
[Sethi Varun-B16395] ok, will change the function names.
> > +/**
> > + * pamu_config_spaace() - Sets up SPAACE entry for specified
> > +subwindow
> > + *
> > + * @liodn: Logical IO device number
> > + * @subwin_cnt: number of sub-windows associated with dma-window
> > + * @subwin_addr: starting address of subwindow
> > + * @subwin_size: size of subwindow
> > + * @omi: Operation mapping index
> > + * @rpn: real (true physical) page number
> > + * @snoopid: snoop id for hardware coherency -- if ~snoopid =3D=3D 0 t=
hen
> > + * snoopid not defined
> > + * @stashid: cache stash id for associated cpu
> > + * @enable: enable/disable subwindow after reconfiguration
> > + * @prot: sub window permissions
> > + *
> > + * Returns 0 upon success else error code < 0 returned */ int
> > +pamu_config_spaace(int liodn, u32 subwin_cnt, u32 subwin_addr,
> > + phys_addr_t subwin_size, u32 omi, unsigned long
> rpn,
> > + u32 snoopid, u32 stashid, int enable, int prot)
> > +{
> > + struct paace *paace;
> > +
> > + /* setup sub-windows */
> > + if (!subwin_cnt) {
> > + pr_err("Invalid subwindow count\n");
> > + return -EINVAL;
> > + }
> > +
> > + paace =3D pamu_get_ppaace(liodn);
> > + if (subwin_addr > 0 && subwin_addr < subwin_cnt && paace) {
>=20
> Why is the comparison subwin_addr < subwin_cnt? Seems wrong...
>=20
[Sethi Varun-B16395] It's actually the subwindow index. I will rename the v=
ariable.
> > + paace =3D pamu_get_spaace(paace, subwin_addr - 1);
> > +
> > + if (paace && !(paace->addr_bitfields & PAACE_V_VALID))
> {
> > + pamu_setup_default_xfer_to_host_spaace(paace);
> > + set_bf(paace->addr_bitfields, SPAACE_AF_LIODN,
> liodn);
> > + }
> > + }
> > +
> > + if (!paace) {
> > + pr_err("Invalid liodn entry\n");
> > + return -ENOENT;
> > + }
> > +
> > + if (!enable && prot =3D=3D PAACE_AP_PERMS_DENIED) {
> > + if (subwin_addr > 0)
> > + set_bf(paace->addr_bitfields, PAACE_AF_V,
> > + PAACE_V_INVALID);
> > + else
> > + set_bf(paace->addr_bitfields, PAACE_AF_AP,
> > + prot);
> > + mb();
> > + return 0;
> > + }
>=20
> Can you add a comment to the above if statement...when is this function
> called with PAACE_AP_PERMS_DENIED?
>=20
[Sethi Varun-B16395] Actually, this piece of code is redundant in case of t=
he window based API. I will remove this. PAACE_AP_PERMS_DENIED is primarily=
used for disabling the primary subwindow.
-Varun
^ permalink raw reply
* 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
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