On Tue, Oct 31, 2023 at 7:33 PM Mi, Dapeng <dapeng1.mi@xxxxxxxxxxxxxxx> wrote: > > > On 11/1/2023 2:47 AM, Jim Mattson wrote: > > On Tue, Oct 31, 2023 at 2:22 AM Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx> 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.