public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: "Mi, Dapeng" <dapeng1.mi@linux.intel.com>
To: Jim Mattson <jmattson@google.com>
Cc: Sean Christopherson <seanjc@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Zhenyu Wang <zhenyuw@linux.intel.com>,
	Zhang Xiong <xiong.y.zhang@intel.com>,
	Mingwei Zhang <mizhang@google.com>,
	Like Xu <like.xu.linux@gmail.com>,
	Dapeng Mi <dapeng1.mi@intel.com>
Subject: Re: [kvm-unit-tests Patch v2 4/5] x86: pmu: Support validation for Intel PMU fixed counter 3
Date: Wed, 1 Nov 2023 11:15:57 +0800	[thread overview]
Message-ID: <fbad1983-5cde-4c7b-aaed-412110fe737f@linux.intel.com> (raw)
In-Reply-To: <CALMp9eRH5pttOA5BApdVeSbbkOU-kWcOWAoGMfK-9f=cy2Jf0g@mail.gmail.com>


On 11/1/2023 10:47 AM, Jim Mattson wrote:
> On Tue, Oct 31, 2023 at 7:33 PM Mi, Dapeng <dapeng1.mi@linux.intel.com> wrote:
>>
>> On 11/1/2023 2:47 AM, Jim Mattson wrote:
>>> On Tue, Oct 31, 2023 at 2:22 AM Dapeng Mi <dapeng1.mi@linux.intel.com> wrote:
>>>> Intel CPUs, like Sapphire Rapids, introduces a new fixed counter
>>>> (fixed counter 3) to counter/sample topdown.slots event, but current
>>>> code still doesn't cover this new fixed counter.
>>>>
>>>> So this patch adds code to validate this new fixed counter can count
>>>> slots event correctly.
>>> I'm not convinced that this actually validates anything.
>>>
>>> Suppose, for example, that KVM used fixed counter 1 when the guest
>>> asked for fixed counter 3. Wouldn't this test still pass?
>>
>> Per my understanding, as long as the KVM returns a valid count in the
>> reasonable count range, we can think KVM works correctly. We don't need
>> to entangle on how KVM really uses the HW, it could be impossible and
>> unnecessary.
> Now, I see how the Pentium FDIV bug escaped notice. Hey, the numbers
> are in a reasonable range. What's everyone upset about?
>
>> Yeah, currently the predefined valid count range may be some kind of
>> loose since I want to cover as much as hardwares and avoid to cause
>> regression. Especially after introducing the random jump and clflush
>> instructions, the cycles and slots become much more hard to predict.
>> Maybe we can have a comparable restricted count range in the initial
>> change, and we can loosen the restriction then if we encounter a failure
>> on some specific hardware. do you think it's better? Thanks.
> I think the test is essentially useless, and should probably just be
> deleted, so that it doesn't give a false sense of confidence.

IMO, I can't say the tests are totally useless. Yes,  passing the tests 
doesn't mean the KVM vPMU must work correctly, but we can say there is 
something probably wrong if it fails to pass these tests. Considering 
the hardware differences, it's impossible to set an exact value for 
these events in advance and it seems there is no better method to verify 
the PMC count as well. I still prefer to keep these tests until we have 
a better method to verify the accuracy of the PMC count.



  reply	other threads:[~2023-11-01  3:16 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-31  9:29 [kvm-unit-tests Patch v2 0/5] Fix PMU test failures on Sapphire Rapids Dapeng Mi
2023-10-31  9:29 ` [kvm-unit-tests Patch v2 1/5] x86: pmu: Remove duplicate code in pmu_init() Dapeng Mi
2023-11-01 12:51   ` Jim Mattson
2023-10-31  9:29 ` [kvm-unit-tests Patch v2 2/5] x86: pmu: Improve loop() to force to generate llc/branch misses Dapeng Mi
2023-10-31  9:29 ` [kvm-unit-tests Patch v2 3/5] x86: pmu: Enlarge cnt array length to 64 in check_counters_many() Dapeng Mi
2023-10-31  9:29 ` [kvm-unit-tests Patch v2 4/5] x86: pmu: Support validation for Intel PMU fixed counter 3 Dapeng Mi
2023-10-31 18:47   ` Jim Mattson
2023-11-01  2:33     ` Mi, Dapeng
2023-11-01  2:47       ` Jim Mattson
2023-11-01  3:15         ` Mi, Dapeng [this message]
2023-11-01  3:24           ` Jim Mattson
2023-11-01  3:57             ` Mi, Dapeng
2023-11-01 13:51               ` Liang, Kan
2023-10-31  9:29 ` [kvm-unit-tests Patch v2 5/5] x86: pmu: Add asserts to warn inconsistent fixed events and counters Dapeng Mi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fbad1983-5cde-4c7b-aaed-412110fe737f@linux.intel.com \
    --to=dapeng1.mi@linux.intel.com \
    --cc=dapeng1.mi@intel.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=like.xu.linux@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mizhang@google.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=xiong.y.zhang@intel.com \
    --cc=zhenyuw@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox