| Message ID | 20210118155153.21520-1-manivannan.sadhasivam@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: kernel: Fix interrupted SMC calls | expand |
On Mon, Jan 18, 2021 at 09:21:53PM +0530, Manivannan Sadhasivam wrote: > @@ -27,10 +29,18 @@ UNWIND( .fnstart) > UNWIND( .save {r4-r7}) > ldm r12, {r4-r7} > \instr > + mov r9, r6 // Copy r6 before popping from stack > pop {r4-r7} > ldr r12, [sp, #(4 * 4)] > stm r12, {r0-r3} > - bx lr > + ldr r10, [sp, #20] > + cmp r10, #0 > + beq 1f // No quirk structure > + ldr r11, [r10, #ARM_SMCCC_QUIRK_ID_OFFS] > + cmp r11, #ARM_SMCCC_QUIRK_QCOM_A6 > + bne 1f // No quirk present > + str r9, [r10, #ARM_SMCCC_QUIRK_STATE_OFFS] > +1: bx lr NAK. This patch shows little regard for the C ABI. You are corrupting registers that must be preserved. Please find out about the C ABIs that are used on ARM. Thanks.
Hi Russel, On Mon, Jan 18, 2021 at 04:08:07PM +0000, Russell King - ARM Linux admin wrote: > On Mon, Jan 18, 2021 at 09:21:53PM +0530, Manivannan Sadhasivam wrote: > > @@ -27,10 +29,18 @@ UNWIND( .fnstart) > > UNWIND( .save {r4-r7}) > > ldm r12, {r4-r7} > > \instr > > + mov r9, r6 // Copy r6 before popping from stack > > pop {r4-r7} > > ldr r12, [sp, #(4 * 4)] > > stm r12, {r0-r3} > > - bx lr > > + ldr r10, [sp, #20] > > + cmp r10, #0 > > + beq 1f // No quirk structure > > + ldr r11, [r10, #ARM_SMCCC_QUIRK_ID_OFFS] > > + cmp r11, #ARM_SMCCC_QUIRK_QCOM_A6 > > + bne 1f // No quirk present > > + str r9, [r10, #ARM_SMCCC_QUIRK_STATE_OFFS] > > +1: bx lr > > NAK. This patch shows little regard for the C ABI. You are corrupting > registers that must be preserved. Please find out about the C ABIs > that are used on ARM. > Sorry, my asm foo is not that great. Should've checked AAPCS. Will fix it in next revision. Thanks, Mani > Thanks. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c index a1570c8bab25..2e2fa6fc2d4f 100644 --- a/arch/arm/kernel/asm-offsets.c +++ b/arch/arm/kernel/asm-offsets.c @@ -23,6 +23,7 @@ #include <asm/vdso_datapage.h> #include <asm/hardware/cache-l2x0.h> #include <linux/kbuild.h> +#include <linux/arm-smccc.h> #include "signal.h" /* @@ -147,6 +148,8 @@ int main(void) DEFINE(SLEEP_SAVE_SP_PHYS, offsetof(struct sleep_save_sp, save_ptr_stash_phys)); DEFINE(SLEEP_SAVE_SP_VIRT, offsetof(struct sleep_save_sp, save_ptr_stash)); #endif + DEFINE(ARM_SMCCC_QUIRK_ID_OFFS, offsetof(struct arm_smccc_quirk, id)); + DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS, offsetof(struct arm_smccc_quirk, state)); BLANK(); DEFINE(DMA_BIDIRECTIONAL, DMA_BIDIRECTIONAL); DEFINE(DMA_TO_DEVICE, DMA_TO_DEVICE); diff --git a/arch/arm/kernel/smccc-call.S b/arch/arm/kernel/smccc-call.S index 00664c78faca..e1ab00b8244b 100644 --- a/arch/arm/kernel/smccc-call.S +++ b/arch/arm/kernel/smccc-call.S @@ -3,7 +3,9 @@ * Copyright (c) 2015, Linaro Limited */ #include <linux/linkage.h> +#include <linux/arm-smccc.h> +#include <asm/asm-offsets.h> #include <asm/opcodes-sec.h> #include <asm/opcodes-virt.h> #include <asm/unwind.h> @@ -27,10 +29,18 @@ UNWIND( .fnstart) UNWIND( .save {r4-r7}) ldm r12, {r4-r7} \instr + mov r9, r6 // Copy r6 before popping from stack pop {r4-r7} ldr r12, [sp, #(4 * 4)] stm r12, {r0-r3} - bx lr + ldr r10, [sp, #20] + cmp r10, #0 + beq 1f // No quirk structure + ldr r11, [r10, #ARM_SMCCC_QUIRK_ID_OFFS] + cmp r11, #ARM_SMCCC_QUIRK_QCOM_A6 + bne 1f // No quirk present + str r9, [r10, #ARM_SMCCC_QUIRK_STATE_OFFS] +1: bx lr UNWIND( .fnend) .endm
On Qualcomm ARM32 platforms, the SMC call can return before it has completed. If this occurs, the call can be restarted, but it requires using the returned session ID value from the interrupted SMC call. The ARM32 SMCC code already has the provision to add platform specific quirks for things like this. So let's make use of it and add the Qualcomm specific quirk (ARM_SMCCC_QUIRK_QCOM_A6) used by the QCOM_SCM driver. This change is similar to the below one added for ARM64 a while ago: commit 82bcd087029f ("firmware: qcom: scm: Fix interrupted SCM calls") Without this change, the Qualcomm ARM32 platforms like SDX55 will return -EINVAL for SMC calls used for modem firmware loading and validation. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- arch/arm/kernel/asm-offsets.c | 3 +++ arch/arm/kernel/smccc-call.S | 12 +++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-)