[PATCH] KVM: arm64: Use raw_smp_processor_id() in kvm_pmu_probe_armpmu()

[PATCH] KVM: arm64: Use raw_smp_processor_id() in kvm_pmu_probe_armpmu()

[Oliver Upton]

Sebastian reports that commit 1c913a1c35aa ("KVM: arm64: Iterate
arm_pmus list to probe for default PMU") introduced the following splat
with CONFIG_DEBUG_PREEMPT enabled:

[70506.110187] BUG: using smp_processor_id() in preemptible [00000000] code: qemu-system-aar/3078242
[70506.119077] caller is debug_smp_processor_id+0x20/0x30
[70506.124229] CPU: 129 PID: 3078242 Comm: qemu-system-aar Tainted: G        W          6.4.0-rc5 #25
[70506.133176] Hardware name: GIGABYTE R181-T92-00/MT91-FS4-00, BIOS F34 08/13/2020
[70506.140559] Call trace:
[70506.142993]  dump_backtrace+0xa4/0x130
[70506.146737]  show_stack+0x20/0x38
[70506.150040]  dump_stack_lvl+0x48/0x60
[70506.153704]  dump_stack+0x18/0x28
[70506.157007]  check_preemption_disabled+0xe4/0x108
[70506.161701]  debug_smp_processor_id+0x20/0x30
[70506.166046]  kvm_arm_pmu_v3_set_attr+0x460/0x628
[70506.170662]  kvm_arm_vcpu_arch_set_attr+0x88/0xd8
[70506.175363]  kvm_arch_vcpu_ioctl+0x258/0x4a8
[70506.179632]  kvm_vcpu_ioctl+0x32c/0x6b8
[70506.183465]  __arm64_sys_ioctl+0xb4/0x100
[70506.187467]  invoke_syscall+0x78/0x108
[70506.191205]  el0_svc_common.constprop.0+0x4c/0x100
[70506.195984]  do_el0_svc+0x34/0x50
[70506.199287]  el0_svc+0x34/0x108
[70506.202416]  el0t_64_sync_handler+0xf4/0x120
[70506.206674]  el0t_64_sync+0x194/0x198

Fix the issue by using the raw variant that bypasses the debug
assertion. While at it, stick all of the nuance and UAPI baggage into a
comment for posterity.

Fixes: 1c913a1c35aa ("KVM: arm64: Iterate arm_pmus list to probe for default PMU")
Reported-by: Sebastian Ott <sebott@redhat.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/pmu-emul.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 491ca7eb2a4c..560650972478 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -700,7 +700,25 @@ static struct arm_pmu *kvm_pmu_probe_armpmu(void)
 
 	mutex_lock(&arm_pmus_lock);
 
-	cpu = smp_processor_id();
+	/*
+	 * It is safe to use a stale cpu to iterate the list of PMUs so long as
+	 * the same value is used for the entirety of the loop. Given this, and
+	 * the fact that no percpu data is used for the lookup there is no need
+	 * to disable preemption.
+	 *
+	 * It is still necessary to get a valid cpu, though, to probe for the
+	 * default PMU instance as userspace is not required to specify a PMU
+	 * type. In order to uphold the preexisting behavior KVM selects the
+	 * PMU instance for the core where the first call to the
+	 * KVM_ARM_VCPU_PMU_V3_CTRL attribute group occurs. A dependent use case
+	 * would be a user with disdain of all things big.LITTLE that affines
+	 * the VMM to a particular cluster of cores.
+	 *
+	 * In any case, userspace should just do the sane thing and use the UAPI
+	 * to select a PMU type directly. But, be wary of the baggage being
+	 * carried here.
+	 */
+	cpu = raw_smp_processor_id();
 	list_for_each_entry(entry, &arm_pmus, entry) {
 		tmp = entry->arm_pmu;
 

base-commit: 9561de3a55bed6bdd44a12820ba81ec416e705a7
-- 
2.41.0.rc2.161.g9c6817b8e7-goog

Re: [PATCH] KVM: arm64: Use raw_smp_processor_id() in kvm_pmu_probe_armpmu()

[Sean Christopherson]

On Tue, Jun 06, 2023, Oliver Upton wrote:
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 491ca7eb2a4c..560650972478 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -700,7 +700,25 @@ static struct arm_pmu *kvm_pmu_probe_armpmu(void)
>  
>  	mutex_lock(&arm_pmus_lock);
>  
> -	cpu = smp_processor_id();
> +	/*
> +	 * It is safe to use a stale cpu to iterate the list of PMUs so long as
> +	 * the same value is used for the entirety of the loop. Given this, and
> +	 * the fact that no percpu data is used for the lookup there is no need
> +	 * to disable preemption.
> +	 *
> +	 * It is still necessary to get a valid cpu, though, to probe for the
> +	 * default PMU instance as userspace is not required to specify a PMU
> +	 * type. In order to uphold the preexisting behavior KVM selects the
> +	 * PMU instance for the core where the first call to the
> +	 * KVM_ARM_VCPU_PMU_V3_CTRL attribute group occurs. A dependent use case
> +	 * would be a user with disdain of all things big.LITTLE that affines
> +	 * the VMM to a particular cluster of cores.
> +	 *
> +	 * In any case, userspace should just do the sane thing and use the UAPI
> +	 * to select a PMU type directly. But, be wary of the baggage being
> +	 * carried here.
> +	 */

Thanks for humoring me :-)

Re: [PATCH] KVM: arm64: Use raw_smp_processor_id() in kvm_pmu_probe_armpmu()

[Marc Zyngier]

On Tue, 6 Jun 2023 18:48:14 +0000, Oliver Upton wrote:
> Sebastian reports that commit 1c913a1c35aa ("KVM: arm64: Iterate
> arm_pmus list to probe for default PMU") introduced the following splat
> with CONFIG_DEBUG_PREEMPT enabled:
> 
> [70506.110187] BUG: using smp_processor_id() in preemptible [00000000] code: qemu-system-aar/3078242
> [70506.119077] caller is debug_smp_processor_id+0x20/0x30
> [70506.124229] CPU: 129 PID: 3078242 Comm: qemu-system-aar Tainted: G        W          6.4.0-rc5 #25
> [70506.133176] Hardware name: GIGABYTE R181-T92-00/MT91-FS4-00, BIOS F34 08/13/2020
> [70506.140559] Call trace:
> [70506.142993]  dump_backtrace+0xa4/0x130
> [70506.146737]  show_stack+0x20/0x38
> [70506.150040]  dump_stack_lvl+0x48/0x60
> [70506.153704]  dump_stack+0x18/0x28
> [70506.157007]  check_preemption_disabled+0xe4/0x108
> [70506.161701]  debug_smp_processor_id+0x20/0x30
> [70506.166046]  kvm_arm_pmu_v3_set_attr+0x460/0x628
> [70506.170662]  kvm_arm_vcpu_arch_set_attr+0x88/0xd8
> [70506.175363]  kvm_arch_vcpu_ioctl+0x258/0x4a8
> [70506.179632]  kvm_vcpu_ioctl+0x32c/0x6b8
> [70506.183465]  __arm64_sys_ioctl+0xb4/0x100
> [70506.187467]  invoke_syscall+0x78/0x108
> [70506.191205]  el0_svc_common.constprop.0+0x4c/0x100
> [70506.195984]  do_el0_svc+0x34/0x50
> [70506.199287]  el0_svc+0x34/0x108
> [70506.202416]  el0t_64_sync_handler+0xf4/0x120
> [70506.206674]  el0t_64_sync+0x194/0x198
> 
> [...]

Applied to fixes, thanks!

[1/1] KVM: arm64: Use raw_smp_processor_id() in kvm_pmu_probe_armpmu()
      commit: 30c60dda219ddda0bc6ff6ac55d493d9db8be4fa

Cheers,

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