On Mon, Aug 01, 2022, Like Xu wrote:
> From: Like Xu <likexu@xxxxxxxxxxx>
>
> The pmu test check_counter_overflow() always fails with the "./configure
> --arch=i386". The cnt.count obtained from the latter run of measure()
> (based on fixed counter 0) is not equal to the expected value (based
> on gp counter 0) and there is a positive error with a value of 2.
>
> The two extra instructions come from inline wrmsr() and inline rdmsr()
> inside the global_disable() binary code block. Specifically, for each msr
> access, the i386 code will have two assembly mov instructions before
> rdmsr/wrmsr (mark it for fixed counter 0, bit 32), but only one assembly
> mov is needed for x86_64 and gp counter 0 on i386.
>
> Fix the expected init cnt.count for fixed counter 0 overflow based on
> the same fixed counter 0, not always using gp counter 0.
>
> Signed-off-by: Like Xu <likexu@xxxxxxxxxxx>
> ---
> x86/pmu.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/x86/pmu.c b/x86/pmu.c
> index 01be1e9..4bb24e9 100644
> --- a/x86/pmu.c
> +++ b/x86/pmu.c
> @@ -304,6 +304,10 @@ static void check_counter_overflow(void)
>
> if (i == nr_gp_counters) {
> cnt.ctr = fixed_events[0].unit_sel;
> + cnt.count = 0;
> + measure(&cnt, 1);
Not directly related to this patch...
Unless I've missed something, every invocation of start_event() and measure() first
sets evt.count=0. Rather than force every caller to ensure count is zeroed, why not
zero the count during start_event() and then drop all of the manual zeroing?
diff --git a/x86/pmu.c b/x86/pmu.c
index 01be1e90..ef804272 100644
--- a/x86/pmu.c
+++ b/x86/pmu.c
@@ -141,7 +141,7 @@ static void global_disable(pmu_counter_t *cnt)
static void start_event(pmu_counter_t *evt)
{
- wrmsr(evt->ctr, evt->count);
+ wrmsr(evt->ctr, 0);
if (is_gp(evt))
wrmsr(MSR_P6_EVNTSEL0 + event_to_global_idx(evt),
evt->config | EVNTSEL_EN);
Accumulating counts can be handled by reading the current count before start_event(),
and doing something like stuffing a high count to test an edge case could be handled
by an inner helper, e.g. by adding __start_event().
