From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> To: Russell King - ARM Linux <linux@arm.linux.org.uk> Cc: "linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>, Santosh Shilimkar <santosh.shilimkar@ti.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH 10/11] ARM: pm: convert some assembly to C Date: Wed, 7 Sep 2011 16:48:28 +0100 [thread overview] Message-ID: <20110907154828.GA21987@e102568-lin.cambridge.arm.com> (raw) In-Reply-To: <E1Qz6k7-00078l-SM@rmk-PC.arm.linux.org.uk> Hi Russell, On Thu, Sep 01, 2011 at 01:51:39PM +0100, Russell King - ARM Linux wrote: > Convert some of the sleep.S guts to C code, which makes it easier to > use our macros and to add L2 cache handling. We provide a helper > function, __cpu_suspend_save(), which deals with saving the common > state, setting up for resume, and flushing caches. > > The remainder left as assembly code is the saving of the CPU general > purpose registers, and allocating space on the stack to save the CPU > specific registers and resume state. > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > arch/arm/include/asm/proc-fns.h | 8 ++++++ > arch/arm/kernel/sleep.S | 53 ++++++++++++-------------------------- > arch/arm/kernel/suspend.c | 24 +++++++++++++++-- > 3 files changed, 46 insertions(+), 39 deletions(-) > [...] > diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c > index 115736a..c78a88f 100644 > --- a/arch/arm/kernel/suspend.c > +++ b/arch/arm/kernel/suspend.c > @@ -8,10 +8,29 @@ > > static pgd_t *suspend_pgd; > > -extern int __cpu_suspend(int, long, unsigned long, int (*)(unsigned long)); > +extern int __cpu_suspend(unsigned long, int (*)(unsigned long)); > extern void cpu_resume_mmu(void); > > /* > + * This is called by __cpu_suspend() to save the state, and do whatever > + * flushing is required to ensure that when the CPU goes to sleep we have > + * the necessary data available when the caches are not searched. > + */ > +void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr) > +{ > + *save_ptr = virt_to_phys(ptr); > + > + /* This must correspond to the LDM in cpu_resume() assembly */ > + *ptr++ = virt_to_phys(suspend_pgd); > + *ptr++ = sp; > + *ptr++ = virt_to_phys(cpu_do_resume); > + > + cpu_do_suspend(ptr); > + > + flush_cache_all(); > +} > + > +/* > * Hide the first two arguments to __cpu_suspend - these are an implementation > * detail which platform code shouldn't have to know about. > */ > @@ -29,8 +48,7 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long)) > * resume (indicated by a zero return code), we need to switch > * back to the correct page tables. > */ > - ret = __cpu_suspend(virt_to_phys(suspend_pgd), > - PHYS_OFFSET - PAGE_OFFSET, arg, fn); > + ret = __cpu_suspend(arg, fn); > if (ret == 0) > cpu_switch_mm(mm->pgd, mm); It is still early testing, but without a local tlb flush here I am getting random segmentation faults in user space. My fear is that 1:1 global TLB entries cause issues if user space processes happen to map those pages at addresses overlapping 1:1 mapping set-up for resume and we do not flush the TLB. With the tlb flush the whole patchset works with nary a blemish, from cpuidle. Still a question mark so please give me the benefit of the doubt. Many thanks, Lorenzo
WARNING: multiple messages have this Message-ID (diff)
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 10/11] ARM: pm: convert some assembly to C Date: Wed, 7 Sep 2011 16:48:28 +0100 [thread overview] Message-ID: <20110907154828.GA21987@e102568-lin.cambridge.arm.com> (raw) In-Reply-To: <E1Qz6k7-00078l-SM@rmk-PC.arm.linux.org.uk> Hi Russell, On Thu, Sep 01, 2011 at 01:51:39PM +0100, Russell King - ARM Linux wrote: > Convert some of the sleep.S guts to C code, which makes it easier to > use our macros and to add L2 cache handling. We provide a helper > function, __cpu_suspend_save(), which deals with saving the common > state, setting up for resume, and flushing caches. > > The remainder left as assembly code is the saving of the CPU general > purpose registers, and allocating space on the stack to save the CPU > specific registers and resume state. > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > arch/arm/include/asm/proc-fns.h | 8 ++++++ > arch/arm/kernel/sleep.S | 53 ++++++++++++-------------------------- > arch/arm/kernel/suspend.c | 24 +++++++++++++++-- > 3 files changed, 46 insertions(+), 39 deletions(-) > [...] > diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c > index 115736a..c78a88f 100644 > --- a/arch/arm/kernel/suspend.c > +++ b/arch/arm/kernel/suspend.c > @@ -8,10 +8,29 @@ > > static pgd_t *suspend_pgd; > > -extern int __cpu_suspend(int, long, unsigned long, int (*)(unsigned long)); > +extern int __cpu_suspend(unsigned long, int (*)(unsigned long)); > extern void cpu_resume_mmu(void); > > /* > + * This is called by __cpu_suspend() to save the state, and do whatever > + * flushing is required to ensure that when the CPU goes to sleep we have > + * the necessary data available when the caches are not searched. > + */ > +void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr) > +{ > + *save_ptr = virt_to_phys(ptr); > + > + /* This must correspond to the LDM in cpu_resume() assembly */ > + *ptr++ = virt_to_phys(suspend_pgd); > + *ptr++ = sp; > + *ptr++ = virt_to_phys(cpu_do_resume); > + > + cpu_do_suspend(ptr); > + > + flush_cache_all(); > +} > + > +/* > * Hide the first two arguments to __cpu_suspend - these are an implementation > * detail which platform code shouldn't have to know about. > */ > @@ -29,8 +48,7 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long)) > * resume (indicated by a zero return code), we need to switch > * back to the correct page tables. > */ > - ret = __cpu_suspend(virt_to_phys(suspend_pgd), > - PHYS_OFFSET - PAGE_OFFSET, arg, fn); > + ret = __cpu_suspend(arg, fn); > if (ret == 0) > cpu_switch_mm(mm->pgd, mm); It is still early testing, but without a local tlb flush here I am getting random segmentation faults in user space. My fear is that 1:1 global TLB entries cause issues if user space processes happen to map those pages at addresses overlapping 1:1 mapping set-up for resume and we do not flush the TLB. With the tlb flush the whole patchset works with nary a blemish, from cpuidle. Still a question mark so please give me the benefit of the doubt. Many thanks, Lorenzo
next prev parent reply other threads:[~2011-09-07 15:48 UTC|newest] Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-09-01 12:47 [PATCH 00/11] Add L2 cache cleaning to generic CPU suspend Russell King - ARM Linux 2011-09-01 12:47 ` Russell King - ARM Linux 2011-09-01 12:48 ` [PATCH 01/11] ARM: pm: CPU specific code should not overwrite r1 (v:p offset) Russell King - ARM Linux 2011-09-01 12:48 ` Russell King - ARM Linux 2011-09-01 12:48 ` [PATCH 02/11] ARM: pm: arm920/926: fix number of registers saved Russell King - ARM Linux 2011-09-01 12:48 ` Russell King - ARM Linux 2011-09-01 12:49 ` [PATCH 03/11] ARM: pm: some ARMv7 requires a dsb in resume to ensure correctness Russell King - ARM Linux 2011-09-01 12:49 ` Russell King - ARM Linux 2011-09-07 15:41 ` Catalin Marinas 2011-09-07 15:41 ` Catalin Marinas 2011-09-07 16:19 ` Russell King - ARM Linux 2011-09-07 16:19 ` Russell King - ARM Linux 2011-09-07 16:26 ` Catalin Marinas 2011-09-07 16:26 ` Catalin Marinas 2011-09-07 16:54 ` Catalin Marinas 2011-09-07 16:54 ` Catalin Marinas 2011-09-01 12:49 ` [PATCH 04/11] ARM: pm: avoid writing the auxillary control register for ARMv7 Russell King - ARM Linux 2011-09-01 12:49 ` Russell King - ARM Linux 2011-09-01 12:49 ` [PATCH 05/11] ARM: pm: force non-zero return value from __cpu_suspend when aborting Russell King - ARM Linux 2011-09-01 12:49 ` Russell King - ARM Linux 2011-09-01 12:50 ` [PATCH 06/11] ARM: pm: preallocate a page table for suspend/resume Russell King - ARM Linux 2011-09-01 12:50 ` Russell King - ARM Linux 2011-09-01 12:50 ` [PATCH 07/11] ARM: pm: only use preallocated page table during resume Russell King - ARM Linux 2011-09-01 12:50 ` Russell King - ARM Linux 2011-09-01 12:50 ` [PATCH 08/11] ARM: pm: no need to save/restore context ID register Russell King - ARM Linux 2011-09-01 12:50 ` Russell King - ARM Linux 2011-09-03 16:33 ` Santosh 2011-09-03 16:33 ` Santosh 2011-09-04 10:08 ` Russell King - ARM Linux 2011-09-04 10:08 ` Russell King - ARM Linux 2011-09-01 12:51 ` [PATCH 09/11] ARM: pm: get rid of cpu_resume_turn_mmu_on Russell King - ARM Linux 2011-09-01 12:51 ` Russell King - ARM Linux 2011-09-01 12:51 ` [PATCH 10/11] ARM: pm: convert some assembly to C Russell King - ARM Linux 2011-09-01 12:51 ` Russell King - ARM Linux 2011-09-07 15:48 ` Lorenzo Pieralisi [this message] 2011-09-07 15:48 ` Lorenzo Pieralisi 2011-09-19 16:32 ` Russell King - ARM Linux 2011-09-19 16:32 ` Russell King - ARM Linux 2011-09-01 12:51 ` [PATCH 11/11] ARM: pm: add L2 cache cleaning for suspend Russell King - ARM Linux 2011-09-01 12:51 ` Russell King - ARM Linux 2011-09-01 15:33 ` [PATCH 00/11] Add L2 cache cleaning to generic CPU suspend Shawn Guo 2011-09-01 15:33 ` Shawn Guo 2011-09-01 15:34 ` Russell King - ARM Linux 2011-09-01 15:34 ` Russell King - ARM Linux 2011-09-01 15:57 ` Shawn Guo 2011-09-01 15:57 ` Shawn Guo 2011-09-10 16:10 ` Shawn Guo 2011-09-10 16:10 ` Shawn Guo 2011-09-19 16:22 ` Russell King - ARM Linux 2011-09-19 16:22 ` Russell King - ARM Linux 2011-09-20 3:24 ` Shawn Guo 2011-09-20 3:24 ` Shawn Guo 2011-09-03 16:36 ` Santosh 2011-09-03 16:36 ` Santosh 2011-09-04 10:12 ` Russell King - ARM Linux 2011-09-04 10:12 ` Russell King - ARM Linux
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20110907154828.GA21987@e102568-lin.cambridge.arm.com \ --to=lorenzo.pieralisi@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-omap@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=santosh.shilimkar@ti.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.