From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DC3ADC7EE24 for ; Tue, 6 Jun 2023 16:18:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=DFPDHub36ENDLJDoQh2HeD8BmA9Mhm77aEC3zEjeVgw=; b=eUFFffI11Mjp4w j2oC0o86LW6EH6zLtiQr22I3gKgCav5C2YOfTuG1NgCu1DoiO661wzXx+5Au8ylKHMzIXfuAgUdp5 Fx9P24E1Vbgng3m4gT5f60L+XTw3ijbR0Rxpkjc2XEpftsOWmq6BXm1jtOTMN//3HbH1IHTsfc2qP ncp4HxrRinQ7jwp0rQjY341si0oZW0juVH9+5c1NalHk3xwd15rJlPn66qfXrc9FPrbu75u5fZfAi 4uphK8DXtcCDEPAwQ6DgeLG1obpnQgksCBf/WF4Kq9lhoUfky0RaQW0SDitCPsSnIZj7v4FAW34GV G1aX+/bXtSQ4f4VV+yyw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q6ZNN-002OUH-2y; Tue, 06 Jun 2023 16:17:41 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q6ZNK-002OSr-2G for linux-arm-kernel@lists.infradead.org; Tue, 06 Jun 2023 16:17:40 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 3CC096292C; Tue, 6 Jun 2023 16:17:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A0DB3C433EF; Tue, 6 Jun 2023 16:17:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1686068257; bh=puBrJe09EWkxcLq1OswbHibnXWpV18HLn2Flaeb+RM4=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=j/G4UlqT33dRc0tFbQ9HONz5HmPtR3zuf6zZN8t8KSLcdTuaxEHgxMZRzLu2lKsHP 8/BLy1pEj6svf+Ja5S3N9LAgat5Ok91t10aPRNKRJl4u3ON0+7qisghC63qaWpYw9/ IR/y/+mr6KdCr9h+PwRkiVenO25FM/6nAUD7itSYc4hggv/zqxjkcogUiARenSmiR+ 5ptZC2RrgfWegAyNeUM5lhy+T0dqmGhOaq4vjHSkEwmQ+h08YrsL75/dJsurEVnnxB Upk3ouV+36+CRfasGUiQlvzt/iNm31KW047CVelLuZWkK5Vqpgy0kL13NWaQgGENWU qUIaDfZ3kBB5A== Received: from 152.5.30.93.rev.sfr.net ([93.30.5.152] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1q6ZNH-003G2u-6D; Tue, 06 Jun 2023 17:17:35 +0100 Date: Tue, 06 Jun 2023 17:17:34 +0100 Message-ID: <87pm68o99d.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Cc: Sebastian Ott , Sean Christopherson , kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] KVM: arm64: Fix smp_processor_id() call in preemptible context In-Reply-To: References: <2f16f83e-ed60-fcb7-7f3d-0fa216c41cb9@redhat.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 93.30.5.152 X-SA-Exim-Rcpt-To: oliver.upton@linux.dev, sebott@redhat.com, seanjc@google.com, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230606_091738_858470_6B85202D X-CRM114-Status: GOOD ( 35.78 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 06 Jun 2023 15:10:44 +0100, Oliver Upton wrote: > > Hi Sebastian, > > On Tue, Jun 06, 2023 at 12:37:30PM +0200, Sebastian Ott wrote: > > Commit 1c913a1c35aa ("KVM: arm64: Iterate arm_pmus list to probe for > > default PMU") introduced a smp_processor_id() call in preemtible context: > > > > [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 > > > > Just disable preemption for this section. > > The call from a preemptible context is intentional, so this really > should just be raw_smp_processor_id(). Do you mind if we fix it with the > following? > > From 2f4680ee6a5aea5c3cf826c84b86172b0b2c1a67 Mon Sep 17 00:00:00 2001 > From: Oliver Upton > Date: Tue, 6 Jun 2023 06:44:54 -0700 > Subject: [PATCH] KVM: arm64: Use raw_smp_processor_id() in > kvm_pmu_probe_armpmu() > > 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 > > Nonetheless, there's no functional requirement for disabling preemption, > as the cpu # is only used to walk the arm_pmus list. Fix it by using > raw_smp_processor_id() instead. > > Fixes: 1c913a1c35aa ("KVM: arm64: Iterate arm_pmus list to probe for default PMU") > Reported-by: Sebastian Ott > Signed-off-by: Oliver Upton > --- > arch/arm64/kvm/pmu-emul.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c > index 491ca7eb2a4c..933a6331168b 100644 > --- a/arch/arm64/kvm/pmu-emul.c > +++ b/arch/arm64/kvm/pmu-emul.c > @@ -700,7 +700,7 @@ static struct arm_pmu *kvm_pmu_probe_armpmu(void) > > mutex_lock(&arm_pmus_lock); > > - cpu = smp_processor_id(); > + cpu = raw_smp_processor_id(); > list_for_each_entry(entry, &arm_pmus, entry) { > tmp = entry->arm_pmu; > > If preemption doesn't matter (and I really don't think it does), why are we looking for a the current CPU? I'd rather we pick the PMU that is associated with CPU0 (we're pretty sure it exists), and be done with it. Something like: diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index 491ca7eb2a4c..fce9d07fe26b 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -696,15 +696,14 @@ static struct arm_pmu *kvm_pmu_probe_armpmu(void) { struct arm_pmu *tmp, *pmu = NULL; struct arm_pmu_entry *entry; - int cpu; mutex_lock(&arm_pmus_lock); - cpu = smp_processor_id(); list_for_each_entry(entry, &arm_pmus, entry) { tmp = entry->arm_pmu; - if (cpumask_test_cpu(cpu, &tmp->supported_cpus)) { + /* Pick the CPU associated with CPU0 as the default */ + if (cpumask_test_cpu(0, &tmp->supported_cpus)) { pmu = tmp; break; } At least, it saves us wondering about the rationale for picking one or the other. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel