On Tue, Nov 28, 2023, Maxim Levitsky wrote:
> On Wed, 2023-11-08 at 11:17 +0000, Nicolas Saenz Julienne wrote:
> > 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)) {
> > +#ifdef CONFIG_X86_64
> > + if (is_64_bit_mode(vcpu)) {
> > + /*
> > + * VTL call 64-bit entry prologue:
> > + * mov %rcx, %rax
> > + * mov $0x11, %ecx
> > + * jmp 0:
>
> This isn't really 'jmp 0' as I first wondered but actually backward jump 32
> bytes back (if I did the calculation correctly). This is very dangerous
> because code that was before can change and in fact I don't think that this
> offset is even correct now, and on top of that it depends on support for xen
> hypercalls as well.
>
> This can be fixed by calculating the offset in runtime, however I am
> thinking:
>
>
> Since userspace will have to be aware of the offsets in this page, and since
> pretty much everything else is done in userspace, it might make sense to
> create the hypercall page in the userspace.
>
> In fact, the fact that KVM currently overwrites the guest page, is a
> violation of the HV spec.
>
> It's more correct regardless of VTL to do userspace vm exit and let the
> userspace put a memslot ("overlay") over the address, and put whatever
> userspace wants there, including the above code.
>
> Then we won't need the new ioctl as well.
>
> To support this I think that we can add a userspace msr filter on the
> HV_X64_MSR_HYPERCALL, although I am not 100% sure if a userspace msr filter
> overrides the in-kernel msr handling.
Yep, userspace MSR filters override in-kernel handling.
