Messages in this thread | | | Date | Wed, 8 Nov 2023 12:53:57 +0100 | Subject | Re: [RFC 05/33] KVM: x86: hyper-v: Introduce VTL call/return prologues in hypercall page | From | Alexander Graf <> |
| |
On 08.11.23 12:17, Nicolas Saenz Julienne wrote: > VTL call/return hypercalls have their own entry points in the hypercall > page because they don't follow normal hyper-v hypercall conventions. > Move the VTL call/return control input into ECX/RAX and set the > hypercall code into EAX/RCX before calling the hypercall instruction in > order to be able to use the Hyper-V hypercall entry function. > > Guests can read an emulated code page offsets register to know the > offsets into the hypercall page for the VTL call/return entries. > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com> > > --- > > My tree has the additional patch, we're still trying to understand under > what conditions Windows expects the offset to be fixed. > > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c > index 54f7f36a89bf..9f2ea8c34447 100644 > --- a/arch/x86/kvm/hyperv.c > +++ b/arch/x86/kvm/hyperv.c > @@ -294,6 +294,7 @@ static int patch_hypercall_page(struct kvm_vcpu *vcpu, u64 data) > > /* VTL call/return entries */ > if (!kvm_xen_hypercall_enabled(kvm) && kvm_hv_vsm_enabled(kvm)) { > + i = 22; > #ifdef CONFIG_X86_64 > if (is_64_bit_mode(vcpu)) { > /* > --- > arch/x86/include/asm/kvm_host.h | 2 + > arch/x86/kvm/hyperv.c | 78 ++++++++++++++++++++++++++++++- > include/asm-generic/hyperv-tlfs.h | 11 +++++ > 3 files changed, 90 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index a2f224f95404..00cd21b09f8c 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1105,6 +1105,8 @@ struct kvm_hv { > u64 hv_tsc_emulation_status; > u64 hv_invtsc_control; > > + union hv_register_vsm_code_page_offsets vsm_code_page_offsets; > + > /* How many vCPUs have VP index != vCPU index */ > atomic_t num_mismatched_vp_indexes; > > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c > index 78d053042667..d4b1b53ea63d 100644 > --- a/arch/x86/kvm/hyperv.c > +++ b/arch/x86/kvm/hyperv.c > @@ -259,7 +259,8 @@ static void synic_exit(struct kvm_vcpu_hv_synic *synic, u32 msr) > static int patch_hypercall_page(struct kvm_vcpu *vcpu, u64 data) > { > struct kvm *kvm = vcpu->kvm; > - u8 instructions[9]; > + struct kvm_hv *hv = to_kvm_hv(kvm); > + u8 instructions[0x30]; > int i = 0; > u64 addr; > > @@ -285,6 +286,81 @@ static int patch_hypercall_page(struct kvm_vcpu *vcpu, u64 data) > /* ret */ > ((unsigned char *)instructions)[i++] = 0xc3; > > + /* VTL call/return entries */ > + if (!kvm_xen_hypercall_enabled(kvm) && kvm_hv_vsm_enabled(kvm)) {
You don't introduce kvm_hv_vsm_enabled() before. Please do a quick test build of all individual commits of your patch set for v1 :).
> +#ifdef CONFIG_X86_64
Why do you need the ifdef here? is_long_mode() already has an ifdef that will always return false for is_64_bit_mode() on 32bit hosts.
> + if (is_64_bit_mode(vcpu)) { > + /* > + * VTL call 64-bit entry prologue: > + * mov %rcx, %rax > + * mov $0x11, %ecx > + * jmp 0: > + */ > + hv->vsm_code_page_offsets.vtl_call_offset = i; > + instructions[i++] = 0x48; > + instructions[i++] = 0x89; > + instructions[i++] = 0xc8; > + instructions[i++] = 0xb9; > + instructions[i++] = 0x11; > + instructions[i++] = 0x00; > + instructions[i++] = 0x00; > + instructions[i++] = 0x00; > + instructions[i++] = 0xeb; > + instructions[i++] = 0xe0;
I think it would be a lot easier to read (because it's denser) if you move the opcodes into a character array:
char vtl_entry[] = { 0x48, 0x89, 0xc8, 0xb9, 0x11, 0x00, 0x00, 0x00. 0xeb, 0xe0 };
and then just memcpy().
Alex
Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
| |