[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC 1/4] arm: add SMC wrapper that is compatible with SMCCC



Hi Volodymyr,

On 11/10/17 20:01, 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>
---
  xen/arch/arm/arm32/Makefile     |  1 +
  xen/arch/arm/arm32/smc.S        | 32 ++++++++++++++++++++++++++++++++
  xen/arch/arm/arm64/Makefile     |  1 +
  xen/arch/arm/arm64/smc.S        | 29 +++++++++++++++++++++++++++++
  xen/include/asm-arm/processor.h |  4 ++++
  5 files changed, 67 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..1cc9528
--- /dev/null
+++ b/xen/arch/arm/arm32/smc.S
@@ -0,0 +1,32 @@
+/*
+ * 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.
+ *
+ * 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, register_t res[4])
+ */
+ENTRY(call_smccc_smc)
+        mov     r12, sp
+        push    {r4-r7}
+        ldm     r12, {r4-r7}
+        smc     #0
+        pop     {r4-r7}
+        ldr     r12, [sp, #(4 * 4)]

Can we get some comment in the code to explain the hardcoded values? And maybe introduce defines?

+        stm     r12, {r0-r3}
+        bx      lr
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 718fe44..58a8ddd 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -8,6 +8,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/smc.S b/xen/arch/arm/arm64/smc.S
new file mode 100644
index 0000000..aa44fba
--- /dev/null
+++ b/xen/arch/arm/arm64/smc.S
@@ -0,0 +1,29 @@
+/*
+ * 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.
+ *
+ * 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, register_t res[4])
+ */
+ENTRY(call_smccc_smc)
+        smc     #0
+        ldr     x4, [sp]
+        stp     x0, x1, [x4, 0]
+        stp     x2, x3, [x4, 16]

Ditto.

+        ret
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index dc6ab62..71f3e60 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -787,6 +787,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, register_t res[4]);

I would much prefer if you introduce a structure describing the result. This would make easier for the caller to understand what the result look like and avoid to hardcode some values in the code.

So how Linux does it.

+
  void do_trap_hyp_serror(struct cpu_user_regs *regs);
void do_trap_guest_serror(struct cpu_user_regs *regs);


Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.