Hello,

The goal of this series is to allow userspace to limit the number
of PMU event counters on the vCPU.  We need this to support migration
across systems that implement different numbers of counters.

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 current PE by default.  Userspace can set PMCR_EL0.N for the
vCPU to any value even with the current KVM 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.

The series is based on kvmarm/next @0a3a1665cbc59 to include the
vCPU reset and feature flags cleanup/fixes series [1] and the
new sysreg definitions [2] that the selftests in this series utilizes.

Patch 1 adds helper functions to set a PMU for the guest. This
helper will make it easier for the following patches to add
modify codes for that process.

Patch 2 makes the default PMU for the guest set before the first
vCPU reset.

Patch 3 adds a helper to read vCPU's PMCR_EL0.

Patch 4 changes the code to use the guest's PMCR_EL0.N, instead
of the PE's PMCR_EL0.N.

Patch 5 adds userspace handlers for PM{C,I}NTEN{SET,CLR} and
PMOVS{SET,CLR} to consider the guest's PMCR.N.

Patch 6 sanitizes the PM{C,I}NTEN{SET,CLR} and PMOVS{SET,CLR} registers
before the first run of the guest based on the number of counters
configured.

Patch 7 adds support userspace modifying PMCR_EL0.N.

Patch 8-13 adds a selftest to verify reading and writing PMU registers
for implemented or unimplemented PMU event counters on the vCPU.

v8: Thanks, Oliver, Sebastian, and Eric for suggestions
- Drop v7 patches 3 and 4, and bring back initializing the
  PM{C,I}NTEN{SET,CLR} and PMOVS{SET,CLR} registers with unknown
  values. (Eric)
- Implement {get,set}_user callbacks for PM{C,I}NTEN{SET,CLR} and
  PMOVS{SET,CLR} registers. (Oliver)
- Sanitize PM{C,I}NTEN{SET,CLR} and PMOVS{SET,CLR} registers
  before starting the first vCPU run. (Oliver)
- Rename kvm_vcpu_set_pmu() to kvm_setup_vcpu(). (Oliver)
- Rename kvm_arm_get_num_counters() to kvm_arm_pmu_get_max_counters()
  and squash it into the caller's patch. (Oliver)
- In set_pmcr() implementation, do not initialize the pmcr register
  with kvm_vcpu_read_pmcr(). (Oliver) 
- Introduce test_create_vpmu_vm_with_pmcr_n() in the selftest to
  carry the commonly used code of creating a VM and configuring
  its PMCR.N field. (Eric)
- Add a selftest scenario to check the immutable behavior of
  the registers. (Sebastian)
- Add a selftest scenario to check the valid behavior of
  PM{C,I}NTEN{SET,CLR} and PMOVS{SET,CLR} registers when accessed
  from userspace.
- Address other nits.

v7: Thanks, Oliver for the suggestions
- Rebase the series onto kvmarm/next.
- Move the logic to set the default PMU for the guest from
  kvm_reset_vcpu() to __kvm_vcpu_set_target() to deal with the
  error returned.
- Add a helper, kvm_arm_get_num_counters(), to read the number
  of general-purpose counters.
- Use this helper to fix the error reported by kernel test robot [3].

v6: Thanks, Oliver and Shaoqin for the suggestions
- Split the previously defined kvm_arm_set_vm_pmu() into separate
  functions: default arm_pmu and a caller requested arm_pmu.
- Send -EINVAL from kvm_reset_vcpu(), instead of -ENODEV for the
  case where KVM fails to set a default arm_pmu, to remain consistent
  with the existing behavior.
- Drop the v5 patch-5/12 that removes ARMV8_PMU_PMCR_N_MASK and adds
  ARMV8_PMU_PMCR_N. Make corresponding changes to v5 patch-6/12.
- Disregard introducing 'pmcr_n_limit' in kvm->arch as a member to
  be accessed later in 'set_pmcr()'. Instead, directly obtain the
  value by accessing the saved 'arm_pmu'.
- 'set_pmcr()' ignores the error when userspace tries to set PMCR.N
  greater than the hardware limit to keep the existing API behavior.
- 'set_pmcr()' ignores modifications to the register after the VM has
  started and returns a success to userspace.
- Introduce [get|set]_pmcr_n() helpers in the selftest to make
  modifications to the field easier.
- Define the 'vpmu_vm' globally in the selftest, instead of allocating
  it every time a VM is created.
- Use the new printf style __GUEST_ASSERT()s in the selftest. 

