* [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points
@ 2018-09-27 19:27 Florian Fainelli
2018-09-27 19:27 ` [PATCH 1/3] firmware/psci: Fix cpu_resume entry points with THUMB2_KERNEL Florian Fainelli
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Florian Fainelli @ 2018-09-27 19:27 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Florian Fainelli, Russell King, Mark Rutland, Lorenzo Pieralisi,
Brian Norris, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Rob Herring,
Doug Berger, open list
Hi all,
While playing with THUMB2_KERNEL on ARCH_BRCMSTB, several issues came up
which are addressed by these 3 patches.
The THUMB() assembler macro is a no-op unless CONFIG_THUMB2_KERNEL so
using it unconditionally for CONFIG_ARM should not cause a problem
AFAICT.
Those patches can all be independently picked up by their respective
maintainers and don't depent on one another.
Thank you!
Florian Fainelli (3):
firmware/psci: Fix cpu_resume entry points with THUMB2_KERNEL
ARM: psci: Fix secondary core boot with THUMB2_KERNEL
soc: bcm: brcmstb: Fix re-entry point with a THUMB2_KERNEL
arch/arm/kernel/psci_smp.c | 4 ++--
drivers/firmware/psci.c | 18 +++++++++++++++---
drivers/soc/bcm/brcmstb/pm/pm-arm.c | 2 +-
3 files changed, 18 insertions(+), 6 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] firmware/psci: Fix cpu_resume entry points with THUMB2_KERNEL
2018-09-27 19:27 [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points Florian Fainelli
@ 2018-09-27 19:27 ` Florian Fainelli
2018-09-28 13:47 ` Mark Rutland
2018-09-27 19:27 ` [PATCH 2/3] ARM: psci: Fix secondary core boot " Florian Fainelli
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2018-09-27 19:27 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Florian Fainelli, Russell King, Mark Rutland, Lorenzo Pieralisi,
Brian Norris, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Rob Herring,
Doug Berger, open list
When THUMB2_KERNEL is enabled, we would be failing to resume from an
idle or system suspend call where the reentry point is set to
cpu_resume() because that function is in Thumb2. Utilize
cpu_resume_arm() for ARM 32-bit kernels which takes care of the mode
switching for us.
Fixes: 8b6f2499ac45 ("ARM: 8511/1: ARM64: kernel: PSCI: move PSCI idle management code to drivers/firmware")
Fixes: faf7ec4a92c0 ("drivers: firmware: psci: add system suspend support")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/firmware/psci.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index c80ec1d03274..f8b154384483 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -400,9 +400,14 @@ int psci_cpu_init_idle(unsigned int cpu)
static int psci_suspend_finisher(unsigned long index)
{
u32 *state = __this_cpu_read(psci_power_state);
+ unsigned long reentry;
- return psci_ops.cpu_suspend(state[index - 1],
- __pa_symbol(cpu_resume));
+#ifdef CONFIG_ARM
+ reentry = __pa_symbol(cpu_resume_arm);
+#else
+ reentry = __pa_symbol(cpu_resume);
+#endif
+ return psci_ops.cpu_suspend(state[index - 1], reentry);
}
int psci_cpu_suspend_enter(unsigned long index)
@@ -437,8 +442,15 @@ CPUIDLE_METHOD_OF_DECLARE(psci, "psci", &psci_cpuidle_ops);
static int psci_system_suspend(unsigned long unused)
{
+ unsigned long reentry;
+
+#ifdef CONFIG_ARM
+ reentry = __pa_symbol(cpu_resume_arm);
+#else
+ reentry = __pa_symbol(cpu_resume);
+#endif
return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
- __pa_symbol(cpu_resume), 0, 0);
+ reentry, 0, 0);
}
static int psci_system_suspend_enter(suspend_state_t state)
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] ARM: psci: Fix secondary core boot with THUMB2_KERNEL
2018-09-27 19:27 [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points Florian Fainelli
2018-09-27 19:27 ` [PATCH 1/3] firmware/psci: Fix cpu_resume entry points with THUMB2_KERNEL Florian Fainelli
@ 2018-09-27 19:27 ` Florian Fainelli
2018-09-28 13:48 ` Mark Rutland
2018-09-27 19:27 ` [PATCH 3/3] soc: bcm: brcmstb: Fix re-entry point with a THUMB2_KERNEL Florian Fainelli
2018-09-28 8:08 ` [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points Robin Murphy
3 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2018-09-27 19:27 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Florian Fainelli, Russell King, Mark Rutland, Lorenzo Pieralisi,
Brian Norris, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Rob Herring,
Doug Berger, open list
When THUMB2_KERNEL is enabled, we would be setting the secondary core's
entry point to secondary_startup() which is already Thumb2 code, utilize
secondary_startup_arm() which takes care of doing the mode switching for
us.
Fixes: 05774088391c ("arm: introduce psci_smp_ops")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
arch/arm/kernel/psci_smp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
index cb3fcaeb2233..547a11b065d2 100644
--- a/arch/arm/kernel/psci_smp.c
+++ b/arch/arm/kernel/psci_smp.c
@@ -47,13 +47,13 @@
*
*/
-extern void secondary_startup(void);
+extern void secondary_startup_arm(void);
static int psci_boot_secondary(unsigned int cpu, struct task_struct *idle)
{
if (psci_ops.cpu_on)
return psci_ops.cpu_on(cpu_logical_map(cpu),
- virt_to_idmap(&secondary_startup));
+ virt_to_idmap(&secondary_startup_arm));
return -ENODEV;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] soc: bcm: brcmstb: Fix re-entry point with a THUMB2_KERNEL
2018-09-27 19:27 [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points Florian Fainelli
2018-09-27 19:27 ` [PATCH 1/3] firmware/psci: Fix cpu_resume entry points with THUMB2_KERNEL Florian Fainelli
2018-09-27 19:27 ` [PATCH 2/3] ARM: psci: Fix secondary core boot " Florian Fainelli
@ 2018-09-27 19:27 ` Florian Fainelli
2018-10-01 18:17 ` Florian Fainelli
2018-09-28 8:08 ` [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points Robin Murphy
3 siblings, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2018-09-27 19:27 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Florian Fainelli, Russell King, Mark Rutland, Lorenzo Pieralisi,
Brian Norris, Gregory Fong,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Rob Herring,
Doug Berger, open list
When the kernel is built with CONFIG_THUMB2_KERNEL we would set the
kernel's resume entry point to be a function that is already built as
Thumb-2 code while the boot agent doing the resume is in ARM mode, so
this does not work. There is a header label defined: cpu_resume_arm
which we can use to do the switching for us.
Fixes: 0b741b8234c8 ("soc: bcm: brcmstb: Add support for S2/S3/S5 suspend states (ARM)")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/soc/bcm/brcmstb/pm/pm-arm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/bcm/brcmstb/pm/pm-arm.c b/drivers/soc/bcm/brcmstb/pm/pm-arm.c
index a5577dd5eb08..8ee06347447c 100644
--- a/drivers/soc/bcm/brcmstb/pm/pm-arm.c
+++ b/drivers/soc/bcm/brcmstb/pm/pm-arm.c
@@ -404,7 +404,7 @@ noinline int brcmstb_pm_s3_finish(void)
{
struct brcmstb_s3_params *params = ctrl.s3_params;
dma_addr_t params_pa = ctrl.s3_params_pa;
- phys_addr_t reentry = virt_to_phys(&cpu_resume);
+ phys_addr_t reentry = virt_to_phys(&cpu_resume_arm);
enum bsp_initiate_command cmd;
u32 flags;
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points
2018-09-27 19:27 [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points Florian Fainelli
` (2 preceding siblings ...)
2018-09-27 19:27 ` [PATCH 3/3] soc: bcm: brcmstb: Fix re-entry point with a THUMB2_KERNEL Florian Fainelli
@ 2018-09-28 8:08 ` Robin Murphy
2018-09-28 17:51 ` Florian Fainelli
3 siblings, 1 reply; 10+ messages in thread
From: Robin Murphy @ 2018-09-28 8:08 UTC (permalink / raw)
To: Florian Fainelli, linux-arm-kernel
Cc: Mark Rutland, Rob Herring, Lorenzo Pieralisi, Russell King,
open list, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
Gregory Fong, Doug Berger, Brian Norris
On 2018-09-27 8:27 PM, Florian Fainelli wrote:
> Hi all,
>
> While playing with THUMB2_KERNEL on ARCH_BRCMSTB, several issues came up
> which are addressed by these 3 patches.
Hmmm, PSCI looks to explicitly support Thumb entrypoints ("T32 support"
in section 6.4.3 of DEN0022D), so these changes smell a little of
papering over a more fundamental problem, which is presumably either
that Thumb symbols are not being resolved correctly, or that the
firmware you're using has a bug.
Robin.
> The THUMB() assembler macro is a no-op unless CONFIG_THUMB2_KERNEL so
> using it unconditionally for CONFIG_ARM should not cause a problem
> AFAICT.
>
> Those patches can all be independently picked up by their respective
> maintainers and don't depent on one another.
>
> Thank you!
>
> Florian Fainelli (3):
> firmware/psci: Fix cpu_resume entry points with THUMB2_KERNEL
> ARM: psci: Fix secondary core boot with THUMB2_KERNEL
> soc: bcm: brcmstb: Fix re-entry point with a THUMB2_KERNEL
>
> arch/arm/kernel/psci_smp.c | 4 ++--
> drivers/firmware/psci.c | 18 +++++++++++++++---
> drivers/soc/bcm/brcmstb/pm/pm-arm.c | 2 +-
> 3 files changed, 18 insertions(+), 6 deletions(-)
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] firmware/psci: Fix cpu_resume entry points with THUMB2_KERNEL
2018-09-27 19:27 ` [PATCH 1/3] firmware/psci: Fix cpu_resume entry points with THUMB2_KERNEL Florian Fainelli
@ 2018-09-28 13:47 ` Mark Rutland
2018-09-28 17:50 ` Florian Fainelli
0 siblings, 1 reply; 10+ messages in thread
From: Mark Rutland @ 2018-09-28 13:47 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-arm-kernel, Russell King, Lorenzo Pieralisi, Brian Norris,
Gregory Fong, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
Rob Herring, Doug Berger, open list
On Thu, Sep 27, 2018 at 12:27:09PM -0700, Florian Fainelli wrote:
> When THUMB2_KERNEL is enabled, we would be failing to resume from an
> idle or system suspend call where the reentry point is set to
> cpu_resume() because that function is in Thumb2. Utilize
> cpu_resume_arm() for ARM 32-bit kernels which takes care of the mode
> switching for us.
Looking at the PSCI spec, if bit[0] of the entry point address is set,
the CPU should be placed into thumb state.
So either there's a FW bug here, or perhaps we're stripping bit[0] when
we do the __pa_symbol() dance.
Which firmware have you seen this with?
Thanks,
Mark.
> Fixes: 8b6f2499ac45 ("ARM: 8511/1: ARM64: kernel: PSCI: move PSCI idle management code to drivers/firmware")
> Fixes: faf7ec4a92c0 ("drivers: firmware: psci: add system suspend support")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/firmware/psci.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index c80ec1d03274..f8b154384483 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -400,9 +400,14 @@ int psci_cpu_init_idle(unsigned int cpu)
> static int psci_suspend_finisher(unsigned long index)
> {
> u32 *state = __this_cpu_read(psci_power_state);
> + unsigned long reentry;
>
> - return psci_ops.cpu_suspend(state[index - 1],
> - __pa_symbol(cpu_resume));
> +#ifdef CONFIG_ARM
> + reentry = __pa_symbol(cpu_resume_arm);
> +#else
> + reentry = __pa_symbol(cpu_resume);
> +#endif
> + return psci_ops.cpu_suspend(state[index - 1], reentry);
> }
>
> int psci_cpu_suspend_enter(unsigned long index)
> @@ -437,8 +442,15 @@ CPUIDLE_METHOD_OF_DECLARE(psci, "psci", &psci_cpuidle_ops);
>
> static int psci_system_suspend(unsigned long unused)
> {
> + unsigned long reentry;
> +
> +#ifdef CONFIG_ARM
> + reentry = __pa_symbol(cpu_resume_arm);
> +#else
> + reentry = __pa_symbol(cpu_resume);
> +#endif
> return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
> - __pa_symbol(cpu_resume), 0, 0);
> + reentry, 0, 0);
> }
>
> static int psci_system_suspend_enter(suspend_state_t state)
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] ARM: psci: Fix secondary core boot with THUMB2_KERNEL
2018-09-27 19:27 ` [PATCH 2/3] ARM: psci: Fix secondary core boot " Florian Fainelli
@ 2018-09-28 13:48 ` Mark Rutland
0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2018-09-28 13:48 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-arm-kernel, Russell King, Lorenzo Pieralisi, Brian Norris,
Gregory Fong, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
Rob Herring, Doug Berger, open list
On Thu, Sep 27, 2018 at 12:27:10PM -0700, Florian Fainelli wrote:
> When THUMB2_KERNEL is enabled, we would be setting the secondary core's
> entry point to secondary_startup() which is already Thumb2 code, utilize
> secondary_startup_arm() which takes care of doing the mode switching for
> us.
As with the first patch, a call to CPU_ON with bit[0] set in the entry
point *should* place the CPU into thumb state, so I'd like to know which
FW you've seen this bug with.
Thanks,
Mark.
>
> Fixes: 05774088391c ("arm: introduce psci_smp_ops")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> arch/arm/kernel/psci_smp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
> index cb3fcaeb2233..547a11b065d2 100644
> --- a/arch/arm/kernel/psci_smp.c
> +++ b/arch/arm/kernel/psci_smp.c
> @@ -47,13 +47,13 @@
> *
> */
>
> -extern void secondary_startup(void);
> +extern void secondary_startup_arm(void);
>
> static int psci_boot_secondary(unsigned int cpu, struct task_struct *idle)
> {
> if (psci_ops.cpu_on)
> return psci_ops.cpu_on(cpu_logical_map(cpu),
> - virt_to_idmap(&secondary_startup));
> + virt_to_idmap(&secondary_startup_arm));
> return -ENODEV;
> }
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] firmware/psci: Fix cpu_resume entry points with THUMB2_KERNEL
2018-09-28 13:47 ` Mark Rutland
@ 2018-09-28 17:50 ` Florian Fainelli
0 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2018-09-28 17:50 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arm-kernel, Russell King, Lorenzo Pieralisi, Brian Norris,
Gregory Fong, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
Rob Herring, Doug Berger, open list
On 09/28/2018 06:47 AM, Mark Rutland wrote:
> On Thu, Sep 27, 2018 at 12:27:09PM -0700, Florian Fainelli wrote:
>> When THUMB2_KERNEL is enabled, we would be failing to resume from an
>> idle or system suspend call where the reentry point is set to
>> cpu_resume() because that function is in Thumb2. Utilize
>> cpu_resume_arm() for ARM 32-bit kernels which takes care of the mode
>> switching for us.
>
> Looking at the PSCI spec, if bit[0] of the entry point address is set,
> the CPU should be placed into thumb state.
>
> So either there's a FW bug here, or perhaps we're stripping bit[0] when
> we do the __pa_symbol() dance.
>
> Which firmware have you seen this with?
This is a custom implementation use with ARCH_BRCMSTB, the same way I
managed to miss it during code review, I missed it again here, sorry
about that and thanks for pointing me in the right direction.
--
Florian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points
2018-09-28 8:08 ` [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points Robin Murphy
@ 2018-09-28 17:51 ` Florian Fainelli
0 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2018-09-28 17:51 UTC (permalink / raw)
To: Robin Murphy, linux-arm-kernel
Cc: Mark Rutland, Rob Herring, Lorenzo Pieralisi, Russell King,
open list, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
Gregory Fong, Doug Berger, Brian Norris
On 09/28/2018 01:08 AM, Robin Murphy wrote:
> On 2018-09-27 8:27 PM, Florian Fainelli wrote:
>> Hi all,
>>
>> While playing with THUMB2_KERNEL on ARCH_BRCMSTB, several issues came up
>> which are addressed by these 3 patches.
>
> Hmmm, PSCI looks to explicitly support Thumb entrypoints ("T32 support"
> in section 6.4.3 of DEN0022D), so these changes smell a little of
> papering over a more fundamental problem, which is presumably either
> that Thumb symbols are not being resolved correctly, or that the
> firmware you're using has a bug.
Yes, I clearly missed that during the internal review of our
implementation and missed it again prior to submitting those patches,
shame on me.
The last patch is valid because it addresses a problem on 32-bit only
platforms which do not have a PSCI firmware at that point.
Thanks!
--
Florian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] soc: bcm: brcmstb: Fix re-entry point with a THUMB2_KERNEL
2018-09-27 19:27 ` [PATCH 3/3] soc: bcm: brcmstb: Fix re-entry point with a THUMB2_KERNEL Florian Fainelli
@ 2018-10-01 18:17 ` Florian Fainelli
0 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2018-10-01 18:17 UTC (permalink / raw)
To: bcm-kernel-feedback-list, linux-arm-kernel
Cc: Russell King, Mark Rutland, Lorenzo Pieralisi, Brian Norris,
Gregory Fong, Rob Herring, Doug Berger, open list
On Thu, 27 Sep 2018 12:27:11 -0700, Florian Fainelli <f.fainelli@gmail.com> wrote:
> When the kernel is built with CONFIG_THUMB2_KERNEL we would set the
> kernel's resume entry point to be a function that is already built as
> Thumb-2 code while the boot agent doing the resume is in ARM mode, so
> this does not work. There is a header label defined: cpu_resume_arm
> which we can use to do the switching for us.
>
> Fixes: 0b741b8234c8 ("soc: bcm: brcmstb: Add support for S2/S3/S5 suspend states (ARM)")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
Applied to drivers/next, thanks!
--
Florian
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-10-01 18:17 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-27 19:27 [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points Florian Fainelli
2018-09-27 19:27 ` [PATCH 1/3] firmware/psci: Fix cpu_resume entry points with THUMB2_KERNEL Florian Fainelli
2018-09-28 13:47 ` Mark Rutland
2018-09-28 17:50 ` Florian Fainelli
2018-09-27 19:27 ` [PATCH 2/3] ARM: psci: Fix secondary core boot " Florian Fainelli
2018-09-28 13:48 ` Mark Rutland
2018-09-27 19:27 ` [PATCH 3/3] soc: bcm: brcmstb: Fix re-entry point with a THUMB2_KERNEL Florian Fainelli
2018-10-01 18:17 ` Florian Fainelli
2018-09-28 8:08 ` [PATCH 0/3] ARM/PSCI: Fix THUMB2_KERNEL entry points Robin Murphy
2018-09-28 17:51 ` Florian Fainelli
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).