Skip to contentSkip to conversation

Arm64: introduce __hyp_func_call

RFC PATCH New Arun Chandran 2 participants 5 messages Aug 27, 2014 – Sep 1, 2014 arm64

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 Arm64: introduce __hyp_func_call

This adds a mechanism to __hyp_stub_vectors to allow a hypercall to call a function at EL2. It is needed for users who want to run a part of code with EL2 permissions. The current usecase is for KVM and kexec.

For kexec we need to move the final CPU up to the mode it started in before we branch to the new kernel. If we don't do that

* We loose EL2 in the next boot
* Arm64 bootwrapper may not be able to put CPUs at the spin-table code. It expects the final jump from kernel to cpu-return-addr to be done in EL2.

KVM can use this to set/get VBAR_EL2

---
Idea is from "Mark Rutland <[email protected]>"
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/280026.html

---
 arch/arm64/include/asm/virt.h |   15 +++++++++++++++
 arch/arm64/kernel/hyp-stub.S  |   33 +++++++++++++++++++++------------
 2 files changed, 36 insertions(+), 12 deletions(-)
arch/arm64/include/asm/virt.h +15 -0
index 7a5df52..910a163 100644
@@ -34,9 +34,24 @@
  */
 extern u32 __boot_cpu_mode[2];
 
+void *__hyp_func_call(u64 __tmp, phys_addr_t func, ...);
 void __hyp_set_vectors(phys_addr_t phys_vector_base);
 phys_addr_t __hyp_get_vectors(void);
 
