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 87735C6FA83 for ; Mon, 12 Sep 2022 14:29:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230191AbiILO3x (ORCPT ); Mon, 12 Sep 2022 10:29:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34562 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230005AbiILO3r (ORCPT ); Mon, 12 Sep 2022 10:29:47 -0400 Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C2565E26 for ; Mon, 12 Sep 2022 07:29:43 -0700 (PDT) Received: by mail-pj1-x1035.google.com with SMTP id a5-20020a17090aa50500b002008eeb040eso4220725pjq.1 for ; Mon, 12 Sep 2022 07:29:43 -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=GFheNGVqLERZhSYCSZk/aU+ENoKVQ3v9+pQbZTcslEY=; b=exu7ea6b5fYdoPyhBfsq0X2/RyFa0nkTMBzKAhu/TECnWWoAiZHfnnwLAwMv3264+V fXHM3vi4u+FYXFDPi8lgH79yRErt5ZSm0OjEyIk4DTUOGRATDN4ye/pFZ0hOBeGwQMm+ W/8jR6Hn+qX+5SqgaVqIG29xbM0agMOTL1bJnT6lT2WXBzCyq/aQvqNns0j1ZNrv5Aqk cZNf41l4yXFpBVe8FuZpCTTUZr0uicnZK4kOofuiYwoL7b5OBsNOiuLq6l2cZ2VKMUKz E3xq9TfZ8xigxuNDAJ6vqCOa67m63/y47yzeWjZcxhPF1Csy5kfCGmBsHaF1fKg4xohQ OHiA== 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=GFheNGVqLERZhSYCSZk/aU+ENoKVQ3v9+pQbZTcslEY=; b=X+pgdlrlYpzz74N7hdkfPvP68fX1fxMkiWeuo51NWvASXXQpZfsX36iyDGhfSeaKWy 5rodrVPimLn3GkseXl4WCRcuIUOQeA3Sunj8L7AEsK+AAVjDpTBhJtHOKKtg6qHIy04q qgT678W2HMAGZdGukNnLLa5aENvbr9pOZIGRwrQ6xO/hmjG0hZ3Aazlht93SFNBPCYoo BxxZNTlOyXHUV47llB+8t6vtMlzIi6eWZzkSznps7vH4CM0zwonnRRRKAT6NUTuTq33R JRv2BnMpyeH5dzHSUVW4DYAKFNsO8Mc7sFpS9qllKiki8VcesvHsgBLfa+zXzcvgXyoP rh8g== X-Gm-Message-State: ACgBeo1FviAyWg51svSwRGfVXGGa6VzA5JL+67l9vnDs3WqxRkV2BPG/ B9hsxao2kl5oFw5oDCnJqats2A== X-Google-Smtp-Source: AA6agR5eQXImfKaiccrP3GiMD3RTxMTyGKrsX6U7ojJdLRpLHqDclHPP5oVBbofCP8Qq/PcjRB6U6A== X-Received: by 2002:a17:902:7d83:b0:170:9353:f299 with SMTP id a3-20020a1709027d8300b001709353f299mr26584625plm.41.1662992982931; Mon, 12 Sep 2022 07:29:42 -0700 (PDT) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id t23-20020a635f17000000b0042b5095b7b4sm5758510pgb.5.2022.09.12.07.29.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 12 Sep 2022 07:29:42 -0700 (PDT) Date: Mon, 12 Sep 2022 14:29:39 +0000 From: Sean Christopherson To: Vitaly Kuznetsov Cc: kvm@vger.kernel.org, Paolo Bonzini , Wanpeng Li , Jim Mattson , Michael Kelley , Maxim Levitsky , linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 3/3] KVM: selftests: Test Hyper-V invariant TSC control Message-ID: References: <20220831085009.1627523-1-vkuznets@redhat.com> <20220831085009.1627523-4-vkuznets@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220831085009.1627523-4-vkuznets@redhat.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Aug 31, 2022, Vitaly Kuznetsov wrote: > Add a test for the newly introduced Hyper-V invariant TSC control feature: > - HV_X64_MSR_TSC_INVARIANT_CONTROL is not available without > HV_ACCESS_TSC_INVARIANT CPUID bit set and available with it. > - BIT(0) of HV_X64_MSR_TSC_INVARIANT_CONTROL controls the filtering of > architectural invariant TSC (CPUID.80000007H:EDX[8]) bit. > > Signed-off-by: Vitaly Kuznetsov > --- > .../selftests/kvm/x86_64/hyperv_features.c | 67 +++++++++++++++++++ > 1 file changed, 67 insertions(+) > > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c > index 4ec4776662a4..26e8c5f7677e 100644 > --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c > @@ -15,6 +15,9 @@ > > #define LINUX_OS_ID ((u64)0x8100 << 48) > > +/* CPUID.80000007H:EDX */ > +#define X86_FEATURE_INVTSC (1 << 8) > + > static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address, > vm_vaddr_t output_address, uint64_t *hv_status) > { > @@ -60,6 +63,24 @@ static void guest_msr(struct msr_data *msr) > GUEST_ASSERT_2(!vector, msr->idx, vector); > else > GUEST_ASSERT_2(vector == GP_VECTOR, msr->idx, vector); > + > + /* Invariant TSC bit appears when TSC invariant control MSR is written to */ > + if (msr->idx == HV_X64_MSR_TSC_INVARIANT_CONTROL) { > + u32 eax, ebx, ecx, edx; > + > + cpuid(0x80000007, &eax, &ebx, &ecx, &edx); Add a proper kvm_x86_cpu_feature so that this is simply this_cpu_has(X86_FEATURE_INVTSC) > + > + /* > + * TSC invariant bit is present without the feature (legacy) or > + * when the feature is present and enabled. > + */ > + if ((!msr->should_not_gp && !msr->write) || (msr->write && msr->write_val == 1)) Relying purely on the inputs is rather nasty as it creates a subtle dependency on the "write 1" testcase coming last. This function already reads the guest MSR value, just use that to check if INVTSC should be enabled. And if we want to verify KVM "wrote" the correct value, then that can be done in the common path. And I think that will make this code self-documenting, e.g. if (msr->idx == HV_X64_MSR_TSC_INVARIANT_CONTROL) GUEST_ASSERT(this_cpu_has(X86_FEATURE_INVTSC) == !!(msr_val & ...)); > + GUEST_ASSERT(edx & X86_FEATURE_INVTSC); > + else > + GUEST_ASSERT(!(edx & X86_FEATURE_INVTSC)); > + } > + > + > GUEST_DONE(); > } > > @@ -104,6 +125,15 @@ static void vcpu_reset_hv_cpuid(struct kvm_vcpu *vcpu) > vcpu_clear_cpuid_entry(vcpu, HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES); > } > > +static bool guest_has_invtsc(void) > +{ > + const struct kvm_cpuid_entry2 *cpuid; > + > + cpuid = kvm_get_supported_cpuid_entry(0x80000007); > + > + return cpuid->edx & X86_FEATURE_INVTSC; > +} > + > static void guest_test_msrs_access(void) > { > struct kvm_cpuid2 *prev_cpuid = NULL; > @@ -115,6 +145,7 @@ static void guest_test_msrs_access(void) > int stage = 0; > vm_vaddr_t msr_gva; > struct msr_data *msr; > + bool has_invtsc = guest_has_invtsc(); Huh, I never added vcpu_has_cpuid_feature()? Can you add that instead of open-coding the check?