v5:
https://lore.kernel.org/all/20230817003029.3073210-1-rananta@google.com/
 - Drop the patches (v4 3,4) related to PMU version fixes as it's
   now being handled in a separate series [4].
 - Switch to config_lock, instead of kvm->lock, while configuring
   the guest PMU.
 - Instead of continuing after a WARN_ON() for the return value of
   kvm_arm_set_vm_pmu() in kvm_arm_pmu_v3_set_pmu(), patch-1 now
   returns from the function immediately with the error code.
 - Fix WARN_ON() logic in kvm_host_pmu_init() (patch v4 9/14).
 - Instead of returning 0, return -ENODEV from the
   kvm_arm_set_vm_pmu() stub function.
 - Do not define the PMEVN_CASE() and PMEVN_SWITCH() macros in
   the selftest code as they are now included in the imported
   arm_pmuv3.h header.
 - Since the (initial) purpose of the selftest is to test the
   accessibility of the counter registers, remove the functional
   test at the end of test_access_pmc_regs(). It'll be added
   later in a separate series.
 - Introduce additional helper functions (destroy_vpmu_vm(),
   PMC_ACC_TO_IDX()) in the selftest for ease of maintenance
   and debugging.
   
v4:
https://lore.kernel.org/all/20230211031506.4159098-1-reijiw@google.com/
 - Fix the selftest bug in patch 13 (Have test_access_pmc_regs() to
   specify pmc index for test_bitmap_pmu_regs() instead of bit-shifted
   value (Thank you Raghavendra for the reporting the issue!).

v3:
https://lore.kernel.org/all/20230203040242.1792453-1-reijiw@google.com/
 - Remove reset_pmu_reg(), and use reset_val() instead. [Marc]
 - Fixed the initial value of PMCR_EL0.N on heterogeneous
   PMU systems. [Oliver]
 - Fixed PMUVer issues on heterogeneous PMU systems.
 - Fixed typos [Shaoqin]

v2:
https://lore.kernel.org/all/20230117013542.371944-1-reijiw@google.com/
 - Added the sys_reg's set_user() handler for the PMCR_EL0 to
   disallow userspace to set PMCR_EL0.N for the vCPU to a value
   that is greater than the host value (and added a new test
   case for this behavior). [Oliver]
 - Added to the commit log of the patch 2 that PMUSERENR_EL0 and
   PMCCFILTR_EL0 have UNKNOWN reset values.

v1:
https://lore.kernel.org/all/20221230035928.3423990-1-reijiw@google.com/

Thank you.
Raghavendra

[1]: https://lore.kernel.org/all/20230920195036.1169791-1-oliver.upton@linux.dev/
[2]: https://lore.kernel.org/all/20231011195740.3349631-5-oliver.upton@linux.dev/
[3]: https://lore.kernel.org/all/202309290607.Qgg05wKw-lkp@intel.com/
[4]: https://lore.kernel.org/all/20230728181907.1759513-1-reijiw@google.com/

Raghavendra Rao Ananta (6):
  KVM: arm64: PMU: Set PMCR_EL0.N for vCPU based on the associated PMU
  KVM: arm64: Add {get,set}_user for PM{C,I}NTEN{SET,CLR},
    PMOVS{SET,CLR}
  KVM: arm64: Sanitize PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR} before first
    run
  tools: Import arm_pmuv3.h
  KVM: selftests: aarch64: vPMU test for validating user accesses
  KVM: selftests: aarch64: vPMU test for immutability

Reiji Watanabe (7):
  KVM: arm64: PMU: Introduce helpers to set the guest's PMU
  KVM: arm64: PMU: Set the default PMU for the guest before vCPU reset
  KVM: arm64: PMU: Add a helper to read a vCPU's PMCR_EL0
  KVM: arm64: PMU: Allow userspace to limit PMCR_EL0.N for the guest
  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/include/asm/kvm_host.h             |   3 +
 arch/arm64/kvm/arm.c                          |  22 +-
 arch/arm64/kvm/pmu-emul.c                     | 112 ++-
 arch/arm64/kvm/sys_regs.c                     | 180 ++++-
 include/kvm/arm_pmu.h                         |  20 +
 tools/include/perf/arm_pmuv3.h                | 308 ++++++++
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../kvm/aarch64/vpmu_counter_access.c         | 726 ++++++++++++++++++
 .../selftests/kvm/include/aarch64/processor.h |   1 +
 9 files changed, 1319 insertions(+), 54 deletions(-)
 create mode 100644 tools/include/perf/arm_pmuv3.h
 create mode 100644 tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c


base-commit: 0a3a1665cbc59ee8d6326aa6c0b4a8d1cd67dda3
-- 
2.42.0.655.g421f12c284-goog

On Fri, 20 Oct 2023 22:40:40 +0100,
Raghavendra Rao Ananta <rananta@google.com> wrote:
> 
> Hello,
> 
> The goal of this series is to allow userspace to limit the number
> of PMU event counters on the vCPU.  We need this to support migration
> across systems that implement different numbers of counters.

FWIW, I've pushed out a branch[1] with a set of fixes that address
some of the comments I had on this series. Feel free to squash them in
your series as you see fit.

	M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/pmu_pmcr_n

-- 
Without deviation from the norm, progress is not possible.

On Mon, Oct 23, 2023 at 07:35:44PM +0100, Marc Zyngier wrote:
> On Fri, 20 Oct 2023 22:40:40 +0100,
> Raghavendra Rao Ananta <rananta@google.com> wrote:
> > 
> > Hello,
> > 
> > The goal of this series is to allow userspace to limit the number
> > of PMU event counters on the vCPU.  We need this to support migration
> > across systems that implement different numbers of counters.
> 
> FWIW, I've pushed out a branch[1] with a set of fixes that address
> some of the comments I had on this series. Feel free to squash them in
> your series as you see fit.

I did a second round of fixes on top of what Marc has and pushed that to
a branch [*]. If everything looks good I'll take it for 6.7.

[*] https://git.kernel.org/pub/scm/linux/kernel/git/oupton/linux.git/log/?h=kvm-arm64/pmu_pmcr_n

-- 
Thanks,
Oliver

On Fri, 20 Oct 2023 21:40:40 +0000, Raghavendra Rao Ananta wrote:
> The goal of this series is to allow userspace to limit the number
> of PMU event counters on the vCPU.  We need this to support migration
> across systems that implement different numbers of counters.
> 
> 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 current PE by default.  Userspace can set PMCR_EL0.N for the
> vCPU to any value even with the current KVM 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.
> 
> [...]

I've applied this with Marc + I's fixes. I'm happy to toss any fixes
on top of this series if folks spot issues.

[01/13] KVM: arm64: PMU: Introduce helpers to set the guest's PMU
        https://git.kernel.org/kvmarm/kvmarm/c/1616ca6f3c10
[02/13] KVM: arm64: PMU: Set the default PMU for the guest before vCPU reset
        https://git.kernel.org/kvmarm/kvmarm/c/427733579744
[03/13] KVM: arm64: PMU: Add a helper to read a vCPU's PMCR_EL0
        https://git.kernel.org/kvmarm/kvmarm/c/57fc267f1b5c
[04/13] KVM: arm64: PMU: Set PMCR_EL0.N for vCPU based on the associated PMU
        https://git.kernel.org/kvmarm/kvmarm/c/4d20debf9ca1
[05/13] KVM: arm64: Add {get,set}_user for PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}
        https://git.kernel.org/kvmarm/kvmarm/c/a45f41d754e0
[06/13] KVM: arm64: Sanitize PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR} before first run
        https://git.kernel.org/kvmarm/kvmarm/c/27131b199f9f
[07/13] KVM: arm64: PMU: Allow userspace to limit PMCR_EL0.N for the guest
        https://git.kernel.org/kvmarm/kvmarm/c/ea9ca904d24f
[08/13] tools: Import arm_pmuv3.h
        https://git.kernel.org/kvmarm/kvmarm/c/9f4b3273dfbe
[09/13] KVM: selftests: aarch64: Introduce vpmu_counter_access test
        https://git.kernel.org/kvmarm/kvmarm/c/8d0aebe1ca2b
[10/13] KVM: selftests: aarch64: vPMU register test for implemented counters
        https://git.kernel.org/kvmarm/kvmarm/c/ada1ae68262d
[11/13] KVM: selftests: aarch64: vPMU register test for unimplemented counters
        https://git.kernel.org/kvmarm/kvmarm/c/e1cc87206348
[12/13] KVM: selftests: aarch64: vPMU test for validating user accesses
        https://git.kernel.org/kvmarm/kvmarm/c/62708be351fe

--
Best,
Oliver

On Fri, 20 Oct 2023 22:40:40 +0100,
Raghavendra Rao Ananta <rananta@google.com> wrote:
> 
> Hello,
> 
> The goal of this series is to allow userspace to limit the number
> of PMU event counters on the vCPU.  We need this to support migration
> across systems that implement different numbers of counters.

[...]

