[PATCH v5 0/5] KVM: arm64: PMU: Fix SET_ONE_REG for vPMC regs

[PATCH v5 0/5] KVM: arm64: PMU: Fix SET_ONE_REG for vPMC regs

[Akihiko Odaki]

Prepare vPMC registers for user-initiated changes after first run. This
is important specifically for debugging Windows on QEMU with GDB; QEMU
tries to write back all visible registers when resuming the VM execution
with GDB, corrupting the PMU state. Windows always uses the PMU so this
can cause adverse effects on that particular OS.

This series also contains patch "KVM: arm64: PMU: Set raw values from
user to PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}", which reverts semantic
changes made for the mentioned registers in the past. It is necessary
to migrate the PMU state properly on Firecracker, QEMU, and crosvm.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Changes in v5:
- Rebased.
- Link to v4: https://lore.kernel.org/r/20250313-pmc-v4-0-2c976827118c@daynix.com

Changes in v4:
- Reverted changes for functions implementing ioctls in patch
  "KVM: arm64: PMU: Assume PMU presence in pmu-emul.c".
- Removed kvm_pmu_vcpu_reset().
- Reordered function calls in kvm_vcpu_reload_pmu() for better style.
- Link to v3: https://lore.kernel.org/r/20250312-pmc-v3-0-0411cab5dc3d@daynix.com

Changes in v3:
- Added patch "KVM: arm64: PMU: Assume PMU presence in pmu-emul.c".
- Added an explanation of this path series' motivation to each patch.
- Explained why userspace register writes and register reset should be
  covered in patch "KVM: arm64: PMU: Reload when user modifies
  registers".
- Marked patch "KVM: arm64: PMU: Set raw values from user to
  PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}" for stable.
- Reoreded so that patch "KVM: arm64: PMU: Set raw values from user to
  PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}" would come first.
- Added patch "KVM: arm64: PMU: Call kvm_pmu_handle_pmcr() after masking
  PMCNTENSET_EL0".
- Added patch "KVM: arm64: Reload PMCNTENSET_EL0".
- Link to v2: https://lore.kernel.org/r/20250307-pmc-v2-0-6c3375a5f1e4@daynix.com

Changes in v2:
- Changed to utilize KVM_REQ_RELOAD_PMU as suggested by Oliver Upton.
- Added patch "KVM: arm64: PMU: Reload when user modifies registers"
  to cover more registers.
- Added patch "KVM: arm64: PMU: Set raw values from user to
  PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}".
- Link to v1: https://lore.kernel.org/r/20250302-pmc-v1-1-caff989093dc@daynix.com

---
Akihiko Odaki (5):
      KVM: arm64: PMU: Set raw values from user to PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}
      KVM: arm64: PMU: Assume PMU presence in pmu-emul.c
      KVM: arm64: PMU: Fix SET_ONE_REG for vPMC regs
      KVM: arm64: PMU: Reload when user modifies registers
      KVM: arm64: PMU: Reload when resetting

 arch/arm64/kvm/arm.c            | 17 ++++++++-----
 arch/arm64/kvm/emulate-nested.c |  6 +++--
 arch/arm64/kvm/pmu-emul.c       | 56 +++++++++++------------------------------
 arch/arm64/kvm/reset.c          |  3 ---
 arch/arm64/kvm/sys_regs.c       | 52 ++++++++++++++++++++++----------------
 include/kvm/arm_pmu.h           |  4 +--
 6 files changed, 62 insertions(+), 76 deletions(-)
---
base-commit: 80e54e84911a923c40d7bee33a34c1b4be148d7a
change-id: 20250302-pmc-b90a86af945c

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>

[PATCH v5 1/5] KVM: arm64: PMU: Set raw values from user to PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}

[Akihiko Odaki]

Commit a45f41d754e0 ("KVM: arm64: Add {get,set}_user for
PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}") changed KVM_SET_ONE_REG to update
the mentioned registers in a way matching with the behavior of guest
register writes. This is a breaking change of a UAPI though the new
semantics looks cleaner and VMMs are not prepared for this.

Firecracker, QEMU, and crosvm perform migration by listing registers
with KVM_GET_REG_LIST, getting their values with KVM_GET_ONE_REG and
setting them with KVM_SET_ONE_REG. This algorithm assumes
KVM_SET_ONE_REG restores the values retrieved with KVM_GET_ONE_REG
without any alteration. However, bit operations added by the earlier
commit do not preserve the values retried with KVM_GET_ONE_REG and
potentially break migration.

Remove the bit operations that alter the values retrieved with
KVM_GET_ONE_REG.

Cc: stable@vger.kernel.org
Fixes: a45f41d754e0 ("KVM: arm64: Add {get,set}_user for PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Acked-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 82430c1e1dd0..ffee72fd1273 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1051,26 +1051,9 @@ static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 
 static int set_pmreg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r, u64 val)
 {
-	bool set;
-
-	val &= kvm_pmu_accessible_counter_mask(vcpu);
-
-	switch (r->reg) {
-	case PMOVSSET_EL0:
-		/* CRm[1] being set indicates a SET register, and CLR otherwise */
-		set = r->CRm & 2;
-		break;
-	default:
-		/* Op2[0] being set indicates a SET register, and CLR otherwise */
-		set = r->Op2 & 1;
-		break;
-	}
-
-	if (set)
-		__vcpu_sys_reg(vcpu, r->reg) |= val;
-	else
-		__vcpu_sys_reg(vcpu, r->reg) &= ~val;
+	u64 mask = kvm_pmu_accessible_counter_mask(vcpu);
 
+	__vcpu_sys_reg(vcpu, r->reg) = val & mask;
 	return 0;
 }
 

