Re: [PATCH 1/1] KVM: arm64: PMU: Avoid inappropriate use of host's PMUVer

[Reiji Watanabe <reijiw@google.com>]

Hi Oliver,

Thank you for the clarification!
But, I still have some questions on your comments.

> > > We emulate reads of PMCEID1_EL0 using the literal value of the CPU. The
> > > _advertised_ PMU version has no bearing on the core PMU version. So,
> > > assuming we hit this on a v3p5+ part with userspace (stupidly)
> > > advertising an older implementation level, we never clear the bit for
> > > STALL_SLOT.
> > 
> > I'm not sure if I understand this comment correctly.
> > When the guest's PMUVer is older than v3p4, I don't think we need
> > to clear the bit for STALL_SLOT, as PMMIR_EL1 is not implemented
> > for the guest (PMMIR_EL1 is implemented only on v3p4 or newer).
> > Or am I missing something ?
> 
> The guest's PMU version has no influence on the *hardware* value of
> PMCEID1_EL0.
> 
> Suppose KVM is running on a v3p5+ implementation, but userspace has set
> ID_AA64DFR0_EL1.PMUVer to v3p0. In this case the read of PMCEID1_EL0 on
> the preceding line would advertise the STALL_SLOT event, and KVM fails
> to mask it due to the ID register value. The fact we do not support the
> event is an invariant, in the worst case we wind up clearing a bit
> that's already 0.

As far as I checked ArmARM, the STALL_SLOT event can be supported on
any PMUv3 version (including on v3p0).  Assuming that is true, I don't
see any reason to not expose the event to the guest in this particular
example. Or can the STALL_SLOT event only be implemented from certain
versions of PMUv3 ?


> This is why I'd suggested just unconditionally clearing the bit. While

When the hardware supports the STALL_SLOT event (again, I assume any
PMUv3 version hardware can support the event), and the guest's PMUVer
is older than v3p4, what is the reason why we want to clear the bit ?


> we're on the topic, doesn't the same reasoning hold for
> STALL_SLOT_{FRONTEND,BACKEND}? We probably want to hide those too.

Yes, I agree on that.  
I will include the fix for that as a part of this series!

Thank you,
Reiji