From patchwork Thu Dec 22 18:38:23 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cristian Marussi X-Patchwork-Id: 35903 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:e747:0:0:0:0:0 with SMTP id c7csp109014wrn; Thu, 22 Dec 2022 10:46:54 -0800 (PST) X-Google-Smtp-Source: AMrXdXtuVh5HLuH6aedVDRDZgrpWsRuGsXYO4DrSt+UOa8ssKKPUED2JVEY0tyDtVnSd1FOZW7r9 X-Received: by 2002:a05:6402:4009:b0:467:dc59:cec6 with SMTP id d9-20020a056402400900b00467dc59cec6mr12020519eda.0.1671734814812; Thu, 22 Dec 2022 10:46:54 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1671734814; cv=none; d=google.com; s=arc-20160816; b=IUyU2KCjy/A0D9vdCuW1bIWI/amhO/vAwZ8TQUMyKuO57tqNiTfe5vmw6pokidVS9c rJLOhRcDbRCNjyZ1wj84CUWhH8rfPRi5YCFuCwOlSwSuKNU62XsJfPZoFzdoF/Yh8sA2 sZ3aCgxqHG6RSkzIIn84PI7S65m4wG6P0Z+YGGtN11MbnCUbhODxUBDZzFTdJ5vRjAR4 7bIZE/eNyKeJ01gfzwBspIH6mvfb0GDsV6Zx0GiYLJfCamIgeE/zeAY+aLaIMTBzjUO0 QdLBDf5VGoi5srL0o9vD3bjs48nLxeGAwpaT88UnKNEH+snnYpKPDP+Swxlla8xgJ5GG fAEw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=q4tCi7tStcUowLDltlSeMcJt8ohL5CBfgI2VYOiT1xs=; b=m88jIKEDJRVx0L1ORoqE2ofHq9XjMfiImZjy1ikS3+FJ2Jucf3FsRCekT4153mrReQ O8iZrg7p1PN5WOY4lyi0jWGjG9xIF/EIGPQq2YCSDnlimbMYVzmTNXEaG65S5bjIAjTB 7YYKcQsw6lILSbOLJZaGuLtM549nBP84diqZVCNNKvkVn2YUmdwNi4dLfae+DzEUGe1j 2uSPuoOIBFgVbVnhZyQdz4Kskjc0g04exULyD2Ix3+RrFVxEM5BJ40c1kf99bovXz4Rk zERA+LIaSGgTG7x6+tfI/8omXqqZBmOXSz4ncvdbiWtPohCejEcdPUho63lTo1ezF7k8 QQkA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id m19-20020a056402511300b0045d15f39bf4si1341454edd.479.2022.12.22.10.46.30; Thu, 22 Dec 2022 10:46:54 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235438AbiLVSjE (ORCPT + 99 others); Thu, 22 Dec 2022 13:39:04 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56368 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235217AbiLVSit (ORCPT ); Thu, 22 Dec 2022 13:38:49 -0500 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id CFCDE1D320 for ; Thu, 22 Dec 2022 10:38:47 -0800 (PST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A32AA16F2; Thu, 22 Dec 2022 10:39:28 -0800 (PST) Received: from e120937-lin.. (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DA57D3FAFB; Thu, 22 Dec 2022 10:38:46 -0800 (PST) From: Cristian Marussi To: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: sudeep.holla@arm.com, cristian.marussi@arm.com Subject: [PATCH 5/5] firmware: arm_scmi: Fix virtio channels cleanup on shutdown Date: Thu, 22 Dec 2022 18:38:23 +0000 Message-Id: <20221222183823.518856-6-cristian.marussi@arm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221222183823.518856-1-cristian.marussi@arm.com> References: <20221222183823.518856-1-cristian.marussi@arm.com> MIME-Version: 1.0 X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1752941005204556613?= X-GMAIL-MSGID: =?utf-8?q?1752941005204556613?= When unloading the SCMI core stack module, configured to use the virtio SCMI transport, LOCKDEP reports the splat down below about unsafe locks dependencies. In order to avoid this possible unsafe locking scenario call upfront virtio_break_device() before getting hold of vioch->lock. ===================================================== WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected 6.1.0-00067-g6b934395ba07-dirty #4 Not tainted ----------------------------------------------------- rmmod/307 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire: ffff000080c510e0 (&dev->vqs_list_lock){+.+.}-{3:3}, at: virtio_break_device+0x28/0x68 and this task is already holding: ffff00008288ada0 (&channels[i].lock){-.-.}-{3:3}, at: virtio_chan_free+0x60/0x168 [scmi_module] which would create a new lock dependency: (&channels[i].lock){-.-.}-{3:3} -> (&dev->vqs_list_lock){+.+.}-{3:3} but this new dependency connects a HARDIRQ-irq-safe lock: (&channels[i].lock){-.-.}-{3:3} ... which became HARDIRQ-irq-safe at: lock_acquire+0x128/0x398 _raw_spin_lock_irqsave+0x78/0x140 scmi_vio_complete_cb+0xb4/0x3b8 [scmi_module] vring_interrupt+0x84/0x120 vm_interrupt+0x94/0xe8 __handle_irq_event_percpu+0xb4/0x3d8 handle_irq_event_percpu+0x20/0x68 handle_irq_event+0x50/0xb0 handle_fasteoi_irq+0xac/0x138 generic_handle_domain_irq+0x34/0x50 gic_handle_irq+0xa0/0xd8 call_on_irq_stack+0x2c/0x54 do_interrupt_handler+0x8c/0x90 el1_interrupt+0x40/0x78 el1h_64_irq_handler+0x18/0x28 el1h_64_irq+0x64/0x68 _raw_write_unlock_irq+0x48/0x80 ep_start_scan+0xf0/0x128 do_epoll_wait+0x390/0x858 do_compat_epoll_pwait.part.34+0x1c/0xb8 __arm64_sys_epoll_pwait+0x80/0xd0 invoke_syscall+0x4c/0x110 el0_svc_common.constprop.3+0x98/0x120 do_el0_svc+0x34/0xd0 el0_svc+0x40/0x98 el0t_64_sync_handler+0x98/0xc0 el0t_64_sync+0x170/0x174 to a HARDIRQ-irq-unsafe lock: (&dev->vqs_list_lock){+.+.}-{3:3} ... which became HARDIRQ-irq-unsafe at: ... lock_acquire+0x128/0x398 _raw_spin_lock+0x58/0x70 __vring_new_virtqueue+0x130/0x1c0 vring_create_virtqueue+0xc4/0x2b8 vm_find_vqs+0x20c/0x430 init_vq+0x308/0x390 virtblk_probe+0x114/0x9b0 virtio_dev_probe+0x1a4/0x248 really_probe+0xc8/0x3a8 __driver_probe_device+0x84/0x190 driver_probe_device+0x44/0x110 __driver_attach+0x104/0x1e8 bus_for_each_dev+0x7c/0xd0 driver_attach+0x2c/0x38 bus_add_driver+0x1e4/0x258 driver_register+0x6c/0x128 register_virtio_driver+0x2c/0x48 virtio_blk_init+0x70/0xac do_one_initcall+0x84/0x420 kernel_init_freeable+0x2d0/0x340 kernel_init+0x2c/0x138 ret_from_fork+0x10/0x20 other info that might help us debug this: Possible interrupt unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&dev->vqs_list_lock); local_irq_disable(); lock(&channels[i].lock); lock(&dev->vqs_list_lock); lock(&channels[i].lock); *** DEADLOCK *** ================ Fixes: 42e90eb53bf3f ("firmware: arm_scmi: Add a virtio channel refcount") Signed-off-by: Cristian Marussi --- drivers/firmware/arm_scmi/virtio.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c index 33c9b81a55cd..1db975c08896 100644 --- a/drivers/firmware/arm_scmi/virtio.c +++ b/drivers/firmware/arm_scmi/virtio.c @@ -160,7 +160,6 @@ static void scmi_vio_channel_cleanup_sync(struct scmi_vio_channel *vioch) } vioch->shutdown_done = &vioch_shutdown_done; - virtio_break_device(vioch->vqueue->vdev); if (!vioch->is_rx && vioch->deferred_tx_wq) /* Cannot be kicked anymore after this...*/ vioch->deferred_tx_wq = NULL; @@ -482,6 +481,12 @@ static int virtio_chan_free(int id, void *p, void *data) struct scmi_chan_info *cinfo = p; struct scmi_vio_channel *vioch = cinfo->transport_info; + /* + * Break device to inhibit further traffic flowing while shutting down + * the channels: doing it later holding vioch->lock creates unsafe + * locking dependency chains as reported by LOCKDEP. + */ + virtio_break_device(vioch->vqueue->vdev); scmi_vio_channel_cleanup_sync(vioch); scmi_free_channel(cinfo, data, id);