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 B13EFEB64DC for ; Wed, 28 Jun 2023 19:46:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231282AbjF1TqZ (ORCPT ); Wed, 28 Jun 2023 15:46:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58510 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230113AbjF1TqW (ORCPT ); Wed, 28 Jun 2023 15:46:22 -0400 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D8B771BE9 for ; Wed, 28 Jun 2023 12:46:20 -0700 (PDT) Received: by mail-pg1-x54a.google.com with SMTP id 41be03b00d2f7-528ab7097afso41439a12.1 for ; Wed, 28 Jun 2023 12:46:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1687981580; x=1690573580; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=I7mcp5/4P8AP0xkXt/thddGLST36j3uQroOasjJw2kQ=; b=3X3J2Y9E1VfBP9Ua7QLQHnf5tWOiCY196KrTDDRGoQT5j5w75S+ef9lJaxQf1I+Dr6 2ng0STjRGjZzYm7EtW7P3AqdvF4BpiTunzeqVHiCrNu8SLnudKTIoKMLwKyJ+dHLPtZx Uy1kq7ORYYrbeGQK/jo2+9FBGCEtb3qnJypIwWIFe5dx+s8TQoFaJ9YZI1UhCqWeNfUG wJ7PZQbwTrRcOtewooOJgYCoEZ9a9ay3tNFjuIrVPzJTK7o6U4C4oXVv4avxQuEpejEU +mrUIUhzaO+zX+OZ8TzVl3OK/CB6e1shtdVkwtZV8PtCKhlGXpsxLKpVzlQEyBGrOjtP Cd9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687981580; x=1690573580; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=I7mcp5/4P8AP0xkXt/thddGLST36j3uQroOasjJw2kQ=; b=WY3zN/h8fJ4b0+8n5qeMYRzhxVmnjCWzb16QL7VhZJcsulWnreXnRY2Buo9cIdbQqy FDCUZEGJH7B1ii1qKV4RZNhS2Xu+tL6qII9nUgYqQ7rQQOhU3uGn9uWck+P/Uhp1cj9H mAU8s8WXLat+X+xTol4a1nqtLefXq79rkwC86lUOnOuwmuk05V1U2RS39ikCT4QcyfYV mHVHBUpEskrA8v0OjeLUs2Hi3cg+OIW+r4OJOy+XXgqVYfiq3Ic1g3eYF3eB2t+lcEv3 TJhjUnCb7DQg1/VLnBApWNztmUMeqjQMNTj+xbWZHmeNjNy5T57q/XOkzQERlVjDF0py 09aQ== X-Gm-Message-State: AC+VfDy2Ib3fQBwM7CJAApgwSHuYFN3jRAR7Pp2ORB+mqtYQtDQXMfpA GV5waOikcAxnfpBgHGFx76pxjHtO6RM= X-Google-Smtp-Source: ACHHUZ63GagN7SvQAnsZIi3Pep1VIp9fjbRzI2L7PvnlNPi9oisw5tsVURUi8r1TgGB2Av+65c6uN1xbQbk= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a63:f50e:0:b0:54f:d3ef:539a with SMTP id w14-20020a63f50e000000b0054fd3ef539amr3666158pgh.4.1687981580106; Wed, 28 Jun 2023 12:46:20 -0700 (PDT) Date: Wed, 28 Jun 2023 12:46:18 -0700 In-Reply-To: <20230530134248.23998-2-cloudliang@tencent.com> Mime-Version: 1.0 References: <20230530134248.23998-1-cloudliang@tencent.com> <20230530134248.23998-2-cloudliang@tencent.com> Message-ID: Subject: Re: [PATCH v2 1/8] KVM: selftests: KVM: selftests: Add macros for fixed counters in processor.h From: Sean Christopherson To: Jinrong Liang Cc: Paolo Bonzini , Like Xu , David Matlack , Aaron Lewis , Vitaly Kuznetsov , Wanpeng Li , Jinrong Liang , kvm@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Heh, duplicate "KVM: selftests:" in the shortlog. On Tue, May 30, 2023, Jinrong Liang wrote: > From: Jinrong Liang > > Add macro in processor.h, providing a efficient way to obtain Try not to describe what the patch literally does in terms of code, the purpose of the shortlog+changelog is to complement the diff, e.g. it's super obvious from the diff that this patch adds macros in processor.h. > the number of fixed counters and fixed counters bit mask. The Wrap closer to 75 chars, 60 is too aggressive. > addition of these macro will simplify the handling of fixed > performance counters, while keeping the code maintainable and > clean. Instead of making assertions, justify the patch by stating the effects on code. Statements like "will simplify the handling" and "keeping the code maintainable and clean" are subjective. In cases like these, it's extremely unlikely anyone will disagree, but getting into the habit of providing concrete justification even for simple cases makes it easier to write changelogs for more complex changes. E.g. Add x86 properties for the number of PMU fixed counters and the bitmask that allows for "discontiguous" fixed counters so that tests don't have to manually retrieve the correct CPUID leaf+register, and so that the resulting code is self-documenting. > Signed-off-by: Jinrong Liang > --- > tools/testing/selftests/kvm/include/x86_64/processor.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h > index aa434c8f19c5..94751bddf1d9 100644 > --- a/tools/testing/selftests/kvm/include/x86_64/processor.h > +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h > @@ -240,6 +240,8 @@ struct kvm_x86_cpu_property { > #define X86_PROPERTY_PMU_VERSION KVM_X86_CPU_PROPERTY(0xa, 0, EAX, 0, 7) > #define X86_PROPERTY_PMU_NR_GP_COUNTERS KVM_X86_CPU_PROPERTY(0xa, 0, EAX, 8, 15) > #define X86_PROPERTY_PMU_EBX_BIT_VECTOR_LENGTH KVM_X86_CPU_PROPERTY(0xa, 0, EAX, 24, 31) > +#define X86_PROPERTY_PMU_FIXED_CTRS_BITMASK KVM_X86_CPU_PROPERTY(0xa, 0, ECX, 0, 31) Please spell out COUNTERS so that all the properties are consistent. > +#define X86_PROPERTY_PMU_NR_FIXED_COUNTERS KVM_X86_CPU_PROPERTY(0xa, 0, EDX, 0, 4) > > #define X86_PROPERTY_SUPPORTED_XCR0_LO KVM_X86_CPU_PROPERTY(0xd, 0, EAX, 0, 31) > #define X86_PROPERTY_XSTATE_MAX_SIZE_XCR0 KVM_X86_CPU_PROPERTY(0xd, 0, EBX, 0, 31) > -- > 2.31.1 >