On Sat, Nov 28, 2020, David Woodhouse wrote: ... > +static int complete_userspace_hypercall(struct kvm_vcpu *vcpu) > +{ > + kvm_rax_write(vcpu, vcpu->run->hypercall.ret); > + > + return kvm_skip_emulated_instruction(vcpu); This should probably verify the linear RIP is unchanged before modifying guest state, e.g. to let userspace reset the vCPU in response to a hypercall. See complete_fast_pio_out(). > +} > + > +int kvm_userspace_hypercall(struct kvm_vcpu *vcpu) > +{ > + struct kvm_run *run = vcpu->run; > + > + if (is_long_mode(vcpu)) { > + run->hypercall.longmode = 1; Should this also capture the CPL? I assume the motivation for grabbing regs and longmode is to avoid having to call back into KVM on every hypercall, and I also assume there are (or will be) hypercalls that are CPL0 only. > + run->hypercall.nr = kvm_rax_read(vcpu); > + run->hypercall.args[0] = kvm_rdi_read(vcpu); > + run->hypercall.args[1] = kvm_rsi_read(vcpu); > + run->hypercall.args[2] = kvm_rdx_read(vcpu); > + run->hypercall.args[3] = kvm_r10_read(vcpu); > + run->hypercall.args[4] = kvm_r8_read(vcpu); > + run->hypercall.args[5] = kvm_r9_read(vcpu); > + run->hypercall.ret = -KVM_ENOSYS; > + } else { > + run->hypercall.longmode = 0; > + run->hypercall.nr = (u32)kvm_rbx_read(vcpu); > + run->hypercall.args[0] = (u32)kvm_rbx_read(vcpu); > + run->hypercall.args[1] = (u32)kvm_rcx_read(vcpu); > + run->hypercall.args[2] = (u32)kvm_rdx_read(vcpu); > + run->hypercall.args[3] = (u32)kvm_rsi_read(vcpu); > + run->hypercall.args[4] = (u32)kvm_rdi_read(vcpu); > + run->hypercall.args[5] = (u32)kvm_rbp_read(vcpu); > + run->hypercall.ret = (u32)-KVM_ENOSYS; > + } Why not get/set all GPRs (except maybe RIP and RSP) as opposed to implementing what I presume is Xen's ABI in a generic KVM user exit? Copying 10 more GPRs to/from memory is likely lost in the noise relative to the cost of the userspace roundtrip. > + run->exit_reason = KVM_EXIT_HYPERCALL; > + vcpu->arch.complete_userspace_io = complete_userspace_hypercall; > + > + return 0; > +} > + > int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) > { > unsigned long nr, a0, a1, a2, a3, ret; > int op_64_bit; > > + if (vcpu->kvm->arch.user_space_hypercall) > + return kvm_userspace_hypercall(vcpu); > + > if (kvm_hv_hypercall_enabled(vcpu->kvm)) > return kvm_hv_hypercall(vcpu); >