-- 
2.48.1

Re: [PATCH v5 0/5] KVM: arm64: PMU: Fix SET_ONE_REG for vPMC regs

[Marc Zyngier]

On Sat, 15 Mar 2025 09:12:09 +0000,
Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> 
> Prepare vPMC registers for user-initiated changes after first run. This
> is important specifically for debugging Windows on QEMU with GDB; QEMU
> tries to write back all visible registers when resuming the VM execution
> with GDB, corrupting the PMU state. Windows always uses the PMU so this
> can cause adverse effects on that particular OS.
> 
> This series also contains patch "KVM: arm64: PMU: Set raw values from
> user to PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}", which reverts semantic
> changes made for the mentioned registers in the past. It is necessary
> to migrate the PMU state properly on Firecracker, QEMU, and crosvm.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> Changes in v5:
> - Rebased.
> - Link to v4: https://lore.kernel.org/r/20250313-pmc-v4-0-2c976827118c@daynix.com
> 
> Changes in v4:
> - Reverted changes for functions implementing ioctls in patch
>   "KVM: arm64: PMU: Assume PMU presence in pmu-emul.c".
> - Removed kvm_pmu_vcpu_reset().
> - Reordered function calls in kvm_vcpu_reload_pmu() for better style.
> - Link to v3: https://lore.kernel.org/r/20250312-pmc-v3-0-0411cab5dc3d@daynix.com
> 
> Changes in v3:
> - Added patch "KVM: arm64: PMU: Assume PMU presence in pmu-emul.c".
> - Added an explanation of this path series' motivation to each patch.
> - Explained why userspace register writes and register reset should be
>   covered in patch "KVM: arm64: PMU: Reload when user modifies
>   registers".
> - Marked patch "KVM: arm64: PMU: Set raw values from user to
>   PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}" for stable.
> - Reoreded so that patch "KVM: arm64: PMU: Set raw values from user to
>   PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}" would come first.
> - Added patch "KVM: arm64: PMU: Call kvm_pmu_handle_pmcr() after masking
>   PMCNTENSET_EL0".
> - Added patch "KVM: arm64: Reload PMCNTENSET_EL0".
> - Link to v2: https://lore.kernel.org/r/20250307-pmc-v2-0-6c3375a5f1e4@daynix.com
> 
> Changes in v2:
> - Changed to utilize KVM_REQ_RELOAD_PMU as suggested by Oliver Upton.
> - Added patch "KVM: arm64: PMU: Reload when user modifies registers"
>   to cover more registers.
> - Added patch "KVM: arm64: PMU: Set raw values from user to
>   PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}".
> - Link to v1: https://lore.kernel.org/r/20250302-pmc-v1-1-caff989093dc@daynix.com
> 
> ---
> Akihiko Odaki (5):
>       KVM: arm64: PMU: Set raw values from user to PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}
>       KVM: arm64: PMU: Assume PMU presence in pmu-emul.c
>       KVM: arm64: PMU: Fix SET_ONE_REG for vPMC regs
>       KVM: arm64: PMU: Reload when user modifies registers
>       KVM: arm64: PMU: Reload when resetting
> 
>  arch/arm64/kvm/arm.c            | 17 ++++++++-----
>  arch/arm64/kvm/emulate-nested.c |  6 +++--
>  arch/arm64/kvm/pmu-emul.c       | 56 +++++++++++------------------------------
>  arch/arm64/kvm/reset.c          |  3 ---
>  arch/arm64/kvm/sys_regs.c       | 52 ++++++++++++++++++++++----------------
>  include/kvm/arm_pmu.h           |  4 +--
>  6 files changed, 62 insertions(+), 76 deletions(-)

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.

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

Re: [PATCH v5 0/5] KVM: arm64: PMU: Fix SET_ONE_REG for vPMC regs

[Oliver Upton]

On Sat, 15 Mar 2025 18:12:09 +0900, Akihiko Odaki wrote:
> Prepare vPMC registers for user-initiated changes after first run. This
> is important specifically for debugging Windows on QEMU with GDB; QEMU
> tries to write back all visible registers when resuming the VM execution
> with GDB, corrupting the PMU state. Windows always uses the PMU so this
> can cause adverse effects on that particular OS.
> 
> This series also contains patch "KVM: arm64: PMU: Set raw values from
> user to PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}", which reverts semantic
> changes made for the mentioned registers in the past. It is necessary
> to migrate the PMU state properly on Firecracker, QEMU, and crosvm.
> 
> [...]

Squashed in a fix for CONFIG_HW_PERF_EVENTS=n build.

Applied to next, thanks!

[1/5] KVM: arm64: PMU: Set raw values from user to PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}
      https://git.kernel.org/kvmarm/kvmarm/c/f2aeb7bbd574
[2/5] KVM: arm64: PMU: Assume PMU presence in pmu-emul.c
      https://git.kernel.org/kvmarm/kvmarm/c/be5ccac3f15e
[3/5] KVM: arm64: PMU: Fix SET_ONE_REG for vPMC regs
      https://git.kernel.org/kvmarm/kvmarm/c/64074ca8ca92
[4/5] KVM: arm64: PMU: Reload when user modifies registers
      https://git.kernel.org/kvmarm/kvmarm/c/1db4aaa05589
[5/5] KVM: arm64: PMU: Reload when resetting
      https://git.kernel.org/kvmarm/kvmarm/c/fe53538069bb

--
Best,
Oliver