[PATCH v3 0/7] riscv: Correctly handle IPIs already pending upon boot

On the K210, the prior stage bootloader does not clear IPIs. This presents a problem, because U-Boot up until this point assumes (with one exception) that IPIs are cleared when it starts. This series attempts to fix this in a robust manner, and fix several concurrency bugs I noticed while fixing these other areas. Heinrich previously submitted a patch addressing part of this problem in [1].
[1] https://patchwork.ozlabs.org/project/uboot/patch/20200811035648.3284-1-xypro...
Changes in v3: - Rename riscv_ipi_init_secondary_hart to dummy_pending_ipi_clear - Only compile dummy_pending_ipi_clear when SMP is enabled - Clarify XIP comment - Updated and expanded most commit messages
Changes in v2: - Use a valid bit instead of addr to validate IPIs - Make riscv_ipi_init_secondary_hart static - Remove fences after amoswaps - Set gp early with XIP - Clarify comments regarding tp
Sean Anderson (7): Revert "riscv: Clear pending interrupts before enabling IPIs" riscv: Match memory barriers between send_ipi_many and handle_ipi riscv: Use a valid bit to ignore already-pending IPIs riscv: Clear pending IPIs on initialization riscv: Consolidate fences into AMOs for available_harts_lock riscv: Ensure gp is NULL or points to valid data riscv: Add some comments to start.S
arch/riscv/cpu/cpu.c | 20 +++++++++++++ arch/riscv/cpu/start.S | 58 ++++++++++++++++++++++++++++-------- arch/riscv/include/asm/smp.h | 7 +++++ arch/riscv/lib/interrupts.c | 3 +- arch/riscv/lib/smp.c | 16 +++++++++- 5 files changed, 89 insertions(+), 15 deletions(-)

Clearing MIP.MSIP is not guaranteed to do anything by the spec. In addition, most existing RISC-V hardware does nothing when this bit is set.
The following commits "riscv: Use a valid bit to ignore already-pending IPIs" and "riscv: Clear pending IPIs on initialization" should implement the original intent of the reverted commit in a more robust manner.
This reverts commit 9472630337e7c4ac442066b5a752aaa8c3b4d4a6.
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com ---
(no changes since v1)
arch/riscv/cpu/start.S | 2 -- 1 file changed, 2 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index bf9fdf369b..e3222b1ea7 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -65,8 +65,6 @@ _start: #else li t0, SIE_SSIE #endif - /* Clear any pending IPIs */ - csrc MODE_PREFIX(ip), t0 csrs MODE_PREFIX(ie), t0 #endif

Clearing MIP.MSIP is not guaranteed to do anything by the spec. In addition, most existing RISC-V hardware does nothing when this bit is set.
The following commits "riscv: Use a valid bit to ignore already-pending IPIs" and "riscv: Clear pending IPIs on initialization" should implement the original intent of the reverted commit in a more robust manner.
This reverts commit 9472630337e7c4ac442066b5a752aaa8c3b4d4a6.
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com
Reviewed-by: Rick Chen rick@andestech.com

