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 smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (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 A5CE0CAC597 for ; Thu, 18 Sep 2025 20:47:45 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 52EBD40B73; Thu, 18 Sep 2025 20:47:45 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id BVJ2GC2e1F_U; Thu, 18 Sep 2025 20:47:44 +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 smtp4.osuosl.org ED7E940B6D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osuosl.org; s=default; t=1758228464; bh=F9lnNfEkDvcZjokYLgamdA2J6AkuI6fP9ftEP6VOZ2o=; h=Date:To:Cc:References:From:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=KydYzT4MwIYKjnkA73nC/i4VEbE7YUTlTQ3E/Pv66rHEIY+0lrT837xxXFYjTS0sq drgS+MqHLSRr/ptyVajS5OZxkKcm8mVLLJ0o3HBhtBUho1F7S1elS/d4bN3hMrSFm+ AuyS7TP8OGlpzg/CLy84qLMb1ZNiqUs4wUedStpX3V9NEHiYi8QzsQ/H3MCGvHJkD/ vbD8Pgs9IW1CF4IY/Qee1MzxCtx8/DkOd/I/x1HKHAzLM/5AMFLHbi+V6zZ+EFlSOM 9GLPd3/5RnnSRlIjd+mdqUa006Y97K+yd7f5MXUV0jKXG1nlWMu6F9RPiBB2ygbNNc XXs1ZhtPttZJw== Received: from lists1.osuosl.org (lists1.osuosl.org [140.211.166.142]) by smtp4.osuosl.org (Postfix) with ESMTP id ED7E940B6D; Thu, 18 Sep 2025 20:47:43 +0000 (UTC) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists1.osuosl.org (Postfix) with ESMTP id 3EC99273 for ; Thu, 18 Sep 2025 20:47:42 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 2FF7B40A2B for ; Thu, 18 Sep 2025 20:47:42 +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 MW28xC9ZrXbT for ; Thu, 18 Sep 2025 20:47:41 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=2001:41d0:1004:224b::b8; helo=out-184.mta0.migadu.com; envelope-from=vadim.fedorenko@linux.dev; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp2.osuosl.org 9ABBB40070 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 9ABBB40070 Received: from out-184.mta0.migadu.com (out-184.mta0.migadu.com [IPv6:2001:41d0:1004:224b::b8]) by smtp2.osuosl.org (Postfix) with ESMTPS id 9ABBB40070 for ; Thu, 18 Sep 2025 20:47:40 +0000 (UTC) Message-ID: <0fc877a5-4b35-4802-9cda-e4eca561c5d1@linux.dev> Date: Thu, 18 Sep 2025 21:47:33 +0100 MIME-Version: 1.0 To: Chwee-Lin Choong , Tony Nguyen , Przemek Kitszel , Andrew Lunn , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Richard Cochran , Vinicius Costa Gomes Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Avi Shalev , Song Yoong Siang References: <20250918183811.31270-1-chwee.lin.choong@intel.com> 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: <20250918183811.31270-1-chwee.lin.choong@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1758228456; 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=F9lnNfEkDvcZjokYLgamdA2J6AkuI6fP9ftEP6VOZ2o=; b=ilw4l79LwaGDHO4b9169Vw87H546/3nWal6LdOQ8mATmyMUyUMxhoqT7phwb0dOBMu95Et FjKejSwvLCW3oK+4IDMvQTyfGYTL7ZiPym7sbZQmNLDCDQ7Qkn2HnDi4ekYFAfRlieixWd HwKrCQOxIaJVZddIs+JEjEa1VirXFPI= 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=ilw4l79L 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 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? From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-181.mta0.migadu.com (out-181.mta0.migadu.com [91.218.175.181]) (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 0D08FD528 for ; Thu, 18 Sep 2025 20:47:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.181 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758228471; cv=none; b=txqLUuYlsA1qCrjc8tTJaXRIYDWnlH05gNrgc3d0HofCeq5XhILLzCI9AjMRXw+OlPFHrxgAvzd40wPz+kbuJr8hFpK02GfyYyW4o1//EkPCSKIYWiPi+VsXZtmMLtdlVzUqClGFprVpYrMBdDcBSILoUbE/po3uDTeLtjdNIYo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758228471; c=relaxed/simple; bh=01aM7kTxYYuOTd2hpW+rvtHoGJAQTBNFNZJgBDduAuY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=AxevyMpReNmLRKglS3hEAdu1i2Tff1VAfU5nIaJvoarW3zG3IqDlxMeDuwXiMOK/hzBwQj5pNwyxv+FEnn8R5BdkOvgfIV76ZEmQ4X39DC/Bgnr/+9RnAwJC280fr/jHcp5iiaVOeocvI6SesvgMSimrM/hDlpFSsWJubrM0rtA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=ilw4l79L; arc=none smtp.client-ip=91.218.175.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="ilw4l79L" Message-ID: <0fc877a5-4b35-4802-9cda-e4eca561c5d1@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1758228456; 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=F9lnNfEkDvcZjokYLgamdA2J6AkuI6fP9ftEP6VOZ2o=; b=ilw4l79LwaGDHO4b9169Vw87H546/3nWal6LdOQ8mATmyMUyUMxhoqT7phwb0dOBMu95Et FjKejSwvLCW3oK+4IDMvQTyfGYTL7ZiPym7sbZQmNLDCDQ7Qkn2HnDi4ekYFAfRlieixWd HwKrCQOxIaJVZddIs+JEjEa1VirXFPI= Date: Thu, 18 Sep 2025 21:47:33 +0100 Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH iwl-net v1] igc: fix race condition in TX timestamp read for register 0 To: Chwee-Lin Choong , Tony Nguyen , Przemek Kitszel , Andrew Lunn , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Richard Cochran , Vinicius Costa Gomes Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Avi Shalev , Song Yoong Siang References: <20250918183811.31270-1-chwee.lin.choong@intel.com> 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: <20250918183811.31270-1-chwee.lin.choong@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT 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?