From patchwork Sun Mar 3 04:45:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Paniakin X-Patchwork-Id: 209281 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:fa17:b0:10a:f01:a869 with SMTP id ju23csp768354dyc; Sat, 2 Mar 2024 20:50:13 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCVJM2sWSfHp/Kev2NAnDINNYhFHizKfpamls9pIw1F7we0JRIlmkrs2SgegL3uRytNK+s5sfaqwaeVZj/SI+J7OATJQJg== X-Google-Smtp-Source: AGHT+IGTdewgU1F7kHiiEdx5G2GmYUlG6fL72z6SsmZujx1j2KQgIHvhPD6rp3L0h3BeIWjATmXk X-Received: by 2002:a05:6e02:1a28:b0:363:812d:d6a3 with SMTP id g8-20020a056e021a2800b00363812dd6a3mr7274915ile.20.1709441412744; Sat, 02 Mar 2024 20:50:12 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1709441412; cv=pass; d=google.com; s=arc-20160816; b=CUg1Hd2LHR6ULevlFSTW0s0sAgKd33K0UohtEGiTBnYPJY1IVHSzZPu/LjHZYNLSDr QDYT9YGa3i91GaRXHEni6hoDVQ8InASzhRzw66ce86NdvAOF1OSp7bnsaSJj5nn//qco Ul2pRiw8ZZwAU/RdDvUVNol/oX2iclgVpkT0GeLdou9CZuHT90zZAqmNFNplYlcZBbpK 3bP/jCXo99tJMZmDYWuZt/4TFMyhCmcvtl1jpeLCzhWdxw2sZ7a/NRqrKHJOGPrNH+Z7 3Hy40mhrq+ZuVKazn3oXSYAr+hwWfPxvXdre08Vd5es5aFqhx1NzECPOIYz+/pYNEFDK W3cQ== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=EcAkR74T37oaMFgDuTvxPlS3yLJ8+qDqwNv46qMyVV4=; fh=JBz4P7rsP/e45jPG5XwMPhGI4U87BeEAn0D85q0eBcI=; b=0JCcZz3L4no3k4UTIVmRe4njhuHyo1zZIsszED+xSXk4/2slsOcP7AF1Nne7++r/jm Ik3zz5dceFWDGRfKmDYSxMi6Expx4daOz3BrmgnB3aXwMc8ThFonmPceOyraW4wHOa9J RA40yBGdndb/h5NtTD2VS6d2BDALqbqNxG6oSchr1yZ/WVTeeV2qCo2ozmCzw6/jHbTR 68FAusS7fe+5rWdBsSooyAxo8VdPhvCIvnsDxPD2cyvzBmIcFBAHdybkwTdkrwSCFCWj efqoNRW0bn6QyQh0i27xovamn1oTpGFBnVbNTM23H0fT1BvmfkhjGhb1CP3wlQAVw14X yw7g==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@amazon.com header.s=amazon201209 header.b=vJvCPUow; arc=pass (i=1 spf=pass spfdomain=amazon.com dkim=pass dkdomain=amazon.com dmarc=pass fromdomain=amazon.com); spf=pass (google.com: domain of linux-kernel+bounces-89655-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) smtp.mailfrom="linux-kernel+bounces-89655-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amazon.com Received: from sy.mirrors.kernel.org (sy.mirrors.kernel.org. [147.75.48.161]) by mx.google.com with ESMTPS id ei27-20020a056a0080db00b006e53b7742c0si6044618pfb.199.2024.03.02.20.50.12 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 02 Mar 2024 20:50:12 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-89655-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161; Authentication-Results: mx.google.com; dkim=pass header.i=@amazon.com header.s=amazon201209 header.b=vJvCPUow; arc=pass (i=1 spf=pass spfdomain=amazon.com dkim=pass dkdomain=amazon.com dmarc=pass fromdomain=amazon.com); spf=pass (google.com: domain of linux-kernel+bounces-89655-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) smtp.mailfrom="linux-kernel+bounces-89655-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amazon.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sy.mirrors.kernel.org (Postfix) with ESMTPS id 4A049B22532 for ; Sun, 3 Mar 2024 04:49:45 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 38D8C8BE7; Sun, 3 Mar 2024 04:49:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=amazon.com header.i=@amazon.com header.b="vJvCPUow" Received: from smtp-fw-52005.amazon.com (smtp-fw-52005.amazon.com [52.119.213.156]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2FAF316FF4C; Sun, 3 Mar 2024 04:49:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=52.119.213.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709441368; cv=none; b=qgK9QULMBRJVNrxjvWW0nBczscrrQWCUmPILZJ21Wu/0MzLHPDTiYTQL15Cv2ZNmI3bpRLhABTnEIyR9MO3ymdD9QISSZZSg2rZ2MQKjVKgfdjS9dqLljLVwAq1hNVPtUAf6Uv677/nmyuxse0X3l1qQrLqUwb8L49jPFtGkuZU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709441368; c=relaxed/simple; bh=/mMBea5qOP8qQVAp1Iyo3m+jJso1XFmBlu1283c3SAs=; h=From:To:CC:Subject:Date:Message-ID:MIME-Version:Content-Type; b=XQl021RW8FNpk1esOc6BPb0bdzlhZspSE3xsCeWPbuSHwlcX/e/d9IfpKQdRA7aNkuly+Ia3yRu3uHfYsCi27MhZ4C5C7+56vVsb2Fv2UGnlWp7fC8N2pJN7Ggyr9Nz052TVYHT6+Gmjpq9AiKupWnzlNfA7oqU7prx4UmxHE8I= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.com; dkim=pass (1024-bit key) header.d=amazon.com header.i=@amazon.com header.b=vJvCPUow; arc=none smtp.client-ip=52.119.213.156 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazon201209; t=1709441366; x=1740977366; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=EcAkR74T37oaMFgDuTvxPlS3yLJ8+qDqwNv46qMyVV4=; b=vJvCPUowcs9mjlz38yg5SeXnN9n2yrVsQ2PWvYwE1JC3irXJ7UFzDF58 bwN80YFRBad1u5MDlHwy4uKyILPE0/UcCVcuMNUKADS5EiqnDdDsGAJrx auyeJGGx7DTWSRZTG+/u9vT63BACf57TqVD5dK0GOBinGxICpufgExClZ k=; X-IronPort-AV: E=Sophos;i="6.06,200,1705363200"; d="scan'208";a="638173303" Received: from iad12-co-svc-p1-lb1-vlan3.amazon.com (HELO smtpout.prod.us-west-2.prod.farcaster.email.amazon.dev) ([10.43.8.6]) by smtp-border-fw-52005.iad7.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Mar 2024 04:49:23 +0000 Received: from EX19MTAEUB002.ant.amazon.com [10.0.43.254:33619] by smtpin.naws.eu-west-1.prod.farcaster.email.amazon.dev [10.0.9.254:2525] with esmtp (Farcaster) id 0f94f4e3-8c8b-42ed-9481-e718686ab622; Sun, 3 Mar 2024 04:49:21 +0000 (UTC) X-Farcaster-Flow-ID: 0f94f4e3-8c8b-42ed-9481-e718686ab622 Received: from EX19D026EUB004.ant.amazon.com (10.252.61.64) by EX19MTAEUB002.ant.amazon.com (10.252.51.59) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1258.28; Sun, 3 Mar 2024 04:49:21 +0000 Received: from uc3ecf78c6baf56.ant.amazon.com (10.187.170.45) by EX19D026EUB004.ant.amazon.com (10.252.61.64) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1258.28; Sun, 3 Mar 2024 04:49:18 +0000 From: Andrew Paniakin To: CC: Benjamin Herrenschmidt , Andrew Panyakin , Maximilian Heyne , Boris Ostrovsky , Juergen Gross , "Stefano Stabellini" , Sasha Levin , "Greg Kroah-Hartman" , Julien Grall , , Subject: [PATCH 5.4] xen/events: close evtchn after mapping cleanup Date: Sat, 2 Mar 2024 20:45:39 -0800 Message-ID: <20240303044539.2673085-1-apanyaki@amazon.com> X-Mailer: git-send-email 2.25.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-ClientProxiedBy: EX19D035UWA004.ant.amazon.com (10.13.139.109) To EX19D026EUB004.ant.amazon.com (10.252.61.64) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1792479239018525900 X-GMAIL-MSGID: 1792479239018525900 From: Andrew Panyakin From: Maximilian Heyne Commit fa765c4b4aed2d64266b694520ecb025c862c5a9 upstream shutdown_pirq and startup_pirq are not taking the irq_mapping_update_lock because they can't due to lock inversion. Both are called with the irq_desc->lock being taking. The lock order, however, is first irq_mapping_update_lock and then irq_desc->lock. This opens multiple races: - shutdown_pirq can be interrupted by a function that allocates an event channel: CPU0 CPU1 shutdown_pirq { xen_evtchn_close(e) __startup_pirq { EVTCHNOP_bind_pirq -> returns just freed evtchn e set_evtchn_to_irq(e, irq) } xen_irq_info_cleanup() { set_evtchn_to_irq(e, -1) } } Assume here event channel e refers here to the same event channel number. After this race the evtchn_to_irq mapping for e is invalid (-1). - __startup_pirq races with __unbind_from_irq in a similar way. Because __startup_pirq doesn't take irq_mapping_update_lock it can grab the evtchn that __unbind_from_irq is currently freeing and cleaning up. In this case even though the event channel is allocated, its mapping can be unset in evtchn_to_irq. The fix is to first cleanup the mappings and then close the event channel. In this way, when an event channel gets allocated it's potential previous evtchn_to_irq mappings are guaranteed to be unset already. This is also the reverse order of the allocation where first the event channel is allocated and then the mappings are setup. On a 5.10 kernel prior to commit 3fcdaf3d7634 ("xen/events: modify internal [un]bind interfaces"), we hit a BUG like the following during probing of NVMe devices. The issue is that during nvme_setup_io_queues, pci_free_irq is called for every device which results in a call to shutdown_pirq. With many nvme devices it's therefore likely to hit this race during boot because there will be multiple calls to shutdown_pirq and startup_pirq are running potentially in parallel. ------------[ cut here ]------------ blkfront: xvda: barrier or flush: disabled; persistent grants: enabled; indirect descriptors: enabled; bounce buffer: enabled kernel BUG at drivers/xen/events/events_base.c:499! invalid opcode: 0000 [#1] SMP PTI CPU: 44 PID: 375 Comm: kworker/u257:23 Not tainted 5.10.201-191.748.amzn2.x86_64 #1 Hardware name: Xen HVM domU, BIOS 4.11.amazon 08/24/2006 Workqueue: nvme-reset-wq nvme_reset_work RIP: 0010:bind_evtchn_to_cpu+0xdf/0xf0 Code: 5d 41 5e c3 cc cc cc cc 44 89 f7 e8 2b 55 ad ff 49 89 c5 48 85 c0 0f 84 64 ff ff ff 4c 8b 68 30 41 83 fe ff 0f 85 60 ff ff ff <0f> 0b 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 0f 1f 44 00 00 RSP: 0000:ffffc9000d533b08 EFLAGS: 00010046 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000006 RDX: 0000000000000028 RSI: 00000000ffffffff RDI: 00000000ffffffff RBP: ffff888107419680 R08: 0000000000000000 R09: ffffffff82d72b00 R10: 0000000000000000 R11: 0000000000000000 R12: 00000000000001ed R13: 0000000000000000 R14: 00000000ffffffff R15: 0000000000000002 FS: 0000000000000000(0000) GS:ffff88bc8b500000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000002610001 CR4: 00000000001706e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: ? show_trace_log_lvl+0x1c1/0x2d9 ? show_trace_log_lvl+0x1c1/0x2d9 ? set_affinity_irq+0xdc/0x1c0 ? __die_body.cold+0x8/0xd ? die+0x2b/0x50 ? do_trap+0x90/0x110 ? bind_evtchn_to_cpu+0xdf/0xf0 ? do_error_trap+0x65/0x80 ? bind_evtchn_to_cpu+0xdf/0xf0 ? exc_invalid_op+0x4e/0x70 ? bind_evtchn_to_cpu+0xdf/0xf0 ? asm_exc_invalid_op+0x12/0x20 ? bind_evtchn_to_cpu+0xdf/0xf0 ? bind_evtchn_to_cpu+0xc5/0xf0 set_affinity_irq+0xdc/0x1c0 irq_do_set_affinity+0x1d7/0x1f0 irq_setup_affinity+0xd6/0x1a0 irq_startup+0x8a/0xf0 __setup_irq+0x639/0x6d0 ? nvme_suspend+0x150/0x150 request_threaded_irq+0x10c/0x180 ? nvme_suspend+0x150/0x150 pci_request_irq+0xa8/0xf0 ? __blk_mq_free_request+0x74/0xa0 queue_request_irq+0x6f/0x80 nvme_create_queue+0x1af/0x200 nvme_create_io_queues+0xbd/0xf0 nvme_setup_io_queues+0x246/0x320 ? nvme_irq_check+0x30/0x30 nvme_reset_work+0x1c8/0x400 process_one_work+0x1b0/0x350 worker_thread+0x49/0x310 ? process_one_work+0x350/0x350 kthread+0x11b/0x140 ? __kthread_bind_mask+0x60/0x60 ret_from_fork+0x22/0x30 Modules linked in: ---[ end trace a11715de1eee1873 ]--- Fixes: d46a78b05c0e ("xen: implement pirq type event channels") Co-debugged-by: Andrew Panyakin Signed-off-by: Maximilian Heyne [apanyaki: backport to v5.4-stable] Signed-off-by: Andrew Paniakin --- Compare to upstream patch this one does not have close_evtchn flag because there is no need to handle static event channels. This feature was added only in 58f6259b7a08f ("xen/evtchn: Introduce new IOCTL to bind static evtchn") drivers/xen/events/events_base.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c index 91806dc1236d..f8554d9a9f28 100644 --- a/drivers/xen/events/events_base.c +++ b/drivers/xen/events/events_base.c @@ -825,8 +825,8 @@ static void shutdown_pirq(struct irq_data *data) return; do_mask(info, EVT_MASK_REASON_EXPLICIT); - xen_evtchn_close(evtchn); xen_irq_info_cleanup(info); + xen_evtchn_close(evtchn); } static void enable_pirq(struct irq_data *data) @@ -869,8 +869,6 @@ static void __unbind_from_irq(unsigned int irq) if (VALID_EVTCHN(evtchn)) { unsigned int cpu = cpu_from_irq(irq); - xen_evtchn_close(evtchn); - switch (type_from_irq(irq)) { case IRQT_VIRQ: per_cpu(virq_to_irq, cpu)[virq_from_irq(irq)] = -1; @@ -883,6 +881,7 @@ static void __unbind_from_irq(unsigned int irq) } xen_irq_info_cleanup(info); + xen_evtchn_close(evtchn); } xen_free_irq(irq);