From patchwork Thu Nov 10 17:31:34 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yaron Micher X-Patchwork-Id: 1702262 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.denx.de (client-ip=2a01:238:438b:c500:173d:9f52:ddab:ee01; helo=phobos.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=hailo.ai header.i=@hailo.ai header.a=rsa-sha256 header.s=selector2 header.b=MG9BGV9j; dkim-atps=neutral Received: from phobos.denx.de (phobos.denx.de [IPv6:2a01:238:438b:c500:173d:9f52:ddab:ee01]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4N7Wcs6Bnxz23lq for ; Fri, 11 Nov 2022 06:10:19 +1100 (AEDT) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B7DD181F4E; Thu, 10 Nov 2022 20:10:05 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=hailo.ai Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=hailo.ai header.i=@hailo.ai header.b="MG9BGV9j"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 4205184DEB; Thu, 10 Nov 2022 18:32:21 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.2 Received: from EUR03-DBA-obe.outbound.protection.outlook.com (mail-dbaeur03on2118.outbound.protection.outlook.com [40.107.104.118]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 7FB0C8502B for ; Thu, 10 Nov 2022 18:32:17 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=hailo.ai Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=yaronm@hailo.ai ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iBJYFO1B/Ox63vrqq8K0ltCbxTBnIXpT2QykW+e4mivoH1YtxiCfPPAjxuZFpBiS1hs+/ftNDpnH9zJn/zHRwaFfhC1p1B8ttIqr9TB2IRLdqlQ+MX/z8NG28vbOiyNsJ2JTwyFV5YX6h+p61QidwNhTxFMjqywASCDlRVhS0rIKp2pxeSQMkYqGv51F/YeNrgi9VxpKJp5qQj++JTDCj4KsWmyfYOTogZgt9UK7mbUuxvwy03iRbT6M9figXgLAjyTAK25hyALAsfuvg3XJ33PPSAv3aRpIg3g3AT2KDy2bSOHvA2zkVEqSLqBV6Tb+k9xU7Op+sQg42E7XHTuACQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=GlKSQ5UY0GgWDdXrqhSjGB28U1fRZ2WySeKpD9F+gFQ=; b=n1j8vqLqugBOBmCaQDXE3aL8MZJL2FkKtTWovpc+tNs3q6ndo3zJWyNmuRGxCD5PAoFMuxbuwm62iAJnbu5mw3yycyir+D5TgzVDF71DngtmCc5lsyjY1Y7f1Mrf5TXbp4sUeQhDA4pYa2y/iBt9pi6LIcQKu14Nh9ITT0QQMTQpARWhvQ+NTHcVom2MANcFjxREd9cEN3IIMlEc+iRf9wCBf5RmUo6Alj2NRTnvPiD2B1tBzieIC4ODdh9at0OtwbaQaO/L+EvslsnC6B9cJwXd9k0qrxrvoIrLxuiWKN1yson6lrF39mWj29gssA7s5G0bUCKaQ1tQNIc7XFjc0w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=hailo.ai; dmarc=pass action=none header.from=hailo.ai; dkim=pass header.d=hailo.ai; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hailo.ai; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=GlKSQ5UY0GgWDdXrqhSjGB28U1fRZ2WySeKpD9F+gFQ=; b=MG9BGV9jJr+H2b/++nj+W2Ek8DHdOI1FiN0MmSq4p3jx24UBe+uTr8TlnU00sAjQ+yqn6OtZT3RczQetjxvz9xxXzmwPsqMo5nY7XEWXH0VsQYGepPCu+oNZoIUPvOUEsSSQh4On1cdR2ivQseqvfb78zBfcKhqsQO/B1BvMwJE= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=hailo.ai; Received: from DU2P194MB1709.EURP194.PROD.OUTLOOK.COM (2603:10a6:10:276::9) by AM8P194MB1595.EURP194.PROD.OUTLOOK.COM (2603:10a6:20b:36e::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5791.27; Thu, 10 Nov 2022 17:32:15 +0000 Received: from DU2P194MB1709.EURP194.PROD.OUTLOOK.COM ([fe80::9b8d:7f74:a504:3103]) by DU2P194MB1709.EURP194.PROD.OUTLOOK.COM ([fe80::9b8d:7f74:a504:3103%6]) with mapi id 15.20.5791.027; Thu, 10 Nov 2022 17:32:15 +0000 From: Yaron Micher To: u-boot@lists.denx.de Cc: joe.hershberger@ni.com, rfried.dev@gmail.com, Yaron Micher Subject: [PATCH] net: macb: Fix race caused by flushing unwanted descriptors Date: Thu, 10 Nov 2022 19:31:34 +0200 Message-Id: <20221110173134.439-1-yaronm@hailo.ai> X-Mailer: git-send-email 2.17.1 X-ClientProxiedBy: AS9PR04CA0172.eurprd04.prod.outlook.com (2603:10a6:20b:530::18) To DU2P194MB1709.EURP194.PROD.OUTLOOK.COM (2603:10a6:10:276::9) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DU2P194MB1709:EE_|AM8P194MB1595:EE_ X-MS-Office365-Filtering-Correlation-Id: 71763c4b-a4a0-4935-e5e5-08dac3418254 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: EFWxc8MjT6GgaoPuJmLbY7XUEF9+lFWIW5tVpBAkrZwMs1JEFi58M0JgGcQ7toJAlUWCY61OBBjPIuqa7jgQZc/1fIHZDCqvPJbc4Sq2UJ5NDwTpuXc5qeDTzBEGtaRQE0zbh94klyZTHw5Q4hvrS19AkhlWtPBA9Pjbdkx12zxkvnNu1gooSYK93ukhorl9xS2eoMpZsIJDRLrEDnBPrsaA9YpmXOVf6a+jMeAokiFx97dUxIZs7vTcLoy0HgHdep5FyLfAwctAMlEAqKsyZZcd20leSET1iES/d1RSZqKXetNXi6zlQgjZHjalRVP8i/3TU9EEhmHcchi9EqkPIZbgDlGO3ICYhuHp3oliz+6ozGIEMptVU9K7KPp88B/Xic6ZG25JtbyPBWbhCYCu0CHSyuLidqETCTVmTPb3Ug+nYCXvC7vTLD5y3DwTPhFpmPuVXKT+DEkZQmuQgwPeIUmxt8h0CVlnwttBofYJhDhGwYyFxnbsmKmyfoIaOeAPPZHNIbIkr801LQEga7KWwXbBk0ICpKV+4FVCzm9tjyneFb6obckasggyz85YgtbGwuRGWGM0q/uF3Ui00djn17a8tyKYKEEmMji88GUy4GEDaSn7IgRLhm58n5vDD6qhjGZEbMSYc2G9P1gDSFZsgxB8WmlPivSeVEFav7xoZ5zOEForyJeLvHV6YmRbOz0fv33GWA6zkrO8Biu4ZuWSwjtUz8M9CsFa8M39n+0Y+LH7Wl1Y3fnZdVmnWtvoR5wjh+YQ9KO1CcqUIQ0lchVlB6OFtFuzNl7dmTbiinN1i1I= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DU2P194MB1709.EURP194.PROD.OUTLOOK.COM; PTR:; CAT:NONE; SFS:(13230022)(4636009)(376002)(396003)(366004)(136003)(346002)(39850400004)(451199015)(107886003)(36756003)(2906002)(6486002)(86362001)(66946007)(52116002)(478600001)(6506007)(38100700002)(38350700002)(186003)(1076003)(66556008)(26005)(6512007)(83380400001)(2616005)(8936002)(5660300002)(4326008)(6916009)(66476007)(6666004)(8676002)(316002)(41300700001)(26123001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: hidFHfxE6hUoYQc7jwVrsm4b8FZYPYoO3OlrsFJx12RmezC3p3aJSvsEPTOQHoOaDlO39o8Nmv2JJCRRifl2o6ATt6mSq7epxe4M+Y0ZpZxXjo04TpleB6K9tHcijbEdru/CUFQ1ld3VVkMfaer3b5pSMVmD8XEJrmWHL/kWwarOx1xnfBxTEcmjLiizEvUOSUPf0SiIjfS7fPSTZLnCS887QkvEfJPwqlcrsXtuP4dNlS/iUooJw5gKECN/4F80lSj59O/kcesWlyhSMGG13Sxv2bKYc9ppcqEcVeiu2mj/aytjhV3f9w5eptiuBilzUDs9mzMjTH57jPgYBJy/ObOTDJLV8GR6K35jmBahWMM0VZB7cCd9F7wj2nglq+R0ZI5jyKHL5VJyzUksIghsJyYi0nvUF7NBWf9LwQ5+TuU7wSQr0Bhddoi8WMyVjxKLGkKrY9FTi90evQZlnOO5Ef/V+rujwpWFJvVaAQGmAUF6y8MCHYuh58H4SvOBAtSpGrdT3V7qdOf8AyIEvr1fEuWx6aWjxGEJa9evp/z2PMFKrfSlCBqNXaEYh1Z63gNKnR0tD44BGSuyae1SowYaezCHTTgV+8EOWmKeED8dgzq1HqgDmcPokD7BccqSV1G+KZNdc64+QBRtPKCMMRSIMC8T+Non38JHsMAbYV0ftYawhuDCaZGMFUEPsCHih7Cwkv3VRZMyIzLPb3eXOjrCZO+FWYbJtJD3YDR1m6bdIbPB9oEGhUcv64Y07+JSd69MW24RM/b91sfMRT1l1Tb7KZ1xUH8eMiBQEAy0oUTRhzo7uIphAvs8AjTVzfmunoR9pc20j64IXTPLA3p3U2gsKybJSgL7dHCNrDgso5cXwqLjDunrv7zyV0GwIiz7Pl6tClTsYqyF8oUPCSdcew7/l8Mk4tcg0sd3yJ1rprQZLzbkqE0yixC/UJbw6ajIZFFoTO0F1VeH1/rjZmxz25gBFxRyi4mab6c0bRhHRyjv2r9n+tp7wE9rVltzZpadj+p+AjnjIREcsIM9hhGfm3wEopQj5LtEDPrT3JJyXaP2wSqX90e3Fhtuem4jrGA0w7TZnInzx9zuteWpufjQKDaIX2QrkIPnn+HQs16PdIuZ+W+DlQTUBR03PGfF6lLUe0r/0GubbmpCFko8U4yE1XsIJ/H+la4GEJTqAmtwgLubAruX0U45wXAWE1ER+o9AV7lsaAslJPlQpyDh0NXsF2JhrL76DzkotNwPB4UbELwDVyXOpmdQKUVS7DIr1lVCWv1wnnNv8DaM6cP3A+flnbduxK+hgPS/OG9xAK33/prUS/FpbvENgNuerjxrsYVSr7rPqp1RzV6H+fu3eavKJeUgESKoPb0J012kWgxLIj7EgIWrud4k8Yp8jwnHegTiHba44H4gnLGUExJFt+oL/fWZFcB2JBa07P2nr6e9/LumKoU4i0MsbwYFNPJu95wAIqxkkwDMMsfUD0mme91qkURktMGuH0lsfpM+l13ptyq5tBe+HieMc0x2d1qc4ubGcWwtr49vFaXcdtIUdzImsodvydaTbYHCxQ6xNPdS0baoqwl7zd7ZZqt+GoXcMjTO1ivB X-OriginatorOrg: hailo.ai X-MS-Exchange-CrossTenant-Network-Message-Id: 71763c4b-a4a0-4935-e5e5-08dac3418254 X-MS-Exchange-CrossTenant-AuthSource: DU2P194MB1709.EURP194.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Nov 2022 17:32:15.4957 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 6ae4a5f7-5467-4189-8f6a-f2928ed536de X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: WPl8+Nqwn7+7NTfWDgiBCmeJJk0gNDVisdK6KwO/4hHoTyg3MftS9fUe9sGlZBC7 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM8P194MB1595 X-Mailman-Approved-At: Thu, 10 Nov 2022 20:10:03 +0100 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean The rx descriptor list is in cached memory, and there may be multiple descriptors per cache-line. After reclaim_rx_buffers marks a descriptor as unused it does a cache flush, which causes the entire cache-line to be written to memory, which may override other descriptors in the same cache-line that the controller may have written to. The fix skips freeing descriptors that are not the last in a cache-line, and if the freed descriptor is the last one in a cache-line, it marks all the descriptors in the cache-line as unused. This is similarly to what is done in drivers/net/fec_mxc.c In my case this bug caused tftpboot to fail some times when other packets are sent to u-boot in addition to the ongoing tftp (e.g. ping). The driver would stop receiving new packets because it is waiting on a descriptor that is marked unused, when in reality the descriptor contains a new unprocessed packet but while freeing the previous buffer descriptor & flushing the cache, the driver accidentally marked the descriptor as unused. Signed-off-by: Yaron Micher --- drivers/net/macb.c | 51 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/drivers/net/macb.c b/drivers/net/macb.c index e02a57b411..65ec1f24ad 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -98,6 +98,9 @@ struct macb_dma_desc_64 { #define MACB_RX_DMA_DESC_SIZE (DMA_DESC_BYTES(MACB_RX_RING_SIZE)) #define MACB_TX_DUMMY_DMA_DESC_SIZE (DMA_DESC_BYTES(1)) +#define DESC_PER_CACHELINE_32 (ARCH_DMA_MINALIGN/sizeof(struct macb_dma_desc)) +#define DESC_PER_CACHELINE_64 (ARCH_DMA_MINALIGN/DMA_DESC_SIZE) + #define RXBUF_FRMLEN_MASK 0x00000fff #define TXBUF_FRMLEN_MASK 0x000007ff @@ -401,32 +404,56 @@ static int _macb_send(struct macb_device *macb, const char *name, void *packet, return 0; } +static void reclaim_rx_buffer(struct macb_device *macb, + unsigned int idx) +{ + unsigned int mask; + unsigned int shift; + unsigned int i; + + /* + * There may be multiple descriptors per CPU cacheline, + * so a cache flush would flush the whole line, meaning the content of other descriptors + * in the cacheline would also flush. If one of the other descriptors had been + * written to by the controller, the flush would cause those changes to be lost. + * + * To circumvent this issue, we do the actual freeing only when we need to free + * the last descriptor in the current cacheline. When the current descriptor is the + * last in the cacheline, we free all the descriptors that belong to that cacheline. + */ + if (macb->config->hw_dma_cap & HW_DMA_CAP_64B) { + mask = DESC_PER_CACHELINE_64 - 1; + shift = 1; + } else { + mask = DESC_PER_CACHELINE_32 - 1; + shift = 0; + } + + /* we exit without freeing if idx is not the last descriptor in the cacheline */ + if ((idx & mask) != mask) + return; + + for (i = idx & (~mask); i <= idx; i++) + macb->rx_ring[i << shift].addr &= ~MACB_BIT(RX_USED); +} + static void reclaim_rx_buffers(struct macb_device *macb, unsigned int new_tail) { unsigned int i; - unsigned int count; i = macb->rx_tail; macb_invalidate_ring_desc(macb, RX); while (i > new_tail) { - if (macb->config->hw_dma_cap & HW_DMA_CAP_64B) - count = i * 2; - else - count = i; - macb->rx_ring[count].addr &= ~MACB_BIT(RX_USED); + reclaim_rx_buffer(macb, i); i++; - if (i > MACB_RX_RING_SIZE) + if (i >= MACB_RX_RING_SIZE) i = 0; } while (i < new_tail) { - if (macb->config->hw_dma_cap & HW_DMA_CAP_64B) - count = i * 2; - else - count = i; - macb->rx_ring[count].addr &= ~MACB_BIT(RX_USED); + reclaim_rx_buffer(macb, i); i++; }