Without a matching barrier on the write side, the barrier in handle_ipi does nothing. It was entirely possible for the boot hart to write to addr, arg0, and arg1 *after* sending the IPI, because there was no barrier on the sending side.
Fixes: 90ae281437 ("riscv: add option to wait for ack from secondary harts in smp functions") Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com Reviewed-by: Rick Chen rick@andestech.com Reviewed-by: Leo Liang ycliang@andestech.com ---
(no changes since v1)
arch/riscv/lib/smp.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c index ac22136314..ab6d8bd7fa 100644 --- a/arch/riscv/lib/smp.c +++ b/arch/riscv/lib/smp.c @@ -54,6 +54,8 @@ static int send_ipi_many(struct ipi_data *ipi, int wait) gd->arch.ipi[reg].arg0 = ipi->arg0; gd->arch.ipi[reg].arg1 = ipi->arg1;
+ __smp_mb(); + ret = riscv_send_ipi(reg); if (ret) { pr_err("Cannot send IPI to hart %d\n", reg);

Some IPIs may already be pending when U-Boot is started. This could be a problem if a secondary hart tries to handle an IPI before the boot hart has initialized the IPI device.
To be specific, the Kendryte K210 ROM-based bootloader does not clear IPIs before passing control to U-Boot. Without this patch, the secondary hart jumps to address 0x0 as soon as it enters secondary_hart_loop, and then hangs in its trap handler.
This commit introduces a valid bit so secondary harts know when and IPI originates from U-Boot, and it is safe to use the IPI API. The valid bit is initialized to 0 by board_init_f_init_reserve. Before this, secondary harts wait in wait_for_gd_init.
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com Reviewed-by: Rick Chen rick@andestech.com Reviewed-by: Leo Liang ycliang@andestech.com ---
(no changes since v2)
Changes in v2: - Use a valid bit instead of addr to validate IPIs
arch/riscv/include/asm/smp.h | 7 +++++++ arch/riscv/lib/smp.c | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h index 1b428856b2..2dae0800ce 100644 --- a/arch/riscv/include/asm/smp.h +++ b/arch/riscv/include/asm/smp.h @@ -18,14 +18,21 @@ * IPI data structure. The hart ID is inserted by the hart handling the IPI and * calling the function. * + * @valid is used to determine whether a sent IPI originated from U-Boot. It is + * initialized to zero by board_init_f_alloc_reserve. When U-Boot sends its + * first IPI, it is set to 1. This prevents already-pending IPIs not sent by + * U-Boot from being taken. + * * @addr: Address of function * @arg0: First argument of function * @arg1: Second argument of function + * @valid: Whether this IPI is valid */ struct ipi_data { ulong addr; ulong arg0; ulong arg1; + unsigned int valid; };
/** diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c index ab6d8bd7fa..8f33ce1fe3 100644 --- a/arch/riscv/lib/smp.c +++ b/arch/riscv/lib/smp.c @@ -54,7 +54,13 @@ static int send_ipi_many(struct ipi_data *ipi, int wait) gd->arch.ipi[reg].arg0 = ipi->arg0; gd->arch.ipi[reg].arg1 = ipi->arg1;
- __smp_mb(); + /* + * Ensure valid only becomes set when the IPI parameters are + * set. An IPI may already be pending on other harts, so we + * need a way to signal that the IPI device has been + * initialized, and that it is ok to call the function. + */ + __smp_store_release(&gd->arch.ipi[reg].valid, 1);
ret = riscv_send_ipi(reg); if (ret) { @@ -83,7 +89,13 @@ void handle_ipi(ulong hart) if (hart >= CONFIG_NR_CPUS) return;
- __smp_mb(); + /* + * If valid is not set, then U-Boot has not requested the IPI. The + * IPI device may not be initialized, so all we can do is wait for + * U-Boot to initialize it and send an IPI + */ + if (!__smp_load_acquire(&gd->arch.ipi[hart].valid)) + return;
smp_function = (void (*)(ulong, ulong, ulong))gd->arch.ipi[hart].addr; invalidate_icache_all();

Even though we no longer call smp_function if an IPI was not sent by U-Boot, we still need to clear any IPIs which were pending from the execution environment. Otherwise, secondary harts will busy-wait in secondary_hart_loop, instead of relaxing.
Along with the previous commit ("riscv: Use a valid bit to ignore already-pending IPIs"), this fixes SMP booting on the Kendryte K210.
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com
---
Changes in v3: - Rename riscv_ipi_init_secondary_hart to dummy_pending_ipi_clear - Only compile dummy_pending_ipi_clear when SMP is enabled
Changes in v2: - Make riscv_ipi_init_secondary_hart static
arch/riscv/cpu/cpu.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index bfa2d4a426..85592f5bee 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -72,6 +72,17 @@ static int riscv_cpu_probe(void) return 0; }
+/* + * This is called on secondary harts just after the IPI is init'd. Currently + * there's nothing to do, since we just need to clear any existing IPIs, and + * that is handled by the sending of an ipi itself. + */ +#if CONFIG_IS_ENABLED(SMP) +static void dummy_pending_ipi_clear(ulong hart, ulong arg0, ulong arg1) +{ +} +#endif + int arch_cpu_init_dm(void) { int ret; @@ -111,6 +122,15 @@ int arch_cpu_init_dm(void) ret = riscv_init_ipi(); if (ret) return ret; + + /* + * Clear all pending IPIs on secondary harts. We don't do anything on + * the boot hart, since we never send an IPI to ourselves, and no + * interrupts are enabled + */ + ret = smp_call_function((ulong)dummy_pending_ipi_clear, 0, 0, 0); + if (ret) + return ret; #endif
return 0;

Even though we no longer call smp_function if an IPI was not sent by U-Boot, we still need to clear any IPIs which were pending from the execution environment. Otherwise, secondary harts will busy-wait in secondary_hart_loop, instead of relaxing.
Along with the previous commit ("riscv: Use a valid bit to ignore already-pending IPIs"), this fixes SMP booting on the Kendryte K210.
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com
Changes in v3:
- Rename riscv_ipi_init_secondary_hart to dummy_pending_ipi_clear
- Only compile dummy_pending_ipi_clear when SMP is enabled
Changes in v2:
- Make riscv_ipi_init_secondary_hart static
arch/riscv/cpu/cpu.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
Reviewed-by: Rick Chen rick@andestech.com

We can reduce the number of instructions needed to use available_harts_lock by using the aq and rl suffixes for AMOs.
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com Reviewed-by: Rick Chen rick@andestech.com ---
(no changes since v2)
Changes in v2: - Remove fences after amoswaps - Reword commit message
arch/riscv/cpu/start.S | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index e3222b1ea7..66ca1c7020 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -125,14 +125,12 @@ call_board_init_f_0:
#ifndef CONFIG_XIP la t0, available_harts_lock - fence rw, w - amoswap.w zero, zero, 0(t0) + amoswap.w.rl zero, zero, 0(t0)
wait_for_gd_init: la t0, available_harts_lock li t1, 1 -1: amoswap.w t1, t1, 0(t0) - fence r, rw +1: amoswap.w.aq t1, t1, 0(t0) bnez t1, 1b
/* register available harts in the available_harts mask */ @@ -142,8 +140,7 @@ wait_for_gd_init: or t2, t2, t1 SREG t2, GD_AVAILABLE_HARTS(gp)
- fence rw, w - amoswap.w zero, zero, 0(t0) + amoswap.w.rl zero, zero, 0(t0)
/* * Continue on hart lottery winner, others branch to

This ensures constructs like `if (gd & gd->...) { ... }` work when accessing the global data pointer. Without this change, it was possible for a very early trap to cause _exit_trap to directly or indirectly (through printf) to read arbitrary memory. This could cause a second trap, preventing show_regs from being printed.
printf (and specifically puts) uses gd to determine what function to print with. These functions in turn use gd to find the serial device, etc. However, before accessing gd, puts first checks to see if it is non-NULL. This indicates an existing (perhaps undocumented) assumption that either gd is NULL or it is completely valid.
Before this patch, gd either points to unexpected data (because it retains the value it did from the prior-stage) or points to uninitialized data (because it has not yet been initialized by board_init_f_init_reserve) until the hart has acquired available_harts_lock. This can cause two problems, depending on the value of gd->flags. If GD_FLG_SERIAL_READY is unset, then some garbage data will be printed to stdout, but there will not be a second trap. However, if GD_FLG_SERIAL_READY is set, then puts will try to print with serial_puts, which will likely cause a second trap.
After this patch, gd is zero up until either a hart has set it in wait_for_gd_init, or until it is set by arch_init_gd. This prevents its usage before its data is initialized because both handle_trap and puts ensure that gd is nonzero before using it. After gd has been set, it is OK to access it because its data has been cleared (and so flags is valid).
XIP cannot use locks because flash is not writable. This leaves it vulnerable to the same class of bugs regarding already-pending IPIs as before this series. Fixing that would require finding another method of synchronization, which is outside the scope of this series.
Fixes: 7c6ca03eae ("riscv: additional crash information") Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com
---
Changes in v3: - Clarify XIP comment
Changes in v2: - Set gp early with XIP
arch/riscv/cpu/start.S | 28 +++++++++++++++++++++++++--- arch/riscv/lib/interrupts.c | 3 ++- 2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 66ca1c7020..eb852538ca 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -47,6 +47,13 @@ _start: mv tp, a0 mv s1, a1
+ /* + * Set the global data pointer to a known value in case we get a very + * early trap. The global data pointer will be set its actual value only + * after it has been initialized. + */ + mv gp, zero + la t0, trap_entry csrw MODE_PREFIX(tvec), t0
@@ -85,10 +92,10 @@ call_board_init_f_0: jal board_init_f_alloc_reserve
/* - * Set global data pointer here for all harts, uninitialized at this - * point. + * Save global data pointer for later. We don't set it here because it + * is not initialized yet. */ - mv gp, a0 + mv s0, a0
/* setup stack */ #if CONFIG_IS_ENABLED(SMP) @@ -109,6 +116,14 @@ call_board_init_f_0: amoswap.w s2, t1, 0(t0) bnez s2, wait_for_gd_init #else + /* + * FIXME: gp is set before it is initialized. If an XIP U-Boot ever + * encounters a pending IPI on boot it is liable to jump to whatever + * memory happens to be in ipi_data.addr on boot. It may also run into + * problems if it encounters an exception too early (because printf/puts + * accesses gd). + */ + mv gp, s0 bnez tp, secondary_hart_loop #endif
@@ -133,6 +148,13 @@ wait_for_gd_init: 1: amoswap.w.aq t1, t1, 0(t0) bnez t1, 1b
+ /* + * Set the global data pointer only when gd_t has been initialized. + * This was already set by arch_setup_gd on the boot hart, but all other + * harts' global data pointers gets set here. + */ + mv gp, s0 + /* register available harts in the available_harts mask */ li t1, 1 sll t1, t1, tp diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index cd47e64487..ad870e98d8 100644 --- a/arch/riscv/lib/interrupts.c +++ b/arch/riscv/lib/interrupts.c @@ -78,7 +78,8 @@ static void _exit_trap(ulong code, ulong epc, ulong tval, struct pt_regs *regs)
printf("EPC: " REG_FMT " RA: " REG_FMT " TVAL: " REG_FMT "\n", epc, regs->ra, tval); - if (gd->flags & GD_FLG_RELOC) + /* Print relocation adjustments, but only if gd is initialized */ + if (gd && gd->flags & GD_FLG_RELOC) printf("EPC: " REG_FMT " RA: " REG_FMT " reloc adjusted\n\n", epc - gd->reloc_off, regs->ra - gd->reloc_off);

This ensures constructs like `if (gd & gd->...) { ... }` work when accessing the global data pointer. Without this change, it was possible for a very early trap to cause _exit_trap to directly or indirectly (through printf) to read arbitrary memory. This could cause a second trap, preventing show_regs from being printed.
printf (and specifically puts) uses gd to determine what function to print with. These functions in turn use gd to find the serial device, etc. However, before accessing gd, puts first checks to see if it is non-NULL. This indicates an existing (perhaps undocumented) assumption that either gd is NULL or it is completely valid.
Before this patch, gd either points to unexpected data (because it retains the value it did from the prior-stage) or points to uninitialized data (because it has not yet been initialized by board_init_f_init_reserve) until the hart has acquired available_harts_lock. This can cause two problems, depending on the value of gd->flags. If GD_FLG_SERIAL_READY is unset, then some garbage data will be printed to stdout, but there will not be a second trap. However, if GD_FLG_SERIAL_READY is set, then puts will try to print with serial_puts, which will likely cause a second trap.
After this patch, gd is zero up until either a hart has set it in wait_for_gd_init, or until it is set by arch_init_gd. This prevents its usage before its data is initialized because both handle_trap and puts ensure that gd is nonzero before using it. After gd has been set, it is OK to access it because its data has been cleared (and so flags is valid).
XIP cannot use locks because flash is not writable. This leaves it vulnerable to the same class of bugs regarding already-pending IPIs as before this series. Fixing that would require finding another method of synchronization, which is outside the scope of this series.
Fixes: 7c6ca03eae ("riscv: additional crash information") Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com
Changes in v3:
- Clarify XIP comment
Changes in v2:
- Set gp early with XIP
arch/riscv/cpu/start.S | 28 +++++++++++++++++++++++++--- arch/riscv/lib/interrupts.c | 3 ++- 2 files changed, 27 insertions(+), 4 deletions(-)
Reviewed-by: Rick Chen rick@andestech.com

This adds comments regarding the ordering and purpose of certain instructions as I understand them.
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com Reviewed-by: Rick Chen rick@andestech.com Reviewed-by: Leo Liang ycliang@andestech.com ---
(no changes since v2)
Changes in v2: - Clarify comments regarding tp
arch/riscv/cpu/start.S | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index eb852538ca..bbc737ed9a 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -43,7 +43,10 @@ _start: csrr a0, CSR_MHARTID #endif
- /* save hart id and dtb pointer */ + /* + * Save hart id and dtb pointer. The thread pointer register is not + * modified by C code. It is used by secondary_hart_loop. + */ mv tp, a0 mv s1, a1
@@ -54,10 +57,18 @@ _start: */ mv gp, zero
+ /* + * Set the trap handler. This must happen after initializing gp because + * the handler may use it. + */ la t0, trap_entry csrw MODE_PREFIX(tvec), t0
- /* mask all interrupts */ + /* + * Mask all interrupts. Interrupts are disabled globally (in m/sstatus) + * for U-Boot, but we will need to read m/sip to determine if we get an + * IPI + */ csrw MODE_PREFIX(ie), zero
#if CONFIG_IS_ENABLED(SMP) @@ -412,6 +423,10 @@ secondary_hart_relocate: mv gp, a2 #endif
+/* + * Interrupts are disabled globally, but they can still be read from m/sip. The + * wfi function will wake us up if we get an IPI, even if we do not trap. + */ secondary_hart_loop: wfi

On 21.09.20 13:51, Sean Anderson wrote:
This adds comments regarding the ordering and purpose of certain instructions as I understand them.
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com Reviewed-by: Rick Chen rick@andestech.com Reviewed-by: Leo Liang ycliang@andestech.com
(no changes since v2)
Changes in v2:
- Clarify comments regarding tp
arch/riscv/cpu/start.S | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index eb852538ca..bbc737ed9a 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -43,7 +43,10 @@ _start: csrr a0, CSR_MHARTID #endif
- /* save hart id and dtb pointer */
- /*
* Save hart id and dtb pointer. The thread pointer register is not
* modified by C code. It is used by secondary_hart_loop.
mv tp, a0 mv s1, a1*/
I would like to understand the implications for the UEFI sub-system.
The current design only takes care of the conservation of the global data gd.
When U-Boot calls a UEFI payload it saves the value of gd (register gp, x3) in a variable.
When the payload calls the UEFI API implemented in U-Boot we store the payload's value of register gp and restore the U-Boot value (see macro EFI_ENTRY).
Before returning from the UEFI call we restore the payload's value of gp (see macro EFI_EXIT).
When a payload returns to the U-Boot shell we restore gp.
Up to now we do not take care of any other register. I am not aware of any specification requiring register tp to be conserved by the UEFI payload.
Is there any other register that has to be conserved except gp?
Best regards
Heinrich
@@ -54,10 +57,18 @@ _start: */ mv gp, zero
- /*
* Set the trap handler. This must happen after initializing gp because
* the handler may use it.
la t0, trap_entry csrw MODE_PREFIX(tvec), t0*/
- /* mask all interrupts */
- /*
* Mask all interrupts. Interrupts are disabled globally (in m/sstatus)
* for U-Boot, but we will need to read m/sip to determine if we get an
* IPI
csrw MODE_PREFIX(ie), zero*/
#if CONFIG_IS_ENABLED(SMP) @@ -412,6 +423,10 @@ secondary_hart_relocate: mv gp, a2 #endif
+/*
- Interrupts are disabled globally, but they can still be read from m/sip. The
- wfi function will wake us up if we get an IPI, even if we do not trap.
- */
secondary_hart_loop: wfi

On 9/21/20 12:23 PM, Heinrich Schuchardt wrote:
On 21.09.20 13:51, Sean Anderson wrote:
This adds comments regarding the ordering and purpose of certain instructions as I understand them.
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bin.meng@windriver.com Reviewed-by: Rick Chen rick@andestech.com Reviewed-by: Leo Liang ycliang@andestech.com
(no changes since v2)
Changes in v2:
- Clarify comments regarding tp
arch/riscv/cpu/start.S | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index eb852538ca..bbc737ed9a 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -43,7 +43,10 @@ _start: csrr a0, CSR_MHARTID #endif
- /* save hart id and dtb pointer */
- /*
* Save hart id and dtb pointer. The thread pointer register is not
* modified by C code. It is used by secondary_hart_loop.
mv tp, a0 mv s1, a1*/
I would like to understand the implications for the UEFI sub-system.
The current design only takes care of the conservation of the global data gd.
When U-Boot calls a UEFI payload it saves the value of gd (register gp, x3) in a variable.
When the payload calls the UEFI API implemented in U-Boot we store the payload's value of register gp and restore the U-Boot value (see macro EFI_ENTRY).
Before returning from the UEFI call we restore the payload's value of gp (see macro EFI_EXIT).
When a payload returns to the U-Boot shell we restore gp.
Up to now we do not take care of any other register. I am not aware of any specification requiring register tp to be conserved by the UEFI payload.
Is there any other register that has to be conserved except gp?
So the core problem here is that there is no way to get the current hart id from S-Mode, except by saving it from a0 on entry. I believe an SBI call was proposed, but it was deemed unnecessary. Unfortunately, gp is already in-use for gd. On the boot hart, the current hart id is stored in gd->arch.boot_hart, so tp is not used. On secondary harts, the hart id is stored in tp, and must be valid for secondary_hart_loop. AFAIK tp is the only other register besides gp which needs to be preserved on secondary harts.
When we spoke on IRC a few weeks ago, I you said that only the boot hart enters UEFI payloads. So there should be no need to save tp for UEFI, because it is unused on the boot hart. If a payload does not return to U-Boot, there is also no need to save tp on secondary harts. The only issue would be if a payload is started on secondary harts and then returns to U-Boot.
--Sean
participants (3)
-
Heinrich Schuchardt
-
Rick Chen
-
Sean Anderson