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 6842BC61DB3 for ; Mon, 9 Jan 2023 18:10:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237187AbjAISKD (ORCPT ); Mon, 9 Jan 2023 13:10:03 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58388 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235387AbjAISIt (ORCPT ); Mon, 9 Jan 2023 13:08:49 -0500 Received: from mail-pg1-x532.google.com (mail-pg1-x532.google.com [IPv6:2607:f8b0:4864:20::532]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 109033C3B7 for ; Mon, 9 Jan 2023 10:07:43 -0800 (PST) Received: by mail-pg1-x532.google.com with SMTP id 78so6437373pgb.8 for ; Mon, 09 Jan 2023 10:07:43 -0800 (PST) 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:message-id:reply-to; bh=cjepIbO6GWlHhX91qulkt2Gx/2Rp33KnfWlDoun7rHc=; b=njVGvA2BBq3lnqckIDLq2Gfu60hjfzhMZj2lCDBA9Q/Lpa3+ceKbvbCs7TKBQE6VJI JcLcW2ERDIL33DzVnKFGiRsQDhsvMqscBoj/k1aNTZeqSd+0O+78Kgx6Vg3zbyValq16 h4mCDaoPitJhps3c/ZPEY8VZWtzGEaa92W68IbCJgZSSUlI2EBgQJ9rJ2rFN5p0HAayC rkBh3/3+LriKDJ3ih4M1SbWqcNNFZctTCctIRO1JQrgWvMNOIt1QhgmzRUAUfQRe+BIg RY2Nh3iZ38UcRLASvxnTDMrmJArGi3ESXXRR3EOtolt7GJydZNIud/QfIf29lt0RhaFH v8HA== 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 :message-id:reply-to; bh=cjepIbO6GWlHhX91qulkt2Gx/2Rp33KnfWlDoun7rHc=; b=THw4vSKVhmL9LAfpDXX7qAUY1qSQqXLNL68jv4SuYFcostrVSfWVue19rLSW25Lpyp aEtnFiZWr6Q8IiLnqHgOqDWQzJM3ka8jWm+3dMSa9PNkKpL83raR7vh3LLMxyLZTNwLU kt3jnx+vwfib01EARLCR4ChgszbMQaw/o2e7xZHGobk4pBARskWg4Cmn4Qimv/kLZPC7 YkxSGgHF2WWhiG725LmSLBGZFy2BB+KnK+f1O2peD05LX4+ELG5bdpn/Lty2H6U1kPwS f1xOzDRUaKGN+hC1f0oqkcnqc9Nc5maxdIzPaDyyF9M0p65QsVGrKcXzMzNcX/rx9y3z 2+aA== X-Gm-Message-State: AFqh2kqqL7wAQ3zRiyhDTK73fatqEQx+F8j9aAvrMx47fQhMknjY/CXw jskJyFw1+MAyT3hldaRbtXIE6w== X-Google-Smtp-Source: AMrXdXs8UZlZ+kaiOTh68EobJZJpxdjrDGhRiljc7mD96iwfDDLVeszhLuzh0X0p8er38fyUS+VXGw== X-Received: by 2002:a05:6a00:1948:b0:581:bfac:7a52 with SMTP id s8-20020a056a00194800b00581bfac7a52mr702100pfk.1.1673287662364; Mon, 09 Jan 2023 10:07:42 -0800 (PST) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id k18-20020a628412000000b0058103f45d9esm3138368pfd.82.2023.01.09.10.07.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Jan 2023 10:07:41 -0800 (PST) Date: Mon, 9 Jan 2023 18:07:38 +0000 From: Sean Christopherson To: Vishal Annapurve Cc: x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, pbonzini@redhat.com, shuah@kernel.org, bgardon@google.com, oupton@google.com, peterx@redhat.com, vkuznets@redhat.com, dmatlack@google.com Subject: Re: [V4 PATCH 1/4] KVM: selftests: x86: use this_cpu_* helpers Message-ID: References: <20221228192438.2835203-1-vannapurve@google.com> <20221228192438.2835203-2-vannapurve@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221228192438.2835203-2-vannapurve@google.com> Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Wed, Dec 28, 2022, Vishal Annapurve wrote: > Use this_cpu_* helpers to query the cpu vendor. Neither the changelog nor the shortlog captures what this patch actually does, or rather what I inteded it to do. Specifically, what I suggested (or intended to suggest) was: KVM: selftests: Rename vendor string helpers to use "this_cpu" prefix > Suggested-by: Sean Christopherson > Signed-off-by: Vishal Annapurve > --- ... > diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c > index acfa1d01e7df..a799af572f3f 100644 > --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c > +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c > @@ -1006,26 +1006,14 @@ void kvm_x86_state_cleanup(struct kvm_x86_state *state) > free(state); > } > > -static bool cpu_vendor_string_is(const char *vendor) > -{ > - const uint32_t *chunk = (const uint32_t *)vendor; > - uint32_t eax, ebx, ecx, edx; > - > - cpuid(0, &eax, &ebx, &ecx, &edx); > - return (ebx == chunk[0] && edx == chunk[1] && ecx == chunk[2]); > -} > - > bool is_intel_cpu(void) Drop the is_intel_cpu() and is_amd_cpu() wrappers. The whole point of the rename was so that it's obvious at the call site that the function is checking the "current" CPU context. That obviously means dropping the is_host_cpu_amd() and is_host_cpu_intel() wrappers too. IMO, the extra layer to jump through (more from a code reading perspective then a code generation perspective) isn't worth protecting the booleans. It's slightly more churn (in between patches, not overall), but the benefit is that it allows squasing patches 2 and 3 into a single patch, e.g. "KVM: selftests: Cache host CPU vendor (AMD vs. Intel)" --- tools/testing/selftests/kvm/include/x86_64/processor.h | 3 +++ tools/testing/selftests/kvm/lib/x86_64/processor.c | 8 ++++---- tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c | 4 ++-- tools/testing/selftests/kvm/x86_64/mmio_warning_test.c | 2 +- .../testing/selftests/kvm/x86_64/pmu_event_filter_test.c | 4 ++-- .../kvm/x86_64/vmx_exception_with_invalid_guest_state.c | 2 +- 6 files changed, 13 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index fdb1af5ca611..c7885f72132a 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -19,6 +19,9 @@ #include "../kvm_util.h" +extern bool host_cpu_is_intel; +extern bool host_cpu_is_amd; + #define NMI_VECTOR 0x02 #define X86_EFLAGS_FIXED (1u << 1) diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c index 0b8de34aa10e..84915bc7d689 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c @@ -19,8 +19,8 @@ #define MAX_NR_CPUID_ENTRIES 100 vm_vaddr_t exception_handlers; -static bool host_cpu_is_amd; -static bool host_cpu_is_intel; +bool host_cpu_is_amd; +bool host_cpu_is_intel; static void regs_dump(FILE *stream, struct kvm_regs *regs, uint8_t indent) { @@ -115,7 +115,7 @@ static void sregs_dump(FILE *stream, struct kvm_sregs *sregs, uint8_t indent) bool kvm_is_tdp_enabled(void) { - if (this_cpu_is_intel()) + if (host_cpu_is_intel) return get_kvm_intel_param_bool("ept"); else return get_kvm_amd_param_bool("npt"); @@ -1218,7 +1218,7 @@ unsigned long vm_compute_max_gfn(struct kvm_vm *vm) max_gfn = (1ULL << (vm->pa_bits - vm->page_shift)) - 1; /* Avoid reserved HyperTransport region on AMD processors. */ - if (!this_cpu_is_amd()) + if (!host_cpu_is_amd) return max_gfn; /* On parts with <40 physical address bits, the area is fully hidden */ diff --git a/tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c b/tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c index 5489c9836ec8..0f728f05ea82 100644 --- a/tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c +++ b/tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c @@ -48,10 +48,10 @@ static void guest_main(void) const uint8_t *other_hypercall_insn; uint64_t ret; - if (this_cpu_is_intel()) { + if (host_cpu_is_intel) { native_hypercall_insn = vmx_vmcall; other_hypercall_insn = svm_vmmcall; - } else if (this_cpu_is_amd()) { + } else if (host_cpu_is_amd) { native_hypercall_insn = svm_vmmcall; other_hypercall_insn = vmx_vmcall; } else { diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c index b0a2a0bae0f3..ce1ccc4c1503 100644 --- a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c +++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c @@ -93,7 +93,7 @@ int main(void) { int warnings_before, warnings_after; - TEST_REQUIRE(this_cpu_is_intel()); + TEST_REQUIRE(host_cpu_is_intel); TEST_REQUIRE(!vm_is_unrestricted_guest(NULL)); diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c index c728822461b2..4dbb454e1760 100644 --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c @@ -363,7 +363,7 @@ static void test_pmu_config_disable(void (*guest_code)(void)) */ static bool use_intel_pmu(void) { - return this_cpu_is_intel() && + return host_cpu_is_intel && kvm_cpu_property(X86_PROPERTY_PMU_VERSION) && kvm_cpu_property(X86_PROPERTY_PMU_NR_GP_COUNTERS) && kvm_pmu_has(X86_PMU_FEATURE_BRANCH_INSNS_RETIRED); @@ -397,7 +397,7 @@ static bool use_amd_pmu(void) uint32_t family = kvm_cpu_family(); uint32_t model = kvm_cpu_model(); - return this_cpu_is_amd() && + return host_cpu_is_amd && (is_zen1(family, model) || is_zen2(family, model) || is_zen3(family, model)); diff --git a/tools/testing/selftests/kvm/x86_64/vmx_exception_with_invalid_guest_state.c b/tools/testing/selftests/kvm/x86_64/vmx_exception_with_invalid_guest_state.c index 53e1ef2fc774..ccdfa5dc1a4d 100644 --- a/tools/testing/selftests/kvm/x86_64/vmx_exception_with_invalid_guest_state.c +++ b/tools/testing/selftests/kvm/x86_64/vmx_exception_with_invalid_guest_state.c @@ -111,7 +111,7 @@ int main(int argc, char *argv[]) struct kvm_vcpu *vcpu; struct kvm_vm *vm; - TEST_REQUIRE(this_cpu_is_intel()); + TEST_REQUIRE(host_cpu_is_intel); TEST_REQUIRE(!vm_is_unrestricted_guest(NULL)); vm = vm_create_with_one_vcpu(&vcpu, guest_code); base-commit: 04b420511919f7b78f17f5fa6dc92975a8b2d7c4 --