Hi Frediano
[add Geoff to cc]
I still have to compare my code with yours, but please note that
some files in arch/arm/kvm are shared with arm64, especially
arm.c, mmu.c and related headers.
So can you please
- submit your new patch without including old e-mail threads.
(don't forget RFC or PATCH.)
- break it up into two pieces, one for common, the other for arm
and a few more comments below:
On 02/06/2015 01:56 AM, Frediano Ziglio wrote:
[snip]
New version. Changes:
- removed saved value and test again
- remove vm flush, useless
- correct compile check on header
- try KVM enabled and not, compile link and tests
- rewrite comments, add more where necessary
- remove code to free and allocate again boot PGD and related, keep in
memory if KEXEC is enabled
Still not trying SMP. Still to be considered RFC. I tried different
compile options (KEXEC and KVM turned on/off). I realized that I
should test if kernel is started with SVC mode code still works
correctly. ARM_VIRT_EXT is always turned on for V7 CPU.
diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
index 3a67bec..985f0a3 100644
--- a/arch/arm/include/asm/kvm_asm.h
+++ b/arch/arm/include/asm/kvm_asm.h
@@ -97,7 +97,19 @@ extern char __kvm_hyp_code_end[];
extern void __kvm_flush_vm_context(void);
extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
+extern void __kvm_set_vectors(unsigned long phys_vector_base);
+
extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+
+#ifndef CONFIG_ARM_VIRT_EXT
Arm64 doesn't have CONFIG_ARM_VIRT_EXT. Why don't use CONFIG_KVM?
I'd rather put this conditional into the place where the function is
actually called for flexible implementation. (See below)
+static void kvm_cpu_reset(void (*phys_reset)(void *), void *addr)
+{
+ phys_reset(addr);
+}
+#else
+extern void kvm_cpu_reset(void (*phys_reset)(void *), void *addr);
+#endif
+
#endif
#endif /* __ARM_KVM_ASM_H__ */
diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
index 2a55373..30339ad 100644
--- a/arch/arm/kernel/hyp-stub.S
+++ b/arch/arm/kernel/hyp-stub.S
@@ -164,13 +164,63 @@ ARM_BE8(orr r7, r7, #(1 << 25)) @ HSCTLR.EE
bx lr @ The boot CPU mode is left in r4.
ENDPROC(__hyp_stub_install_secondary)
+#undef CPU_RESET
+#if defined(CONFIG_ARM_VIRT_EXT) && !defined(CONFIG_KVM) && !defined(ZIMAGE)
+# define CPU_RESET 1
+#endif
+
__hyp_stub_do_trap:
+#ifdef CPU_RESET
+ cmp r0, #-2
+ bne 1f
+
+ mrc p15, 4, r0, c1, c0, 0 @ HSCR
+ ldr r2, =0x40003807
+ bic r0, r0, r2
+ mcr p15, 4, r0, c1, c0, 0 @ HSCR
+ isb
+
+ @ Disable MMU, caches and Thumb in SCTLR
+ mrc p15, 0, r0, c1, c0, 0 @ SCTLR
+ bic r0, r0, r2
+ mcr p15, 0, r0, c1, c0, 0
+
+ bx r1
+ .ltorg
+1:
+#endif
cmp r0, #-1
mrceq p15, 4, r0, c12, c0, 0 @ get HVBAR
mcrne p15, 4, r0, c12, c0, 0 @ set HVBAR
__ERET
ENDPROC(__hyp_stub_do_trap)
+#ifdef CPU_RESET
+/*
+ * r0 cpu_reset function
+ * r1 address to jump to
+ */
+ENTRY(kvm_cpu_reset)
+ mov r2, r0
+
+ @ __boot_cpu_mode should be HYP_MODE
+ adr r0, .L__boot_cpu_mode_offset
+ ldr r3, [r0]
+ ldr r0, [r0, r3]
+ cmp r0, #HYP_MODE
+ beq reset_to_hyp
+
+ @ Call SVC mode cpu_reset
+ mov r0, r1
+THUMB( orr r2, r2, 1 )
+ bx r2
+
+reset_to_hyp:
+ mov r0, #-2
+ b __hyp_set_vectors
+ENDPROC(kvm_cpu_reset)
+#endif
+
/*
* __hyp_set_vectors: Call this after boot to set the initial hypervisor
* vectors as part of hypervisor installation. On an SMP system, this should
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index fdfa3a7..c018719 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -41,6 +41,7 @@
#include <asm/system_misc.h>
#include <asm/mach/time.h>
#include <asm/tls.h>
+#include <asm/kvm_asm.h>
#ifdef CONFIG_CC_STACKPROTECTOR
#include <linux/stackprotector.h>
@@ -60,7 +61,7 @@ static const char *isa_modes[] __maybe_unused = {
};
extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
-typedef void (*phys_reset_t)(unsigned long);
+typedef void (*phys_reset_t)(void *);
/*
* A temporary stack to use for CPU reset. This is static so that we
@@ -89,7 +90,8 @@ static void __soft_restart(void *addr)
/* Switch to the identity mapping. */
phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
- phys_reset((unsigned long)addr);
+
+ kvm_cpu_reset(phys_reset, addr);
How about this?
#ifdef CONFIG_KVM
kvm_cpu_reset(...);
#endif
phys_reset(addr);
It will clearify the purpose of kvm_cpu_reset().
The name of kvm_cpu_reset() might better be cpu_*de*init_hyp_mode or similar
given that the function would be called in hyp_init_cpu_notify()
once kvm supports cpu hotplug in the future.
/* Should never get here. */
BUG();
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 0b0d58a..e4d4a2b 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -1009,7 +1009,7 @@ static int init_hyp_mode(void)
if (err)
goto out_free_mappings;
-#ifndef CONFIG_HOTPLUG_CPU
+#if !defined(CONFIG_HOTPLUG_CPU) && !defined(CONFIG_KEXEC)
free_boot_hyp_pgd();
#endif
@@ -1079,6 +1079,53 @@ out_err:
return err;
}
+void kvm_cpu_reset(void (*phys_reset)(void *), void *addr)
I'm not sure, but it might be better to put this code into arm/kernel/init.S
as it is a counterpart of _do_hyp_init from cpu_init_hyp_mode().
+{
+ unsigned long vector;
+
+ if (!is_hyp_mode_available()) {
+ phys_reset(addr);
+ return;
+ }
+
+ vector = (unsigned long) kvm_get_idmap_vector();
+
+ /*
+ * Set vectors so we call initialization routines.
+ * trampoline is mapped at TRAMPOLINE_VA but not at its physical
+ * address so we don't have an identity map to be able to
+ * disable MMU. We need to set exception vector at trampoline
+ * at TRAMPOLINE_VA address which is mapped.
+ */
+ kvm_call_hyp(__kvm_set_vectors,(unsigned long) (TRAMPOLINE_VA +
(vector & (PAGE_SIZE-1))));
+
+ /*
+ * Set HVBAR to physical address, page table to identity to do the switch
+ */
+ kvm_call_hyp((void*) 4, (unsigned long) vector, kvm_mmu_get_boot_httbr());
+
+ /*
+ * Flush to make sure code after the cache are disabled see updated values
+ */
+ flush_cache_all();
+
+ /*
+ * Turn off caching on Hypervisor mode
+ */
+ kvm_call_hyp((void*) 1);
+
+ /*
+ * Flush to make sude code don't see old cached values after cache is
+ * enabled
+ */
+ flush_cache_all();
+
+ /*
+ * Restore execution
+ */
+ kvm_call_hyp((void*) 3, addr);
+}
+
If you like kvm_call_hyp-like sequences, please specify what each kvm_call_hyp() should do.
(we can do all in one kvm_call_hyp() though.)
Thanks,
-Takahiro AKASHI
/* NOP: Compiling as a module not supported */
void kvm_arch_exit(void)
{
diff --git a/arch/arm/kvm/init.S b/arch/arm/kvm/init.S
index 3988e72..c2f1d4d 100644
--- a/arch/arm/kvm/init.S
+++ b/arch/arm/kvm/init.S
@@ -68,6 +68,12 @@ __kvm_hyp_init:
W(b) .
__do_hyp_init:
+ @ check for special values, odd numbers can't be a stack pointer
+ cmp r0, #1
+ beq cpu_proc_fin
+ cmp r0, #3
+ beq restore
+
cmp r0, #0 @ We have a SP?
bne phase2 @ Yes, second stage init
@@ -151,6 +157,33 @@ target: @ We're now in the trampoline code,
switch page tables
eret
+cpu_proc_fin:
+ mrc p15, 4, r0, c1, c0, 0 @ ctrl register
+ bic r0, r0, #0x1000 @ ...i............
+ bic r0, r0, #0x0006 @ .............ca.
+ mcr p15, 4, r0, c1, c0, 0 @ disable caches
+ eret
+
+restore:
+ @ Disable MMU, caches and Thumb in HSCTLR
+ mrc p15, 4, r0, c1, c0, 0 @ HSCR
+ ldr r2, =0x40003807
+ bic r0, r0, r2
+ mcr p15, 4, r0, c1, c0, 0 @ HSCR
+ isb
+
+ @ Invalidate old TLBs
+ mcr p15, 4, r0, c8, c7, 0 @ TLBIALLH
+ isb
+ dsb ish
+
+ @ Disable MMU, caches and Thumb in SCTLR
+ mrc p15, 0, r0, c1, c0, 0 @ SCTLR
+ bic r0, r0, r2
+ mcr p15, 0, r0, c1, c0, 0
+
+ bx r1
+
.ltorg
.globl __kvm_hyp_init_end
diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
index 01dcb0e..449e7dd 100644
--- a/arch/arm/kvm/interrupts.S
+++ b/arch/arm/kvm/interrupts.S
@@ -87,6 +87,18 @@ ENDPROC(__kvm_flush_vm_context)
/********************************************************************
+ * Set HVBAR
+ *
+ * void __kvm_set_vectors(unsigned long phys_vector_base);
+ */
+ENTRY(__kvm_set_vectors)
+ mcr p15, 4, r0, c12, c0, 0 @ set HVBAR
+ dsb ish
+ isb
+ bx lr
+ENDPROC(__kvm_set_vectors)
+
+/********************************************************************
* Hypervisor world-switch code
*
*
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index 1366625..f853858 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -369,6 +369,11 @@ void free_boot_hyp_pgd(void)
free_page((unsigned long)init_bounce_page);
init_bounce_page = NULL;
+ /* avoid to reuse possibly invalid values if bounce page is freed */
+ hyp_idmap_start = 0;
+ hyp_idmap_end = 0;
+ hyp_idmap_vector = 0;
+
mutex_unlock(&kvm_hyp_pgd_mutex);
}
Frediano
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
