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

2017-10-16 Thread Julien Grall 

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" 

Signed-off-by: Volodymyr Babchuk 
---
  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 000..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 
+
+/*
+ * 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 000..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 
+
+/*
+ * 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);

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

2017-10-11 Thread Volodymyr Babchuk 
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" 

Signed-off-by: Volodymyr Babchuk 
---
 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 000..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 
+
+/*
+ * 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)]
+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 000..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 
+
+/*
+ * 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]
+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]);
+
 void do_trap_hyp_serror(struct cpu_user_regs *regs);
 
 void do_trap_guest_serror(struct cpu_user_regs *regs);
-- 
2.7.4


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