Re: [PATCH 0/9] KVM: arm64: PMU: Fixing chained events, and PMUv3p5 support

[Oliver Upton <oliver.upton@linux.dev>]

Hi Ricardo,

On Wed, Aug 10, 2022 at 11:46:22AM -0700, Ricardo Koller wrote:
> On Fri, Aug 05, 2022 at 02:58:04PM +0100, Marc Zyngier wrote:
> > Ricardo recently reported[1] that our PMU emulation was busted when it
> > comes to chained events, as we cannot expose the overflow on a 32bit
> > boundary (which the architecture requires).
> > 
> > This series aims at fixing this (by deleting a lot of code), and as a
> > bonus adds support for PMUv3p5, as this requires us to fix a few more
> > things.
> > 
> > Tested on A53 (PMUv3) and FVP (PMUv3p5).
> > 
> > [1] https://lore.kernel.org/r/20220805004139.990531-1-ricarkol@google.com
> > 
> > Marc Zyngier (9):
> >   KVM: arm64: PMU: Align chained counter implementation with
> >     architecture pseudocode
> >   KVM: arm64: PMU: Distinguish between 64bit counter and 64bit overflow
> >   KVM: arm64: PMU: Only narrow counters that are not 64bit wide
> >   KVM: arm64: PMU: Add counter_index_to_*reg() helpers
> >   KVM: arm64: PMU: Simplify setting a counter to a specific value
> >   KVM: arm64: PMU: Move the ID_AA64DFR0_EL1.PMUver limit to VM creation
> >   KVM: arm64: PMU: Aleven ID_AA64DFR0_EL1.PMUver to be set from userspace
> >   KVM: arm64: PMU: Implement PMUv3p5 long counter support
> >   KVM: arm64: PMU: Aleven PMUv3p5 to be exposed to the guest
> > 
> >  arch/arm64/include/asm/kvm_host.h |   1 +
> >  arch/arm64/kvm/arm.c              |   6 +
> >  arch/arm64/kvm/pmu-emul.c         | 372 ++++++++++--------------------
> >  arch/arm64/kvm/sys_regs.c         |  65 +++++-
> >  include/kvm/arm_pmu.h             |  16 +-
> >  5 files changed, 208 insertions(+), 252 deletions(-)
> > 
> > -- 
> > 2.34.1
> > 
> 
> Hi Marc,
> 
> There is one extra potential issue with exposing PMUv3p5. I see this
> weird behavior when doing passthrough ("bare metal") on the fast-model
> configured to emulate PMUv3p5: the [63:32] half of the counters
> overflowing at 32-bits is still incremented.
> 
>   Fast model - ARMv8.5:
>    
> 	Assuming the initial state is even=0xFFFFFFFF and odd=0x0,
> 	incrementing the even counter leads to:
> 
> 	0x00000001_00000000	0x00000000_00000001		0x1
> 	even counter		odd counter			PMOVSET
> 
> 	Assuming the initial state is even=0xFFFFFFFF and odd=0xFFFFFFFF,
> 	incrementing the even counter leads to:
> 
> 	0x00000001_00000000	0x00000001_00000000		0x3
> 	even counter		odd counter			PMOVSET

This is to be expected, actually. PMUv8p5 counters are always 64 bit,
regardless of the configured overflow.

DDI 0487H D8.3 Behavior on overflow

  If FEAT_PMUv3p5 is implemented, 64-bit event counters are implemented,
  HDCR.HPMN is not 0, and either n is in the range [0 .. (HDCR.HPMN-1)]
  or EL2 is not implemented, then event counter overflow is configured
  by PMCR.LP:

  — When PMCR.LP is set to 0, if incrementing PMEVCNTR<n> causes an unsigned
    overflow of bits [31:0] of the event counter, the PE sets PMOVSCLR[n] to 1.
  — When PMCR.LP is set to 1, if incrementing PMEVCNTR<n> causes an unsigned
    overflow of bits [63:0] of the event counter, the PE sets PMOVSCLR[n] to 1.

  [...]

  For all 64-bit counters, incrementing the counter is the same whether an
  unsigned overflow occurs at [31:0] or [63:0]. If the counter increments
  for an event, bits [63:0] are always incremented.

Do you see this same (expected) failure w/ Marc's series?

--
Thanks,
Oliver
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm