[07/13] arm64: KVM: VHE: Patch out use of HVC
diff mbox

Message ID 1436372356-30410-8-git-send-email-marc.zyngier@arm.com
State New
Headers show

Commit Message

Marc Zyngier July 8, 2015, 4:19 p.m. UTC
With VHE, the host never issues an HVC instruction to get into the
KVM code, as we can simply branch there.

Use runtime code patching to simplify things a bit.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp.S        | 43 ++++++++++++++++++++++++++++++++++++-------
 arch/arm64/kvm/vhe-macros.h | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm64/kvm/vhe-macros.h

Comments

Paolo Bonzini July 8, 2015, 5:14 p.m. UTC | #1
On 08/07/2015 18:19, Marc Zyngier wrote:
> +/* Hack to allow stringification of macros... */
> +#define __S__(a,args...)	__stringify(a, ##args)
> +#define _S_(a,args...)		__S__(a, args)
> +
> +.macro ifnvhe nonvhe vhe
> +	alternative_insn	"\nonvhe", "\vhe", ARM64_HAS_VIRT_HOST_EXTN
> +.endm

Why not use this in patch 6 too?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier July 8, 2015, 5:54 p.m. UTC | #2
On 08/07/15 18:14, Paolo Bonzini wrote:
> 
> 
> On 08/07/2015 18:19, Marc Zyngier wrote:
>> +/* Hack to allow stringification of macros... */
>> +#define __S__(a,args...)	__stringify(a, ##args)
>> +#define _S_(a,args...)		__S__(a, args)
>> +
>> +.macro ifnvhe nonvhe vhe
>> +	alternative_insn	"\nonvhe", "\vhe", ARM64_HAS_VIRT_HOST_EXTN
>> +.endm
> 
> Why not use this in patch 6 too?

I tried, and ended up in #include hell. vhe-macros.h is local to
arch/arm64/kvm, and including it from asm/kvm_mmu.h breaks (we include
it from arch/arm/kvm/ and virt/kvm/arm/).

Alternatively, I could move it to arch/arm64/include/asm (renamed to
kvm_vhe_macros.h?), which would solve this issue. I just gave it a go,
and that seems sensible enough.

Thanks for the suggestion!

	M.
Paolo Bonzini July 10, 2015, 11:02 a.m. UTC | #3
On 08/07/2015 19:54, Marc Zyngier wrote:
> Alternatively, I could move it to arch/arm64/include/asm (renamed to
> kvm_vhe_macros.h?), which would solve this issue. I just gave it a go,
> and that seems sensible enough.

Yes, that would do.  Alternatively:

* create kvm_hyp_macros.h and put more stuff in there from kvm_mmu.h

* just put it in kvm_host.h

Whatever looks better to you.

Paolo

> Thanks for the suggestion!
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Catalin Marinas Aug. 5, 2015, 5:57 p.m. UTC | #4
On Wed, Jul 08, 2015 at 05:19:10PM +0100, Marc Zyngier wrote:
> --- /dev/null
> +++ b/arch/arm64/kvm/vhe-macros.h
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright (C) 2015 - ARM Ltd
> + * Author: Marc Zyngier <marc.zyngier@arm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __ARM64_VHE_MACROS_H__
> +#define __ARM64_VHE_MACROS_H__
> +
> +#include <asm/alternative.h>
> +#include <asm/cpufeature.h>
> +
> +#ifdef __ASSEMBLY__
> +
> +/* Hack to allow stringification of macros... */
> +#define __S__(a,args...)	__stringify(a, ##args)
> +#define _S_(a,args...)		__S__(a, args)
> +
> +.macro ifnvhe nonvhe vhe
> +	alternative_insn	"\nonvhe", "\vhe", ARM64_HAS_VIRT_HOST_EXTN
> +.endm

I always found the alternative_insn tricks hard to read but with
Daniel's patch queued in -next it gets slightly better. If you are not
targeting 4.3, could you do something like (untested):

.macro	vhe_if_not
	alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
.endm

.macro	vhe_else
	alternative_else CONFIG_ARM64_VHE
.endm

.macro	vhe_endif
	alternative_endif CONFIG_ARM64_VHE
.endm


The kvm_call_hyp, for example, would become:

ENTRY(kvm_call_hyp)
vhe_if_not
	hvc	#0
	ret
vhe_else
	nop
	push	lr, xzr
