Geoff,
On 03/24/2015 01:46 AM, Geoff Levand wrote:
Hi Takahiro,
On Mon, 2015-03-23 at 20:53 +0900, AKASHI Takahiro wrote:
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index 3e6859b..428f41c 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
...
+phys_addr_t kvm_get_stub_vectors(void)
+{
+ return virt_to_phys(__hyp_stub_vectors);
+}
The stub vectors are not part of KVM, but part of kernel,
so to me a routine get_hyp_stub_vectors() with a prototype
in asm/virt.h, then a definition in maybe
kernel/process.c, or a new file kernel/virt.c makes more
sense.
Right.
Will rename the function to get_hyp_stub_vectors() and put it in asm/virt.h.
+unsigned long kvm_reset_func_entry(void)
+{
+ /* VA of __kvm_hyp_reset in trampline code */
+ return TRAMPOLINE_VA + (__kvm_hyp_reset - __hyp_idmap_text_start);
+}
+
int kvm_mmu_init(void)
{
int err;
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 4f7310f..97ee2fc 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -116,8 +116,11 @@
struct kvm;
struct kvm_vcpu;
+extern char __hyp_stub_vectors[];
I think this should at least be in asm/virt.h, or better,
have a get_hyp_stub_vectors().
See above.
extern char __kvm_hyp_init[];
extern char __kvm_hyp_init_end[];
+extern char __kvm_hyp_reset[];
extern char __kvm_hyp_vector[];
diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
index 8ac3c70..97f88fe 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -199,6 +199,7 @@ struct kvm_vcpu *kvm_arm_get_running_vcpu(void);
struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void);
u64 kvm_call_hyp(void *hypfn, ...);
+void kvm_call_reset(unsigned long reset_func, ...);
kvm_call_reset() takes a fixed number of args, so we shouldn't
have it as a variadic here. I think a variadic routine
complicates things for my kvm_call_reset() suggestion below.
void force_vm_exit(const cpumask_t *mask);
void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
@@ -223,6 +224,15 @@ static inline void __cpu_init_hyp_mode(phys_addr_t
boot_pgd_ptr,
hyp_stack_ptr, vector_ptr);
}
+static inline void __cpu_reset_hyp_mode(unsigned long reset_func,
+ phys_addr_t boot_pgd_ptr,
+ phys_addr_t phys_idmap_start,
+ unsigned long stub_vector_ptr)
+ kvm_call_reset(reset_func, boot_pgd_ptr,
+ phys_idmap_start, stub_vector_ptr);
Why not switch the order of the args here to:
kvm_call_reset(boot_pgd_ptr, phys_idmap_start, stub_vector_ptr, reset_func)
This will eliminate the register shifting in the HVC_RESET
hcall vector which becomes just 'br x3'.
Looks nice.
FYI, initially I wanted to implement kvm_cpu_reset() using kvm_call_hyp()
and so both have similar code.
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index fd085ec..aee75f9 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -1136,6 +1136,11 @@ ENTRY(kvm_call_hyp)
ret
ENDPROC(kvm_call_hyp)
+ENTRY(kvm_call_reset)
+ hvc #HVC_RESET
+ ret
+ENDPROC(kvm_call_reset)
+
.macro invalid_vector label, target
.align 2
\label:
@@ -1179,10 +1184,10 @@ el1_sync: //
Guest trapped into EL2
cmp x18, #HVC_GET_VECTORS
b.ne 1f
mrs x0, vbar_el2
- b 2f
-
-1: /* Default to HVC_CALL_HYP. */
It seems you are deleting this comment and also
removing the logic that makes HVC_CALL_HYP the default.
Yeah, I didn't think of that.
But IIUC, we don't have to handle it as default case because
all interfaces come from kvm_call_hyp() once KVM is initialized.
+ b 3f
+1: cmp x18, #HVC_CALL_HYP
+ b.ne 2f
push lr, xzr
/*
@@ -1196,7 +1201,23 @@ el1_sync: //
Guest trapped into EL2
blr lr
pop lr, xzr
-2: eret
+ b 3f
+
+ /*
+ * shuffle the parameters and jump into trampline code.
+ */
+2: cmp x18, #HVC_RESET
+ b.ne 3f
+
+ mov x18, x0
+ mov x0, x1
+ mov x1, x2
+ mov x2, x3
+ mov x3, x4
+ br x18
+ /* not reach here */
+
+3: eret
We don't need to change labels for each new hcall, I
think just this is OK:
cmp x18, #HVC_GET_VECTORS
b.ne 1f
mrs x0, vbar_el2
b 2f
+1: cmp x18, #HVC_RESET
+ b.ne 1f
+ br x3 // No return
1: /* Default to HVC_CALL_HYP. */
...
Will fix it.
Thanks,
-Takahiro AKASHI
-Geoff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