I've gone through the initial patches, and stopped before the tests
(which I usually can't be bothered to review anyway).

The comments I have a relatively minor and could be applied as fixes
on top if Oliver can be convinced to do so. Note that patch #4 has an
attribution issue.

> base-commit: 0a3a1665cbc59ee8d6326aa6c0b4a8d1cd67dda3

maz@valley-girl:~/hot-poop/arm-platforms$ git describe 0a3a1665cbc59ee8d6326aa6c0b4a8d1cd67dda3
fatal: 0a3a1665cbc59ee8d6326aa6c0b4a8d1cd67dda3 is neither a commit nor blob

Can you please make an effort to base your postings on a known, stable
commit? A tagged -rc would be best. but certainly not a random commit.

This sort of information is just as useful as "No functional change
intended"...

	M.

-- 
Without deviation from the norm, progress is not possible.

On Mon, Oct 23, 2023 at 6:09 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Fri, 20 Oct 2023 22:40:40 +0100,
> Raghavendra Rao Ananta <rananta@google.com> wrote:
> >
> > Hello,
> >
> > The goal of this series is to allow userspace to limit the number
> > of PMU event counters on the vCPU.  We need this to support migration
> > across systems that implement different numbers of counters.
>
> [...]
>
> I've gone through the initial patches, and stopped before the tests
> (which I usually can't be bothered to review anyway).
>
> The comments I have a relatively minor and could be applied as fixes
> on top if Oliver can be convinced to do so. Note that patch #4 has an
> attribution issue.
>
> > base-commit: 0a3a1665cbc59ee8d6326aa6c0b4a8d1cd67dda3
>
> maz@valley-girl:~/hot-poop/arm-platforms$ git describe 0a3a1665cbc59ee8d6326aa6c0b4a8d1cd67dda3
> fatal: 0a3a1665cbc59ee8d6326aa6c0b4a8d1cd67dda3 is neither a commit nor blob
>
> Can you please make an effort to base your postings on a known, stable
> commit? A tagged -rc would be best. but certainly not a random commit.
>
I usually do base on a known -rc. But this series needed a couple of
series from kvmarm/next (mentioned in the original patch), and hence I
rebased on top of them. How do you suggest I handle this in the
future? Rebase to a known -rc on mainline, apply the required series,
and then my series on top?

Thank you.
Raghavendra

On Mon, 23 Oct 2023 18:58:19 +0100,
Raghavendra Rao Ananta <rananta@google.com> wrote:
> 
> On Mon, Oct 23, 2023 at 6:09 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Fri, 20 Oct 2023 22:40:40 +0100,
> > Raghavendra Rao Ananta <rananta@google.com> wrote:
> > >
> > > Hello,
> > >
> > > The goal of this series is to allow userspace to limit the number
> > > of PMU event counters on the vCPU.  We need this to support migration
> > > across systems that implement different numbers of counters.
> >
> > [...]
> >
> > I've gone through the initial patches, and stopped before the tests
> > (which I usually can't be bothered to review anyway).
> >
> > The comments I have a relatively minor and could be applied as fixes
> > on top if Oliver can be convinced to do so. Note that patch #4 has an
> > attribution issue.
> >
> > > base-commit: 0a3a1665cbc59ee8d6326aa6c0b4a8d1cd67dda3
> >
> > maz@valley-girl:~/hot-poop/arm-platforms$ git describe 0a3a1665cbc59ee8d6326aa6c0b4a8d1cd67dda3
> > fatal: 0a3a1665cbc59ee8d6326aa6c0b4a8d1cd67dda3 is neither a commit nor blob
> >
> > Can you please make an effort to base your postings on a known, stable
> > commit? A tagged -rc would be best. but certainly not a random commit.
> >
> I usually do base on a known -rc. But this series needed a couple of
> series from kvmarm/next (mentioned in the original patch), and hence I
> rebased on top of them.

Well, that commit has since disappeared, as git cannot find it (as
demonstrated above). Which is why I insist on a public tag as a base,
as everything else is completely volatile.

> How do you suggest I handle this in the future? Rebase to a known
> -rc on mainline, apply the required series, and then my series on
> top?

No. You base your own series on an -rc (ideally, -rc3). If there is a
conflict with another series, it is our job (Oliver and I) to fix it
(bonus points if you indicate a resolution for the conflict in the
cover letter).

If there is a hard dependency (something that would actively prevent
your series from working at all), you cherry-pick the minimal set of
patches that makes your own series functional as a *prefix*, and post
the whole thing, including the patches you depend on. Oliver and I
will make sure the common prefix is dealt with without duplication.

And for what it is worth, this series directly applies on v6.6-rc3
without a conflict.

	M.

-- 
Without deviation from the norm, progress is not possible.