+#define __hyp_set_vectors(__vbase)					    \
+({									    \
+	u64 __tmp = 0;							    \
+	__hyp_func_call(__tmp, virt_to_phys(__hyp_set_vectors), __vbase);   \
+})
+
+#define __hyp_get_vectors()						    \
+({									    \
+	u64 __tmp = 0;							    \
+	phys_addr_t ret = (phys_addr_t) __hyp_func_call(__tmp,		    \
+					virt_to_phys(__hyp_get_vectors));   \
+	ret;								    \
+})
+
 /* Reports the availability of HYP mode */
 static inline bool is_hyp_mode_available(void)
 {
arch/arm64/kernel/hyp-stub.S +21 -12
index a272f33..2144a3f 100644
@@ -53,15 +53,14 @@ ENDPROC(__hyp_stub_vectors)
 	.align 11
 
 el1_sync:
-	mrs	x1, esr_el2
-	lsr	x1, x1, #26
-	cmp	x1, #0x16
+	mrs	x0, esr_el2
+	lsr	x0, x0, #26
+	cmp	x0, #0x16
 	b.ne	2f				// Not an HVC trap
-	cbz	x0, 1f
-	msr	vbar_el2, x0			// Set vbar_el2
-	b	2f
-1:	mrs	x0, vbar_el2			// Return vbar_el2
-2:	eret
+
+1:	blr	x1 				// Jump to the function
+2:	mov	x0, xzr				// esr_el2 not readable <= el2
+	eret
 ENDPROC(el1_sync)
 
 .macro invalid_vector	label
@@ -101,10 +100,20 @@ ENDPROC(\label)
  */
 
 ENTRY(__hyp_get_vectors)
-	mov	x0, xzr
-	// fall through
-ENTRY(__hyp_set_vectors)
-	hvc	#0
+	mrs	x0, vbar_el2			// Return vbar_el2
 	ret
 ENDPROC(__hyp_get_vectors)
+
+ENTRY(__hyp_set_vectors)
+	msr	vbar_el2, x2
+	ret
 ENDPROC(__hyp_set_vectors)
+
+/* Call a function @x1 */
+ENTRY(__hyp_func_call)
+	/* Save lr here */
+	msr	elr_el1, x30
+	hvc	#0
+	mrs	x30, elr_el1
+	ret
+ENDPROC(__hyp_func_call)
Signature · 1 line
1.7.9.5
#1 lore raw

RFC PATCH 1/1 Arm64: introduce __hyp_func_call

Quoted 8 lines · Arun Chandran

This adds a mechanism to __hyp_stub_vectors to allow a hypercall to call a function at EL2. It is needed for users who want to run a part of code with EL2 permissions. The current usecase is for KVM and kexec.

For kexec we need to move the final CPU up to the mode it started in before we branch to the new kernel. If we don't do that

Quoted 6 lines

* We loose EL2 in the next boot
* Arm64 bootwrapper may not be able to put CPUs at the spin-table code. It expects the final jump from kernel to cpu-return-addr to be done in EL2.

KVM can use this to set/get VBAR_EL2

So you've now broken KVM on arm64, since this patch doesn't address the callers. But if you do so, you also break KVM on 32bit ARM, as we share the calling code.

So you'll have to work a bit more before this can be an acceptable solution.

Thanks,

	M.
Quoted 90 lines

---
Idea is from "Mark Rutland <[email protected]>"
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/280026.html

---
 arch/arm64/include/asm/virt.h |   15 +++++++++++++++
 arch/arm64/kernel/hyp-stub.S  |   33 +++++++++++++++++++++------------
 2 files changed, 36 insertions(+), 12 deletions(-)
arch/arm64/include/asm/virt.h +15 -0
index 7a5df52..910a163 100644
@@ -34,9 +34,24 @@
  */
 extern u32 __boot_cpu_mode[2];
 
+void *__hyp_func_call(u64 __tmp, phys_addr_t func, ...);
 void __hyp_set_vectors(phys_addr_t phys_vector_base);
 phys_addr_t __hyp_get_vectors(void);
 
+#define __hyp_set_vectors(__vbase)					    \
+({									    \
+	u64 __tmp = 0;							    \
+	__hyp_func_call(__tmp, virt_to_phys(__hyp_set_vectors), __vbase);   \
+})
+
+#define __hyp_get_vectors()						    \
+({									    \
+	u64 __tmp = 0;							    \
+	phys_addr_t ret = (phys_addr_t) __hyp_func_call(__tmp,		    \
+					virt_to_phys(__hyp_get_vectors));   \
+	ret;								    \
+})
+
 /* Reports the availability of HYP mode */
 static inline bool is_hyp_mode_available(void)
 {
arch/arm64/kernel/hyp-stub.S +21 -12
index a272f33..2144a3f 100644
@@ -53,15 +53,14 @@ ENDPROC(__hyp_stub_vectors)
 	.align 11
 
 el1_sync:
-	mrs	x1, esr_el2
-	lsr	x1, x1, #26
-	cmp	x1, #0x16
+	mrs	x0, esr_el2
+	lsr	x0, x0, #26
+	cmp	x0, #0x16
 	b.ne	2f				// Not an HVC trap
-	cbz	x0, 1f
-	msr	vbar_el2, x0			// Set vbar_el2
-	b	2f
-1:	mrs	x0, vbar_el2			// Return vbar_el2
-2:	eret
+
+1:	blr	x1 				// Jump to the function
+2:	mov	x0, xzr				// esr_el2 not readable <= el2
+	eret
 ENDPROC(el1_sync)
 
 .macro invalid_vector	label
@@ -101,10 +100,20 @@ ENDPROC(\label)
  */
 
 ENTRY(__hyp_get_vectors)
-	mov	x0, xzr
-	// fall through
-ENTRY(__hyp_set_vectors)
-	hvc	#0
+	mrs	x0, vbar_el2			// Return vbar_el2
 	ret
 ENDPROC(__hyp_get_vectors)
+
+ENTRY(__hyp_set_vectors)
+	msr	vbar_el2, x2
+	ret
 ENDPROC(__hyp_set_vectors)
+
+/* Call a function @x1 */
+ENTRY(__hyp_func_call)
+	/* Save lr here */
+	msr	elr_el1, x30
+	hvc	#0
+	mrs	x30, elr_el1
+	ret
+ENDPROC(__hyp_func_call)
Signature · 1 line
Jazz is not dead. It just smells funny.
#2 lore raw

RFC PATCH 1/1 Arm64: introduce __hyp_func_call

In reply to Arun Chandran
Quoted 8 lines · Arun Chandran

This adds a mechanism to __hyp_stub_vectors to allow a hypercall to call a function at EL2. It is needed for users who want to run a part of code with EL2 permissions. The current usecase is for KVM and kexec.

For kexec we need to move the final CPU up to the mode it started in before we branch to the new kernel. If we don't do that

Quoted 6 lines

* We loose EL2 in the next boot
* Arm64 bootwrapper may not be able to put CPUs at the spin-table code. It expects the final jump from kernel to cpu-return-addr to be done in EL2.

KVM can use this to set/get VBAR_EL2

Ah, looking at this a bit more, I see what you've done (missed the #define trickery below).

Quoted 25 lines

---
Idea is from "Mark Rutland <[email protected]>"
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/280026.html

---
 arch/arm64/include/asm/virt.h |   15 +++++++++++++++
 arch/arm64/kernel/hyp-stub.S  |   33 +++++++++++++++++++++------------
 2 files changed, 36 insertions(+), 12 deletions(-)
arch/arm64/include/asm/virt.h +5 -0
index 7a5df52..910a163 100644
@@ -34,9 +34,24 @@
  */
 extern u32 __boot_cpu_mode[2];
 
+void *__hyp_func_call(u64 __tmp, phys_addr_t func, ...);
 void __hyp_set_vectors(phys_addr_t phys_vector_base);
 phys_addr_t __hyp_get_vectors(void);
 
+#define __hyp_set_vectors(__vbase)					    \
+({									    \
+	u64 __tmp = 0;							    \
+	__hyp_func_call(__tmp, virt_to_phys(__hyp_set_vectors), __vbase);   \

arch/arm64/include/asm/virt.h line 54

Quoted 10 lines
arch/arm64/include/asm/virt.h +10 -0
+})
+
+#define __hyp_get_vectors()						    \
+({									    \
+	u64 __tmp = 0;							    \
+	phys_addr_t ret = (phys_addr_t) __hyp_func_call(__tmp,		    \
+					virt_to_phys(__hyp_get_vectors));   \
+	ret;								    \
+})
+

This is what has thrown me off the wrong path. Don't do that, this is horrid. Just rename the assembly entry points, it will make things a lot clearer.

Thanks,

	M.
Quoted 56 lines
arch/arm64/include/asm/virt.h +0 -0
 /* Reports the availability of HYP mode */
 static inline bool is_hyp_mode_available(void)
 {
arch/arm64/kernel/hyp-stub.S +21 -12
index a272f33..2144a3f 100644
@@ -53,15 +53,14 @@ ENDPROC(__hyp_stub_vectors)
 	.align 11
 
 el1_sync:
-	mrs	x1, esr_el2
-	lsr	x1, x1, #26
-	cmp	x1, #0x16
+	mrs	x0, esr_el2
+	lsr	x0, x0, #26
+	cmp	x0, #0x16
 	b.ne	2f				// Not an HVC trap
-	cbz	x0, 1f
-	msr	vbar_el2, x0			// Set vbar_el2
-	b	2f
-1:	mrs	x0, vbar_el2			// Return vbar_el2
-2:	eret
+
+1:	blr	x1 				// Jump to the function
+2:	mov	x0, xzr				// esr_el2 not readable <= el2
+	eret
 ENDPROC(el1_sync)
 
 .macro invalid_vector	label
@@ -101,10 +100,20 @@ ENDPROC(\label)
  */
 
 ENTRY(__hyp_get_vectors)
-	mov	x0, xzr
-	// fall through
-ENTRY(__hyp_set_vectors)
-	hvc	#0
+	mrs	x0, vbar_el2			// Return vbar_el2
 	ret
 ENDPROC(__hyp_get_vectors)
+
+ENTRY(__hyp_set_vectors)
+	msr	vbar_el2, x2
+	ret
 ENDPROC(__hyp_set_vectors)
+
+/* Call a function @x1 */
+ENTRY(__hyp_func_call)
+	/* Save lr here */
+	msr	elr_el1, x30
+	hvc	#0
+	mrs	x30, elr_el1
+	ret
+ENDPROC(__hyp_func_call)
Signature · 1 line
Jazz is not dead. It just smells funny...
#3 lore raw

RFC PATCH 1/1 Arm64: introduce __hyp_func_call

Hi Marc,

Quoted 50 lines · Marc Zyngier
Quoted 8 lines · Arun Chandran

This adds a mechanism to __hyp_stub_vectors to allow a hypercall to call a function at EL2. It is needed for users who want to run a part of code with EL2 permissions. The current usecase is for KVM and kexec.

For kexec we need to move the final CPU up to the mode it started in before we branch to the new kernel. If we don't do that

Quoted 6 lines

* We loose EL2 in the next boot
* Arm64 bootwrapper may not be able to put CPUs at the spin-table code. It expects the final jump from kernel to cpu-return-addr to be done in EL2.

KVM can use this to set/get VBAR_EL2

Ah, looking at this a bit more, I see what you've done (missed the #define trickery below).

Quoted 31 lines

---
Idea is from "Mark Rutland <[email protected]>"
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/280026.html

---
 arch/arm64/include/asm/virt.h |   15 +++++++++++++++
 arch/arm64/kernel/hyp-stub.S  |   33 +++++++++++++++++++++------------
 2 files changed, 36 insertions(+), 12 deletions(-)
arch/arm64/include/asm/virt.h +11 -0
index 7a5df52..910a163 100644
@@ -34,9 +34,24 @@
  */
 extern u32 __boot_cpu_mode[2];
 
+void *__hyp_func_call(u64 __tmp, phys_addr_t func, ...);
 void __hyp_set_vectors(phys_addr_t phys_vector_base);
 phys_addr_t __hyp_get_vectors(void);
 
+#define __hyp_set_vectors(__vbase)                                       \
+({                                                                       \
+     u64 __tmp = 0;                                                      \
+     __hyp_func_call(__tmp, virt_to_phys(__hyp_set_vectors), __vbase);   \
+})
+
+#define __hyp_get_vectors()                                              \
+({                                                                       \
+     u64 __tmp = 0;                                                      \
+     phys_addr_t ret = (phys_addr_t) __hyp_func_call(__tmp,              \
Quoted 9 lines

arch/arm64/include/asm/virt.h line 54

Quoted 4 lines
arch/arm64/include/asm/virt.h +4 -0
+                                     virt_to_phys(__hyp_get_vectors));   \
+     ret;                                                                \
+})
+

This is what has thrown me off the wrong path. Don't do that, this is horrid. Just rename the assembly entry points, it will make things a lot clearer.

Ok. I will post another one.

--Arun

Quoted 64 lines

Thanks,

        M.
Quoted 46 lines
arch/arm64/include/asm/virt.h +0 -0
 /* Reports the availability of HYP mode */
 static inline bool is_hyp_mode_available(void)
 {
arch/arm64/kernel/hyp-stub.S +12 -12
index a272f33..2144a3f 100644
@@ -53,15 +53,14 @@ ENDPROC(__hyp_stub_vectors)
      .align 11
 
 el1_sync:
-     mrs     x1, esr_el2
-     lsr     x1, x1, #26
-     cmp     x1, #0x16
+     mrs     x0, esr_el2
+     lsr     x0, x0, #26
+     cmp     x0, #0x16
      b.ne    2f                              // Not an HVC trap
-     cbz     x0, 1f
-     msr     vbar_el2, x0                    // Set vbar_el2
-     b       2f
-1:   mrs     x0, vbar_el2                    // Return vbar_el2
-2:   eret
+
+1:   blr     x1                              // Jump to the function
+2:   mov     x0, xzr                         // esr_el2 not readable <= el2
+     eret
 ENDPROC(el1_sync)
 
 .macro invalid_vector        label
@@ -101,10 +100,20 @@ ENDPROC(\label)
  */
 
 ENTRY(__hyp_get_vectors)
-     mov     x0, xzr
-     // fall through
-ENTRY(__hyp_set_vectors)
-     hvc     #0
+     mrs     x0, vbar_el2                    // Return vbar_el2
      ret
 ENDPROC(__hyp_get_vectors)
+
+ENTRY(__hyp_set_vectors)
+     msr     vbar_el2, x2
+     ret

arch/arm64/kernel/hyp-stub.S ENDPROC(\label) · line 119

Quoted 10 lines
arch/arm64/kernel/hyp-stub.S +9 -0
 ENDPROC(__hyp_set_vectors)
+
+/* Call a function @x1 */
+ENTRY(__hyp_func_call)
+     /* Save lr here */
+     msr     elr_el1, x30
+     hvc     #0
+     mrs     x30, elr_el1
+     ret
+ENDPROC(__hyp_func_call)

--
Jazz is not dead. It just smells funny...

#4 lore raw

RFC PATCH 1/1 Arm64: introduce __hyp_func_call

Quoted 63 lines · Arun Chandran

Hi Marc,

Quoted 50 lines · Marc Zyngier
Quoted 8 lines · Arun Chandran

This adds a mechanism to __hyp_stub_vectors to allow a hypercall to call a function at EL2. It is needed for users who want to run a part of code with EL2 permissions. The current usecase is for KVM and kexec.

For kexec we need to move the final CPU up to the mode it started in before we branch to the new kernel. If we don't do that

Quoted 6 lines

* We loose EL2 in the next boot
* Arm64 bootwrapper may not be able to put CPUs at the spin-table code. It expects the final jump from kernel to cpu-return-addr to be done in EL2.

KVM can use this to set/get VBAR_EL2

Ah, looking at this a bit more, I see what you've done (missed the #define trickery below).

Quoted 31 lines

---
Idea is from "Mark Rutland <[email protected]>"
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/280026.html

---
 arch/arm64/include/asm/virt.h |   15 +++++++++++++++
 arch/arm64/kernel/hyp-stub.S  |   33 +++++++++++++++++++++------------
 2 files changed, 36 insertions(+), 12 deletions(-)
arch/arm64/include/asm/virt.h +11 -0
index 7a5df52..910a163 100644
@@ -34,9 +34,24 @@
  */
 extern u32 __boot_cpu_mode[2];
 
+void *__hyp_func_call(u64 __tmp, phys_addr_t func, ...);
 void __hyp_set_vectors(phys_addr_t phys_vector_base);
 phys_addr_t __hyp_get_vectors(void);
 
+#define __hyp_set_vectors(__vbase)                                       \
+({                                                                       \
+     u64 __tmp = 0;                                                      \
+     __hyp_func_call(__tmp, virt_to_phys(__hyp_set_vectors), __vbase);   \
+})
+
+#define __hyp_get_vectors()                                              \
+({                                                                       \
+     u64 __tmp = 0;                                                      \
+     phys_addr_t ret = (phys_addr_t) __hyp_func_call(__tmp,              \
Quoted 9 lines

arch/arm64/include/asm/virt.h line 54

Quoted 4 lines
arch/arm64/include/asm/virt.h +4 -0
+                                     virt_to_phys(__hyp_get_vectors));   \
+     ret;                                                                \
+})
+

This is what has thrown me off the wrong path. Don't do that, this is horrid. Just rename the assembly entry points, it will make things a lot clearer.

Quoted 1 line

Ok. I will post another one.

Hold on, there are more problems.

You've changed the case that works when KVM is *not* running. But if you look at the KVM code, you'll see that it has its own implementation of __hyp_get_vectors. What will happen when KVM calls this version of __hyp_get_vectors (on a CPU hotplug event, for example)? The code isn't mapped in KVM's HYP code, so it will simply burst into flames.

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...