On Wed, Jul 20, 2022 at 02:17:09PM -0700, Ricardo Koller wrote: > On Wed, Jul 20, 2022 at 10:45:20AM +0100, Marc Zyngier wrote: > > On Wed, 20 Jul 2022 09:40:01 +0100, > > Ricardo Koller <ricarkol@xxxxxxxxxx> wrote: > > > > > > On Tue, Jul 19, 2022 at 12:34:05PM +0100, Marc Zyngier wrote: > > > > On Mon, 18 Jul 2022 16:49:10 +0100, > > > > Ricardo Koller <ricarkol@xxxxxxxxxx> 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 > > > > > > I think it's the second, as those two "= '1'" apply to the non-chained > > > counters case as well, which should definitely set the bit in PMOVSSET. > > > > > > > (though this looks pretty involved for > > > > anything that isn't a SWINC event). > > > > > > Ah, I see, there's a pretty convenient kvm_pmu_software_increment() for > > > SWINC, but a non-SWINC event is implemented as a single 64-bit perf > > > event. > > > > Indeed. Which means we need to de-optimise chained counters to being > > 32bit events, which is pretty annoying (for rapidly firing events, the > > interrupt rate is going to be significantly higher). > > > > I guess we should also investigate the support for FEAT_PMUv3p5 and > > native 64bit counters. Someone is bound to build it at some point. > > The kernel perf event is implementing 64-bit counters using chained > counters. I assume this is already firing an interrupt for the low > counter overflow; we might need to just hook into that, investigating... > Additionally, given that the kernel is already emulating 64-bit counters, can KVM just expose FEAT_PMUv3p5? Assuming all the other new features can be emulated. Thanks, Ricardo > > > > Thanks, > > > > M. > > > > -- > > Without deviation from the norm, progress is not possible.
