* Re: kernel panic during kernel module load (powerpc specific part)
From: Benjamin Herrenschmidt @ 2012-06-05 22:14 UTC (permalink / raw)
To: Gabriel Paubert
Cc: Wrobel Heinz-R39252, Michael Ellerman, Steffen Rumler,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20120605113225.GA11215@visitor2.iram.es>
On Tue, 2012-06-05 at 13:32 +0200, Gabriel Paubert wrote:
> - gcc-4.6 and gcc-4.7 behave identically, if -Os is set, they
> generate by default lmw/stmw. But if I combine -Os with
> -mno-multiple, they call the helper functions.
>
> In other words, on this system, gcc-4.4 is broken but should not
> cause any harm. gcc-4.6 and gcc-4.7 look correct, but are there
> any processors on which -mno-multiple is worth setting?
It could be an artifact of the -mtune we use, dunno. And yes, I'm pretty
sure some embedded processors don't like multiple load/store.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc: Fix assmption of end_of_DRAM() returns end address
From: Benjamin Herrenschmidt @ 2012-06-05 22:17 UTC (permalink / raw)
To: Bharat Bhushan; +Cc: Bharat Bhushan, linuxppc-dev, linux-kernel
In-Reply-To: <1338904504-2750-1-git-send-email-bharat.bhushan@freescale.com>
On Tue, 2012-06-05 at 19:25 +0530, Bharat Bhushan wrote:
> memblock_end_of_DRAM() returns end_address + 1, not end address.
> While some code assumes that it returns end address.
Shouldn't we instead fix it the other way around ? IE, make
memblock_end_of_DRAM() does what the name implies, which is to return
the last byte of DRAM, and fix the -other- callers not to make bad
assumptions ?
Cheers,
Ben.
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> ---
> This patch is based on next branch of https://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git
>
> arch/powerpc/platforms/44x/currituck.c | 2 +-
> arch/powerpc/platforms/85xx/corenet_ds.c | 2 +-
> arch/powerpc/platforms/85xx/ge_imp3a.c | 2 +-
> arch/powerpc/platforms/85xx/mpc8536_ds.c | 2 +-
> arch/powerpc/platforms/85xx/mpc85xx_ds.c | 2 +-
> arch/powerpc/platforms/85xx/mpc85xx_mds.c | 2 +-
> arch/powerpc/platforms/85xx/p1022_ds.c | 2 +-
> arch/powerpc/platforms/86xx/mpc86xx_hpcn.c | 2 +-
> 8 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/platforms/44x/currituck.c b/arch/powerpc/platforms/44x/currituck.c
> index 583e67f..9f6c33d 100644
> --- a/arch/powerpc/platforms/44x/currituck.c
> +++ b/arch/powerpc/platforms/44x/currituck.c
> @@ -160,7 +160,7 @@ static void __init ppc47x_setup_arch(void)
> /* No need to check the DMA config as we /know/ our windows are all of
> * RAM. Lets hope that doesn't change */
> #ifdef CONFIG_SWIOTLB
> - if (memblock_end_of_DRAM() > 0xffffffff) {
> + if ((memblock_end_of_DRAM() - 1) > 0xffffffff) {
> ppc_swiotlb_enable = 1;
> set_pci_dma_ops(&swiotlb_dma_ops);
> ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
> diff --git a/arch/powerpc/platforms/85xx/corenet_ds.c b/arch/powerpc/platforms/85xx/corenet_ds.c
> index dd3617c..925b028 100644
> --- a/arch/powerpc/platforms/85xx/corenet_ds.c
> +++ b/arch/powerpc/platforms/85xx/corenet_ds.c
> @@ -77,7 +77,7 @@ void __init corenet_ds_setup_arch(void)
> #endif
>
> #ifdef CONFIG_SWIOTLB
> - if (memblock_end_of_DRAM() > max) {
> + if ((memblock_end_of_DRAM() - 1) > max) {
> ppc_swiotlb_enable = 1;
> set_pci_dma_ops(&swiotlb_dma_ops);
> ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
> diff --git a/arch/powerpc/platforms/85xx/ge_imp3a.c b/arch/powerpc/platforms/85xx/ge_imp3a.c
> index 1801462..b6a728b 100644
> --- a/arch/powerpc/platforms/85xx/ge_imp3a.c
> +++ b/arch/powerpc/platforms/85xx/ge_imp3a.c
> @@ -125,7 +125,7 @@ static void __init ge_imp3a_setup_arch(void)
> mpc85xx_smp_init();
>
> #ifdef CONFIG_SWIOTLB
> - if (memblock_end_of_DRAM() > max) {
> + if ((memblock_end_of_DRAM() - 1) > max) {
> ppc_swiotlb_enable = 1;
> set_pci_dma_ops(&swiotlb_dma_ops);
> ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
> diff --git a/arch/powerpc/platforms/85xx/mpc8536_ds.c b/arch/powerpc/platforms/85xx/mpc8536_ds.c
> index 585bd22..767c7cf 100644
> --- a/arch/powerpc/platforms/85xx/mpc8536_ds.c
> +++ b/arch/powerpc/platforms/85xx/mpc8536_ds.c
> @@ -75,7 +75,7 @@ static void __init mpc8536_ds_setup_arch(void)
> #endif
>
> #ifdef CONFIG_SWIOTLB
> - if (memblock_end_of_DRAM() > max) {
> + if ((memblock_end_of_DRAM() - 1) > max) {
> ppc_swiotlb_enable = 1;
> set_pci_dma_ops(&swiotlb_dma_ops);
> ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> index 1fd91e9..d30f6c4 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
> @@ -173,7 +173,7 @@ static void __init mpc85xx_ds_setup_arch(void)
> mpc85xx_smp_init();
>
> #ifdef CONFIG_SWIOTLB
> - if (memblock_end_of_DRAM() > max) {
> + if ((memblock_end_of_DRAM() - 1) > max) {
> ppc_swiotlb_enable = 1;
> set_pci_dma_ops(&swiotlb_dma_ops);
> ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> index d208ebc..8e4b094 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> @@ -359,7 +359,7 @@ static void __init mpc85xx_mds_setup_arch(void)
> mpc85xx_mds_qe_init();
>
> #ifdef CONFIG_SWIOTLB
> - if (memblock_end_of_DRAM() > max) {
> + if ((memblock_end_of_DRAM() - 1) > max) {
> ppc_swiotlb_enable = 1;
> set_pci_dma_ops(&swiotlb_dma_ops);
> ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
> diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c b/arch/powerpc/platforms/85xx/p1022_ds.c
> index f700c81..74e310b 100644
> --- a/arch/powerpc/platforms/85xx/p1022_ds.c
> +++ b/arch/powerpc/platforms/85xx/p1022_ds.c
> @@ -450,7 +450,7 @@ static void __init p1022_ds_setup_arch(void)
> mpc85xx_smp_init();
>
> #ifdef CONFIG_SWIOTLB
> - if (memblock_end_of_DRAM() > max) {
> + if ((memblock_end_of_DRAM() - 1) > max) {
> ppc_swiotlb_enable = 1;
> set_pci_dma_ops(&swiotlb_dma_ops);
> ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
> diff --git a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
> index 3755e61..817245b 100644
> --- a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
> +++ b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
> @@ -102,7 +102,7 @@ mpc86xx_hpcn_setup_arch(void)
> #endif
>
> #ifdef CONFIG_SWIOTLB
> - if (memblock_end_of_DRAM() > max) {
> + if ((memblock_end_of_DRAM() - 1) > max) {
> ppc_swiotlb_enable = 1;
> set_pci_dma_ops(&swiotlb_dma_ops);
> ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
^ permalink raw reply
* Re: [PATCH] powerpc: Fix assmption of end_of_DRAM() returns end address
From: David Miller @ 2012-06-05 22:20 UTC (permalink / raw)
To: benh; +Cc: bharat.bhushan, linuxppc-dev, linux-kernel, r65777
In-Reply-To: <1338934659.7150.113.camel@pasglop>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Wed, 06 Jun 2012 08:17:39 +1000
> On Tue, 2012-06-05 at 19:25 +0530, Bharat Bhushan wrote:
>> memblock_end_of_DRAM() returns end_address + 1, not end address.
>> While some code assumes that it returns end address.
>
> Shouldn't we instead fix it the other way around ? IE, make
> memblock_end_of_DRAM() does what the name implies, which is to return
> the last byte of DRAM, and fix the -other- callers not to make bad
> assumptions ?
That was my impression too when I saw this patch.
^ permalink raw reply
* Re: [PATCH] perf: Don't use SIAR for user addresses
From: Benjamin Herrenschmidt @ 2012-06-05 22:43 UTC (permalink / raw)
To: Sukadev Bhattiprolu; +Cc: mpjohn, linuxppc-dev, Anton Blanchard
In-Reply-To: <20120605203247.GA1525@us.ibm.com>
On Tue, 2012-06-05 at 13:32 -0700, Sukadev Bhattiprolu wrote:
> From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Date: Fri, 1 Jun 2012 20:56:02 -0400
> Subject: [PATCH] perf: Don't use SIAR for user-space addresses
>
> With the pipelining on Power7, the SIAR can be off by several instructions
> leading to incorrect callgraphs. For user space code at least we can be more
> accurate by just using the next instruction pointer.
>
> Based on code/input from Anton Blanchard.
Had a chat with him, we still want to use SIAR for marked events, which
I think off the top of my head is mmcra & MMCRA_SAMPLE_ENABLE.
There's a few more considerations we discussed, such as wanting also
more precise backtraces in kernel space, but still capture in pHyp when
SIHV is set and have maybe some kind of sysfs option to revert back to
SIAR kernel-wide if we are trying to track down hard irq off stuff.
I'll let Anton followup. I'm still technically not working for a couple
of weeks :-)
Cheers,
Ben.
> Before this fon Power7, we see callgraphs like:
>
> 1.90% sprintft sprintft [.] do_my_sprintf
> |
> --- 00000011.plt_call.strlen@@GLIBC_2.3+0
> do_my_sprintf
> main
> generic_start_main
> __libc_start_main
> (nil)
> ...snip...
>
> |
> --- __random
> rand
> |
> |--63.16%-- do_my_sprintf
> | main
> | generic_start_main
> | __libc_start_main
> | (nil)
> |
> --36.84%-- rand
> do_my_sprintf
> main
> generic_start_main
> __libc_start_main
> (nil)
>
> which seems to indicate the libc __random() calls do_my_sprintf(), instead
> of the other way around.
>
> After the fix, the same trace looks like this:
>
> 4.08% sprintft sprintft [.] do_my_sprintf
> |
> --- do_my_sprintf
> do_my_sprintf
> main
> generic_start_main
> __libc_start_main
> (nil)
>
> Cc: Anton Blanchard <anton@samba.org>
> Reported-by: Maynard Johnson <mpjohn@us.ibm.com>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
> arch/powerpc/perf/core-book3s.c | 39 ++++++++++++++++++++++++++++++---------
> 1 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 8f84bcb..846bd68 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -116,6 +116,26 @@ static inline void perf_get_data_addr(struct pt_regs *regs, u64 *addrp)
> *addrp = mfspr(SPRN_SDAR);
> }
>
> +static bool mmcra_sihv(unsigned long mmcra)
> +{
> + unsigned long sihv = MMCRA_SIHV;
> +
> + if (ppmu->flags & PPMU_ALT_SIPR)
> + sihv = POWER6_MMCRA_SIHV;
> +
> + return !!(mmcra & sihv);
> +}
> +
> +static bool mmcra_sipr(unsigned long mmcra)
> +{
> + unsigned long sipr = MMCRA_SIPR;
> +
> + if (ppmu->flags & PPMU_ALT_SIPR)
> + sipr = POWER6_MMCRA_SIPR;
> +
> + return !!(mmcra & sipr);
> +}
> +
> static inline u32 perf_flags_from_msr(struct pt_regs *regs)
> {
> if (regs->msr & MSR_PR)
> @@ -128,8 +148,6 @@ static inline u32 perf_flags_from_msr(struct pt_regs *regs)
> static inline u32 perf_get_misc_flags(struct pt_regs *regs)
> {
> unsigned long mmcra = regs->dsisr;
> - unsigned long sihv = MMCRA_SIHV;
> - unsigned long sipr = MMCRA_SIPR;
>
> /* Not a PMU interrupt: Make up flags from regs->msr */
> if (TRAP(regs) != 0xf00)
> @@ -156,15 +174,10 @@ static inline u32 perf_get_misc_flags(struct pt_regs *regs)
> return PERF_RECORD_MISC_USER;
> }
>
> - if (ppmu->flags & PPMU_ALT_SIPR) {
> - sihv = POWER6_MMCRA_SIHV;
> - sipr = POWER6_MMCRA_SIPR;
> - }
> -
> /* PR has priority over HV, so order below is important */
> - if (mmcra & sipr)
> + if (mmcra_sipr(mmcra))
> return PERF_RECORD_MISC_USER;
> - if ((mmcra & sihv) && (freeze_events_kernel != MMCR0_FCHV))
> + if (mmcra_sihv(mmcra) && (freeze_events_kernel != MMCR0_FCHV))
> return PERF_RECORD_MISC_HYPERVISOR;
> return PERF_RECORD_MISC_KERNEL;
> }
> @@ -1340,6 +1353,14 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
> !(mmcra & MMCRA_SAMPLE_ENABLE))
> return regs->nip;
>
> + /*
> + * With pipelining on P7, addresses in the SIAR can be off by several
> + * instructions. For user space use the next instruction pointer as
> + * that will be more accurate.
> + */
> + if (mmcra_sipr(mmcra))
> + return regs->nip;
> +
> return mfspr(SPRN_SIAR) + perf_ip_adjust(regs);
> }
>
^ permalink raw reply
* Re: kernel panic during kernel module load (powerpc specific part)
From: Benjamin Herrenschmidt @ 2012-06-05 22:47 UTC (permalink / raw)
To: Gabriel Paubert
Cc: Wrobel Heinz-R39252, Michael Ellerman, Steffen Rumler,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20120605104449.GA32032@visitor2.iram.es>
On Tue, 2012-06-05 at 12:44 +0200, Gabriel Paubert wrote:
> On Tue, Jun 05, 2012 at 08:00:42AM +1000, Benjamin Herrenschmidt wrote:
> > On Mon, 2012-06-04 at 13:03 +0200, Gabriel Paubert wrote:
> > > There is no conflict to the ABI. These functions are supposed to be
> > > directly reachable from whatever code
> > > section may need them.
> > >
> > > Now I have a question: how did you get the need for this?
> > >
> > > None of my kernels uses them:
> > > - if I compile with -O2, the compiler simply expands epilogue and
> > > prologue to series of lwz and stw
> > > - if I compile with -Os, the compiler generates lmw/stmw which give
> > > the smallest possible cache footprint
> > >
> > > Neither did I find a single reference to these functions in several
> > > systems that I grepped for.
> >
> > Newer gcc's ... even worse, with -Os, it does it for a single register
> > spill afaik. At least that's how I hit it with grub2 using whatever gcc
> > is in fc17.
>
> Ok, it's beyond stupid, and at this point must be considered a gcc bug.
Probably, I haven't had a chance to report it...
It would make more sense if the out of line functions also handled
creating the stack frame & disposing of it (ie, maybe a tail call for
return) but they don't even do that so yes it's quite gross.
Cheers,
Ben.
^ permalink raw reply
* RE: [PATCH] powerpc: Fix assmption of end_of_DRAM() returns end address
From: Bhushan Bharat-R65777 @ 2012-06-06 0:46 UTC (permalink / raw)
To: David Miller, benh@kernel.crashing.org, Andrea Arcangeli
Cc: linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20120605.152058.828742127223799137.davem@davemloft.net>
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, June 06, 2012 3:51 AM
> To: benh@kernel.crashing.org
> Cc: Bhushan Bharat-R65777; linuxppc-dev@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; galak@kernel.crashing.org; Bhushan Bharat-R65777
> Subject: Re: [PATCH] powerpc: Fix assmption of end_of_DRAM() returns end =
address
>=20
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Wed, 06 Jun 2012 08:17:39 +1000
>=20
> > On Tue, 2012-06-05 at 19:25 +0530, Bharat Bhushan wrote:
> >> memblock_end_of_DRAM() returns end_address + 1, not end address.
> >> While some code assumes that it returns end address.
> >
> > Shouldn't we instead fix it the other way around ? IE, make
> > memblock_end_of_DRAM() does what the name implies, which is to return
> > the last byte of DRAM, and fix the -other- callers not to make bad
> > assumptions ?
>=20
> That was my impression too when I saw this patch.
Initially I also intended to do so. I initiated a email on linux-mm@ subje=
ct "memblock_end_of_DRAM() return end address + 1" and the only response I=
received from Andrea was:
"
It's normal that "end" means "first byte offset out of the range". End =3D =
not ok.
end =3D start+size.
This is true for vm_end too. So it's better to keep it that way.
My suggestion is to just fix point 1 below and audit the rest :)
"
Thanks
-Bharat
^ permalink raw reply
* Re: Access to http://ozlabs.org/people/dgibson/dldwd/
From: David Gibson @ 2012-06-06 3:13 UTC (permalink / raw)
To: R.P. Burrasca; +Cc: linuxppc-dev
In-Reply-To: <SNT125-DS202B61C3B77896C6E6D393B00C0@phx.gbl>
On Tue, Jun 05, 2012 at 07:27:44AM -0600, R.P. Burrasca wrote:
>
> Would it be possible to access the above referenced webpage in order to see
> what's available as the Orinoco & Prism 2 Wireless Driver?
Uh, that site hasn't existed for 5 years or more. The latest versions
of the orinoco driver are in the mainline kernel tree.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
From: Ben Collins @ 2012-06-06 3:50 UTC (permalink / raw)
To: linuxppc-dev
The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
64-bit sign extention, which is the case on ppc32 with 64-bit resource
addresses. This only seems to have shown up while running under QEMU for
e500mc target. It may or may be suboptimal that QEMU has an IO base
address > 32-bits for the e500-pci implementation, but 1) it's still a
regression and 2) it's more correct to handle things this way.
Signed-off-by: Ben Collins <bcollins@ubuntu.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/kernel/pci-common.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/pci-common.c =
b/arch/powerpc/kernel/pci-common.c
index 8e78e93..be9ced7 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, =
int mask)
return pci_enable_resources(dev, mask);
}
=20
+/* Before assuming too much here, take care to realize that we need =
sign
+ * extension from 32-bit pointers to 64-bit resource addresses to work.
+ */
resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
{
- return (unsigned long) hose->io_base_virt - _IO_BASE;
+ long vbase =3D (long)hose->io_base_virt;
+ long io_base =3D _IO_BASE;
+
+ return (resource_size_t)(vbase - io_base);
}
=20
static void __devinit pcibios_setup_phb_resources(struct pci_controller =
*hose, struct list_head *resources)
--=20
1.7.9.5
--
Ben Collins
Servergy, Inc.
(757) 243-7557
CONFIDENTIALITY NOTICE: This communication contains privileged and/or =
confidential information; and should be maintained with the strictest =
confidence. It is intended solely for the use of the person or entity in =
which it is addressed. If you are not the intended recipient, you are =
STRICTLY PROHIBITED from disclosing, copying, distributing or using any =
of this information. If you received this communication in error, please =
contact the sender immediately and destroy the material in its entirety, =
whether electronic or hard copy.
^ permalink raw reply related
* Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source
From: Li Yang @ 2012-06-06 4:06 UTC (permalink / raw)
To: Scott Wood
Cc: Wood Scott-B07421, Li Yang-R58472, Zhao Chenhui-B35336,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4FCE4A5A.3020603@freescale.com>
On Wed, Jun 6, 2012 at 2:05 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 06/05/2012 11:49 AM, Li Yang-R58472 wrote:
>>
>>
>>>>> On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
>>>>>> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
>>>>>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>>>>>>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool
>>>>>>>> +enable)
>>>>>>>
>>>>>>> Why does it have to be a platform_device? =C2=A0Would a bare device=
_node
>>>>> work
>>>>>>> here? =C2=A0If it's for stuff like device_may_wakeup() that could b=
e in
>>>>>>> a platform_device wrapper function.
>>>>>>
>>>>>> It does not have to be a platform_device. I think it can be a struct
>>>>> device.
>>>>>
>>>>> Why does it even need that? =C2=A0The low level mechanism for influen=
cing
>>>>> PMCDR should only need a device node, not a Linux device struct.
>>>>
>>>> It does no harm to pass the device structure and makes more sense for
>>> object oriented interface design.
>>>
>>> It does do harm if you don't have a device structure to pass, if for so=
me
>>> reason you found the device by directly looking for it rather than goin=
g
>>> through the device model.
>>
>> Whether or not a device is a wakeup source not only depends on the
>> SoC specification but also the configuration and current state for
>> the device. =C2=A0I only expect the device driver to have this knowledge
>> and call this function rather than some standalone platform code.
>> Therefore I don't think your concern matters.
>
> First, I think it's bad API to force the passing of a higher level
> object when a lower level object would suffice (and there are no
> legitimate future-proofing or abstraction reasons for hiding the lower
> level object).
>
> But regardless, the entity you call a "device driver" may or may not use
> the standard driver model (e.g. look at PCI root complexes), and your
> assumption about what platform code knows may or may not be correct. =C2=
=A0I
> could just as well say that only platform code knows about the SoC
> clock/power routing during a given low power state.
Good point. We need to fix such non-standard drivers. The new PM
framework depends a lot on the standard Linux Driver Model. We need
to change our drivers to make them work better with PM. Also we have
already submitted a patch series to change the PCI root complex driver
in that regard.
>
>>>>>>> Who is setting can_wakeup for these devices?
>>>>>>
>>>>>> The device driver is responsible to set can_wakeup.
>>>>>
>>>>> How would the device driver know how to set it? =C2=A0Wouldn't this d=
epend
>>>>> on the particular SoC and low power mode?
>>>>
>>>> It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device
>>> tree properties.
>>>
>>> fsl,magic-packet was a mistake. =C2=A0It is equivalent to checking the
>>> compatible for etsec. =C2=A0It does not convey any information about wh=
ether
>>
>> It can be described either by explicit feature property or by the
>> compatible. =C2=A0I don't think it is a problem that we choose one again=
st
>> another.
>
> I do think it's a problem, because it's unnecessarily complicated, and
> more error prone (we probably didn't have fsl,magic-packet on all the
> SoCs that support it before the .dtsi refactoring). =C2=A0But my point wa=
s
> that it says nothing about whether the eTSEC will still be functioning
> during a low power state.
>
>>> the eTSEC is still active in a given low power mode.
>>
>> Whether or not the eTSEC is still active in both sleep and deep sleep
>> is only depending on if we set it to be a wakeup source.
>
> Only because we happen to support eTSEC as a wakeup source on all SoCs.
>
>> If it behaves differently for low power modes in the future, we could
>> address that by adding additional property.
>>
>>>
>>> How is fsl,wake-os-filer relevant to this decision? =C2=A0When will it =
be set
>>> but not fsl,magic-packet?
>>
>> I mean either fsl,magic-packet or fsl,wake-on-filer shows it can be a
>> wakeup source. =C2=A0Currently we don't have an SoC to have wake-on-file=
r
>> while not wake-on-magic. =C2=A0But I think it's better to consider both =
as
>> they are independent features.
>
> You're not willing to consider an SoC where waking on eTSEC is
> unsupported, but you're willing to consider an eTSEC that has
> wake-on-filer but magic packet support has for some reason been dropped?
Good findings. :) I think it's fine to keep the extra that have
already been done as long as it does no harm. But we should stop
adding more extras that are not necessary for now.
>
>>> What about devices other than ethernet? =C2=A0What about differences be=
tween
>>> ordinary sleep and deep sleep?
>>
>> There is no difference for sleep and deep sleep for all wakeup sources c=
urrently.
>
> I recall being able to wake an mpc85xx chip (maybe mpc8544?) from
> ordinary sleep using the DUART (even though the manual suggests that
> only external interrupts can be used -- not even eTSEC). =C2=A0You can't =
do
> that in deep sleep.
I doubt that as the blocks are clock gated in sleep. We only have
MPC8536 and P1022 in the e500 family to support deep sleep now. I
agree with you the sleep and deep sleep can imply different wakeup
source in the future. But can we worry about it later?
>
> You ignored "what about devices other than ethernet".
No, I haven't. Other devices are so at least for now.
- Leo
^ permalink raw reply
* Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
From: Benjamin Herrenschmidt @ 2012-06-06 5:15 UTC (permalink / raw)
To: Ben Collins; +Cc: Bjorn Helgaas, Paul Mackerras, linuxppc-dev
In-Reply-To: <4DC27253-67FC-4A55-8C78-7782D9D0CF53@servergy.com>
On Tue, 2012-06-05 at 23:50 -0400, Ben Collins wrote:
> The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
> 64-bit sign extention, which is the case on ppc32 with 64-bit resource
> addresses. This only seems to have shown up while running under QEMU for
> e500mc target. It may or may be suboptimal that QEMU has an IO base
> address > 32-bits for the e500-pci implementation, but 1) it's still a
> regression and 2) it's more correct to handle things this way.
See Bjorn, we both did end up getting it wrong :-)
> Signed-off-by: Ben Collins <bcollins@ubuntu.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> arch/powerpc/kernel/pci-common.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 8e78e93..be9ced7 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> return pci_enable_resources(dev, mask);
> }
>
> +/* Before assuming too much here, take care to realize that we need sign
> + * extension from 32-bit pointers to 64-bit resource addresses to work.
> + */
> resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
> {
> - return (unsigned long) hose->io_base_virt - _IO_BASE;
> + long vbase = (long)hose->io_base_virt;
> + long io_base = _IO_BASE;
> +
> + return (resource_size_t)(vbase - io_base);
> }
>
> static void __devinit pcibios_setup_phb_resources(struct pci_controller *hose, struct list_head *resources)
^ permalink raw reply
* Re: [PATCH v2 0/2] Add pcibios_device_change_notifier
From: Benjamin Herrenschmidt @ 2012-06-06 5:26 UTC (permalink / raw)
To: Hiroo Matsumoto
Cc: Bjorn Helgaas, linux-pci, linuxppc-dev, jbarnes, Kenji Kaneshige
In-Reply-To: <4FBC4C86.9050206@jp.fujitsu.com>
On Wed, 2012-05-23 at 11:33 +0900, Hiroo Matsumoto wrote:
> This patchset is for PCI hotplug.
>
>
> pcibios_setup_bus_devices which sets DMA and IRQs of PCI device is called
> only when boot. DMA setting in probe for PCI driver, like dma_set_mask,
> does not work on powerpc platform. So it is need to set DMA and IRQs of
> PCI device when hotplug.
>
> 1. Moving pcibios_setup_bus_devices code to pcibios_device_change_notifier
> which is registered to bus notifier in pcibios_init.
> 2. Removing caller and callee of pcibios_setup_bus_devices bus notifier
> works instead of pcibios_setup_bus_devices.
> 3. Using this bus notifier for microblaze because microblaze/PCI is similer
> with powerpc/PCI.
This makes me a bit nervous (that doesn't mean it's not right, but
we need some careful auditing & testing here, which I won't be
able to do until I'm back from leave). Mostly due to the change in when
we do the work.
pcibios_fixup_bus() used to be called early on in the initial scan pass.
Your code causes the code to be called -much- later when registering the
device with the device model. Are we 100% certain nothing will happen in
between that might rely on the stuff being setup already ? It might well
be ok, but I want us to triple check that.
Now, if we are ok to do the setup that late (basically right before the
driver probe() routine gets called), would it make sense to simplify
things even further ... and do it from pcibios_enable_device() ? Thus
avoiding the notifier business completely or is that pushing it too
far ?
Also you seem to add:
+ /* Setup OF node pointer in the device */
+ dev->dev.of_node = pci_device_to_OF_node(dev);
This shouldn't be needed anymore, the device node should be setup by the
core nowadays. Is this just a remnant of you rebasing an old patch or do
you have a good reason to add this statement ?
Cheers,
Ben.
^ permalink raw reply
* RE: [PATCH] powerpc: Fix assmption of end_of_DRAM() returns end address
From: Benjamin Herrenschmidt @ 2012-06-06 5:30 UTC (permalink / raw)
To: Bhushan Bharat-R65777
Cc: Andrea Arcangeli, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D03D68F08@039-SN2MPN1-022.039d.mgd.msft.net>
On Wed, 2012-06-06 at 00:46 +0000, Bhushan Bharat-R65777 wrote:
> > >> memblock_end_of_DRAM() returns end_address + 1, not end address.
> > >> While some code assumes that it returns end address.
> > >
> > > Shouldn't we instead fix it the other way around ? IE, make
> > > memblock_end_of_DRAM() does what the name implies, which is to
> return
> > > the last byte of DRAM, and fix the -other- callers not to make bad
> > > assumptions ?
> >
> > That was my impression too when I saw this patch.
>
> Initially I also intended to do so. I initiated a email on linux-mm@
> subject "memblock_end_of_DRAM() return end address + 1" and the only
> response I received from Andrea was:
>
> "
> It's normal that "end" means "first byte offset out of the range". End
> = not ok.
> end = start+size.
> This is true for vm_end too. So it's better to keep it that way.
> My suggestion is to just fix point 1 below and audit the rest :)
> "
Oh well, I don't care enough to fight this battle in my current state so
unless Dave has more stamina than I have today, I'm ok with the patch.
Cheers,
Ben.
^ permalink raw reply
* [RFC PATCH] sched/numa: do load balance between remote nodes
From: Alex Shi @ 2012-06-06 6:52 UTC (permalink / raw)
To: a.p.zijlstra
Cc: linux-mips, linux-ia64, linux-sh, dhowells, paulus, hpa,
sparclinux, mingo, sivanich, x86, greg.pearson, chris.mason,
arjan.van.de.ven, mattst88, pjt, fenghua.yu, seto.hidetoshi,
cmetcalf, ak, ink, anton, tglx, kamezawa.hiroyu, rth, tony.luck,
torvalds, linux-kernel, ralf, lethal, linux-alpha, bob.picco,
akpm, linuxppc-dev, davem
commit cb83b629b remove the NODE sched domain and check if the node
distance in SLIT table is farther than REMOTE_DISTANCE, if so, it will
lose the load balance chance at exec/fork/wake_affine points.
But actually, even the node distance is farther than REMOTE_DISTANCE,
Modern CPUs also has QPI like connections, that make memory access is
not too slow between nodes. So above losing on NUMA machine make a
huge performance regression on benchmark: hackbench, tbench, netperf
and oltp etc.
This patch will recover the scheduler behavior to old mode on all my
Intel platforms: NHM EP/EX, WSM EP, SNB EP/EP4S, and so remove the
perfromance regressions. (all of them just has 2 kinds distance, 10 21)
Signed-off-by: Alex Shi <alex.shi@intel.com>
---
kernel/sched/core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 39eb601..b2ee41a 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6286,7 +6286,7 @@ static int sched_domains_curr_level;
static inline int sd_local_flags(int level)
{
- if (sched_domains_numa_distance[level] > REMOTE_DISTANCE)
+ if (sched_domains_numa_distance[level] > RECLAIM_DISTANCE)
return 0;
return SD_BALANCE_EXEC | SD_BALANCE_FORK | SD_WAKE_AFFINE;
--
1.7.5.4
^ permalink raw reply related
* Re: kernel panic during kernel module load (powerpc specific part)
From: Steffen Rumler @ 2012-06-06 7:36 UTC (permalink / raw)
To: ext Gabriel Paubert
Cc: Wrobel Heinz-R39252, Michael Ellerman,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20120604110305.GA16707@visitor2.iram.es>
> On Fri, Jun 01, 2012 at 11:33:37AM +0000, Wrobel Heinz-R39252 wrote:
>>>> I believe that the basic premise is that you should provide a directly
>>>> reachable copy of the save/rstore functions, even if this means that
>>> you need several copies of the functions.
>>>
>>> I just fixed a very similar problem with grub2 in fact. It was using r0
>>> and trashing the saved LR that way.
>>>
>>> The real fix is indeed to statically link those gcc "helpers", we
>>> shouldn't generate things like cross-module calls inside function prologs
>>> and epilogues, when stackframes aren't even guaranteed to be reliable.
>>>
>>> However, in the grub2 case, it was easier to just use r12 :-)
>> For not just the module loading case, I believe r12 is the only real solution now. I checked one debugger capable of doing ELF download. It also uses r12 for trampoline code. I am guessing for the reason that prompted this discussion.
>>
> I disagree. Look carefully at Be's answer: cross-module calls
> are intrinsically dangerous when stack frames are in a transient
> state.
>
>> Without r12 we'd have to change standard libraries to automagically link in gcc helpers for any conceivable non-.text section, which I am not sure is feasible. How would you write section independent helper functions which link to any section needing them?!
> I don't thnk that it is tha bad: the helpers should be linked to the default .text section
> when needed, typically the init code and so on are mapped within the reach of that
> section (otherwise you'll end up with the linker complaining that it finds overflowing
> branch offsets between .text and .init.text).
>
>> Asking users to create their own section specific copy of helper functions is definitely not portable if the module or other code is not architecture dependent.
> Well, it automagically works on 64 bit. There is is performed by magic built into the linker.
>
>> It is a normal gcc feature that you can assign specific code to non-.text sections and it is not documented that it may crash depending on the OS arch the ELF is built for, so asking for a Power Architecture specific change on tool libs to make Power Architecture Linux happy seems a bit much to ask.
>>
> Once again I disagree.
>
>> Using r12 in any Linux related trampoline code seems a reachable goal, and it would eliminate the conflict to the ABI.
>>
> There is no conflict to the ABI. These functions are supposed to be directly reachable from whatever code
> section may need them.
>
> Now I have a question: how did you get the need for this?
>
> None of my kernels uses them:
> - if I compile with -O2, the compiler simply expands epilogue and prologue to series of lwz and stw
> - if I compile with -Os, the compiler generates lmw/stmw which give the smallest possible cache footprint
>
> Neither did I find a single reference to these functions in several systems that I grepped for.
>
> Regards,
> Gabriel
>
Hi,
how should we continue here ?
There is the kernel panic, I've described.
Technically, there is an conflict between the code generated by the compiler and the loader in module_32.c, at least by using -Os.
Because the prologue/epilogue is part of the .text and init_module() is part of .init.text (in the case __init is applied, as usual),
a directly reachable call is not always possible.
Thanks
Steffen
^ permalink raw reply
* Re: [RFC PATCH] sched/numa: do load balance between remote nodes
From: Peter Zijlstra @ 2012-06-06 9:01 UTC (permalink / raw)
To: Alex Shi
Cc: linux-mips, linux-ia64, linux-sh, dhowells, paulus, hpa,
sparclinux, mingo, sivanich, x86, greg.pearson, chris.mason,
arjan.van.de.ven, mattst88, pjt, fenghua.yu, seto.hidetoshi,
cmetcalf, ak, ink, anton, tglx, kamezawa.hiroyu, rth, tony.luck,
torvalds, linux-kernel, ralf, lethal, linux-alpha, bob.picco,
akpm, linuxppc-dev, davem
In-Reply-To: <1338965571-9812-1-git-send-email-alex.shi@intel.com>
On Wed, 2012-06-06 at 14:52 +0800, Alex Shi wrote:
> - if (sched_domains_numa_distance[level] > REMOTE_DISTANCE)
> + if (sched_domains_numa_distance[level] > RECLAIM_DISTANCE)=20
I actually considered this.. I just felt a little uneasy re-purposing
the RECLAIM_DISTANCE for this, but I guess its all the same anyway. Both
mean expensive-away-distance.
So I've taken this.
thanks!
^ permalink raw reply
* [PATCH 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()
From: Ananth N Mavinakayanahalli @ 2012-06-06 9:19 UTC (permalink / raw)
To: linuxppc-dev, lkml
Cc: Srikar Dronamraju, peterz, oleg, Paul Mackerras, Anton Blanchard,
Ingo Molnar
From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
On RISC architectures like powerpc, instructions are fixed size.
Instruction analysis on such platforms is just a matter of (insn % 4).
Pass the vaddr at which the uprobe is to be inserted so that
arch_uprobe_analyze_insn() can flag misaligned registration requests.
Signed-off-by: Ananth N Mavinakaynahalli <ananth@in.ibm.com>
---
arch/x86/include/asm/uprobes.h | 2 +-
arch/x86/kernel/uprobes.c | 3 ++-
kernel/events/uprobes.c | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
Index: uprobes-24may/arch/x86/include/asm/uprobes.h
===================================================================
--- uprobes-24may.orig/arch/x86/include/asm/uprobes.h
+++ uprobes-24may/arch/x86/include/asm/uprobes.h
@@ -48,7 +48,7 @@ struct arch_uprobe_task {
#endif
};
-extern int arch_uprobe_analyze_insn(struct arch_uprobe *aup, struct mm_struct *mm);
+extern int arch_uprobe_analyze_insn(struct arch_uprobe *aup, struct mm_struct *mm, loff_t vaddr);
extern int arch_uprobe_pre_xol(struct arch_uprobe *aup, struct pt_regs *regs);
extern int arch_uprobe_post_xol(struct arch_uprobe *aup, struct pt_regs *regs);
extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
Index: uprobes-24may/arch/x86/kernel/uprobes.c
===================================================================
--- uprobes-24may.orig/arch/x86/kernel/uprobes.c
+++ uprobes-24may/arch/x86/kernel/uprobes.c
@@ -409,9 +409,10 @@ static int validate_insn_bits(struct arc
* arch_uprobe_analyze_insn - instruction analysis including validity and fixups.
* @mm: the probed address space.
* @arch_uprobe: the probepoint information.
+ * @vaddr: virtual address at which to install the probepoint
* Return 0 on success or a -ve number on error.
*/
-int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm)
+int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, loff_t vaddr)
{
int ret;
struct insn insn;
Index: uprobes-24may/kernel/events/uprobes.c
===================================================================
--- uprobes-24may.orig/kernel/events/uprobes.c
+++ uprobes-24may/kernel/events/uprobes.c
@@ -697,7 +697,7 @@ install_breakpoint(struct uprobe *uprobe
if (is_swbp_insn((uprobe_opcode_t *)uprobe->arch.insn))
return -EEXIST;
- ret = arch_uprobe_analyze_insn(&uprobe->arch, mm);
+ ret = arch_uprobe_analyze_insn(&uprobe->arch, mm, vaddr);
if (ret)
return ret;
^ permalink raw reply
* [PATCH 2/2] [POWERPC] uprobes: powerpc port
From: Ananth N Mavinakayanahalli @ 2012-06-06 9:21 UTC (permalink / raw)
To: linuxppc-dev, lkml
Cc: Srikar Dronamraju, peterz, oleg, Paul Mackerras, Anton Blanchard,
Ingo Molnar
In-Reply-To: <20120606091950.GB6745@in.ibm.com>
From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
This is the port of uprobes to powerpc. Usage is similar to x86.
One TODO in this port compared to x86 is the uprobe abort_xol() logic.
x86 depends on the thread_struct.trap_nr (absent in powerpc) to determine
if a signal was caused when the uprobed instruction was single-stepped/
emulated, in which case, we reset the instruction pointer to the probed
address and retry the probe again.
[root@xxxx ~]# ./bin/perf probe -x /lib64/libc.so.6 malloc
Added new event:
probe_libc:malloc (on 0xb4860)
You can now use it in all perf tools, such as:
perf record -e probe_libc:malloc -aR sleep 1
[root@xxxx ~]# ./bin/perf record -e probe_libc:malloc -aR sleep 20
[ perf record: Woken up 22 times to write data ]
[ perf record: Captured and wrote 5.843 MB perf.data (~255302 samples) ]
[root@xxxx ~]# ./bin/perf report --stdio
# ========
# captured on: Mon Jun 4 05:26:31 2012
# hostname : xxxx.ibm.com
# os release : 3.4.0-uprobe
# perf version : 3.4.0
# arch : ppc64
# nrcpus online : 4
# nrcpus avail : 4
# cpudesc : POWER6 (raw), altivec supported
# cpuid : 62,769
# total memory : 7310528 kB
# cmdline : /root/bin/perf record -e probe_libc:malloc -aR sleep 20
# event : name = probe_libc:malloc, type = 2, config = 0x124, config1 = 0x0, con
# HEADER_CPU_TOPOLOGY info available, use -I to display
# HEADER_NUMA_TOPOLOGY info available, use -I to display
# ========
#
# Samples: 83K of event 'probe_libc:malloc'
# Event count (approx.): 83484
#
# Overhead Command Shared Object Symbol
# ........ ............ ............. ..........
#
69.05% tar libc-2.12.so [.] malloc
28.57% rm libc-2.12.so [.] malloc
1.32% avahi-daemon libc-2.12.so [.] malloc
0.58% bash libc-2.12.so [.] malloc
0.28% sshd libc-2.12.so [.] malloc
0.08% irqbalance libc-2.12.so [.] malloc
0.05% bzip2 libc-2.12.so [.] malloc
0.04% sleep libc-2.12.so [.] malloc
0.03% multipathd libc-2.12.so [.] malloc
0.01% sendmail libc-2.12.so [.] malloc
0.01% automount libc-2.12.so [.] malloc
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Index: linux-3.5-rc1/arch/powerpc/include/asm/thread_info.h
===================================================================
--- linux-3.5-rc1.orig/arch/powerpc/include/asm/thread_info.h 2012-06-03 06:59:26.000000000 +0530
+++ linux-3.5-rc1/arch/powerpc/include/asm/thread_info.h 2012-06-03 21:05:48.226233001 +0530
@@ -96,6 +96,7 @@
#define TIF_RESTOREALL 11 /* Restore all regs (implies NOERROR) */
#define TIF_NOERROR 12 /* Force successful syscall return */
#define TIF_NOTIFY_RESUME 13 /* callback before returning to user */
+#define TIF_UPROBE 14 /* breakpointed or single-stepping */
#define TIF_SYSCALL_TRACEPOINT 15 /* syscall tracepoint instrumentation */
/* as above, but as bit values */
@@ -112,12 +113,13 @@
#define _TIF_RESTOREALL (1<<TIF_RESTOREALL)
#define _TIF_NOERROR (1<<TIF_NOERROR)
#define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME)
+#define _TIF_UPROBE (1<<TIF_UPROBE)
#define _TIF_SYSCALL_TRACEPOINT (1<<TIF_SYSCALL_TRACEPOINT)
#define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
_TIF_SECCOMP | _TIF_SYSCALL_TRACEPOINT)
#define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
- _TIF_NOTIFY_RESUME)
+ _TIF_NOTIFY_RESUME | _TIF_UPROBE)
#define _TIF_PERSYSCALL_MASK (_TIF_RESTOREALL|_TIF_NOERROR)
/* Bits in local_flags */
Index: linux-3.5-rc1/arch/powerpc/include/asm/uprobes.h
===================================================================
--- linux-3.5-rc1.orig/arch/powerpc/include/asm/uprobes.h 2012-05-31 02:23:15.746233001 +0530
+++ linux-3.5-rc1/arch/powerpc/include/asm/uprobes.h 2012-06-03 21:05:48.226233001 +0530
@@ -0,0 +1,49 @@
+#ifndef _ASM_UPROBES_H
+#define _ASM_UPROBES_H
+/*
+ * User-space Probes (UProbes) for powerpc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) IBM Corporation, 2007-2012
+ *
+ * Adapted from the x86 port by Ananth N Mavinakayanahalli <ananth@in.ibm.com>
+ */
+
+#include <linux/notifier.h>
+
+typedef unsigned int uprobe_opcode_t;
+
+#define MAX_UINSN_BYTES 4
+#define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES)
+
+#define UPROBE_SWBP_INSN 0x7fe00008
+#define UPROBE_SWBP_INSN_SIZE 4 /* swbp insn size in bytes */
+
+struct arch_uprobe {
+ u8 insn[MAX_UINSN_BYTES];
+};
+
+struct arch_uprobe_task {
+};
+
+extern int arch_uprobe_analyze_insn(struct arch_uprobe *aup, struct mm_struct *mm, loff_t vaddr);
+extern unsigned long uprobe_get_swbp_addr(struct pt_regs *regs);
+extern int arch_uprobe_pre_xol(struct arch_uprobe *aup, struct pt_regs *regs);
+extern int arch_uprobe_post_xol(struct arch_uprobe *aup, struct pt_regs *regs);
+extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
+extern int arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
+extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
+#endif /* _ASM_UPROBES_H */
Index: linux-3.5-rc1/arch/powerpc/kernel/Makefile
===================================================================
--- linux-3.5-rc1.orig/arch/powerpc/kernel/Makefile 2012-06-03 06:59:26.000000000 +0530
+++ linux-3.5-rc1/arch/powerpc/kernel/Makefile 2012-06-03 21:05:48.226233001 +0530
@@ -96,6 +96,7 @@
obj-$(CONFIG_BOOTX_TEXT) += btext.o
obj-$(CONFIG_SMP) += smp.o
obj-$(CONFIG_KPROBES) += kprobes.o
+obj-$(CONFIG_UPROBES) += uprobes.o
obj-$(CONFIG_PPC_UDBG_16550) += legacy_serial.o udbg_16550.o
obj-$(CONFIG_STACKTRACE) += stacktrace.o
obj-$(CONFIG_SWIOTLB) += dma-swiotlb.o
Index: linux-3.5-rc1/arch/powerpc/kernel/signal.c
===================================================================
--- linux-3.5-rc1.orig/arch/powerpc/kernel/signal.c 2012-06-03 06:59:26.000000000 +0530
+++ linux-3.5-rc1/arch/powerpc/kernel/signal.c 2012-06-03 21:05:48.236233000 +0530
@@ -11,6 +11,7 @@
#include <linux/tracehook.h>
#include <linux/signal.h>
+#include <linux/uprobes.h>
#include <linux/key.h>
#include <asm/hw_breakpoint.h>
#include <asm/uaccess.h>
@@ -157,6 +158,11 @@
void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags)
{
+ if (thread_info_flags & _TIF_UPROBE) {
+ clear_thread_flag(TIF_UPROBE);
+ uprobe_notify_resume(regs);
+ }
+
if (thread_info_flags & _TIF_SIGPENDING)
do_signal(regs);
Index: linux-3.5-rc1/arch/powerpc/kernel/uprobes.c
===================================================================
--- linux-3.5-rc1.orig/arch/powerpc/kernel/uprobes.c 2012-05-31 02:23:15.746233001 +0530
+++ linux-3.5-rc1/arch/powerpc/kernel/uprobes.c 2012-06-03 21:05:48.236233000 +0530
@@ -0,0 +1,163 @@
+/*
+ * User-space Probes (UProbes) for powerpc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) IBM Corporation, 2007-2012
+ *
+ * Adapted from the x86 port by Ananth N Mavinakayanahalli <ananth@in.ibm.com>
+ */
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/ptrace.h>
+#include <linux/uprobes.h>
+#include <linux/uaccess.h>
+
+#include <linux/kdebug.h>
+#include <asm/sstep.h>
+
+/**
+ * arch_uprobe_analyze_insn
+ * @mm: the probed address space.
+ * @arch_uprobe: the probepoint information.
+ * Return 0 on success or a -ve number on error.
+ */
+int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, loff_t vaddr)
+{
+ if (vaddr & 0x03)
+ return -EINVAL;
+ return 0;
+}
+
+/*
+ * arch_uprobe_pre_xol - prepare to execute out of line.
+ * @auprobe: the probepoint information.
+ * @regs: reflects the saved user state of current task.
+ */
+int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
+{
+ /* FIXME: We don't support abort_xol on powerpc for now */
+ regs->nip = current->utask->xol_vaddr;
+ return 0;
+}
+
+/**
+ * uprobe_get_swbp_addr - compute address of swbp given post-swbp regs
+ * @regs: Reflects the saved state of the task after it has hit a breakpoint
+ * instruction.
+ * Return the address of the breakpoint instruction.
+ */
+unsigned long uprobe_get_swbp_addr(struct pt_regs *regs)
+{
+ return instruction_pointer(regs);
+}
+
+/*
+ * If xol insn itself traps and generates a signal (SIGILL/SIGSEGV/etc),
+ * then detect the case where a singlestepped instruction jumps back to its
+ * own address. It is assumed that anything like do_page_fault/do_trap/etc
+ * sets thread.trap_nr != -1.
+ *
+ * FIXME: powerpc however doesn't have thread.trap_nr yet.
+ *
+ * arch_uprobe_pre_xol/arch_uprobe_post_xol save/restore thread.trap_nr,
+ * arch_uprobe_xol_was_trapped() simply checks that ->trap_nr is not equal to
+ * UPROBE_TRAP_NR == -1 set by arch_uprobe_pre_xol().
+ */
+bool arch_uprobe_xol_was_trapped(struct task_struct *t)
+{
+ /* FIXME: We don't support abort_xol on powerpc for now */
+ return false;
+}
+
+/*
+ * Called after single-stepping. To avoid the SMP problems that can
+ * occur when we temporarily put back the original opcode to
+ * single-step, we single-stepped a copy of the instruction.
+ *
+ * This function prepares to resume execution after the single-step.
+ */
+int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
+{
+ /* FIXME: We don't support abort_xol on powerpc for now */
+
+ /*
+ * On powerpc, except for loads and stores, most instructions
+ * including ones that alter code flow (branches, calls, returns)
+ * are emulated in the kernel. We get here only if the emulation
+ * support doesn't exist and have to fix-up the next instruction
+ * to be executed.
+ */
+ regs->nip = current->utask->vaddr + MAX_UINSN_BYTES;
+ return 0;
+}
+
+/* callback routine for handling exceptions. */
+int arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data)
+{
+ struct die_args *args = data;
+ struct pt_regs *regs = args->regs;
+ int ret = NOTIFY_DONE;
+
+ /* We are only interested in userspace traps */
+ if (regs && !user_mode(regs))
+ return NOTIFY_DONE;
+
+ switch (val) {
+ case DIE_BPT:
+ if (uprobe_pre_sstep_notifier(regs))
+ ret = NOTIFY_STOP;
+ break;
+ case DIE_SSTEP:
+ if (uprobe_post_sstep_notifier(regs))
+ ret = NOTIFY_STOP;
+ default:
+ break;
+ }
+ return ret;
+}
+
+/*
+ * This function gets called when XOL instruction either gets trapped or
+ * the thread has a fatal signal, so reset the instruction pointer to its
+ * probed address.
+ */
+void arch_uprobe_abort_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
+{
+ /* FIXME: We don't support abort_xol on powerpc for now */
+ return;
+}
+
+/*
+ * See if the instruction can be emulated.
+ * Returns true if instruction was emulated, false otherwise.
+ */
+bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
+{
+ int ret;
+ unsigned int insn;
+
+ memcpy(&insn, auprobe->insn, MAX_UINSN_BYTES);
+
+ /*
+ * emulate_step() returns 1 if the insn was successfully emulated.
+ * For all other cases, we need to single-step in hardware.
+ */
+ ret = emulate_step(regs, insn);
+ if (ret > 0)
+ return true;
+
+ return false;
+}
Index: linux-3.5-rc1/arch/powerpc/Kconfig
===================================================================
--- linux-3.5-rc1.orig/arch/powerpc/Kconfig 2012-06-03 06:59:26.000000000 +0530
+++ linux-3.5-rc1/arch/powerpc/Kconfig 2012-06-03 21:05:48.246233000 +0530
@@ -237,6 +237,9 @@
config ARCH_SUPPORTS_DEBUG_PAGEALLOC
def_bool y
+config ARCH_SUPPORTS_UPROBES
+ def_bool y
+
config PPC_ADV_DEBUG_REGS
bool
depends on 40x || BOOKE
^ permalink raw reply
* Re: [PATCH 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()
From: Peter Zijlstra @ 2012-06-06 9:23 UTC (permalink / raw)
To: ananth
Cc: Srikar Dronamraju, lkml, oleg, Paul Mackerras, Anton Blanchard,
Ingo Molnar, linuxppc-dev
In-Reply-To: <20120606091950.GB6745@in.ibm.com>
On Wed, 2012-06-06 at 14:49 +0530, Ananth N Mavinakayanahalli wrote:
> +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_stru=
ct *mm, loff_t vaddr)
Don't we traditionally use unsigned long to pass vaddrs?
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] uprobes: powerpc port
From: Peter Zijlstra @ 2012-06-06 9:27 UTC (permalink / raw)
To: ananth
Cc: Srikar Dronamraju, lkml, oleg, Paul Mackerras, Anton Blanchard,
Ingo Molnar, linuxppc-dev
In-Reply-To: <20120606092150.GC6745@in.ibm.com>
On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
> One TODO in this port compared to x86 is the uprobe abort_xol() logic.
> x86 depends on the thread_struct.trap_nr (absent in powerpc) to determine
> if a signal was caused when the uprobed instruction was single-stepped/
> emulated, in which case, we reset the instruction pointer to the probed
> address and retry the probe again.=20
Another curious difference is that x86 uses an instruction decoder and
contains massive tables to validate we can probe a particular
instruction.
Can we probe all possible PPC instructions?
^ permalink raw reply
* Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync
From: Zhao Chenhui @ 2012-06-06 9:31 UTC (permalink / raw)
To: Scott Wood; +Cc: Matthew McClintock, linuxppc-dev, linux-kernel
In-Reply-To: <4FCE2ECD.4050107@freescale.com>
On Tue, Jun 05, 2012 at 11:07:41AM -0500, Scott Wood wrote:
> On 06/05/2012 04:08 AM, Zhao Chenhui wrote:
> > On Fri, Jun 01, 2012 at 10:40:00AM -0500, Scott Wood wrote:
> >> I know you say this is for dual-core chips only, but it would be nice if
> >> you'd write this in a way that doesn't assume that (even if the
> >> corenet-specific timebase freezing comes later).
> >
> > At this point, I have not thought about how to implement the cornet-specific timebase freezing.
>
> I wasn't asking you to. I was asking you to not have logic that breaks
> with more than 2 CPUs.
These routines only called in the dual-core case.
>
> >> Do we need an isync after setting the timebase, to ensure it's happened
> >> before we enable the timebase? Likewise, do we need a readback after
> >> disabling the timebase to ensure it's disabled before we read the
> >> timebase in give_timebase?
> >
> > I checked the e500 core manual (Chapter 2.16 Synchronization Requirements for SPRs).
> > Only some SPR registers need an isync. The timebase registers do not.
>
> I don't trust that, and the consequences of having the sync be imperfect
> are too unpleasant to chance it.
>
> > I did a readback in mpc85xx_timebase_freeze().
>
> Sorry, missed that somehow.
>
> >>> +#ifdef CONFIG_KEXEC
> >>> + np = of_find_matching_node(NULL, guts_ids);
> >>> + if (np) {
> >>> + guts = of_iomap(np, 0);
> >>> + smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
> >>> + smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
> >>> + of_node_put(np);
> >>> + } else {
> >>> + smp_85xx_ops.give_timebase = smp_generic_give_timebase;
> >>> + smp_85xx_ops.take_timebase = smp_generic_take_timebase;
> >>> + }
> >>
> >> Do not use smp_generic_give/take_timebase, ever. If you don't have the
> >> guts node, then just assume the timebase is already synced.
> >>
> >> -Scott
> >
> > smp_generic_give/take_timebase is the default in KEXEC before.
>
> That was a mistake.
>
> > If do not set them, it may make KEXEC fail on other platforms.
>
> What platforms?
>
> -Scott
Such as P4080, P3041, etc.
-Chenhui
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] uprobes: powerpc port
From: Ananth N Mavinakayanahalli @ 2012-06-06 9:35 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Srikar Dronamraju, lkml, oleg, Paul Mackerras, Anton Blanchard,
Ingo Molnar, linuxppc-dev
In-Reply-To: <1338974822.2749.89.camel@twins>
On Wed, Jun 06, 2012 at 11:27:02AM +0200, Peter Zijlstra wrote:
> On Wed, 2012-06-06 at 14:51 +0530, Ananth N Mavinakayanahalli wrote:
> > One TODO in this port compared to x86 is the uprobe abort_xol() logic.
> > x86 depends on the thread_struct.trap_nr (absent in powerpc) to determine
> > if a signal was caused when the uprobed instruction was single-stepped/
> > emulated, in which case, we reset the instruction pointer to the probed
> > address and retry the probe again.
>
> Another curious difference is that x86 uses an instruction decoder and
> contains massive tables to validate we can probe a particular
> instruction.
>
> Can we probe all possible PPC instructions?
For the kernel, the only ones that are off limits are rfi (return from
interrupt), mtmsr (move to msr). All other instructions can be probed.
Both those instructions are supervisor level, so we won't see them in
userspace at all; so we should be able to probe all user level
instructions.
I am not aware of specific caveats for vector/altivec instructions;
maybe Paul or Ben are more suitable to comment on that.
Ananth
^ permalink raw reply
* Re: [PATCH 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()
From: Ananth N Mavinakayanahalli @ 2012-06-06 9:37 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Srikar Dronamraju, lkml, oleg, Paul Mackerras, Anton Blanchard,
Ingo Molnar, linuxppc-dev
In-Reply-To: <1338974632.2749.87.camel@twins>
On Wed, Jun 06, 2012 at 11:23:52AM +0200, Peter Zijlstra wrote:
> On Wed, 2012-06-06 at 14:49 +0530, Ananth N Mavinakayanahalli wrote:
> > +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, loff_t vaddr)
>
> Don't we traditionally use unsigned long to pass vaddrs?
Right. But the vaddr we pass here is vma_info->vaddr which is loff_t.
I guess I should've made that clear in the patch description.
Ananth
^ permalink raw reply
* Re: [PATCH 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()
From: Ingo Molnar @ 2012-06-06 9:40 UTC (permalink / raw)
To: Ananth N Mavinakayanahalli
Cc: Srikar Dronamraju, Peter Zijlstra, lkml, oleg, Paul Mackerras,
Anton Blanchard, Ingo Molnar, linuxppc-dev
In-Reply-To: <20120606093744.GB29580@in.ibm.com>
* Ananth N Mavinakayanahalli <ananth@in.ibm.com> wrote:
> On Wed, Jun 06, 2012 at 11:23:52AM +0200, Peter Zijlstra wrote:
> > On Wed, 2012-06-06 at 14:49 +0530, Ananth N Mavinakayanahalli wrote:
> > > +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, loff_t vaddr)
> >
> > Don't we traditionally use unsigned long to pass vaddrs?
>
> Right. But the vaddr we pass here is vma_info->vaddr which is loff_t.
> I guess I should've made that clear in the patch description.
Why not fix struct vma_info's vaddr type?
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support
From: Zhao Chenhui @ 2012-06-06 9:59 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4FCE30B8.2070902@freescale.com>
On Tue, Jun 05, 2012 at 11:15:52AM -0500, Scott Wood wrote:
> On 06/05/2012 06:18 AM, Zhao Chenhui wrote:
> > On Mon, Jun 04, 2012 at 11:32:47AM -0500, Scott Wood wrote:
> >> On 06/04/2012 06:04 AM, Zhao Chenhui wrote:
> >>> On Fri, Jun 01, 2012 at 04:27:27PM -0500, Scott Wood wrote:
> >>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>>>> -#ifdef CONFIG_KEXEC
> >>>>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
> >>>>
> >>>> Let's not grow lists like this. Is there any harm in building it
> >>>> unconditionally?
> >>>>
> >>>> -Scott
> >>>
> >>> We need this ifdef. We only set give_timebase/take_timebase
> >>> when CONFIG_KEXEC or CONFIG_HOTPLUG_CPU is defined.
> >>
> >> If we really need this to be a compile-time decision, make a new symbol
> >> for it, but I really think this should be decided at runtime. Just
> >> because we have kexec or hotplug support enabled doesn't mean that's
> >> actually what we're doing at the moment.
> >>
> >> -Scott
> >
> > If user does not enable kexec or hotplug, these codes are redundant.
> > So use CONFIG_KEXEC and CONFIG_HOTPLUG_CPU to gard them.
>
> My point is that these lists tend to grow and be a maintenance pain.
> For small things it's often better to not worry about saving a few
> bytes. For larger things that need to be conditional, define a new
> symbol rather than growing ORed lists like this.
>
> -Scott
I agree with you in principle. But there are only two config options
in this patch, and it is unlikely to grow.
-Chenhui
^ permalink raw reply
* Re: [PATCH v5 5/5] powerpc/85xx: add support to JOG feature using cpufreq interface
From: Zhao Chenhui @ 2012-06-06 10:19 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4FCE2CB1.5090308@freescale.com>
On Tue, Jun 05, 2012 at 10:58:41AM -0500, Scott Wood wrote:
> On 06/05/2012 05:59 AM, Zhao Chenhui wrote:
> > On Fri, Jun 01, 2012 at 06:30:55PM -0500, Scott Wood wrote:
> >> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>> The jog mode frequency transition process on the MPC8536 is similar to
> >>> the deep sleep process. The driver need save the CPU state and restore
> >>> it after CPU warm reset.
> >>>
> >>> Note:
> >>> * The I/O peripherals such as PCIe and eTSEC may lose packets during
> >>> the jog mode frequency transition.
> >>
> >> That might be acceptable for eTSEC, but it is not acceptable to lose
> >> anything on PCIe. Especially not if you're going to make this "default y".
> >
> > It is a hardware limitation.
>
> Then make sure jog isn't used if PCIe is used.
>
> Maybe you could do something with the suspend infrastructure, but this
> is sufficiently heavyweight that transitions should be manually
> requested, not triggered by the automatic cpufreq governor.
>
> Does this apply to p1022, or just mpc8536?
Both of them.
>
> > Peripherals in the platform will not be operating
> > during the jog mode frequency transition process.
>
> What ensures this?
>
> -Scott
Hardware ensures it without software intervention.
-Chenhui
^ 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