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 smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 ABE62CAC592 for ; Fri, 19 Sep 2025 10:56:04 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 769C28218C; Fri, 19 Sep 2025 10:56:04 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id dJ_0tbhPoX23; Fri, 19 Sep 2025 10:56:03 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.166.142; helo=lists1.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org E294B81E2E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osuosl.org; s=default; t=1758279362; bh=5mmmnjsu9xXx/i0h/Qsh8uWABNz0Q9My1OhrKczY0Xc=; h=Date:To:Cc:References:From:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=SCq+N+qww0/5bP2o29Gv4WE3UfSKiLzmOOlxBfOUqfCw5FzYBAkqK8sRBEU0PKOJ3 3KuTKtN8tLZJ3SDRiVUyCaQAE5Ic4M9P+lIlkzWiEan33dlvM7F3L7PYRNxgtJQDtT iHYBBM1EOKhCT+8rrx+UMfXL2YHJcsYfB5Vb7WXyK8P0BZP1GFG0OZiP4KYOoho7W/ OF/WJb3aPjgIlNsu6BTwOTPrDIyS2DGuQflWs/5YjFt5SCA3GAGg+7GYYfAaN3GfyM mk8RLFdRG6FrZsyKQpedqgkCEzd56DK3w0N1IZ0Dak8GVrc+EnOWSDMQF/QrqI1BAY UxKajUW9h3l5g== Received: from lists1.osuosl.org (lists1.osuosl.org [140.211.166.142]) by smtp1.osuosl.org (Postfix) with ESMTP id E294B81E2E; Fri, 19 Sep 2025 10:56:02 +0000 (UTC) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists1.osuosl.org (Postfix) with ESMTP id 70C3AE7 for ; Fri, 19 Sep 2025 10:56:02 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 6B39C40BA8 for ; Fri, 19 Sep 2025 10:56:02 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id lG6LbJX0k2Ts for ; Fri, 19 Sep 2025 10:56:01 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=2001:41d0:203:375::bd; helo=out-189.mta1.migadu.com; envelope-from=vadim.fedorenko@linux.dev; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp2.osuosl.org A412540BFD DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org A412540BFD Received: from out-189.mta1.migadu.com (out-189.mta1.migadu.com [IPv6:2001:41d0:203:375::bd]) by smtp2.osuosl.org (Postfix) with ESMTPS id A412540BFD for ; Fri, 19 Sep 2025 10:56:01 +0000 (UTC) Message-ID: <9bf5066a-a006-4f93-93fd-38e4c063e59e@linux.dev> Date: Fri, 19 Sep 2025 11:55:56 +0100 MIME-Version: 1.0 To: "Choong, Chwee Lin" , "Keller, Jacob E" , "Nguyen, Anthony L" , "Kitszel, Przemyslaw" , Andrew Lunn , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Richard Cochran , "Gomes, Vinicius" Cc: "intel-wired-lan@lists.osuosl.org" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "Shalev, Avi" , "Song, Yoong Siang" References: <20250918183811.31270-1-chwee.lin.choong@intel.com> <0fc877a5-4b35-4802-9cda-e4eca561c5d1@linux.dev> Content-Language: en-US X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Vadim Fedorenko In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1758279359; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5mmmnjsu9xXx/i0h/Qsh8uWABNz0Q9My1OhrKczY0Xc=; b=Y9rD8d5Sbz9S1OKeW/Jp5GeodEl2YYtUqvdsIE1Yb/Dkhie9iiUVtroew6SRci8G1AQzxU ccyz5npLbFup2oDOEjJZKJjvK7qEJaJnyMZpKfZW+BdC5yjWCe4cVbvHrEQw+l9w5cssY+ asYNJxLiThEz/qK2fBy9glSQECLa5ws= X-Mailman-Original-Authentication-Results: smtp2.osuosl.org; dmarc=pass (p=none dis=none) header.from=linux.dev X-Mailman-Original-Authentication-Results: smtp2.osuosl.org; dkim=pass (1024-bit key, unprotected) header.d=linux.dev header.i=@linux.dev header.a=rsa-sha256 header.s=key1 header.b=Y9rD8d5S Subject: Re: [Intel-wired-lan] [PATCH iwl-net v1] igc: fix race condition in TX timestamp read for register 0 X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" On 19/09/2025 08:17, Choong, Chwee Lin wrote: > > On Friday, September 19, 2025 6:11 AM, Keller, Jacob E wrote: >> On 9/18/2025 1:47 PM, Vadim Fedorenko wrote: >>> On 18/09/2025 19:38, Chwee-Lin Choong wrote: >>>> The current HW bug workaround checks the TXTT_0 ready bit first, then >>>> reads LOW -> HIGH -> LOW from register 0 to detect if a timestamp was >>>> captured. >>>> >>>> This sequence has a race: if a new timestamp is latched after reading >>>> the TXTT mask but before the first LOW read, both old and new >>>> timestamp match, causing the driver to drop a valid timestamp. >>>> >>>> Fix by reading the LOW register first, then the TXTT mask, so a newly >>>> latched timestamp will always be detected. >>>> >>>> This fix also prevents TX unit hangs observed under heavy >>>> timestamping load. >>>> >>>> Fixes: c789ad7cbebc ("igc: Work around HW bug causing missing >>>> timestamps") >>>> Suggested-by: Avi Shalev >>>> Signed-off-by: Song Yoong Siang >>>> Signed-off-by: Chwee-Lin Choong >>>> --- >>>> drivers/net/ethernet/intel/igc/igc_ptp.c | 10 ++++++++-- >>>> 1 file changed, 8 insertions(+), 2 deletions(-) >>>> >>> >>> [...] >>> >>>> * timestamp was captured, we can read the "high" >>>> * register again. >>>> */ >>> >>> This comment begins with 'read the "high" register (to latch a new >>> timestamp)' ... >>> >>>> - u32 txstmpl_old, txstmpl_new; >>>> + u32 txstmpl_new; >>>> >>>> - txstmpl_old = rd32(IGC_TXSTMPL); >>>> rd32(IGC_TXSTMPH); >>>> txstmpl_new = rd32(IGC_TXSTMPL); >>> >>> and a couple of lines later in this function you have >>> >>> regval = txstmpl_new; >>> regval |= (u64)rd32(IGC_TXSTMPH) << 32; >>> >>> According to the comment above, the value in the register will be >>> latched after reading IGC_TXSTMPH. As there will be no read of "low" >>> part of the register, it will stay latched with old value until the >>> next call to the same function. Could it be the reason of unit hangs? >>> >>> It looks like the value of previous read of IGC_TXSTMPH should be >>> stored and used to construct new timestamp, right? >>> >> >> I wouldn't trust the comment, but instead double check the data sheets. >> Unfortunately, I don't seem to have a copy of the igc hardware data sheet handy :( >> >> Thanks, >> Jake > > Flow before this patch: > 1. Read the TXTT bits into mask > 2. if TXTT_0 == 0, go to workaround ->If at this point register 0 captures TX timestamp, and TXTT_0 is set but we think it is 0. > 3. Read LOW to OLD > 4. Read HIGH – this clears the TXTT_0 > 5. Read LOW again , now to NEW. > 6. NEW==OLD, so the timestamp is discarded -> causing timestamp timeout > > Flow after this patch: > 1. Read LOW to OLD > 2. Read the TXTT bits into mask > 3. if TXTT_0 == 0, go to workaround -> If at this point register 0 captures TX timestamp, and TXTT_0 is set but we think it is 0. > 4. Read HIGH – this clears the TXTT_0 > 5. Read LOW again , now to NEW. > 6. NEW!=OLD, so we detect this is a valid timestamp > 7. Read HIGH again and use the timestamp > > Let me know if this address your questions? Unfortunately, it doesn't. The question is "what will happen to register after step 7?" The comment above says it will stay latched until LOW is read, will it affect performance/stability?