| Message ID | 20221230035928.3423990-1-reijiw@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: arm64: PMU: Allow userspace to limit the number of PMCs on vCPU | expand |
On Thu, 29 Dec 2022 19:59:21 -0800 Reiji Watanabe <reijiw@google.com> wrote: > The goal of this series is to allow userspace to limit the number > of PMU event counters on the vCPU. Hi Rieji, Why do you want to do this? I can conjecture a bunch of possible reasons, but they may not match up with your use case. It would be useful to have that information in the cover letter. Jonathan > > The number of PMU event counters is indicated in PMCR_EL0.N. > For a vCPU with PMUv3 configured, its value will be the same as > the host value by default. Userspace can set PMCR_EL0.N for the > vCPU to a lower value than the host value, using KVM_SET_ONE_REG. > However, it is practically unsupported, as KVM resets PMCR_EL0.N > to the host value on vCPU reset and some KVM code uses the host > value to identify (un)implemented event counters on the vCPU. > > This series will ensure that the PMCR_EL0.N value is preserved > on vCPU reset and that KVM doesn't use the host value > to identify (un)implemented event counters on the vCPU. > This allows userspace to limit the number of the PMU event > counters on the vCPU. > > Patch 1 fixes reset_pmu_reg() to ensure that (RAZ) bits of > {PMCNTEN,PMOVS}{SET,CLR}_EL1 corresponding to unimplemented event > counters on the vCPU are reset to zero even when PMCR_EL0.N for > the vCPU is different from the host. > > Patch 2 is a minor refactoring to use the default PMU register reset > function (reset_pmu_reg()) for PMUSERENR_EL0 and PMCCFILTR_EL0. > (With the Patch 1 change, reset_pmu_reg() can now be used for > those registers) > > Patch 3 fixes reset_pmcr() to preserve PMCR_EL0.N for the vCPU on > vCPU reset. > > Patch 4-7 adds a selftest to verify reading and writing PMU registers > for implemented or unimplemented PMU event counters on the vCPU. > > The series is based on kvmarm/fixes at the following commit: > commit aff234839f8b ("KVM: arm64: PMU: Fix PMCR_EL0 reset value") > > Reiji Watanabe (7): > KVM: arm64: PMU: Have reset_pmu_reg() to clear a register > KVM: arm64: PMU: Use reset_pmu_reg() for PMUSERENR_EL0 and > PMCCFILTR_EL0 > KVM: arm64: PMU: Preserve vCPU's PMCR_EL0.N value on vCPU reset > tools: arm64: Import perf_event.h > KVM: selftests: aarch64: Introduce vpmu_counter_access test > KVM: selftests: aarch64: vPMU register test for implemented counters > KVM: selftests: aarch64: vPMU register test for unimplemented counters > > arch/arm64/kvm/pmu-emul.c | 6 + > arch/arm64/kvm/sys_regs.c | 18 +- > tools/arch/arm64/include/asm/perf_event.h | 258 ++++++++ > tools/testing/selftests/kvm/.gitignore | 1 + > tools/testing/selftests/kvm/Makefile | 1 + > .../kvm/aarch64/vpmu_counter_access.c | 613 ++++++++++++++++++ > .../selftests/kvm/include/aarch64/processor.h | 1 + > 7 files changed, 886 insertions(+), 12 deletions(-) > create mode 100644 tools/arch/arm64/include/asm/perf_event.h > create mode 100644 tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c > > > base-commit: aff234839f8b80ac101e6c2f14d0e44b236efa48
On Tue, 03 Jan 2023 12:40:34 +0000, Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote: > > On Thu, 29 Dec 2022 19:59:21 -0800 > Reiji Watanabe <reijiw@google.com> wrote: > > > The goal of this series is to allow userspace to limit the number > > of PMU event counters on the vCPU. > > Hi Rieji, > > Why do you want to do this? > > I can conjecture a bunch of possible reasons, but they may not > match up with your use case. It would be useful to have that information > in the cover letter. The most obvious use case is to support migration across systems that implement different numbers of counters. Similar reasoning could also apply to the debug infrastructure (watchpoints, breakpoints). In any case, being able to decouple the VM from the underlying HW within the extent that the architecture permits it seems like a valuable goal. Thanks, M.
Hi Jonathan, On Tue, Jan 3, 2023 at 4:47 AM Marc Zyngier <maz@kernel.org> wrote: > > On Tue, 03 Jan 2023 12:40:34 +0000, > Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote: > > > > On Thu, 29 Dec 2022 19:59:21 -0800 > > Reiji Watanabe <reijiw@google.com> wrote: > > > > > The goal of this series is to allow userspace to limit the number > > > of PMU event counters on the vCPU. > > > > Hi Rieji, > > > > Why do you want to do this? > > > > I can conjecture a bunch of possible reasons, but they may not > > match up with your use case. It would be useful to have that information > > in the cover letter. > > The most obvious use case is to support migration across systems that > implement different numbers of counters. Similar reasoning could also > apply to the debug infrastructure (watchpoints, breakpoints). Exactly, this is to unblock migration support between systems that implement different numbers of counters. I will include this information in the cover letter when I update the series for the v2. Thanks, Reiji > > In any case, being able to decouple the VM from the underlying HW > within the extent that the architecture permits it seems like a > valuable goal. > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
Hi Reiji, On Thu, Dec 29, 2022 at 07:59:21PM -0800, Reiji Watanabe wrote: > The goal of this series is to allow userspace to limit the number > of PMU event counters on the vCPU. > > The number of PMU event counters is indicated in PMCR_EL0.N. > For a vCPU with PMUv3 configured, its value will be the same as > the host value by default. Userspace can set PMCR_EL0.N for the > vCPU to a lower value than the host value, using KVM_SET_ONE_REG. > However, it is practically unsupported, as KVM resets PMCR_EL0.N > to the host value on vCPU reset and some KVM code uses the host > value to identify (un)implemented event counters on the vCPU. > > This series will ensure that the PMCR_EL0.N value is preserved > on vCPU reset and that KVM doesn't use the host value > to identify (un)implemented event counters on the vCPU. > This allows userspace to limit the number of the PMU event > counters on the vCPU. I just wanted to bring up the conversation we had today on the list as it is a pretty relevant issue. KVM currently allows any value to be written to PMCR_EL0.N, meaning that userspace could advertize more PMCs than are supported by the system. IDK if Marc feels otherwise, but it doesn't seem like we should worry about ABI change here (i.e. userspace can no longer write junk to the register) as KVM has advertized the correct value to userspace. The only case that breaks would be a userspace that intentionally sets PMCR_EL0.N to something larger than the host. As accesses to unadvertized PMC indices is CONSTRAINED UNPRED behavior, I'm having a hard time coming up with a use case. -- Thanks, Oliver
Hi Oliver, On Mon, Jan 9, 2023 at 6:01 PM Oliver Upton <oliver.upton@linux.dev> wrote: > > Hi Reiji, > > On Thu, Dec 29, 2022 at 07:59:21PM -0800, Reiji Watanabe wrote: > > The goal of this series is to allow userspace to limit the number > > of PMU event counters on the vCPU. > > > > The number of PMU event counters is indicated in PMCR_EL0.N. > > For a vCPU with PMUv3 configured, its value will be the same as > > the host value by default. Userspace can set PMCR_EL0.N for the > > vCPU to a lower value than the host value, using KVM_SET_ONE_REG. > > However, it is practically unsupported, as KVM resets PMCR_EL0.N > > to the host value on vCPU reset and some KVM code uses the host > > value to identify (un)implemented event counters on the vCPU. > > > > This series will ensure that the PMCR_EL0.N value is preserved > > on vCPU reset and that KVM doesn't use the host value > > to identify (un)implemented event counters on the vCPU. > > This allows userspace to limit the number of the PMU event > > counters on the vCPU. > > I just wanted to bring up the conversation we had today on the list as > it is a pretty relevant issue. > > KVM currently allows any value to be written to PMCR_EL0.N, meaning that > userspace could advertize more PMCs than are supported by the system. > > IDK if Marc feels otherwise, but it doesn't seem like we should worry > about ABI change here (i.e. userspace can no longer write junk to the > register) as KVM has advertized the correct value to userspace. The only > case that breaks would be a userspace that intentionally sets PMCR_EL0.N > to something larger than the host. As accesses to unadvertized PMC > indices is CONSTRAINED UNPRED behavior, I'm having a hard time coming up > with a use case. Yes, I agree with that. I will be looking at adding the validation of the register field for the v2. Thank you, Reiji
The goal of this series is to allow userspace to limit the number of PMU event counters on the vCPU. The number of PMU event counters is indicated in PMCR_EL0.N. For a vCPU with PMUv3 configured, its value will be the same as the host value by default. Userspace can set PMCR_EL0.N for the vCPU to a lower value than the host value, using KVM_SET_ONE_REG. However, it is practically unsupported, as KVM resets PMCR_EL0.N to the host value on vCPU reset and some KVM code uses the host value to identify (un)implemented event counters on the vCPU. This series will ensure that the PMCR_EL0.N value is preserved on vCPU reset and that KVM doesn't use the host value to identify (un)implemented event counters on the vCPU. This allows userspace to limit the number of the PMU event counters on the vCPU. Patch 1 fixes reset_pmu_reg() to ensure that (RAZ) bits of {PMCNTEN,PMOVS}{SET,CLR}_EL1 corresponding to unimplemented event counters on the vCPU are reset to zero even when PMCR_EL0.N for the vCPU is different from the host. Patch 2 is a minor refactoring to use the default PMU register reset function (reset_pmu_reg()) for PMUSERENR_EL0 and PMCCFILTR_EL0. (With the Patch 1 change, reset_pmu_reg() can now be used for those registers) Patch 3 fixes reset_pmcr() to preserve PMCR_EL0.N for the vCPU on vCPU reset. Patch 4-7 adds a selftest to verify reading and writing PMU registers for implemented or unimplemented PMU event counters on the vCPU. The series is based on kvmarm/fixes at the following commit: commit aff234839f8b ("KVM: arm64: PMU: Fix PMCR_EL0 reset value") Reiji Watanabe (7): KVM: arm64: PMU: Have reset_pmu_reg() to clear a register KVM: arm64: PMU: Use reset_pmu_reg() for PMUSERENR_EL0 and PMCCFILTR_EL0 KVM: arm64: PMU: Preserve vCPU's PMCR_EL0.N value on vCPU reset tools: arm64: Import perf_event.h KVM: selftests: aarch64: Introduce vpmu_counter_access test KVM: selftests: aarch64: vPMU register test for implemented counters KVM: selftests: aarch64: vPMU register test for unimplemented counters arch/arm64/kvm/pmu-emul.c | 6 + arch/arm64/kvm/sys_regs.c | 18 +- tools/arch/arm64/include/asm/perf_event.h | 258 ++++++++ tools/testing/selftests/kvm/.gitignore | 1 + tools/testing/selftests/kvm/Makefile | 1 + .../kvm/aarch64/vpmu_counter_access.c | 613 ++++++++++++++++++ .../selftests/kvm/include/aarch64/processor.h | 1 + 7 files changed, 886 insertions(+), 12 deletions(-) create mode 100644 tools/arch/arm64/include/asm/perf_event.h create mode 100644 tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c base-commit: aff234839f8b80ac101e6c2f14d0e44b236efa48