From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (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 2717D38AC86 for ; Tue, 7 Apr 2026 10:48:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775558936; cv=none; b=U7mxpUnYfCNXusNmPVG9uAN/mqCLsf0Bc0yaJq//fK2kZb8gwwuzIxrqztouyVy2M+MzxPeQ5zwZaJCqofl7QCz4c268U81vHz5T2sL4Y8/vEHvN5Mhc91ZoHA0Whv9Pt1mYdZAdV+H7QQbt+cgSPXYektGKH/5R+Qm4ZzWNTnc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775558936; c=relaxed/simple; bh=Ytn6pR4N3uMVNtXoQo8Gr27tjtQpaIHKh+ULAxWMXXo=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=A5S+EqcbojaV41E5eIs9bfWRETmIt8VfoiZUMTqGgmBb4MBmdpH7smoGROPOxKl3BLDMIC0xadfEPp6i4acpYzoM+3eWz/g3bzRLA5DZ8gWMC0Em2CEDNyubEuBjdTBN2pjkubQQ5zy6wGKbuRptuYcU959ieCpI1v1KPQI7NLI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=Jpb4PSu1; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=WIHk8h5z; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Jpb4PSu1"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="WIHk8h5z" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1775558934; 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=fwuWWKD1gpIC79YNQto4eiR0rGIBiL3Ai2I/8/jRRlE=; b=Jpb4PSu1WPp/82wKHS1A5pLSXCUK2cVZjNd0VKqy/Szrd3Kq9ZQ7MruLu8E1Vg0OhagsWN a+0W6402TOADyWes7lTZdlq16TdHimkWlRpIDYOPFZEPTxUDbk41vkco+wL8bIWp2tpu+2 Dp5hYubL1e8wYqRMftQKXz7zZURZyFU= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-649-tTRiluIZOI6r39U70KaigQ-1; Tue, 07 Apr 2026 06:48:53 -0400 X-MC-Unique: tTRiluIZOI6r39U70KaigQ-1 X-Mimecast-MFC-AGG-ID: tTRiluIZOI6r39U70KaigQ_1775558932 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-43cff5bc312so3689994f8f.1 for ; Tue, 07 Apr 2026 03:48:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1775558932; x=1776163732; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=fwuWWKD1gpIC79YNQto4eiR0rGIBiL3Ai2I/8/jRRlE=; b=WIHk8h5zpmAfvLzEZvB7xge8JA75yeaXjpxNn5ILvi5TUdctjYSHDbW1KLPZuFZeS6 fOn7VN4rNJwZFIlt7kO9MGpirIMXv9+zEnZXY5YvU6og5ln86KaPU9PhVVkiEUBhKrGa tvNyLxfr9+TbyOlbmVW+P/cFxSend14gY8IiH2EOelDxK468xHwgBqI0XF6L8hCFO2tI yrMR+rxdism5cEwm9Xft+Px4qhFMSCIx+W3Y+PBuWbCIx4wxmTvrToDECwGQJAg34P/o O21RSip3ded6sDeBfuJ6OF7egyZRYY3cuksSk7I//K5uffRAxocLlnW0yDeUDDxyar91 u7yA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775558932; x=1776163732; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=fwuWWKD1gpIC79YNQto4eiR0rGIBiL3Ai2I/8/jRRlE=; b=Fm0uJPQlEcWgYF9O1siPCwCZ8w3WQFPmOIvLZWcz7d6fGTZvOXwoW51emN42OdJKbl jhgEfqp5cYnqBCqf1sGMvygs/igp3F1u3FOfEFFZnyTyoeT/THy2Zf5ZYbhvtwj4gHn5 rYLsbsia/PFaTUozrb+MXz9xBCrdyoHRaOB85En6Jtts1JoakZOu/0IKO+pC1EZtkrbZ SeMoN18AHC1iNleGhgMI9r+mMfmG0g0/cu8L4rtTS1d4HNhbMyXsAwXwn7/fM/BLCVJg BvQsq9oZAvLdV0t0aUmtvruYT4dVLvjCpxxe1p6N8ewQZpduH9wscIHTHrDYd/7f0BYO CM7g== X-Gm-Message-State: AOJu0YyKVdPRrKGYCi4SoXMnUU9Tc1XaUz92Blto15e/+V9+wyYgZbbi wGenV/KvHu80xxzdKFT6Cp8Duh2+vKv0E44/ojocxMSrflvqMDFBjPy/udmBll8i14C4TeH0Bfv 9E21Gkw6tpO0jUFPKnca7UY2EUoffXERciU9dwa6kUsGPIUE2enMCLUc7ScF2SsN6dw== X-Gm-Gg: AeBDievQsMkfmkz9f7jhB1/lfBxM/muHMMZJx/f6MKfFhnU9JDOhai0O+Az5GP6UokI hV9KY95X0MTlBRO/8Cg5ancyKpOa2UxvgFuLjG2LZxcspGVjBtKZrLyud8wkwZR6vFHC3To0Ixe mPh94i0b87sGXS8clEB7s1G02NVQwkcGNyRs5jUcYv2xbx7FIDrI/mDlJK6YGhNyJW4b1xP6yej jRrlIv3UnesP3WUgArrtFYWI56LRciQH6GD5OtA0N9JrWLbz/ENhW/FF1W95NLdlyKcrcKkzuB8 wta3X3HO0R0dY9VIkmCcQYciZMANoD/DhjQiNgh3RZGev6xQVgADB2F38Vf7m+xgIxvqOXECC0P 6oIKF2CcCgmjagIPleCKyXZNrpcP2PlfIlsf6puQg/8fjz/6d8to9pnAX6w== X-Received: by 2002:a05:600c:4743:b0:487:219e:42d with SMTP id 5b1f17b1804b1-4889970642emr229188255e9.11.1775558931657; Tue, 07 Apr 2026 03:48:51 -0700 (PDT) X-Received: by 2002:a05:600c:4743:b0:487:219e:42d with SMTP id 5b1f17b1804b1-4889970642emr229187935e9.11.1775558931166; Tue, 07 Apr 2026 03:48:51 -0700 (PDT) Received: from [192.168.88.32] ([212.105.153.231]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48895e19c10sm348616065e9.8.2026.04.07.03.48.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 07 Apr 2026 03:48:50 -0700 (PDT) Message-ID: Date: Tue, 7 Apr 2026 12:48:43 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 net] amd-xgbe: synchronize KR training with device operations To: Raju Rangoju , netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org, kuba@kernel.org, edumazet@google.com, davem@davemloft.net, andrew+netdev@lunn.ch, Thomas.Lendacky@amd.com, maxime.chevallier@bootlin.com References: <20260403072157.1042806-1-Raju.Rangoju@amd.com> Content-Language: en-US From: Paolo Abeni In-Reply-To: <20260403072157.1042806-1-Raju.Rangoju@amd.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 4/3/26 9:21 AM, Raju Rangoju wrote: > +static bool xgbe_kr_training_in_progress(struct xgbe_prv_data *pdata) > +{ > + struct xgbe_phy_data *phy_data = pdata->phy_data; > + unsigned long kr_start, kr_end; > + > + /* Only wait for KR training in specific conditions: > + * - Inphi re-driver is present, OR > + * - Currently in KR mode with autoneg enabled > + */ > + if (!xgbe_phy_port_is_inphi(pdata) && > + !(phy_data->cur_mode == XGBE_MODE_KR && > + pdata->phy.autoneg == AUTONEG_ENABLE)) > + return false; > + > + /* If training hasn't completed, ensure it actually started */ > + kr_start = READ_ONCE(pdata->kr_start_time); > + if (!kr_start) > + return false; AFAICS kr_start_time is set to the current jiffies value at initialization time and 0 is a valid - even if unlikely - jiffies value. The above test is false-positive prone. > + > + /* Training is complete - no need to wait */ > + if (READ_ONCE(pdata->an_result) == XGBE_AN_COMPLETE) > + return false; > + > + kr_end = kr_start + > + msecs_to_jiffies(XGBE_AN_MS_TIMEOUT + XGBE_KRTR_TIME); > + > + /* If we're already past the training window, it's not "in progress" */ > + if (time_after(jiffies, kr_end)) > + return false; Sashiko notes wrap-around (32 bits systems) will lead to wrong return value. > + > + return true; > +} > + > +static void xgbe_wait_for_kr_training_inprogress(struct xgbe_prv_data *pdata) > +{ > + unsigned long kr_end; > + > + if (!xgbe_kr_training_in_progress(pdata)) > + return; > + > + /* Don't block the auto-negotiation state machine work item */ > + if (current_work() == &pdata->an_work) > + return; > + > + kr_end = READ_ONCE(pdata->kr_start_time) + > + msecs_to_jiffies(XGBE_AN_MS_TIMEOUT + XGBE_KRTR_TIME); > + > + /* Poll until training completes or the training window expires */ > + while (time_before(jiffies, kr_end)) { > + if (READ_ONCE(pdata->an_result) == XGBE_AN_COMPLETE) > + break; > + > + usleep_range(10000, 11000); > + } > +} > + > static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, > - enum xgbe_mb_cmd cmd, enum xgbe_mb_subcmd sub_cmd) > + enum xgbe_mb_cmd cmd, > + enum xgbe_mb_subcmd sub_cmd) > { > unsigned int s0 = 0; > unsigned int wait; > @@ -2104,6 +2171,13 @@ static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, > /* Disable PLL re-initialization during FW command processing */ > xgbe_phy_pll_ctrl(pdata, false); > > + /* Serialize firmware mailbox access. > + * Protects entire command sequence including busy check, PLL control, > + * and command execution. Uses explicit lock/unlock for compatibility > + * with goto-based cleanup (per cleanup.h guidelines). > + */ > + mutex_lock(&pdata->mailbox_lock); The comment is confusing, due the the `xgbe_phy_pll_ctrl()` invocation just before acquiring the lock. > + > /* Log if a previous command did not complete */ > if (XP_IOREAD_BITS(pdata, XP_DRIVER_INT_RO, STATUS)) { > netif_dbg(pdata, link, pdata->netdev, > @@ -2115,7 +2189,7 @@ static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, > XP_SET_BITS(s0, XP_DRIVER_SCRATCH_0, COMMAND, cmd); > XP_SET_BITS(s0, XP_DRIVER_SCRATCH_0, SUB_COMMAND, sub_cmd); > > - /* Issue the command */ > + /* Issue the firmware command */ > XP_IOWRITE(pdata, XP_DRIVER_SCRATCH_0, s0); > XP_IOWRITE(pdata, XP_DRIVER_SCRATCH_1, 0); > XP_IOWRITE_BITS(pdata, XP_DRIVER_INT_REQ, REQUEST, 1); > @@ -2123,11 +2197,13 @@ static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, > /* Wait for command to complete */ > wait = XGBE_RATECHANGE_COUNT; > while (wait--) { > - if (!XP_IOREAD_BITS(pdata, XP_DRIVER_INT_RO, STATUS)) > + if (!XP_IOREAD_BITS(pdata, XP_DRIVER_INT_RO, STATUS)) { > + mutex_unlock(&pdata->mailbox_lock); > goto do_rx_adaptation; > - > + } > usleep_range(1000, 2000); > } > + mutex_unlock(&pdata->mailbox_lock); > > netif_dbg(pdata, link, pdata->netdev, > "firmware mailbox command did not complete\n"); A `xgbe_phy_rx_reset()` invocation will happen here, outside the lock. Can that race? will that corrupt the nic status? /P