vhe_endif


Or you can even ignore defining new vhe_* macros altogether and just use
"alternative_if_not ARM64_HAS_VIRT_HOST_EXTN" directly (more to type)
but you may be able to avoid a vhe-macros.h file.

Patch
diff mbox

diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 17a8fb1..a65e053 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -30,6 +30,8 @@ 
 #include <asm/kvm_mmu.h>
 #include <asm/memory.h>
 
+#include "vhe-macros.h"
+
 #define CPU_GP_REG_OFFSET(x)	(CPU_GP_REGS + x)
 #define CPU_XREG_OFFSET(x)	CPU_GP_REG_OFFSET(CPU_USER_PT_REGS + 8*x)
 #define CPU_SPSR_OFFSET(x)	CPU_GP_REG_OFFSET(CPU_SPSR + 8*x)
@@ -39,6 +41,19 @@ 
 	.pushsection	.hyp.text, "ax"
 	.align	PAGE_SHIFT
 
+.macro do_el2_call
+	/*
+	 * Shuffle the parameters before calling the function
+	 * pointed to in x0. Assumes parameters in x[1,2,3],
+	 * clobbers lr.
+	 */
+	mov     lr, x0
+	mov     x0, x1
+	mov     x1, x2
+	mov     x2, x3
+	blr     lr
+.endm
+
 .macro save_common_regs
 	// x2: base address for cpu context
 	// x3: tmp register
@@ -1124,7 +1139,23 @@  __hyp_panic_str:
  * arch/arm64/kernel/hyp_stub.S.
  */
 ENTRY(kvm_call_hyp)
-	hvc	#0
+	/*
+	 * NOP out the hvc/ret sequence on VHE, and fall through.
+	 */
+ifnvhe	hvc #0,					nop
+ifnvhe	ret,				       "push	lr, xzr"
+
+	do_el2_call
+
+	/*
+	 * We used to rely on having an exception return to get
+	 * an implicit isb. In the E2H case, we don't have it anymore.
+	 * rather than changing all the leaf functions, just do it here
+	 * before returning to the rest of the kernel.
+	 */
+	isb
+
+	pop     lr, xzr
 	ret
 ENDPROC(kvm_call_hyp)
 
@@ -1156,7 +1187,9 @@  el1_sync:					// Guest trapped into EL2
 	mrs	x1, esr_el2
 	lsr	x2, x1, #ESR_ELx_EC_SHIFT
 
-	cmp	x2, #ESR_ELx_EC_HVC64
+	// Ugly shortcut for VHE. We can do this early because the
+	// host cannot do an HVC.
+ifnvhe  _S_(cmp	x2, #ESR_ELx_EC_HVC64),		"b	el1_trap"
 	b.ne	el1_trap
 
 	mrs	x3, vttbr_el2			// If vttbr is valid, the 64bit guest
@@ -1177,11 +1210,7 @@  el1_sync:					// Guest trapped into EL2
 	 * Compute the function address in EL2, and shuffle the parameters.
 	 */
 	kern_hyp_va	x0
-	mov	lr, x0
-	mov	x0, x1
-	mov	x1, x2
-	mov	x2, x3
-	blr	lr
+	do_el2_call
 
 	pop	lr, xzr
 2:	eret
diff --git a/arch/arm64/kvm/vhe-macros.h b/arch/arm64/kvm/vhe-macros.h
new file mode 100644
index 0000000..da7f9da
--- /dev/null
+++ b/arch/arm64/kvm/vhe-macros.h
@@ -0,0 +1,36 @@ 
+/*
+ * Copyright (C) 2015 - ARM Ltd
+ * Author: Marc Zyngier <marc.zyngier@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ARM64_VHE_MACROS_H__
+#define __ARM64_VHE_MACROS_H__
+
+#include <asm/alternative.h>
+#include <asm/cpufeature.h>
+
+#ifdef __ASSEMBLY__
+
+/* Hack to allow stringification of macros... */
+#define __S__(a,args...)	__stringify(a, ##args)
+#define _S_(a,args...)		__S__(a, args)
+
+.macro ifnvhe nonvhe vhe
+	alternative_insn	"\nonvhe", "\vhe", ARM64_HAS_VIRT_HOST_EXTN
+.endm
+
+#endif
+
+#endif	/*__ARM64_VHE_MACROS_H__  */