* [PATCH] ARM: Enter CPU in ARM state for cpu_resume @ 2015-06-01 20:22 Stephen Boyd 2015-06-01 21:45 ` Russell King - ARM Linux 0 siblings, 1 reply; 5+ messages in thread From: Stephen Boyd @ 2015-06-01 20:22 UTC (permalink / raw) To: Russell King; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, Lina Iyer The standard boot protocol on ARM requires CPUs to be entered in the ARM state, unless they don't support the ARM instruction set (see Documentation/arm/Booting). On THUMB2 kernels, we assume the firmware can determine what state to enter the kernel in, but some firmwares don't honor the thumb bit. Make the cpu_resume symbol an ARM symbol, so that firmwares that honor the thumb bit will enter the kernel in ARM state and firmwares that don't honor the thumb bit will be able to enter the kernel in ARM state without more changes. This fixes a problem reported by Kevin Hilman where the ifc6410 fails to boot for THUMB2 kernels because the platform's firmware always enters the kernel in ARM mode from deep idle states. Reported-by: Kevin Hilman <khilman@linaro.org> Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Lina Iyer <lina.iyer@linaro.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- arch/arm/kernel/sleep.S | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S index 76bb3128e135..7c289ebb0b45 100644 --- a/arch/arm/kernel/sleep.S +++ b/arch/arm/kernel/sleep.S @@ -118,7 +118,12 @@ ENDPROC(cpu_resume_after_mmu) .text .align + .arm ENTRY(cpu_resume) + THUMB( badr r9, 1f ) @ Kernel is entered in ARM. + THUMB( bx r9 ) @ If this is a Thumb-2 kernel, + THUMB( .thumb ) @ switch to Thumb now. + THUMB(1: ) ARM_BE8(setend be) @ ensure we are in BE mode #ifdef CONFIG_ARM_VIRT_EXT bl __hyp_stub_install_secondary -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: Enter CPU in ARM state for cpu_resume 2015-06-01 20:22 [PATCH] ARM: Enter CPU in ARM state for cpu_resume Stephen Boyd @ 2015-06-01 21:45 ` Russell King - ARM Linux 2015-06-02 6:18 ` Ard Biesheuvel 0 siblings, 1 reply; 5+ messages in thread From: Russell King - ARM Linux @ 2015-06-01 21:45 UTC (permalink / raw) To: Stephen Boyd; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, Lina Iyer On Mon, Jun 01, 2015 at 01:22:00PM -0700, Stephen Boyd wrote: > The standard boot protocol on ARM requires CPUs to be entered in > the ARM state, unless they don't support the ARM instruction set > (see Documentation/arm/Booting). On THUMB2 kernels, we assume > the firmware can determine what state to enter the kernel in, but > some firmwares don't honor the thumb bit. Make the cpu_resume > symbol an ARM symbol, so that firmwares that honor the thumb bit > will enter the kernel in ARM state and firmwares that don't honor > the thumb bit will be able to enter the kernel in ARM state > without more changes. > > This fixes a problem reported by Kevin Hilman where the ifc6410 > fails to boot for THUMB2 kernels because the platform's firmware > always enters the kernel in ARM mode from deep idle states. Please do this differently. The default should be (as we do with the SMP secondary entry path) to assume that the firmware does the right thing. So, if we want an ARM-mode entry point, please use: + .arm +ENTRY(cpu_resume_arm) + THUMB( badr r9, 1f ) @ Kernel is entered in ARM. + THUMB( bx r9 ) @ If this is a Thumb-2 kernel, + THUMB( .thumb ) @ switch to Thumb now. + THUMB(1: ) Don't forget an ENDPROC() for the new symbol. Buggy platforms then use cpu_resume_arm instead of cpu_resume. Thanks. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: Enter CPU in ARM state for cpu_resume 2015-06-01 21:45 ` Russell King - ARM Linux @ 2015-06-02 6:18 ` Ard Biesheuvel 2015-06-02 8:34 ` Russell King - ARM Linux 0 siblings, 1 reply; 5+ messages in thread From: Ard Biesheuvel @ 2015-06-02 6:18 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Stephen Boyd, linux-arm-msm, linux-kernel, linux-arm-kernel, Lina Iyer On 1 June 2015 at 23:45, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Mon, Jun 01, 2015 at 01:22:00PM -0700, Stephen Boyd wrote: >> The standard boot protocol on ARM requires CPUs to be entered in >> the ARM state, unless they don't support the ARM instruction set >> (see Documentation/arm/Booting). On THUMB2 kernels, we assume >> the firmware can determine what state to enter the kernel in, but >> some firmwares don't honor the thumb bit. Make the cpu_resume >> symbol an ARM symbol, so that firmwares that honor the thumb bit >> will enter the kernel in ARM state and firmwares that don't honor >> the thumb bit will be able to enter the kernel in ARM state >> without more changes. >> >> This fixes a problem reported by Kevin Hilman where the ifc6410 >> fails to boot for THUMB2 kernels because the platform's firmware >> always enters the kernel in ARM mode from deep idle states. > > Please do this differently. The default should be (as we do with > the SMP secondary entry path) to assume that the firmware does the > right thing. > > So, if we want an ARM-mode entry point, please use: > > + .arm > +ENTRY(cpu_resume_arm) > + THUMB( badr r9, 1f ) @ Kernel is entered in ARM. > + THUMB( bx r9 ) @ If this is a Thumb-2 kernel, > + THUMB( .thumb ) @ switch to Thumb now. > + THUMB(1: ) > > Don't forget an ENDPROC() for the new symbol. Buggy platforms then > use cpu_resume_arm instead of cpu_resume. > OK, I think that was Stephen intention at first, but I suggested this instead. The point is that it is safer and more tidy to make these entry points ARM only throughout, and switch to Thumb2 only if THUMB2_KERNEL. This way, since all firmwares (except ARMv7-M, but let's disregard that for now) are known to be able to enter/resume into the kernel in ARM mode, this is more robust in the face of new platforms and firmware revisions of existing platforms. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: Enter CPU in ARM state for cpu_resume 2015-06-02 6:18 ` Ard Biesheuvel @ 2015-06-02 8:34 ` Russell King - ARM Linux 2015-06-02 10:34 ` Ard Biesheuvel 0 siblings, 1 reply; 5+ messages in thread From: Russell King - ARM Linux @ 2015-06-02 8:34 UTC (permalink / raw) To: Ard Biesheuvel Cc: Stephen Boyd, linux-arm-msm, linux-kernel, linux-arm-kernel, Lina Iyer On Tue, Jun 02, 2015 at 08:18:18AM +0200, Ard Biesheuvel wrote: > On 1 June 2015 at 23:45, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > Please do this differently. The default should be (as we do with > > the SMP secondary entry path) to assume that the firmware does the > > right thing. > > > > So, if we want an ARM-mode entry point, please use: > > > > + .arm > > +ENTRY(cpu_resume_arm) > > + THUMB( badr r9, 1f ) @ Kernel is entered in ARM. > > + THUMB( bx r9 ) @ If this is a Thumb-2 kernel, > > + THUMB( .thumb ) @ switch to Thumb now. > > + THUMB(1: ) > > > > Don't forget an ENDPROC() for the new symbol. Buggy platforms then > > use cpu_resume_arm instead of cpu_resume. > > > > OK, I think that was Stephen intention at first, but I suggested this instead. > > The point is that it is safer and more tidy to make these entry points > ARM only throughout, and switch to Thumb2 only if THUMB2_KERNEL. This > way, since all firmwares (except ARMv7-M, but let's disregard that for > now) are known to be able to enter/resume into the kernel in ARM mode, > this is more robust in the face of new platforms and firmware > revisions of existing platforms. Stop creating random interfaces with differing ways to call them. Consistency is one of the important things. We have three interfaces to the kernel: - Boot - Secondary CPU - Resume Out of those three, boot is the special one, as we have no way to communicate what mode it is required - so we specify a mode, which is ARM mode, except for CPUs which have no ARM mode support. Secondary CPU is not defined in the booting document because it's not relevant, and it's not relevant for the same reason that the resume entry point isn't. These entry points are not at a fixed location in the kernel image, and the kernel has to communicate that, along with their entry mode to the firmware. Therefore, firmware _does_ have the information to discover whether the address should be called in ARM or Thumb mode, and what's more, it should just work on those platforms which have no ARM mode support. If we start forcing these interfaces to be ARM mode only, we then need the same work-arounds for them as for the boot interface. In other words, boot has a _valid_ reason to be different, the other two are very similar in how they should be called. Besides, I'm not pandering to broken firmware. We should do the right thing by default, which is to have sane interfaces, and only work around stuff where needed. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: Enter CPU in ARM state for cpu_resume 2015-06-02 8:34 ` Russell King - ARM Linux @ 2015-06-02 10:34 ` Ard Biesheuvel 0 siblings, 0 replies; 5+ messages in thread From: Ard Biesheuvel @ 2015-06-02 10:34 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Stephen Boyd, linux-arm-msm, linux-kernel, linux-arm-kernel, Lina Iyer On 2 June 2015 at 10:34, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Tue, Jun 02, 2015 at 08:18:18AM +0200, Ard Biesheuvel wrote: >> On 1 June 2015 at 23:45, Russell King - ARM Linux >> <linux@arm.linux.org.uk> wrote: >> > Please do this differently. The default should be (as we do with >> > the SMP secondary entry path) to assume that the firmware does the >> > right thing. >> > >> > So, if we want an ARM-mode entry point, please use: >> > >> > + .arm >> > +ENTRY(cpu_resume_arm) >> > + THUMB( badr r9, 1f ) @ Kernel is entered in ARM. >> > + THUMB( bx r9 ) @ If this is a Thumb-2 kernel, >> > + THUMB( .thumb ) @ switch to Thumb now. >> > + THUMB(1: ) >> > >> > Don't forget an ENDPROC() for the new symbol. Buggy platforms then >> > use cpu_resume_arm instead of cpu_resume. >> > >> >> OK, I think that was Stephen intention at first, but I suggested this instead. >> >> The point is that it is safer and more tidy to make these entry points >> ARM only throughout, and switch to Thumb2 only if THUMB2_KERNEL. This >> way, since all firmwares (except ARMv7-M, but let's disregard that for >> now) are known to be able to enter/resume into the kernel in ARM mode, >> this is more robust in the face of new platforms and firmware >> revisions of existing platforms. > > Stop creating random interfaces with differing ways to call them. > Consistency is one of the important things. > Consistency was actually the point of my suggestion, but I see how the pre-existence of secondary_startup_arm() and your point regarding treating broken firmware as the exception may lead to the opposite conclusion. > We have three interfaces to the kernel: > > - Boot > - Secondary CPU > - Resume > > Out of those three, boot is the special one, as we have no way to > communicate what mode it is required - so we specify a mode, which > is ARM mode, except for CPUs which have no ARM mode support. > > Secondary CPU is not defined in the booting document because it's > not relevant, and it's not relevant for the same reason that the > resume entry point isn't. These entry points are not at a fixed > location in the kernel image, and the kernel has to communicate that, > along with their entry mode to the firmware. > > Therefore, firmware _does_ have the information to discover whether > the address should be called in ARM or Thumb mode, and what's more, > it should just work on those platforms which have no ARM mode support. > > If we start forcing these interfaces to be ARM mode only, we then need > the same work-arounds for them as for the boot interface. > > In other words, boot has a _valid_ reason to be different, the other > two are very similar in how they should be called. > > Besides, I'm not pandering to broken firmware. We should do the right > thing by default, which is to have sane interfaces, and only work around > stuff where needed. > > -- > FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up > according to speedtest.net. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-02 10:34 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-06-01 20:22 [PATCH] ARM: Enter CPU in ARM state for cpu_resume Stephen Boyd 2015-06-01 21:45 ` Russell King - ARM Linux 2015-06-02 6:18 ` Ard Biesheuvel 2015-06-02 8:34 ` Russell King - ARM Linux 2015-06-02 10:34 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).