From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CAAA3C32771 for ; Wed, 21 Sep 2022 21:51:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230368AbiIUVvg (ORCPT ); Wed, 21 Sep 2022 17:51:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58216 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230337AbiIUVv1 (ORCPT ); Wed, 21 Sep 2022 17:51:27 -0400 Received: from mail-pl1-x62f.google.com (mail-pl1-x62f.google.com [IPv6:2607:f8b0:4864:20::62f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 996C7A6C13 for ; Wed, 21 Sep 2022 14:51:24 -0700 (PDT) Received: by mail-pl1-x62f.google.com with SMTP id t3so6994152ply.2 for ; Wed, 21 Sep 2022 14:51:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=NgbONa/JLrIJmOkn0CBqxCxDowwxjWoWs2Rsui1X0KI=; b=FWvucp8C7NSejl3BHBuBIg1NxNS9DZ7c3Jqiurti4tcdCRdol5m+vQG7QltXtbcpzO 3W8gW2CM5N/6HEp9HPytppWqpt5fBPeCs+S6iBx4sgOwxARDlAap6uU8dwB7mR4Y517/ FXVYd6oYAFBlRI595Ey9sRfrSmUc4V/5ETJRgnmBaggNifM7/By7p2mz3nFaH6Cx0U+F 4i0qjMYR2VoMH7ymgzavCcCWQ52bQPD5/gPQhzyrqpv9jI5BPMMtE1pn+vP3+vRGQ7Jr hBwB1GqpxY+XzwuwUMUivArA4YEw91XTSZYBHS0hbbZgVbzHMz2fgrLg3bOv1uRCbGbU i8Dw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date; bh=NgbONa/JLrIJmOkn0CBqxCxDowwxjWoWs2Rsui1X0KI=; b=AvMmmpt/Av5FfcoFp15n4b+gZTd6T40HHAwKJbbkgBeB7h426u2Rqty93AArUnPrs+ 92l581GVpz8UhZ/k+sXWZ0zgAm8PW5gTWzoTBqSg3Y1E72lOMm2HfnmwxxZrYmuzKv9s XIirC8O9ZypueUiiWBFuHsBqy10HQf+jpV8uQZ/8SAzSzGa0jAHc+RLGxTtAbVB3HHuk 297IVqb4Df5KQhYsjtfkPZTGPWyZnhgzpd3g3hdYRxY8WZxuDRLoN6oxdRhTe+MGH82A ci1IlpTmXewMqs0C+47Lpmt1c+pVpe0o2IuzyOE19gZT1GlPBpg9MWCY9Py07UiHdvNF sHfA== X-Gm-Message-State: ACrzQf0J8FbdVsJ3eDnmHPQWV9V4uLGqU7x/X1LcXvusDAFyi2v9MJnq Az2wI/ZZOmJ6vOk4oYRexNf8fA== X-Google-Smtp-Source: AMsMyM4aIDcK41T2ZRt2R6AWvsRZzPd8M+xtr9rkQdV+bvj+aCThLN/hvruZIVlUapXGsf2BGYbBQQ== X-Received: by 2002:a17:902:7e42:b0:178:489:86ac with SMTP id a2-20020a1709027e4200b00178048986acmr214779pln.68.1663797084005; Wed, 21 Sep 2022 14:51:24 -0700 (PDT) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id j6-20020a17090276c600b001789b724712sm2439580plt.232.2022.09.21.14.51.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Sep 2022 14:51:23 -0700 (PDT) Date: Wed, 21 Sep 2022 21:51:20 +0000 From: Sean Christopherson To: Vitaly Kuznetsov Cc: kvm@vger.kernel.org, Paolo Bonzini , Wanpeng Li , Jim Mattson , Michael Kelley , Siddharth Chandrasekaran , Yuan Yao , Maxim Levitsky , linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org, Vipin Sharma Subject: Re: [PATCH v10 25/39] KVM: selftests: Move the function doing Hyper-V hypercall to a common header Message-ID: References: <20220921152436.3673454-1-vkuznets@redhat.com> <20220921152436.3673454-26-vkuznets@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220921152436.3673454-26-vkuznets@redhat.com> Precedence: bulk List-ID: X-Mailing-List: linux-hyperv@vger.kernel.org +Vipin On Wed, Sep 21, 2022, Vitaly Kuznetsov wrote: > All Hyper-V specific tests issuing hypercalls need this. > > Reviewed-by: Maxim Levitsky > Signed-off-by: Vitaly Kuznetsov > --- > .../selftests/kvm/include/x86_64/hyperv.h | 16 ++++++++++++++++ > .../selftests/kvm/x86_64/hyperv_features.c | 18 +----------------- > 2 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h > index f0a8a93694b2..285e9ff73573 100644 > --- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h > +++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h > @@ -185,6 +185,22 @@ > /* hypercall options */ > #define HV_HYPERCALL_FAST_BIT BIT(16) > > +static inline uint8_t hyperv_hypercall(u64 control, vm_vaddr_t input_address, > + vm_vaddr_t output_address, > + uint64_t *hv_status) > +{ > + uint8_t vector; Newline after the variable declaration. > + /* Note both the hypercall and the "asm safe" clobber r9-r11. */ > + asm volatile("mov %[output_address], %%r8\n\t" > + KVM_ASM_SAFE("vmcall") > + : "=a" (*hv_status), > + "+c" (control), "+d" (input_address), > + KVM_ASM_SAFE_OUTPUTS(vector) > + : [output_address] "r"(output_address) > + : "cc", "memory", "r8", KVM_ASM_SAFE_CLOBBERS); > + return vector; > +} > + > /* Proper HV_X64_MSR_GUEST_OS_ID value */ > #define HYPERV_LINUX_OS_ID ((u64)0x8100 << 48) > > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c > index 1144bd1ea626..c464d324cde0 100644 > --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c > @@ -13,22 +13,6 @@ > #include "processor.h" > #include "hyperv.h" > > -static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address, > - vm_vaddr_t output_address, uint64_t *hv_status) > -{ > - uint8_t vector; > - > - /* Note both the hypercall and the "asm safe" clobber r9-r11. */ > - asm volatile("mov %[output_address], %%r8\n\t" > - KVM_ASM_SAFE("vmcall") > - : "=a" (*hv_status), > - "+c" (control), "+d" (input_address), > - KVM_ASM_SAFE_OUTPUTS(vector) > - : [output_address] "r"(output_address) > - : "cc", "memory", "r8", KVM_ASM_SAFE_CLOBBERS); > - return vector; > -} > - > struct msr_data { > uint32_t idx; > bool available; > @@ -78,7 +62,7 @@ static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall) > input = output = 0; > } > > - vector = hypercall(hcall->control, input, output, &res); > + vector = hyperv_hypercall(hcall->control, input, output, &res); > if (hcall->ud_expected) > GUEST_ASSERT_2(vector == UD_VECTOR, hcall->control, vector); > else Just out of sight here, but I broke this code in commit cc5851c6be86 ("KVM: selftests: Use exception fixup for #UD/#GP Hyper-V MSR/hcall tests"). I got too fancy and inverted the ud_expected logic when checking the result. The broken code skips the check when #UD _not_ expected. I.e. this if (hcall->ud_expected) GUEST_ASSERT_2(vector == UD_VECTOR, hcall->control, vector); else GUEST_ASSERT_2(!vector, hcall->control, vector); GUEST_ASSERT_2(!hcall->ud_expected || res == hcall->expect, hcall->expect, res); should be if (hcall->ud_expected) { GUEST_ASSERT_2(vector == UD_VECTOR, hcall->control, vector); } else { GUEST_ASSERT_2(!vector, hcall->control, vector); GUEST_ASSERT_2(res == hcall->expect, hcall->expect, res); } The reason I bring this up here is because of the reason the test passes: gcc zeros RAX before the hypercall (not entirely sure why), and so res=0 on #UD due to nothing changing RAX. But clang doesn't zero RAX and so the test fails due to RAX holding garbage (probably '1' from the lower 32 bits of HV_X64_MSR_HYPERCALL). So, what do you think about explicitly setting hv_status, e.g. to -EFAULT, prior to the hypercall, both to defend against selftest bugs and to verify that _KVM_ actually zeros the result?