[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v1 1/6] arm: add SMC wrapper that is compatible with SMCCC
- To: Julien Grall <julien.grall@xxxxxxx>, xen-devel@xxxxxxxxxxxxx, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>
- From: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
- Date: Thu, 23 Aug 2018 17:35:25 +0300
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Volodymyr_Babchuk@xxxxxxxx;
- Cc: tee-dev@xxxxxxxxxxxxxxxx, "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
- Delivery-date: Thu, 23 Aug 2018 14:35:56 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
Hi,
On 22.08.18 19:46, Julien Grall wrote:
Hi Volodymyr,
On 22/08/18 15:11, Volodymyr Babchuk wrote:
Existing SMC wrapper call_smc() allows only 4 parameters and
returns only one value. This is enough for existing
use in PSCI code, but TEE mediator will need a call that is
fully compatible with ARM SMCCC.
This patch adds this call for both arm32 and arm64.
There was similar patch by Edgar E. Iglesias ([1]), but looks
like it is abandoned.
[1]
https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg00636.html
CC: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
---
changes from "RFC":
- response now stored in structure instead of array
- added comments for arm32 assembly code
- added offset (instead of magic values) for arm32 asm code
Did you mean arm64?
Yes, you are right.
xen/arch/arm/arm32/Makefile | 1 +
xen/arch/arm/arm32/smc.S | 39
+++++++++++++++++++++++++++++++++++++++
xen/arch/arm/arm64/Makefile | 1 +
xen/arch/arm/arm64/asm-offsets.c | 4 ++++
xen/arch/arm/arm64/smc.S | 30 ++++++++++++++++++++++++++++++
xen/include/asm-arm/processor.h | 11 +++++++++++
6 files changed, 86 insertions(+)
create mode 100644 xen/arch/arm/arm32/smc.S
create mode 100644 xen/arch/arm/arm64/smc.S
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 0ac254f..c69f35e 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -10,4 +10,5 @@ obj-y += proc-v7.o proc-caxx.o
obj-y += smpboot.o
obj-y += traps.o
obj-y += vfp.o
+obj-y += smc.o
diff --git a/xen/arch/arm/arm32/smc.S b/xen/arch/arm/arm32/smc.S
new file mode 100644
index 0000000..56f7982
--- /dev/null
+++ b/xen/arch/arm/arm32/smc.S
@@ -0,0 +1,39 @@
+/*
+ * xen/arch/arm/arm32/smc.S
+ *
+ * Wrapper for Secure Monitors Calls
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
Xen is GPLv2 only. Can you please update the copyright accordingly?
Also, the code is pretty much a copy of the Linux part. So it is
probably worth to keep the copy-right around.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/macros.h>
+
+/*
+ * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
+ * register_t a3, register_t a4, register_t a5,
+ * register_t a6, register_t a7, struct smccc_res
*res)
+ */
+ENTRY(call_smccc_smc)
I would rather avoid to have 2 function to handle the same convention.
Can you please either drop call_smc or rework it to handle more parameters?
But it looks like you could re-use arm_smccc_1_1 as it follows the 1.0
convention for 32-bits.
Oops, I missed introduction of arm_smccc_1_1. I think, it will be
sufficient for my tasks. Looks like, I'll drop this patch at all.
I'm sorry for bothering you.
+ mov r12, sp
+ push {r4-r7}
+ /*
+ * According to ARMv7 (or aarch32) ABI, first 4 parameters of
+ * call_smccc_smc are passed in r0-r3 and other 4 are on stack.
+ * Load a4-a7 from stack to r4-r7.
+ */
+ ldm r12, {r4-r7}
+ smc #0
+ pop {r4-r7}
+ /* Load pointer to res (4th parameter on stack) to r12 */
+ ldr r12, [sp, #(4 * 4)]
+ /* Store returned results from r0-r3 to res */
+ stm r12, {r0-r3}
+ bx lr
Something looks wrong with the indentation. There seem to be a mix of
hard tab and soft tab.
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index bb5c610..7ecdad7 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -9,6 +9,7 @@ obj-y += entry.o
obj-y += insn.o
obj-$(CONFIG_LIVEPATCH) += livepatch.o
obj-y += smpboot.o
+obj-y += smc.o
obj-y += traps.o
obj-y += vfp.o
obj-y += vsysreg.o
diff --git a/xen/arch/arm/arm64/asm-offsets.c
b/xen/arch/arm/arm64/asm-offsets.c
index ce24e44..5353fe8 100644
--- a/xen/arch/arm/arm64/asm-offsets.c
+++ b/xen/arch/arm/arm64/asm-offsets.c
@@ -50,6 +50,10 @@ void __dummy__(void)
BLANK();
OFFSET(INITINFO_stack, struct init_info, stack);
+
+ BLANK();
+ OFFSET(SMCCC_RES_a0, struct smccc_res, a0);
+ OFFSET(SMCCC_RES_a2, struct smccc_res, a2);
}
/*
diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
new file mode 100644
index 0000000..40843c0
--- /dev/null
+++ b/xen/arch/arm/arm64/smc.S
@@ -0,0 +1,30 @@
+/*
+ * xen/arch/arm/arm64/smc.S
+ *
+ * Wrapper for Secure Monitors Calls
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
Xen is GPLv2 only. Can you please update the copyright accordingly?
Also, the code is pretty much a copy of the Linux part. So it is
probably worth to keep the copy-right around.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/asm_defns.h>
+#include <asm/macros.h>
+
+/*
+ * void call_smccc_smc(register_t a0, register_t a1, register_t a2,
+ * register_t a3, register_t a4, register_t a5,
+ * register_t a6, register_t a7, struct smccc_res
*res)
+ */
+ENTRY(call_smccc_smc)
+ smc #0
+ ldr x4, [sp]
+ stp x0, x1, [x4, #SMCCC_RES_a0]
+ stp x2, x3, [x4, #SMCCC_RES_a2]
+ ret
diff --git a/xen/include/asm-arm/processor.h
b/xen/include/asm-arm/processor.h
index 222a02d..946a2db 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -650,6 +650,13 @@ union hsr {
};
+
+struct smccc_res {
+ register_t a0;
+ register_t a1;
+ register_t a2;
+ register_t a3;
+};
Any reason to not use arm_smccc_res?
#endif
/* HSR.EC == HSR_CP{15,14,10}_32 */
@@ -815,6 +822,10 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
int call_smc(register_t function_id, register_t arg0, register_t arg1,
register_t arg2);
+void call_smccc_smc(register_t a0, register_t a1, register_t a2,
+ register_t a3, register_t a4, register_t a5,
+ register_t a6, register_t a7, struct smccc_res
*res);
+
void do_trap_hyp_serror(struct cpu_user_regs *regs);
void do_trap_guest_serror(struct cpu_user_regs *regs);
Cheers,
--
Volodymyr Babchuk
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
