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

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

[Oliver Upton]

On Sun, Jun 11, 2023 at 09:01:05AM -0700, Reiji Watanabe wrote:

[...]

> > 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 ?

Well, users of the event don't get the full picture w/o PMMIR_EL1.SLOTS,
which is only available on v3p4+. We probably should start exposing the
register + event (separate from this change).

> > 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 ?

What's the value of the event w/o PMMIR_EL1? I agree there's no
fundamental issue with letting it past, but I'd rather we start
exposing the feature when we provide all the necessary detail.

--
Thanks,
Oliver

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Reiji Watanabe]

On Mon, Jun 12, 2023 at 09:36:38PM +0200, Oliver Upton wrote:
> On Sun, Jun 11, 2023 at 09:01:05AM -0700, Reiji Watanabe wrote:
> 
> [...]
> 
> > > 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 ?
> 
> Well, users of the event don't get the full picture w/o PMMIR_EL1.SLOTS,
> which is only available on v3p4+. We probably should start exposing the
> register + event (separate from this change).
> 
> > > 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 ?
> 
> What's the value of the event w/o PMMIR_EL1? I agree there's no

I agree that the value of the event w/o PMMIR_EL1 is pretty limited.


> fundamental issue with letting it past, but I'd rather we start
> exposing the feature when we provide all the necessary detail.

To confirm, are you suggesting to stop exposing the event even on hosts
w/o PMMIR_EL1 until KVM gets ready to support PMMIR_EL1 ?
(guests on those hosts won't get PMMIR_EL1 in any case though?)
Could you please explain why ?

Perhaps I think I would rather keep the code as it is?
(since I'm simply not sure what would be the benefits of that)

Thank you,
Reiji

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Oliver Upton]

On Mon, Jun 12, 2023 at 05:26:33PM -0700, Reiji Watanabe wrote:
> On Mon, Jun 12, 2023 at 09:36:38PM +0200, Oliver Upton wrote:
> > I'd rather we start exposing the feature when we provide all the
> > necessary detail.
> 
> To confirm, are you suggesting to stop exposing the event even on hosts
> w/o PMMIR_EL1 until KVM gets ready to support PMMIR_EL1 ?
> (guests on those hosts won't get PMMIR_EL1 in any case though?)
> Could you please explain why ?
> 
> Perhaps I think I would rather keep the code as it is?
> (since I'm simply not sure what would be the benefits of that)

I'd rather not keep confusing code hanging around. The fact that KVM
does not support the STALL_SLOTS event is invariant of both the hardware
PMU implementation and the userspace value for the ID register field.
Let's make sure the implementation exactly matches this position.

-- 
Thanks,
Oliver

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel