Re: [kvm-unit-tests PATCH 3/3] arm: pmu: Remove checks for !overflow in chained counters tests

From: Marc Zyngier <maz@kernel.org>
To: Ricardo Koller <ricarkol@google.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu,
	drjones@redhat.com, alexandru.elisei@arm.com,
	eric.auger@redhat.com, oliver.upton@linux.dev, reijiw@google.com
Subject: Re: [kvm-unit-tests PATCH 3/3] arm: pmu: Remove checks for !overflow in chained counters tests
Date: Tue, 19 Jul 2022 12:34:05 +0100	[thread overview]
Message-ID: <87edyhz68i.wl-maz@kernel.org> (raw)
In-Reply-To: <20220718154910.3923412-4-ricarkol@google.com>

On Mon, 18 Jul 2022 16:49:10 +0100,
Ricardo Koller <ricarkol@google.com> wrote:
> 
> A chained event overflowing on the low counter can set the overflow flag
> in PMOVS.  KVM does not set it, but real HW and the fast-model seem to.
> Moreover, the AArch64.IncrementEventCounter() pseudocode in the ARM ARM
> (DDI 0487H.a, J1.1.1 "aarch64/debug") also sets the PMOVS bit on
> overflow.

Isn't this indicative of a bug in the KVM emulation? To be honest, the
pseudocode looks odd. It says:

<quote>
	if old_value<64:ovflw> != new_value<64:ovflw> then
	    PMOVSSET_EL0<idx> = '1';
	    PMOVSCLR_EL0<idx> = '1';
</quote>

which I find remarkably ambiguous. Is this setting and clearing the
overflow bit? Or setting it in the single register that backs the two
accessors in whatever way it can?

If it is the second interpretation that is correct, then KVM
definitely needs fixing (though this looks pretty involved for
anything that isn't a SWINC event).

	M.

-- 
Without deviation from the norm, progress is not possible.