On Thu, May 24, 2018 at 05:50:56PM +0200, Radim Krčmář wrote:
> If the hypercall was called from userspace or real mode, KVM injects #UD
> and then advances RIP, so it looks like #UD was caused by the following
> instruction. This probably won't cause more than confusion, but could
> give an unexpected access to guest OS' instruction emulator.
What does the SDM say about this? Or does the HyperV specification give
a firm statement that this is the expectation ?
>
> Also, refactor the code to count hv hypercalls that were handled by the
> virt userspace.
>
> Fixes: 6356ee0c9602 ("x86: Delay skip of emulated hypercall instruction")
> Signed-off-by: Radim Krčmář <rkrcmar@xxxxxxxxxx>
> ---
> arch/x86/kvm/hyperv.c | 19 +++++++++++--------
> arch/x86/kvm/x86.c | 12 ++++--------
> 2 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 5708e951a5c6..46ff64da44ca 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1260,12 +1260,16 @@ static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
> }
> }
>
> +static int kvm_hv_hypercall_complete(struct kvm_vcpu *vcpu, u64 result)
> +{
> + kvm_hv_hypercall_set_result(vcpu, result);
> + ++vcpu->stat.hypercalls;
> + return kvm_skip_emulated_instruction(vcpu);
> +}
> +
> static int kvm_hv_hypercall_complete_userspace(struct kvm_vcpu *vcpu)
> {
> - struct kvm_run *run = vcpu->run;
> -
> - kvm_hv_hypercall_set_result(vcpu, run->hyperv.u.hcall.result);
> - return kvm_skip_emulated_instruction(vcpu);
> + return kvm_hv_hypercall_complete(vcpu, vcpu->run->hyperv.u.hcall.result);
> }
>
> static u16 kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, bool fast, u64 param)
> @@ -1350,7 +1354,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
> /* Hypercall continuation is not supported yet */
> if (rep_cnt || rep_idx) {
> ret = HV_STATUS_INVALID_HYPERCALL_CODE;
> - goto set_result;
> + goto out;
> }
>
> switch (code) {
> @@ -1381,9 +1385,8 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
> break;
> }
>
> -set_result:
> - kvm_hv_hypercall_set_result(vcpu, ret);
> - return 1;
> +out:
> + return kvm_hv_hypercall_complete(vcpu, ret);
> }
>
> void kvm_hv_init_vm(struct kvm *kvm)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 37dd9a9d050a..35a20fcfb661 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6676,11 +6676,8 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> unsigned long nr, a0, a1, a2, a3, ret;
> int op_64_bit;
>
> - if (kvm_hv_hypercall_enabled(vcpu->kvm)) {
> - if (!kvm_hv_hypercall(vcpu))
> - return 0;
> - goto out;
> - }
> + if (kvm_hv_hypercall_enabled(vcpu->kvm))
> + return kvm_hv_hypercall(vcpu);
>
> nr = kvm_register_read(vcpu, VCPU_REGS_RAX);
> a0 = kvm_register_read(vcpu, VCPU_REGS_RBX);
> @@ -6701,7 +6698,7 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
>
> if (kvm_x86_ops->get_cpl(vcpu) != 0) {
> ret = -KVM_EPERM;
> - goto out_error;
> + goto out;
> }
>
> switch (nr) {
> @@ -6721,12 +6718,11 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> ret = -KVM_ENOSYS;
> break;
> }
> -out_error:
> +out:
> if (!op_64_bit)
> ret = (u32)ret;
> kvm_register_write(vcpu, VCPU_REGS_RAX, ret);
>
> -out:
> ++vcpu->stat.hypercalls;
> return kvm_skip_emulated_instruction(vcpu);
> }
> --
> 2.17.0
>
