From: Andre Przywara <andre.przywara@arm.com>
To: Tom Rini <trini@konsulko.com>
Cc: Simon Glass <sjg@chromium.org>,
Heinrich Schuchardt <heinrich.schuchardt@canonical.com>,
Alison Wang <alison.wang@nxp.com>,
Michael Walle <michael@walle.cc>, Nishanth Menon <nm@ti.com>,
Priyanka Singh <priyanka.singh@nxp.com>,
Peter Hoyes <Peter.Hoyes@arm.com>,
Marek Vasut <marek.vasut+renesas@gmail.com>,
u-boot@lists.denx.de, Nobuhiro Iwamatsu <iwamatsu@nigauri.org>,
Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>,
Tien Fong Chee <tien.fong.chee@intel.com>,
Alex Nemirovsky <alex.nemirovsky@cortina-access.com>
Subject: [PATCH 6/6] armv8: Fix and simplify branch_if_master/branch_if_slave
Date: Sun, 9 Jan 2022 17:30:09 +0000 [thread overview]
Message-ID: <20220109173009.25522-7-andre.przywara@arm.com> (raw)
In-Reply-To: <20220109173009.25522-1-andre.przywara@arm.com>
The branch_if_master macro jumps to a label if the CPU is the "master"
core, which we define as having all affinity levels set to 0. To check
for this condition, we need to mask off some bits from the MPIDR
register, then compare the remaining register value against zero.
The implementation of this was slighly broken (it preserved the upper
RES0 bits), overly complicated and hard to understand, especially since
it lacked comments. The same was true for the very similar
branch_if_slave macro.
Use a much shorter assembly sequence for those checks, use the same
masking for both macros (just negate the final branch), and put some
comments on them, to make it clear what the code does.
This allows to drop the second temporary register for branch_if_master,
so we adjust all call sites as well.
Also use the opportunity to remove a misleading comment: the macro
works fine on SoCs with multiple clusters. Judging by the commit
message, the original problem with the Juno SoC stems from the fact that
the master CPU *can* be configured to be from cluster 1, so the
assumption that the master CPU has all affinity values set to 0 does not
hold there. But this is already mentioned above in a comment, so remove
the extra comment.
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S | 2 +-
arch/arm/cpu/armv8/start.S | 6 ++--
arch/arm/include/asm/macro.h | 29 ++++++--------------
arch/arm/mach-rmobile/lowlevel_init_gen3.S | 2 +-
arch/arm/mach-socfpga/lowlevel_init_soc64.S | 2 +-
board/cortina/presidio-asic/lowlevel_init.S | 2 +-
6 files changed, 15 insertions(+), 28 deletions(-)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S
index 0929c58d43f..2fb4e404a24 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S
+++ b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S
@@ -200,7 +200,7 @@ ENTRY(lowlevel_init)
#endif
100:
- branch_if_master x0, x1, 2f
+ branch_if_master x0, 2f
#if defined(CONFIG_MP) && defined(CONFIG_ARMV8_MULTIENTRY)
/*
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
index 47a612883c5..b02fb76d627 100644
--- a/arch/arm/cpu/armv8/start.S
+++ b/arch/arm/cpu/armv8/start.S
@@ -179,11 +179,11 @@ pie_fixup_done:
bl lowlevel_init
#if defined(CONFIG_ARMV8_SPIN_TABLE) && !defined(CONFIG_SPL_BUILD)
- branch_if_master x0, x1, master_cpu
+ branch_if_master x0, master_cpu
b spin_table_secondary_jump
/* never return */
#elif defined(CONFIG_ARMV8_MULTIENTRY)
- branch_if_master x0, x1, master_cpu
+ branch_if_master x0, master_cpu
/*
* Slave CPUs
@@ -342,7 +342,7 @@ WEAK(lowlevel_init)
#endif
#ifdef CONFIG_ARMV8_MULTIENTRY
- branch_if_master x0, x1, 2f
+ branch_if_master x0, 2f
/*
* Slave should wait for master clearing spin table.
diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h
index acd519020d7..1a1edc98703 100644
--- a/arch/arm/include/asm/macro.h
+++ b/arch/arm/include/asm/macro.h
@@ -121,19 +121,10 @@ lr .req x30
*/
.macro branch_if_slave, xreg, slave_label
#ifdef CONFIG_ARMV8_MULTIENTRY
- /* NOTE: MPIDR handling will be erroneous on multi-cluster machines */
mrs \xreg, mpidr_el1
- tst \xreg, #0xff /* Test Affinity 0 */
- b.ne \slave_label
- lsr \xreg, \xreg, #8
- tst \xreg, #0xff /* Test Affinity 1 */
- b.ne \slave_label
- lsr \xreg, \xreg, #8
- tst \xreg, #0xff /* Test Affinity 2 */
- b.ne \slave_label
- lsr \xreg, \xreg, #16
- tst \xreg, #0xff /* Test Affinity 3 */
- b.ne \slave_label
+ and \xreg, \xreg, 0xffffffffff /* clear bits [63:40] */
+ and \xreg, \xreg, ~0x00ff000000 /* also clear bits [31:24] */
+ cbnz \xreg, \slave_label
#endif
.endm
@@ -141,16 +132,12 @@ lr .req x30
* Branch if current processor is a master,
* choose processor with all zero affinity value as the master.
*/
-.macro branch_if_master, xreg1, xreg2, master_label
+.macro branch_if_master, xreg, master_label
#ifdef CONFIG_ARMV8_MULTIENTRY
- /* NOTE: MPIDR handling will be erroneous on multi-cluster machines */
- mrs \xreg1, mpidr_el1
- lsr \xreg2, \xreg1, #32
- lsl \xreg2, \xreg2, #32
- lsl \xreg1, \xreg1, #40
- lsr \xreg1, \xreg1, #40
- orr \xreg1, \xreg1, \xreg2
- cbz \xreg1, \master_label
+ mrs \xreg, mpidr_el1
+ and \xreg, \xreg, 0xffffffffff /* clear bits [63:40] */
+ and \xreg, \xreg, ~0x00ff000000 /* also clear bits [31:24] */
+ cbz \xreg, \master_label
#else
b \master_label
#endif
diff --git a/arch/arm/mach-rmobile/lowlevel_init_gen3.S b/arch/arm/mach-rmobile/lowlevel_init_gen3.S
index 1df2c403453..0d7780031ac 100644
--- a/arch/arm/mach-rmobile/lowlevel_init_gen3.S
+++ b/arch/arm/mach-rmobile/lowlevel_init_gen3.S
@@ -64,7 +64,7 @@ ENTRY(lowlevel_init)
#endif
#endif
- branch_if_master x0, x1, 2f
+ branch_if_master x0, 2f
/*
* Slave should wait for master clearing spin table.
diff --git a/arch/arm/mach-socfpga/lowlevel_init_soc64.S b/arch/arm/mach-socfpga/lowlevel_init_soc64.S
index 612ea8a0371..875927cc4d9 100644
--- a/arch/arm/mach-socfpga/lowlevel_init_soc64.S
+++ b/arch/arm/mach-socfpga/lowlevel_init_soc64.S
@@ -38,7 +38,7 @@ slave_wait_atf:
#endif
#ifdef CONFIG_ARMV8_MULTIENTRY
- branch_if_master x0, x1, 2f
+ branch_if_master x0, 2f
/*
* Slave should wait for master clearing spin table.
diff --git a/board/cortina/presidio-asic/lowlevel_init.S b/board/cortina/presidio-asic/lowlevel_init.S
index 4450a5df79f..cbf8134346d 100644
--- a/board/cortina/presidio-asic/lowlevel_init.S
+++ b/board/cortina/presidio-asic/lowlevel_init.S
@@ -50,7 +50,7 @@ skip_smp_setup:
#endif
#ifdef CONFIG_ARMV8_MULTIENTRY
- branch_if_master x0, x1, 2f
+ branch_if_master x0, 2f
/*
* Slave should wait for master clearing spin table.
--
2.17.6
next prev parent reply other threads:[~2022-01-09 17:32 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-09 17:30 [PATCH 0/6] armv8: fixes and cleanups Andre Przywara
2022-01-09 17:30 ` [PATCH 1/6] cmd: exception: arm64: fix undefined, add faults Andre Przywara
2022-01-09 18:43 ` Heinrich Schuchardt
2022-01-09 19:08 ` Heinrich Schuchardt
2022-01-09 21:31 ` Andre Przywara
2022-01-09 22:19 ` Heinrich Schuchardt
2022-01-09 22:35 ` Andre Przywara
2022-01-09 22:47 ` Mark Kettenis
2022-01-09 23:19 ` Andre Przywara
2022-01-09 23:23 ` Heinrich Schuchardt
2022-01-09 23:49 ` Andre Przywara
2022-01-10 22:37 ` Mark Kettenis
2022-01-11 10:28 ` Andre Przywara
2022-01-09 17:30 ` [PATCH 2/6] armv8: Always unmask SErrors Andre Przywara
2022-01-09 17:30 ` [PATCH 3/6] armv8: Force SP_ELx stack pointer usage Andre Przywara
2022-01-09 17:30 ` [PATCH 4/6] arm: Clean up asm/io.h Andre Przywara
2022-01-09 21:39 ` Andre Przywara
2022-01-13 6:44 ` Leo Liang
2022-01-09 17:30 ` [PATCH 5/6] armv8: Simplify switch_el macro Andre Przywara
2022-01-09 17:30 ` Andre Przywara [this message]
2022-01-09 19:14 ` [PATCH 0/6] armv8: fixes and cleanups Michael Walle
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=20220109173009.25522-7-andre.przywara@arm.com \
--to=andre.przywara@arm.com \
--cc=Peter.Hoyes@arm.com \
--cc=alex.nemirovsky@cortina-access.com \
--cc=alison.wang@nxp.com \
--cc=heinrich.schuchardt@canonical.com \
--cc=iwamatsu@nigauri.org \
--cc=marek.vasut+renesas@gmail.com \
--cc=michael@walle.cc \
--cc=nm@ti.com \
--cc=priyanka.singh@nxp.com \
--cc=simon.k.r.goldschmidt@gmail.com \
--cc=sjg@chromium.org \
--cc=tien.fong.chee@intel.com \
--cc=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox