Hi Marc, On Thu, Jun 01, 2023 at 06:02:41AM +0100, Marc Zyngier wrote: > 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. Thank you for the information! Just curious, how about a mix of cores with and without PMU ? (with the same ARMv8.x version) I'm guessing there are very few if any though :) > > > 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. Thank you for the comment! I will go with the latter. > 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... While I'm working on fixing the series based on the recent comment from Oliver (https://lore.kernel.org/all/ZG%2Fw95pYjWnMJB62@xxxxxxxxx/), I have a new PMU EL0 issue, which blocked my testing of the series. So, I am debugging the new PMU EL0 issue. It appears that arch_perf_update_userpage() defined in drivers/perf/arm_pmuv3.c isn't used, and instead, the weak one in kernel/events/core.c is used. This prevents cap_user_rdpmc (, etc) from being set (This prevented my test program from directly accessing counters). This seems to be caused by the commit 7755cec63ade ("arm64: perf: Move PMUv3 driver to drivers/perf"). I have not yet figured out why the one in arm_pmuv3.c isn't used though (The weak one in core.c seems to take precedence over strong ones under drivers/ somehow...). Anyway, I worked around the new issue for now, and ran the test for my series though. I will post the new version of the EL0 series tomorrow hopefully. Thank you, Reiji > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
