Hey Reiji, On Tue, 30 May 2023 13:53:24 +0100, Reiji Watanabe <reijiw@xxxxxxxxxx> wrote: > > Hi Marc, > > On Mon, May 29, 2023 at 02:39:28PM +0100, Marc Zyngier wrote: > > On Sat, 27 May 2023 05:02:32 +0100, > > Reiji Watanabe <reijiw@xxxxxxxxxx> wrote: > > > > > > This series fixes issues with PMUVer handling for a guest with > > > PMU configured on heterogeneous PMU systems. > > > Specifically, it addresses the following two issues. > > > > > > [A] The default value of ID_AA64DFR0_EL1.PMUVer of the vCPU is set > > > to its sanitized value. This could be inappropriate on > > > heterogeneous PMU systems, as arm64_ftr_bits for PMUVer is defined > > > as FTR_EXACT with safe_val == 0 (when ID_AA64DFR0_EL1.PMUVer of all > > > PEs on the host is not uniform, the sanitized value will be 0). > > > > Why is this a problem? The CPUs don't implement the same version of > > the architecture, we don't get a PMU. Why should we try to do anything > > better? I really don't think we should go out or out way and make the > > code more complicated for something that doesn't really exist. > > Even when the CPUs don't implement the same version of the architecture, > if one of them implement PMUv3, KVM advertises KVM_CAP_ARM_PMU_V3, > and allows userspace to configure PMU (KVM_ARM_VCPU_PMU_V3) for vCPUs. Ah, I see it now. The kernel will register the PMU even if it decides that advertising it is wrong, and then we pick it up. Great :-/. > In this case, although KVM provides PMU emulations for the guest, > the guest's ID_AA64DFR0_EL1.PMUVer will be zero. Also, > KVM_SET_ONE_REG for ID_AA64DFR0_EL1 will never work for vCPUs > with PMU configured on such systems (since KVM also doesn't allow > userspace to set the PMUVer to 0 for the vCPUs with PMU configured). > > I would think either ID_AA64DFR0_EL1.PMUVer for the guest should > indicate PMUv3, or KVM should not allow userspace to configure PMU, > in this case. My vote is on the latter. Even if a PMU is available, we should rely on the feature exposed by the kernel to decide whether exposing a PMU or not. To be honest, this will affect almost nobody (I only know of a single one, an obscure ARMv8.0+ARMv8.2 system which is very unlikely to ever use KVM). I'm happy to take the responsibility to actively break those. > This series is a fix for the former, mainly to keep the current > behavior of KVM_CAP_ARM_PMU_V3 and KVM_ARM_VCPU_PMU_V3 on such > systems, since I wasn't sure if such systems don't really exist :) > (Also, I plan to implement a similar fix for PMCR_EL0.N on top of > those changes) > > I could make a fix for the latter instead though. What do you think ? I think this would be valuable. Also, didn't you have patches for the EL0 side of the PMU? I've been trying to look for a new version, but couldn't find it... Thanks, M. -- Without deviation from the norm, progress is not possible.
