[Intel-wired-lan] [PATCH 3/3] igb: Get rid of spurious interrupts
When running the igc with XDP/ZC in busy polling mode with deferral of hard interrupts, interrupts still happen from time to time. That is caused by the igc task watchdog which triggers Rx interrupts periodically. That mechanism has been introduced to overcome skb/memory allocation failures [1]. So the Rx clean functions stop processing the Rx ring in case of such failure. The task watchdog triggers Rx interrupts periodically in the hope that memory became available in the mean time. The current behavior is undesirable for real time applications, because the driver induced Rx interrupts trigger also the softirq processing. However, all real time packets should be processed by the application which uses the busy polling method. Therefore, only trigger the Rx interrupts in case of real allocation failures. Introduce a new flag for signaling that condition. Follow the same logic as in commit 8dcf2c212078 ("igc: Get rid of spurious interrupts"). [1] - https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=3be507547e6177e5c808544bd6a2efa2c7f1d436 Signed-off-by: Kurt Kanzenbach --- drivers/net/ethernet/intel/igb/igb.h | 3 ++- drivers/net/ethernet/intel/igb/igb_main.c | 29 + drivers/net/ethernet/intel/igb/igb_xsk.c | 1 + 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h index 79eca385a751bfdafdf384928b6cc1b350b22560..f34ead8243e9f0176a068299138c5c16f7faab2e 100644 --- a/drivers/net/ethernet/intel/igb/igb.h +++ b/drivers/net/ethernet/intel/igb/igb.h @@ -391,7 +391,8 @@ enum e1000_ring_flags_t { IGB_RING_FLAG_RX_LB_VLAN_BSWAP, IGB_RING_FLAG_TX_CTX_IDX, IGB_RING_FLAG_TX_DETECT_HANG, - IGB_RING_FLAG_TX_DISABLED + IGB_RING_FLAG_TX_DISABLED, + IGB_RING_FLAG_RX_ALLOC_FAILED, }; #define ring_uses_large_buffer(ring) \ diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 8e964484f4c9854e4e3e0b4f3e8785fe93bd1207..96da3e2ddc9a67f799ee55d9621d98f80a6b449c 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -5754,11 +5754,29 @@ static void igb_watchdog_task(struct work_struct *work) if (adapter->flags & IGB_FLAG_HAS_MSIX) { u32 eics = 0; - for (i = 0; i < adapter->num_q_vectors; i++) - eics |= adapter->q_vector[i]->eims_value; - wr32(E1000_EICS, eics); + for (i = 0; i < adapter->num_q_vectors; i++) { + struct igb_q_vector *q_vector = adapter->q_vector[i]; + struct igb_ring *rx_ring; + + if (!q_vector->rx.ring) + continue; + + rx_ring = adapter->rx_ring[q_vector->rx.ring->queue_index]; + + if (test_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring->flags)) { + eics |= q_vector->eims_value; + clear_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring->flags); + } + } + if (eics) + wr32(E1000_EICS, eics); } else { - wr32(E1000_ICS, E1000_ICS_RXDMT0); + struct igb_ring *rx_ring = adapter->rx_ring[0]; + + if (test_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring->flags)) { + clear_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring->flags); + wr32(E1000_ICS, E1000_ICS_RXDMT0); + } } igb_spoof_check(adapter); @@ -9089,6 +9107,7 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget) if (!xdp_res && !skb) { rx_ring->rx_stats.alloc_failed++; rx_buffer->pagecnt_bias++; + set_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring->flags); break; } @@ -9148,6 +9167,7 @@ static bool igb_alloc_mapped_page(struct igb_ring *rx_ring, page = dev_alloc_pages(igb_rx_pg_order(rx_ring)); if (unlikely(!page)) { rx_ring->rx_stats.alloc_failed++; + set_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring->flags); return false; } @@ -9164,6 +9184,7 @@ static bool igb_alloc_mapped_page(struct igb_ring *rx_ring, __free_pages(page, igb_rx_pg_order(rx_ring)); rx_ring->rx_stats.alloc_failed++; + set_bit(IGB_RING_FLAG_RX_ALLOC_FAILED, &rx_ring->flags); return false; } diff --git a/drivers/net/ethernet/intel/igb/igb_xsk.c b/drivers/net/ethernet/intel/igb/igb_xsk.c index a5ad090dfe94b6afc8194fe39d28cdd51c7067b0..47344ee1ed7f29bd68055485702a87df3b8922e8 100644 --- a/drivers/net/ethernet/intel/igb/igb_xsk.c +++ b/dri
[Intel-wired-lan] [PATCH 0/3] igb: XDP/ZC follow up
This is a follow up for the igb XDP/ZC implementation. The first two patches link the IRQs and queues to NAPI instances. This is required to bring back the XDP/ZC busy polling support. The last patch removes undesired IRQs (injected via igb watchdog) while busy polling with napi_defer_hard_irqs and gro_flush_timeout set. Signed-off-by: Kurt Kanzenbach --- Kurt Kanzenbach (3): igb: Link IRQs to NAPI instances igb: Link queues to NAPI instances igb: Get rid of spurious interrupts drivers/net/ethernet/intel/igb/igb.h | 5 ++- drivers/net/ethernet/intel/igb/igb_main.c | 67 ++- drivers/net/ethernet/intel/igb/igb_xsk.c | 3 ++ 3 files changed, 65 insertions(+), 10 deletions(-) --- base-commit: acdefab0dcbc3833b5a734ab80d792bb778517a0 change-id: 20250206-igb_irq-f5a4d4deb207 Best regards, -- Kurt Kanzenbach
[Intel-wired-lan] [PATCH 1/3] igb: Link IRQs to NAPI instances
Link IRQs to NAPI instances via netdev-genl API. This allows users to query that information via netlink: |$ ./tools/net/ynl/pyynl/cli.py --spec Documentation/netlink/specs/netdev.yaml \ | --dump napi-get --json='{"ifindex": 2}' |[{'defer-hard-irqs': 0, | 'gro-flush-timeout': 0, | 'id': 8204, | 'ifindex': 2, | 'irq': 127, | 'irq-suspend-timeout': 0}, | {'defer-hard-irqs': 0, | 'gro-flush-timeout': 0, | 'id': 8203, | 'ifindex': 2, | 'irq': 126, | 'irq-suspend-timeout': 0}, | {'defer-hard-irqs': 0, | 'gro-flush-timeout': 0, | 'id': 8202, | 'ifindex': 2, | 'irq': 125, | 'irq-suspend-timeout': 0}, | {'defer-hard-irqs': 0, | 'gro-flush-timeout': 0, | 'id': 8201, | 'ifindex': 2, | 'irq': 124, | 'irq-suspend-timeout': 0}] |$ cat /proc/interrupts | grep enp2s0 |123: 0 1 IR-PCI-MSIX-:02:00.0 0-edge enp2s0 |124: 0 7 IR-PCI-MSIX-:02:00.0 1-edge enp2s0-TxRx-0 |125: 0 0 IR-PCI-MSIX-:02:00.0 2-edge enp2s0-TxRx-1 |126: 0 5 IR-PCI-MSIX-:02:00.0 3-edge enp2s0-TxRx-2 |127: 0 0 IR-PCI-MSIX-:02:00.0 4-edge enp2s0-TxRx-3 Signed-off-by: Kurt Kanzenbach --- drivers/net/ethernet/intel/igb/igb_main.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index d368b753a4675d01b5dfa50dee4cd218e6a5e14b..d4128d19cc08f62f95682069bb5ed9b8bbbf10cb 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -947,6 +947,9 @@ static int igb_request_msix(struct igb_adapter *adapter) q_vector); if (err) goto err_free; + + netif_napi_set_irq(&q_vector->napi, + adapter->msix_entries[vector].vector); } igb_configure_msix(adapter); -- 2.39.5
[Intel-wired-lan] [PATCH 2/3] igb: Link queues to NAPI instances
Link queues to NAPI instances via netdev-genl API. This is required to use XDP/ZC busy polling. See commit 5ef44b3cb43b ("xsk: Bring back busy polling support") for details. This also allows users to query the info with netlink: |$ ./tools/net/ynl/pyynl/cli.py --spec Documentation/netlink/specs/netdev.yaml \ | --dump queue-get --json='{"ifindex": 2}' |[{'id': 0, 'ifindex': 2, 'napi-id': 8201, 'type': 'rx'}, | {'id': 1, 'ifindex': 2, 'napi-id': 8202, 'type': 'rx'}, | {'id': 2, 'ifindex': 2, 'napi-id': 8203, 'type': 'rx'}, | {'id': 3, 'ifindex': 2, 'napi-id': 8204, 'type': 'rx'}, | {'id': 0, 'ifindex': 2, 'napi-id': 8201, 'type': 'tx'}, | {'id': 1, 'ifindex': 2, 'napi-id': 8202, 'type': 'tx'}, | {'id': 2, 'ifindex': 2, 'napi-id': 8203, 'type': 'tx'}, | {'id': 3, 'ifindex': 2, 'napi-id': 8204, 'type': 'tx'}] While at __igb_open() use RCT coding style. Signed-off-by: Kurt Kanzenbach --- drivers/net/ethernet/intel/igb/igb.h | 2 ++ drivers/net/ethernet/intel/igb/igb_main.c | 35 ++- drivers/net/ethernet/intel/igb/igb_xsk.c | 2 ++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h index 02f340280d20a6f7e32bbd3dfcbb9c1c7b4c6662..79eca385a751bfdafdf384928b6cc1b350b22560 100644 --- a/drivers/net/ethernet/intel/igb/igb.h +++ b/drivers/net/ethernet/intel/igb/igb.h @@ -722,6 +722,8 @@ enum igb_boards { extern char igb_driver_name[]; +void igb_set_queue_napi(struct igb_adapter *adapter, int q_idx, + struct napi_struct *napi); int igb_xmit_xdp_ring(struct igb_adapter *adapter, struct igb_ring *ring, struct xdp_frame *xdpf); diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index d4128d19cc08f62f95682069bb5ed9b8bbbf10cb..8e964484f4c9854e4e3e0b4f3e8785fe93bd1207 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -2099,6 +2099,22 @@ static void igb_check_swap_media(struct igb_adapter *adapter) wr32(E1000_CTRL_EXT, ctrl_ext); } +void igb_set_queue_napi(struct igb_adapter *adapter, int vector, + struct napi_struct *napi) +{ + struct igb_q_vector *q_vector = adapter->q_vector[vector]; + + if (q_vector->rx.ring) + netif_queue_set_napi(adapter->netdev, +q_vector->rx.ring->queue_index, +NETDEV_QUEUE_TYPE_RX, napi); + + if (q_vector->tx.ring) + netif_queue_set_napi(adapter->netdev, +q_vector->tx.ring->queue_index, +NETDEV_QUEUE_TYPE_TX, napi); +} + /** * igb_up - Open the interface and prepare it to handle traffic * @adapter: board private structure @@ -2106,6 +2122,7 @@ static void igb_check_swap_media(struct igb_adapter *adapter) int igb_up(struct igb_adapter *adapter) { struct e1000_hw *hw = &adapter->hw; + struct napi_struct *napi; int i; /* hardware has been reset, we need to reload some things */ @@ -2113,8 +2130,11 @@ int igb_up(struct igb_adapter *adapter) clear_bit(__IGB_DOWN, &adapter->state); - for (i = 0; i < adapter->num_q_vectors; i++) - napi_enable(&(adapter->q_vector[i]->napi)); + for (i = 0; i < adapter->num_q_vectors; i++) { + napi = &adapter->q_vector[i]->napi; + napi_enable(napi); + igb_set_queue_napi(adapter, i, napi); + } if (adapter->flags & IGB_FLAG_HAS_MSIX) igb_configure_msix(adapter); @@ -2184,6 +2204,7 @@ void igb_down(struct igb_adapter *adapter) for (i = 0; i < adapter->num_q_vectors; i++) { if (adapter->q_vector[i]) { napi_synchronize(&adapter->q_vector[i]->napi); + igb_set_queue_napi(adapter, i, NULL); napi_disable(&adapter->q_vector[i]->napi); } } @@ -4116,8 +4137,9 @@ static int igb_sw_init(struct igb_adapter *adapter) static int __igb_open(struct net_device *netdev, bool resuming) { struct igb_adapter *adapter = netdev_priv(netdev); - struct e1000_hw *hw = &adapter->hw; struct pci_dev *pdev = adapter->pdev; + struct e1000_hw *hw = &adapter->hw; + struct napi_struct *napi; int err; int i; @@ -4169,8 +4191,11 @@ static int __igb_open(struct net_device *netdev, bool resuming) /* From here on the code is the same as igb_up() */ clear_bit(__IGB_DOWN, &adapter->state); - for (i = 0; i < adapter->num_q_vectors; i++) - napi_enable(&(adapter->q_vector[i]->napi)); + for (i = 0; i < adapter->num_q_vectors; i++) { + napi = &adapter->q_vector[i]->napi; + napi_
Re: [Intel-wired-lan] [PATCH v2] net: e1000e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
On 08/02/2025 15:43, Piotr Wejman wrote: Update the driver to use the new hardware timestamping API added in commit 66f7223039c0 ("net: add NDOs for configuring hardware timestamping"). Use Netlink extack for error reporting in e1000e_hwtstamp_set. Align the indentation of net_device_ops. Signed-off-by: Piotr Wejman --- Changes in v2: - amend commit message - use extack for error reporting - rename e1000_mii_ioctl to e1000_ioctl - Link to v1: https://lore.kernel.org/netdev/20250202170839.47375-1-piotrwejma...@gmail.com/ Reviewed-by: Vadim Fedorenko
Re: [Intel-wired-lan] [iwl-next, rdma v3 00/24] Add RDMA support for Intel IPU E2000 (GEN3)
On 2/7/25 20:49, Tatyana Nikolova wrote: This patch series is based on 6.14-rc1 and includes both netdev and RDMA patches for ease of review. It can also be viewed here [1]. A shared pull request will be sent for patches 1-7 following review. [...] TLDR of my mail: could be take 1st patch prior to the rest? V2 RFC series is at https://lwn.net/Articles/987141/. code there was mostly the same, and noone commented, I bet due to the sheer size of the series 58 files changed, 6693 insertions(+), 1137 deletions(-) While it is very good to have a common cover letter and all the changes available for inspection/cross reference, it is just too much for doing a practical review. I think it would be much easier to follow and split into multiple series if we will apply the first patch, is that feasible now? (It was a no-no back when there were no idpf at all, no it is not a theoretical case anymore, and the patch makes code better anyway)? The first patch: Dave Ertman (1): iidc/ice/irdma: Update IDC to support multiple consumers @Tatyana The rest of the patches needs to go via our internal e1000 ML, and get the Reviewed-by tag prior to being ready for wider review (now the community would have to put substantial effor to get rid of style distractions, etc, and, as we have seen, there was not much eagerness to do that for your RFC v2).
[Intel-wired-lan] [PATCH iwl-next v3] ixgbe: add support for thermal sensor event reception
E610 NICs unlike the previous devices utilising ixgbe driver are notified in the case of overheatning by the FW ACI event. In event of overheat when threshold is exceeded, FW suspends all traffic and sends overtemp event to the driver. Then driver logs appropriate message and closes the adapter instance. The card remains in that state until the platform is rebooted. Reviewed-by: Przemek Kitszel Reviewed-by: Mateusz Polchlopek Signed-off-by: Jedrzej Jagielski --- v2,3 : commit msg tweaks --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 + drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h | 3 +++ 2 files changed, 8 insertions(+) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 7236f20c9a30..5c804948dd1f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -3165,6 +3165,7 @@ static void ixgbe_aci_event_cleanup(struct ixgbe_aci_event *event) static void ixgbe_handle_fw_event(struct ixgbe_adapter *adapter) { struct ixgbe_aci_event event __cleanup(ixgbe_aci_event_cleanup); + struct net_device *netdev = adapter->netdev; struct ixgbe_hw *hw = &adapter->hw; bool pending = false; int err; @@ -3185,6 +3186,10 @@ static void ixgbe_handle_fw_event(struct ixgbe_adapter *adapter) case ixgbe_aci_opc_get_link_status: ixgbe_handle_link_status_event(adapter, &event); break; + case ixgbe_aci_opc_temp_tca_event: + e_crit(drv, "%s\n", ixgbe_overheat_msg); + ixgbe_close(netdev); + break; default: e_warn(hw, "unknown FW async event captured\n"); break; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h index 8d06ade3c7cd..617e07878e4f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h @@ -171,6 +171,9 @@ enum ixgbe_aci_opc { ixgbe_aci_opc_done_alt_write= 0x0904, ixgbe_aci_opc_clear_port_alt_write = 0x0906, + /* TCA Events */ + ixgbe_aci_opc_temp_tca_event= 0x0C94, + /* debug commands */ ixgbe_aci_opc_debug_dump_internals = 0xFF08, -- 2.31.1
Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: Add sync delay for E825C
> -Original Message- > From: Simon Horman > Sent: Friday, February 7, 2025 11:04 AM > To: Nitka, Grzegorz > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; Nguyen, > Anthony L ; Kitszel, Przemyslaw > ; Kolacinski, Karol > > Subject: Re: [PATCH iwl-next v1 1/3] ice: Add sync delay for E825C > > On Thu, Feb 06, 2025 at 09:36:53AM +0100, Grzegorz Nitka wrote: > > From: Karol Kolacinski > > > > Implement setting GLTSYN_SYNC_DLAY for E825C products. > > This is the execution delay compensation of SYNC command between > > PHC and PHY. > > Also, refactor the code by changing ice_ptp_init_phc_eth56g function > > name to ice_ptp_init_phc_e825, to be consistent with the naming pattern > > for other devices. > > Adding support for GLTSYN_SYNC_DLAY and the refactor seem > to be two distinct changes, albeit touching common code. > > I think it would be slightly better to split this into two patches. > Sure, will exclude this commit from the series and will submit it as separate patch. Thanks for your review and valuable feedback. > > Reviewed-by: Przemek Kitszel > > Signed-off-by: Karol Kolacinski > > Signed-off-by: Grzegorz Nitka > > ...
Re: [Intel-wired-lan] [PATCH iwl-next v1 2/3] ice: Refactor E825C PHY registers info struct
> -Original Message- > From: Simon Horman > Sent: Friday, February 7, 2025 11:06 AM > To: Nitka, Grzegorz > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; Nguyen, > Anthony L ; Kitszel, Przemyslaw > ; Kolacinski, Karol > > Subject: Re: [PATCH iwl-next v1 2/3] ice: Refactor E825C PHY registers info > struct > > On Fri, Feb 07, 2025 at 10:03:45AM +, Simon Horman wrote: > > On Thu, Feb 06, 2025 at 09:36:54AM +0100, Grzegorz Nitka wrote: > > > From: Karol Kolacinski > > > > > > Simplify ice_phy_reg_info_eth56g struct definition to include base > > > address for the very first quad. Use base address info and 'step' > > > value to determine address for specific PHY quad. > > > > > > Reviewed-by: Przemek Kitszel > > > Signed-off-by: Karol Kolacinski > > > Signed-off-by: Grzegorz Nitka > > > > Reviewed-by: Simon Horman > > Sorry, I failed to notice that the kdoc for ice_phy_reg_info_eth56g > needs to be updated to document base_addr instead of base. Good catch! Thanks! To be fixed in v2.
Re: [Intel-wired-lan] [PATCH iwl-next v1 01/13] ixgbe: add initial devlink support
From: Simon Horman Sent: Friday, February 7, 2025 11:47 AM >On Mon, Feb 03, 2025 at 04:03:16PM +0100, Jedrzej Jagielski wrote: >> Add an initial support for devlink interface to ixgbe driver. >> >> Similarly to i40e driver the implementation doesn't enable >> devlink to manage device-wide configuration. Devlink instance >> is created for each physical function of PCIe device. >> >> Create separate directory for devlink related ixgbe files >> and use naming scheme similar to the one used in the ice driver. >> >> Add a stub for Documentation, to be extended by further patches. >> >> Reviewed-by: Mateusz Polchlopek >> Signed-off-by: Jedrzej Jagielski > >... > >> diff --git a/Documentation/networking/devlink/ixgbe.rst >> b/Documentation/networking/devlink/ixgbe.rst >> new file mode 100644 >> index ..ca920d421d42 >> --- /dev/null >> +++ b/Documentation/networking/devlink/ixgbe.rst >> @@ -0,0 +1,8 @@ >> +.. SPDX-License-Identifier: GPL-2.0 >> + >> + >> +ixgbe devlink support >> + > >nit: the '=' lines are one character too short wrt the text they decorate. > >Flagged by make htmldocs. Thanks for catching it. Will be fixed. > >> + >> +This document describes the devlink features implemented by the ``ixgbe`` >> +device driver. > >...
Re: [Intel-wired-lan] [PATCH iwl-next v1 02/13] ixgbe: add handler for devlink .info_get()
From: Simon Horman Sent: Sunday, February 9, 2025 5:27 PM >On Mon, Feb 03, 2025 at 04:03:17PM +0100, Jedrzej Jagielski wrote: >> Provide devlink .info_get() callback implementation to allow the >> driver to report detailed version information. The following info >> is reported: >> >> "serial_number" -> The PCI DSN of the adapter >> "fw.bundle_id" -> Unique identifier for the combined flash image >> "fw.undi" -> Version of the Option ROM containing the UEFI driver >> "board.id" -> The PBA ID string >> >> Reviewed-by: Mateusz Polchlopek >> Signed-off-by: Jedrzej Jagielski > >... > >> diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c >> b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c > >... > >> +static void ixgbe_info_nvm_ver(struct ixgbe_adapter *adapter, >> + struct ixgbe_info_ctx *ctx) >> +{ >> +struct ixgbe_hw *hw = &adapter->hw; >> +struct ixgbe_nvm_version nvm_ver; >> + >> +ixgbe_get_oem_prod_version(hw, &nvm_ver); >> +if (nvm_ver.oem_valid) { >> +snprintf(ctx->buf, sizeof(ctx->buf), "%x.%x.%x", >> + nvm_ver.oem_major, nvm_ver.oem_minor, >> + nvm_ver.oem_release); >> + >> +return; >> +} >> + >> +ixgbe_get_orom_version(hw, &nvm_ver); >> +if (nvm_ver.or_valid) >> +snprintf(ctx->buf, sizeof(ctx->buf), "%d.%d.%d", >> + nvm_ver.or_major, nvm_ver.or_build, nvm_ver.or_patch); > >Hi Jedrzej, > >If neither of the conditions above are met then it seems that ctx->buf will >contain whatever string was present when the function was called. Is >something like the following needed here? > > ctx->buf[0] = '\0'; Hi Simon, thanks for suggestion, it's definitely worth do be incorporated in the next revision. Thanks! > >> +} >> + >> +static void ixgbe_info_eetrack(struct ixgbe_adapter *adapter, >> + struct ixgbe_info_ctx *ctx) >> +{ >> +struct ixgbe_hw *hw = &adapter->hw; >> +struct ixgbe_nvm_version nvm_ver; >> + >> +ixgbe_get_oem_prod_version(hw, &nvm_ver); >> +/* No ETRACK version for OEM */ >> +if (nvm_ver.oem_valid) >> +return; > >Likewise, here. > >> + >> +ixgbe_get_etk_id(hw, &nvm_ver); >> +snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", nvm_ver.etk_id); >> +} >> + >> +static int ixgbe_devlink_info_get(struct devlink *devlink, >> + struct devlink_info_req *req, >> + struct netlink_ext_ack *extack) >> +{ >> +struct ixgbe_devlink_priv *devlink_private = devlink_priv(devlink); >> +struct ixgbe_adapter *adapter = devlink_private->adapter; >> +struct ixgbe_hw *hw = &adapter->hw; >> +struct ixgbe_info_ctx *ctx; >> +int err; >> + >> +ctx = kmalloc(sizeof(*ctx), GFP_KERNEL); >> +if (!ctx) >> +return -ENOMEM; >> + >> +ixgbe_info_get_dsn(adapter, ctx); >> +err = devlink_info_serial_number_put(req, ctx->buf); >> +if (err) >> +goto free_ctx; >> + >> +ixgbe_info_nvm_ver(adapter, ctx); >> +err = ixgbe_devlink_info_put(req, IXGBE_DL_VERSION_RUNNING, >> + DEVLINK_INFO_VERSION_GENERIC_FW_UNDI, >> + ctx->buf); >> +if (err) >> +goto free_ctx; >> + >> +ixgbe_info_eetrack(adapter, ctx); >> +err = ixgbe_devlink_info_put(req, IXGBE_DL_VERSION_RUNNING, >> + DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID, >> + ctx->buf); >> +if (err) >> +goto free_ctx; >> + >> +err = ixgbe_read_pba_string_generic(hw, ctx->buf, sizeof(ctx->buf)); >> +if (err) >> +goto free_ctx; >> + >> +err = ixgbe_devlink_info_put(req, IXGBE_DL_VERSION_FIXED, >> + DEVLINK_INFO_VERSION_GENERIC_BOARD_ID, >> + ctx->buf); >> +free_ctx: >> +kfree(ctx); >> +return err; >> +} > >...
Re: [Intel-wired-lan] [iwl-next, rdma v3 00/24] Add RDMA support for Intel IPU E2000 (GEN3)
On Mon, Feb 10, 2025 at 11:41:31AM +0100, Przemek Kitszel wrote: > On 2/7/25 20:49, Tatyana Nikolova wrote: > > This patch series is based on 6.14-rc1 and includes both netdev and RDMA > > patches for ease of review. It can also be viewed here [1]. A shared pull > > request will be sent for patches 1-7 following review. > > > > [...] > TLDR of my mail: could be take 1st patch prior to the rest? > > > V2 RFC series is at https://lwn.net/Articles/987141/. > > code there was mostly the same, and noone commented, I bet due > to the sheer size of the series It was very optimistic to expect for a review during holiday season and merge window, especially series of 25 patches which are marked as RFC. Thanks
Re: [Intel-wired-lan] [PATCH] ixgbe: remove self assignment
On Sun, Feb 09, 2025 at 11:47:24PM -0500, Ethan Carter Edwards wrote: > Variable self assignment does not have any effect. Hi Ethan As a general rule, it would be good to explain in the comment message what research you did to find out why there is a self assignment, and why just deleting it is the correct solution. There are somewhat legitimate reasons to do a self assign, some older compilers would warn about variables which were set but then never used, for example. Or it could be a dumb copy/paste error when writing the code. But more likely than not, the developer had something in mind, got distracted, and never finished the code. Which appears to the issue here. If you cannot figure out what the correct fix is, please just email to the list, Cc: the Maintainer of the file, pointing out the problem. Andrew --- pw-bot: cr
Re: [Intel-wired-lan] [PATCH net-next v7 1/5] net: move ARFS rmap management to core
On 2025-02-06 7:29 p.m., Jakub Kicinski wrote: Speaking of which, why do the auto-removal in napi_disable() rather than netif_napi_del() ? We don't reinstall on napi_enable() and doing a disable() + enable() is fairly common during driver reconfig. The patch does not re-install the notifiers in napi_add either, they are installed in set_irq() : napi_add_config() -> napi_set_irq() -> napi_enable() so napi_disable or napi_del seemed both OK to me. However, I moved notifier auto-removal to npi_del() and did some testing on ice but it seems the driver does not delete napi on "ip link down" and that generates warnings on free_irq(). It only disables the napis. So is this a bug? Do we need to ask drivers to disable __and__ delete napis before freeing the IRQs? If not, then we have to keep notifier aut-removal in napi_diasable().
Re: [Intel-wired-lan] [PATCH iwl-next v3] ixgbe: add support for thermal sensor event reception
Dear Jedrzej, Thank you for the quick reply. Am 10.02.25 um 12:59 schrieb Jagielski, Jedrzej: From: Paul Menzel Sent: Monday, February 10, 2025 12:40 PM Am 10.02.25 um 11:40 schrieb Jedrzej Jagielski: E610 NICs unlike the previous devices utilising ixgbe driver are notified in the case of overheatning by the FW ACI event. overheating (without n) In event of overheat when threshold is exceeded, FW suspends all traffic and sends overtemp event to the driver. I guess this was already the behavior before your patch, and now it’s only logged, and the device stopped properly? Without this patch driver cannot receive the overtemp info. FW handles the event - device stops but user has no clue what has happened. FW does the major work but this patch adds this minimal piece of overtemp handling done by SW - informing user at the very end. Thank you for the confirmation. Maybe add a small note, that it’s not logged to make it crystal clear for people like myself. Then driver logs appropriate message and closes the adapter instance. The card remains in that state until the platform is rebooted. As a user I’d be interested what the threshold is, and what the measured temperature is. Currently, the log seems to be just generic? These details are FW internals. Driver just gets info that such event has happened. There's no additional information. In that case driver's job is just to inform user that such scenario has happened and tell what should be the next steps. From a user perspective that is a suboptimal behavior, and shows another time that the Linux kernel should have all the control, and stuff like this should be moved *out* of the firmware and not into the firmware. It’d be great if you could talk to the card designers/engineers to take that into account, and maybe revert this decision for future versions or future adapters. Kind regards, Paul drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:static const char ixgbe_overheat_msg[] = "Network adapter has been stopped because it has over heated. Restart the computer. If the problem persists, power off the system and replace the adapter"; Reviewed-by: Przemek Kitszel Reviewed-by: Mateusz Polchlopek Signed-off-by: Jedrzej Jagielski --- v2,3 : commit msg tweaks --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 + drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h | 3 +++ 2 files changed, 8 insertions(+) Kind regards, Paul diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 7236f20c9a30..5c804948dd1f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -3165,6 +3165,7 @@ static void ixgbe_aci_event_cleanup(struct ixgbe_aci_event *event) static void ixgbe_handle_fw_event(struct ixgbe_adapter *adapter) { struct ixgbe_aci_event event __cleanup(ixgbe_aci_event_cleanup); + struct net_device *netdev = adapter->netdev; struct ixgbe_hw *hw = &adapter->hw; bool pending = false; int err; @@ -3185,6 +3186,10 @@ static void ixgbe_handle_fw_event(struct ixgbe_adapter *adapter) case ixgbe_aci_opc_get_link_status: ixgbe_handle_link_status_event(adapter, &event); break; + case ixgbe_aci_opc_temp_tca_event: + e_crit(drv, "%s\n", ixgbe_overheat_msg); + ixgbe_close(netdev); + break; default: e_warn(hw, "unknown FW async event captured\n"); break; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h index 8d06ade3c7cd..617e07878e4f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h @@ -171,6 +171,9 @@ enum ixgbe_aci_opc { ixgbe_aci_opc_done_alt_write= 0x0904, ixgbe_aci_opc_clear_port_alt_write = 0x0906, + /* TCA Events */ + ixgbe_aci_opc_temp_tca_event= 0x0C94, + /* debug commands */ ixgbe_aci_opc_debug_dump_internals = 0xFF08,
[Intel-wired-lan] [PATCH iwl-next v2 03/13] ixgbe: add E610 functions for acquiring flash data
From: Slawomir Mrozowicz Read NVM related info from the flash. Add several helper functions used to access the flash data, find memory banks, calculate offsets, calculate the flash size. Reviewed-by: Przemek Kitszel Reviewed-by: Mateusz Polchlopek Signed-off-by: Slawomir Mrozowicz Co-developed-by: Piotr Kwapulinski Signed-off-by: Piotr Kwapulinski Co-developed-by: Jedrzej Jagielski Signed-off-by: Jedrzej Jagielski --- drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 509 +- drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 1 + drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 + .../ethernet/intel/ixgbe/ixgbe_type_e610.h| 40 +- 4 files changed, 552 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c index 9ec1f4a8284b..e2121eec4f36 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c @@ -2261,6 +2261,513 @@ int ixgbe_nvm_validate_checksum(struct ixgbe_hw *hw) return err; } +/** + * ixgbe_discover_flash_size - Discover the available flash size + * @hw: pointer to the HW struct + * + * The device flash could be up to 16MB in size. However, it is possible that + * the actual size is smaller. Use bisection to determine the accessible size + * of flash memory. + * + * Return: the exit code of the operation. + */ +static int ixgbe_discover_flash_size(struct ixgbe_hw *hw) +{ + u32 min_size = 0, max_size = IXGBE_ACI_NVM_MAX_OFFSET + 1; + int err; + + err = ixgbe_acquire_nvm(hw, IXGBE_RES_READ); + if (err) + return err; + + while ((max_size - min_size) > 1) { + u32 offset = (max_size + min_size) / 2; + u32 len = 1; + u8 data; + + err = ixgbe_read_flat_nvm(hw, offset, &len, &data, false); + if (err == -EIO && + hw->aci.last_status == IXGBE_ACI_RC_EINVAL) { + err = 0; + max_size = offset; + } else if (!err) { + min_size = offset; + } else { + /* an unexpected error occurred */ + goto err_read_flat_nvm; + } + } + + hw->flash.flash_size = max_size; + +err_read_flat_nvm: + ixgbe_release_nvm(hw); + + return err; +} + +/** + * ixgbe_read_sr_base_address - Read the value of a Shadow RAM pointer word + * @hw: pointer to the HW structure + * @offset: the word offset of the Shadow RAM word to read + * @pointer: pointer value read from Shadow RAM + * + * Read the given Shadow RAM word, and convert it to a pointer value specified + * in bytes. This function assumes the specified offset is a valid pointer + * word. + * + * Each pointer word specifies whether it is stored in word size or 4KB + * sector size by using the highest bit. The reported pointer value will be in + * bytes, intended for flat NVM reads. + * + * Return: the exit code of the operation. + */ +static int ixgbe_read_sr_base_address(struct ixgbe_hw *hw, u16 offset, + u32 *pointer) +{ + u16 value; + int err; + + err = ixgbe_read_ee_aci_e610(hw, offset, &value); + if (err) + return err; + + /* Determine if the pointer is in 4KB or word units */ + if (value & IXGBE_SR_NVM_PTR_4KB_UNITS) + *pointer = (value & ~IXGBE_SR_NVM_PTR_4KB_UNITS) * SZ_4K; + else + *pointer = value * sizeof(u16); + + return 0; +} + +/** + * ixgbe_read_sr_area_size - Read an area size from a Shadow RAM word + * @hw: pointer to the HW structure + * @offset: the word offset of the Shadow RAM to read + * @size: size value read from the Shadow RAM + * + * Read the given Shadow RAM word, and convert it to an area size value + * specified in bytes. This function assumes the specified offset is a valid + * area size word. + * + * Each area size word is specified in 4KB sector units. This function reports + * the size in bytes, intended for flat NVM reads. + * + * Return: the exit code of the operation. + */ +static int ixgbe_read_sr_area_size(struct ixgbe_hw *hw, u16 offset, u32 *size) +{ + u16 value; + int err; + + err = ixgbe_read_ee_aci_e610(hw, offset, &value); + if (err) + return err; + + /* Area sizes are always specified in 4KB units */ + *size = value * SZ_4K; + + return 0; +} + +/** + * ixgbe_determine_active_flash_banks - Discover active bank for each module + * @hw: pointer to the HW struct + * + * Read the Shadow RAM control word and determine which banks are active for + * the NVM, OROM, and Netlist modules. Also read and calculate the associated + * pointer and size. These values are then cached into the ixgbe_flash_info + * structure for later use in order to calculate the correct offset to read + * from the active mo
[Intel-wired-lan] [PATCH iwl-next v2 08/13] ixgbe: extend .info_get with stored versions
Add functions reading inactive versions from the inactive flash banks. Print stored NVM, OROM and netlist versions by devlink when there is an ongoing update for E610 devices. Reviewed-by: Mateusz Polchlopek Reviewed-by: Przemek Kitszel Co-developed-by: Slawomir Mrozowicz Signed-off-by: Slawomir Mrozowicz Co-developed-by: Piotr Kwapulinski Signed-off-by: Piotr Kwapulinski Signed-off-by: Jedrzej Jagielski --- .../ethernet/intel/ixgbe/devlink/devlink.c| 180 ++ drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 59 ++ drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 3 + 3 files changed, 242 insertions(+) diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c index 69d878fe5986..bd27de229ae1 100644 --- a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c +++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c @@ -6,11 +6,16 @@ struct ixgbe_info_ctx { char buf[128]; + struct ixgbe_orom_info pending_orom; + struct ixgbe_nvm_info pending_nvm; + struct ixgbe_netlist_info pending_netlist; + struct ixgbe_hw_dev_caps dev_caps; }; enum ixgbe_devlink_version_type { IXGBE_DL_VERSION_FIXED, IXGBE_DL_VERSION_RUNNING, + IXGBE_DL_VERSION_STORED, }; static int ixgbe_devlink_info_put(struct devlink_info_req *req, @@ -25,6 +30,8 @@ static int ixgbe_devlink_info_put(struct devlink_info_req *req, return devlink_info_version_fixed_put(req, key, value); case IXGBE_DL_VERSION_RUNNING: return devlink_info_version_running_put(req, key, value); + case IXGBE_DL_VERSION_STORED: + return devlink_info_version_stored_put(req, key, value); } return 0; @@ -96,6 +103,15 @@ static void ixgbe_info_eetrack(struct ixgbe_adapter *adapter, snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", nvm_ver.etk_id); } +static void ixgbe_info_pending_eetrack(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) +{ + struct ixgbe_nvm_info *nvm = &ctx->pending_nvm; + + if (ctx->dev_caps.common_cap.nvm_update_pending_nvm) + snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", nvm->eetrack); +} + static void ixgbe_info_fw_api(struct ixgbe_adapter *adapter, struct ixgbe_info_ctx *ctx) { @@ -121,6 +137,25 @@ static void ixgbe_info_fw_srev(struct ixgbe_adapter *adapter, snprintf(ctx->buf, sizeof(ctx->buf), "%u", nvm->srev); } +static void ixgbe_info_pending_fw_srev(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) +{ + struct ixgbe_nvm_info *nvm = &ctx->pending_nvm; + + if (ctx->dev_caps.common_cap.nvm_update_pending_nvm) + snprintf(ctx->buf, sizeof(ctx->buf), "%u", nvm->srev); +} + +static void ixgbe_info_pending_orom_ver(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) +{ + struct ixgbe_orom_info *orom = &ctx->pending_orom; + + if (ctx->dev_caps.common_cap.nvm_update_pending_orom) + snprintf(ctx->buf, sizeof(ctx->buf), "%u.%u.%u", +orom->major, orom->build, orom->patch); +} + static void ixgbe_info_orom_srev(struct ixgbe_adapter *adapter, struct ixgbe_info_ctx *ctx) { @@ -129,6 +164,15 @@ static void ixgbe_info_orom_srev(struct ixgbe_adapter *adapter, snprintf(ctx->buf, sizeof(ctx->buf), "%u", orom->srev); } +static void ixgbe_info_pending_orom_srev(struct ixgbe_adapter *adapter, +struct ixgbe_info_ctx *ctx) +{ + struct ixgbe_orom_info *orom = &ctx->pending_orom; + + if (ctx->dev_caps.common_cap.nvm_update_pending_orom) + snprintf(ctx->buf, sizeof(ctx->buf), "%u", orom->srev); +} + static void ixgbe_info_nvm_ver(struct ixgbe_adapter *adapter, struct ixgbe_info_ctx *ctx) { @@ -137,6 +181,16 @@ static void ixgbe_info_nvm_ver(struct ixgbe_adapter *adapter, snprintf(ctx->buf, sizeof(ctx->buf), "%x.%02x", nvm->major, nvm->minor); } +static void ixgbe_info_pending_nvm_ver(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) +{ + struct ixgbe_nvm_info *nvm = &ctx->pending_nvm; + + if (ctx->dev_caps.common_cap.nvm_update_pending_nvm) + snprintf(ctx->buf, sizeof(ctx->buf), "%x.%02x", +nvm->major, nvm->minor); +} + static void ixgbe_info_netlist_ver(struct ixgbe_adapter *adapter, struct ixgbe_info_ctx *ctx) { @@ -149,6 +203,19 @@ static void ixgbe_info_netlist_ver(struct ixgbe_adapter *adapter, netlist->rev, netlist->cust_ver); } +static void ixgbe_info_pending_netlist_ver(struct ixgbe_adapter *adapter, +
[Intel-wired-lan] [PATCH] ixgbe: remove self assignment
Variable self assignment does not have any effect. Addresses-Coverity-ID: 1641823 ("Self assignment") Fixes: 46761fd52a886 ("ixgbe: Add support for E610 FW Admin Command Interface") Signed-off-by: Ethan Carter Edwards --- drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c index 683c668672d65535fca3b2fe6f58a9deda1188fa..6b0bce92476c3c5ec3cf7ab79864b394b592c6d4 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c @@ -145,7 +145,6 @@ static int ixgbe_aci_send_cmd_execute(struct ixgbe_hw *hw, if ((hicr & IXGBE_PF_HICR_SV)) { for (i = 0; i < IXGBE_ACI_DESC_SIZE_IN_DWORDS; i++) { raw_desc[i] = IXGBE_READ_REG(hw, IXGBE_PF_HIDA(i)); - raw_desc[i] = raw_desc[i]; } } --- base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3 change-id: 20250209-e610-self-85eac1f0e338 Best regards, -- Ethan Carter Edwards
[Intel-wired-lan] [PATCH v2] net: e1000e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
Update the driver to use the new hardware timestamping API added in commit 66f7223039c0 ("net: add NDOs for configuring hardware timestamping"). Use Netlink extack for error reporting in e1000e_hwtstamp_set. Align the indentation of net_device_ops. Signed-off-by: Piotr Wejman --- Changes in v2: - amend commit message - use extack for error reporting - rename e1000_mii_ioctl to e1000_ioctl - Link to v1: https://lore.kernel.org/netdev/20250202170839.47375-1-piotrwejma...@gmail.com/ drivers/net/ethernet/intel/e1000e/e1000.h | 2 +- drivers/net/ethernet/intel/e1000e/netdev.c | 68 ++ 2 files changed, 31 insertions(+), 39 deletions(-) diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h index ba9c19e6994c..952898151565 100644 --- a/drivers/net/ethernet/intel/e1000e/e1000.h +++ b/drivers/net/ethernet/intel/e1000e/e1000.h @@ -319,7 +319,7 @@ struct e1000_adapter { u16 tx_ring_count; u16 rx_ring_count; - struct hwtstamp_config hwtstamp_config; + struct kernel_hwtstamp_config hwtstamp_config; struct delayed_work systim_overflow_work; struct sk_buff *tx_hwtstamp_skb; unsigned long tx_hwtstamp_start; diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index 286155efcedf..43933e64819b 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -3574,6 +3574,7 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca) * e1000e_config_hwtstamp - configure the hwtstamp registers and enable/disable * @adapter: board private structure * @config: timestamp configuration + * @extack: netlink extended ACK for error report * * Outgoing time stamping can be enabled and disabled. Play nice and * disable it when requested, although it shouldn't cause any overhead @@ -3587,7 +3588,8 @@ s32 e1000e_get_base_timinca(struct e1000_adapter *adapter, u32 *timinca) * exception of "all V2 events regardless of level 2 or 4". **/ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, - struct hwtstamp_config *config) + struct kernel_hwtstamp_config *config, + struct netlink_ext_ack *extack) { struct e1000_hw *hw = &adapter->hw; u32 tsync_tx_ctl = E1000_TSYNCTXCTL_ENABLED; @@ -3598,8 +3600,10 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, bool is_l2 = false; u32 regval; - if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) + if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) { + NL_SET_ERR_MSG(extack, "No HW timestamp support\n"); return -EINVAL; + } switch (config->tx_type) { case HWTSTAMP_TX_OFF: @@ -3608,6 +3612,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, case HWTSTAMP_TX_ON: break; default: + NL_SET_ERR_MSG(extack, "Unsupported TX HW timestamp type\n"); return -ERANGE; } @@ -3681,6 +3686,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, config->rx_filter = HWTSTAMP_FILTER_ALL; break; default: + NL_SET_ERR_MSG(extack, "Unsupported RX HW timestamp filter\n"); return -ERANGE; } @@ -3693,7 +3699,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, ew32(TSYNCTXCTL, regval); if ((er32(TSYNCTXCTL) & E1000_TSYNCTXCTL_ENABLED) != (regval & E1000_TSYNCTXCTL_ENABLED)) { - e_err("Timesync Tx Control register not set as expected\n"); + NL_SET_ERR_MSG(extack, "Timesync Tx Control register not set as expected\n"); return -EAGAIN; } @@ -3706,7 +3712,7 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter, E1000_TSYNCRXCTL_TYPE_MASK)) != (regval & (E1000_TSYNCRXCTL_ENABLED | E1000_TSYNCRXCTL_TYPE_MASK))) { - e_err("Timesync Rx Control register not set as expected\n"); + NL_SET_ERR_MSG(extack, "Timesync Rx Control register not set as expected\n"); return -EAGAIN; } @@ -3932,7 +3938,7 @@ static void e1000e_systim_reset(struct e1000_adapter *adapter) spin_unlock_irqrestore(&adapter->systim_lock, flags); /* restore the previous hwtstamp configuration settings */ - e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config); + e1000e_config_hwtstamp(adapter, &adapter->hwtstamp_config, NULL); } /** @@ -6079,8 +6085,8 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu) return 0; } -static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, - int cmd
Re: [Intel-wired-lan] [PATCH iwl-next v2 02/13] ixgbe: add handler for devlink .info_get()
Mon, Feb 10, 2025 at 02:56:28PM +0100, jedrzej.jagiel...@intel.com wrote: [...] >+enum ixgbe_devlink_version_type { >+ IXGBE_DL_VERSION_FIXED, >+ IXGBE_DL_VERSION_RUNNING, >+}; >+ >+static int ixgbe_devlink_info_put(struct devlink_info_req *req, >+enum ixgbe_devlink_version_type type, >+const char *key, const char *value) I may be missing something, but what's the benefit of having this helper instead of calling directly devlink_info_version_*_put()? >+{ >+ if (!*value) >+ return 0; >+ >+ switch (type) { >+ case IXGBE_DL_VERSION_FIXED: >+ return devlink_info_version_fixed_put(req, key, value); >+ case IXGBE_DL_VERSION_RUNNING: >+ return devlink_info_version_running_put(req, key, value); >+ } >+ >+ return 0; >+} >+ [...] >+static int ixgbe_devlink_info_get(struct devlink *devlink, >+struct devlink_info_req *req, >+struct netlink_ext_ack *extack) >+{ >+ struct ixgbe_devlink_priv *devlink_private = devlink_priv(devlink); >+ struct ixgbe_adapter *adapter = devlink_private->adapter; >+ struct ixgbe_hw *hw = &adapter->hw; >+ struct ixgbe_info_ctx *ctx; >+ int err; >+ >+ ctx = kmalloc(sizeof(*ctx), GFP_KERNEL); >+ if (!ctx) >+ return -ENOMEM; >+ >+ ixgbe_info_get_dsn(adapter, ctx); >+ err = devlink_info_serial_number_put(req, ctx->buf); >+ if (err) >+ goto free_ctx; >+ >+ ixgbe_info_nvm_ver(adapter, ctx); >+ err = ixgbe_devlink_info_put(req, IXGBE_DL_VERSION_RUNNING, >+ DEVLINK_INFO_VERSION_GENERIC_FW_UNDI, >+ ctx->buf); >+ if (err) >+ goto free_ctx; >+ >+ ixgbe_info_eetrack(adapter, ctx); >+ err = ixgbe_devlink_info_put(req, IXGBE_DL_VERSION_RUNNING, >+ DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID, >+ ctx->buf); >+ if (err) >+ goto free_ctx; >+ >+ err = ixgbe_read_pba_string_generic(hw, ctx->buf, sizeof(ctx->buf)); >+ if (err) >+ goto free_ctx; >+ >+ err = ixgbe_devlink_info_put(req, IXGBE_DL_VERSION_FIXED, >+ DEVLINK_INFO_VERSION_GENERIC_BOARD_ID, >+ ctx->buf); >+free_ctx: >+ kfree(ctx); >+ return err; >+} >+ > static const struct devlink_ops ixgbe_devlink_ops = { >+ .info_get = ixgbe_devlink_info_get, > }; > > /** >-- >2.31.1 > >
Re: [Intel-wired-lan] [PATCH iwl-next v3] ixgbe: add support for thermal sensor event reception
Dear Jedrzej, Thank you for your patch. Am 10.02.25 um 11:40 schrieb Jedrzej Jagielski: E610 NICs unlike the previous devices utilising ixgbe driver are notified in the case of overheatning by the FW ACI event. overheating (without n) In event of overheat when threshold is exceeded, FW suspends all traffic and sends overtemp event to the driver. I guess this was already the behavior before your patch, and now it’s only logged, and the device stopped properly? Then driver logs appropriate message and closes the adapter instance. The card remains in that state until the platform is rebooted. As a user I’d be interested what the threshold is, and what the measured temperature is. Currently, the log seems to be just generic? drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:static const char ixgbe_overheat_msg[] = "Network adapter has been stopped because it has over heated. Restart the computer. If the problem persists, power off the system and replace the adapter"; Reviewed-by: Przemek Kitszel Reviewed-by: Mateusz Polchlopek Signed-off-by: Jedrzej Jagielski --- v2,3 : commit msg tweaks --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 + drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h | 3 +++ 2 files changed, 8 insertions(+) Kind regards, Paul diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 7236f20c9a30..5c804948dd1f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -3165,6 +3165,7 @@ static void ixgbe_aci_event_cleanup(struct ixgbe_aci_event *event) static void ixgbe_handle_fw_event(struct ixgbe_adapter *adapter) { struct ixgbe_aci_event event __cleanup(ixgbe_aci_event_cleanup); + struct net_device *netdev = adapter->netdev; struct ixgbe_hw *hw = &adapter->hw; bool pending = false; int err; @@ -3185,6 +3186,10 @@ static void ixgbe_handle_fw_event(struct ixgbe_adapter *adapter) case ixgbe_aci_opc_get_link_status: ixgbe_handle_link_status_event(adapter, &event); break; + case ixgbe_aci_opc_temp_tca_event: + e_crit(drv, "%s\n", ixgbe_overheat_msg); + ixgbe_close(netdev); + break; default: e_warn(hw, "unknown FW async event captured\n"); break; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h index 8d06ade3c7cd..617e07878e4f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h @@ -171,6 +171,9 @@ enum ixgbe_aci_opc { ixgbe_aci_opc_done_alt_write= 0x0904, ixgbe_aci_opc_clear_port_alt_write = 0x0906, + /* TCA Events */ + ixgbe_aci_opc_temp_tca_event= 0x0C94, + /* debug commands */ ixgbe_aci_opc_debug_dump_internals = 0xFF08,
Re: [Intel-wired-lan] [PATCH iwl-next v3] ixgbe: add support for thermal sensor event reception
From: Paul Menzel Sent: Monday, February 10, 2025 12:40 PM >Dear Jedrzej, > > >Thank you for your patch. > >Am 10.02.25 um 11:40 schrieb Jedrzej Jagielski: >> E610 NICs unlike the previous devices utilising ixgbe driver >> are notified in the case of overheatning by the FW ACI event. > >overheating (without n) > >> In event of overheat when threshold is exceeded, FW suspends all >> traffic and sends overtemp event to the driver. > >I guess this was already the behavior before your patch, and now it’s >only logged, and the device stopped properly? Hi Paul, Without this patch driver cannot receive the overtemp info. FW handles the event - device stops but user has no clue what has happened. FW does the major work but this patch adds this minimal piece of overtemp handling done by SW - informing user at the very end. > >> Then driver >> logs appropriate message and closes the adapter instance. >> The card remains in that state until the platform is rebooted. > >As a user I’d be interested what the threshold is, and what the measured >temperature is. Currently, the log seems to be just generic? These details are FW internals. Driver just gets info that such event has happened. There's no additional information. In that case driver's job is just to inform user that such scenario has happened and tell what should be the next steps. > > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:static const char >ixgbe_overheat_msg[] = "Network adapter has been stopped because it has >over heated. Restart the computer. If the problem persists, power off >the system and replace the adapter"; > >> Reviewed-by: Przemek Kitszel >> Reviewed-by: Mateusz Polchlopek >> Signed-off-by: Jedrzej Jagielski >> --- >> v2,3 : commit msg tweaks >> --- >> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 + >> drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h | 3 +++ >> 2 files changed, 8 insertions(+) > > >Kind regards, > >Paul > > >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> index 7236f20c9a30..5c804948dd1f 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> @@ -3165,6 +3165,7 @@ static void ixgbe_aci_event_cleanup(struct >> ixgbe_aci_event *event) >> static void ixgbe_handle_fw_event(struct ixgbe_adapter *adapter) >> { >> struct ixgbe_aci_event event __cleanup(ixgbe_aci_event_cleanup); >> +struct net_device *netdev = adapter->netdev; >> struct ixgbe_hw *hw = &adapter->hw; >> bool pending = false; >> int err; >> @@ -3185,6 +3186,10 @@ static void ixgbe_handle_fw_event(struct >> ixgbe_adapter *adapter) >> case ixgbe_aci_opc_get_link_status: >> ixgbe_handle_link_status_event(adapter, &event); >> break; >> +case ixgbe_aci_opc_temp_tca_event: >> +e_crit(drv, "%s\n", ixgbe_overheat_msg); >> +ixgbe_close(netdev); >> +break; >> default: >> e_warn(hw, "unknown FW async event captured\n"); >> break; >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h >> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h >> index 8d06ade3c7cd..617e07878e4f 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h >> @@ -171,6 +171,9 @@ enum ixgbe_aci_opc { >> ixgbe_aci_opc_done_alt_write= 0x0904, >> ixgbe_aci_opc_clear_port_alt_write = 0x0906, >> >> +/* TCA Events */ >> +ixgbe_aci_opc_temp_tca_event= 0x0C94, >> + >> /* debug commands */ >> ixgbe_aci_opc_debug_dump_internals = 0xFF08, >>
Re: [Intel-wired-lan] [PATCH iwl-next v3] ixgbe: add support for thermal sensor event reception
From: Paul Menzel Sent: Monday, February 10, 2025 1:07 PM >Dear Jedrzej, > > >Thank you for the quick reply. > > >Am 10.02.25 um 12:59 schrieb Jagielski, Jedrzej: >> From: Paul Menzel >> Sent: Monday, February 10, 2025 12:40 PM > >>> Am 10.02.25 um 11:40 schrieb Jedrzej Jagielski: E610 NICs unlike the previous devices utilising ixgbe driver are notified in the case of overheatning by the FW ACI event. >>> >>> overheating (without n) >>> In event of overheat when threshold is exceeded, FW suspends all traffic and sends overtemp event to the driver. >>> >>> I guess this was already the behavior before your patch, and now it’s >>> only logged, and the device stopped properly? > >> Without this patch driver cannot receive the overtemp info. FW handles >> the event - device stops but user has no clue what has happened. >> FW does the major work but this patch adds this minimal piece of >> overtemp handling done by SW - informing user at the very end. > >Thank you for the confirmation. Maybe add a small note, that it’s not >logged to make it crystal clear for people like myself. OK, i will extend the commit msg. > Then driver logs appropriate message and closes the adapter instance. The card remains in that state until the platform is rebooted. >>> >>> As a user I’d be interested what the threshold is, and what the measured >>> temperature is. Currently, the log seems to be just generic? >> >> These details are FW internals. >> Driver just gets info that such event has happened. >> There's no additional information. >> >> In that case driver's job is just to inform user that such scenario >> has happened and tell what should be the next steps. > > From a user perspective that is a suboptimal behavior, and shows >another time that the Linux kernel should have all the control, and >stuff like this should be moved *out* of the firmware and not into the >firmware. > >It’d be great if you could talk to the card designers/engineers to take >that into account, and maybe revert this decision for future versions or >future adapters. I will have it on my mind. Thanks, Jedrek > > >Kind regards, > >Paul > > >>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:static const char >>> ixgbe_overheat_msg[] = "Network adapter has been stopped because it has >>> over heated. Restart the computer. If the problem persists, power off the >>> system and replace the adapter"; >>> Reviewed-by: Przemek Kitszel Reviewed-by: Mateusz Polchlopek Signed-off-by: Jedrzej Jagielski --- v2,3 : commit msg tweaks --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 + drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h | 3 +++ 2 files changed, 8 insertions(+) >>> >>> >>> Kind regards, >>> >>> Paul >>> >>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 7236f20c9a30..5c804948dd1f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -3165,6 +3165,7 @@ static void ixgbe_aci_event_cleanup(struct ixgbe_aci_event *event) static void ixgbe_handle_fw_event(struct ixgbe_adapter *adapter) { struct ixgbe_aci_event event __cleanup(ixgbe_aci_event_cleanup); + struct net_device *netdev = adapter->netdev; struct ixgbe_hw *hw = &adapter->hw; bool pending = false; int err; @@ -3185,6 +3186,10 @@ static void ixgbe_handle_fw_event(struct ixgbe_adapter *adapter) case ixgbe_aci_opc_get_link_status: ixgbe_handle_link_status_event(adapter, &event); break; + case ixgbe_aci_opc_temp_tca_event: + e_crit(drv, "%s\n", ixgbe_overheat_msg); + ixgbe_close(netdev); + break; default: e_warn(hw, "unknown FW async event captured\n"); break; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h index 8d06ade3c7cd..617e07878e4f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h @@ -171,6 +171,9 @@ enum ixgbe_aci_opc { ixgbe_aci_opc_done_alt_write= 0x0904, ixgbe_aci_opc_clear_port_alt_write = 0x0906, + /* TCA Events */ + ixgbe_aci_opc_temp_tca_event= 0x0C94, + /* debug commands */ ixgbe_aci_opc_debug_dump_internals = 0xFF08,
[Intel-wired-lan] [PATCH iwl-next v2 11/13] ixgbe: add FW API version check
Add E610 specific function checking whether the FW API version is compatible with the driver expectations. The major API version should be less than or equal to the expected API version. If not the driver won't be fully operational. Check the minor version, and if it is more than two versions lesser or greater than the expected version, print a message indicating that the NVM or driver should be updated respectively. Reviewed-by: Mateusz Polchlopek Co-developed-by: Piotr Kwapulinski Signed-off-by: Piotr Kwapulinski Signed-off-by: Jedrzej Jagielski --- .../ethernet/intel/ixgbe/devlink/devlink.c| 2 ++ drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 + drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 31 +++ .../ethernet/intel/ixgbe/ixgbe_type_e610.h| 4 +++ 4 files changed, 38 insertions(+) diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c index b0396141eb9b..4ba72ec5422b 100644 --- a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c +++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c @@ -540,6 +540,8 @@ static int ixgbe_devlink_reload_empr_finish(struct devlink *devlink, *actions_performed = BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE); + adapter->flags2 &= ~IXGBE_FLAG2_API_MISMATCH; + return 0; } diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h index 0dbb2e205557..59dceb96e76a 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h @@ -671,6 +671,7 @@ struct ixgbe_adapter { #define IXGBE_FLAG2_PHY_FW_LOAD_FAILED BIT(20) #define IXGBE_FLAG2_NO_MEDIA BIT(21) #define IXGBE_FLAG2_MOD_POWER_UNSUPPORTED BIT(22) +#define IXGBE_FLAG2_API_MISMATCH BIT(23) /* Tx fast path data */ int num_tx_queues; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 4523f7d5a12e..0a4922e4e9cf 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -8361,6 +8361,31 @@ static void ixgbe_reset_subtask(struct ixgbe_adapter *adapter) rtnl_unlock(); } +static int ixgbe_check_fw_api_mismatch(struct ixgbe_adapter *adapter) +{ + struct ixgbe_hw *hw = &adapter->hw; + + if (hw->mac.type != ixgbe_mac_e610) + return 0; + + if (hw->api_maj_ver > IXGBE_FW_API_VER_MAJOR) { + e_dev_err("The driver for the device stopped because the NVM image is newer than expected. You must install the most recent version of the network driver.\n"); + + adapter->flags2 |= IXGBE_FLAG2_API_MISMATCH; + return -EOPNOTSUPP; + } else if (hw->api_maj_ver == IXGBE_FW_API_VER_MAJOR && + hw->api_min_ver > IXGBE_FW_API_VER_MINOR + IXGBE_FW_API_VER_DIFF_ALLOWED) { + e_dev_info("The driver for the device detected a newer version of the NVM image than expected. Please install the most recent version of the network driver.\n"); + adapter->flags2 |= IXGBE_FLAG2_API_MISMATCH; + } else if (hw->api_maj_ver < IXGBE_FW_API_VER_MAJOR || + hw->api_min_ver < IXGBE_FW_API_VER_MINOR - IXGBE_FW_API_VER_DIFF_ALLOWED) { + e_dev_info("The driver for the device detected an older version of the NVM image than expected. Please update the NVM image.\n"); + adapter->flags2 |= IXGBE_FLAG2_API_MISMATCH; + } + + return 0; +} + /** * ixgbe_check_fw_error - Check firmware for errors * @adapter: the adapter private structure @@ -8371,6 +8396,7 @@ static bool ixgbe_check_fw_error(struct ixgbe_adapter *adapter) { struct ixgbe_hw *hw = &adapter->hw; u32 fwsm; + int err; /* read fwsm.ext_err_ind register and log errors */ fwsm = IXGBE_READ_REG(hw, IXGBE_FWSM(hw)); @@ -8385,6 +8411,11 @@ static bool ixgbe_check_fw_error(struct ixgbe_adapter *adapter) e_dev_err("Firmware recovery mode detected. Limiting functionality. Refer to the Intel(R) Ethernet Adapters and Devices User Guide for details on firmware recovery mode.\n"); return true; } + if (!(adapter->flags2 & IXGBE_FLAG2_API_MISMATCH)) { + err = ixgbe_check_fw_api_mismatch(adapter); + if (err) + return true; + } return false; } diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h index 93d854b8a92e..4d591019dd07 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h @@ -112,6 +112,10 @@ #define IXGBE_PF_HICR_SV BIT(2) #define IXGBE_PF_HICR_EV BIT(3) +#define IXGBE_FW_API_VER_MAJOR 0x01 +#define IXGBE_FW_
[Intel-wired-lan] [PATCH iwl-next v2 10/13] ixgbe: add support for devlink reload
The E610 adapters contain an embedded chip with firmware which can be updated using devlink flash. The firmware which runs on this chip is referred to as the Embedded Management Processor firmware (EMP firmware). Activating the new firmware image currently requires that the system be rebooted. This is not ideal as rebooting the system can cause unwanted downtime. The EMP firmware itself can be reloaded by issuing a special update to the device called an Embedded Management Processor reset (EMP reset). This reset causes the device to reset and reload the EMP firmware. Implement support for devlink reload with the "fw_activate" flag. This allows user space to request the firmware be activated immediately. Reviewed-by: Mateusz Polchlopek Co-developed-by: Slawomir Mrozowicz Signed-off-by: Slawomir Mrozowicz Co-developed-by: Piotr Kwapulinski Signed-off-by: Piotr Kwapulinski Co-developed-by: Stefan Wegrzyn Signed-off-by: Stefan Wegrzyn Signed-off-by: Jedrzej Jagielski --- Documentation/networking/devlink/ixgbe.rst| 15 +++ .../ethernet/intel/ixgbe/devlink/devlink.c| 114 ++ drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 + drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 18 +++ drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 1 + .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 12 ++ .../ethernet/intel/ixgbe/ixgbe_fw_update.c| 37 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +- 8 files changed, 199 insertions(+), 7 deletions(-) diff --git a/Documentation/networking/devlink/ixgbe.rst b/Documentation/networking/devlink/ixgbe.rst index 41aedf4b8017..e5fef951c6f5 100644 --- a/Documentation/networking/devlink/ixgbe.rst +++ b/Documentation/networking/devlink/ixgbe.rst @@ -88,3 +88,18 @@ combined flash image that contains the ``fw.mgmt``, ``fw.undi``, and and device serial number. It is expected that this combination be used with an image customized for the specific device. +Reload +== + +The ``ixgbe`` driver supports activating new firmware after a flash update +using ``DEVLINK_CMD_RELOAD`` with the ``DEVLINK_RELOAD_ACTION_FW_ACTIVATE`` +action. + +.. code:: shell +$ devlink dev reload pci/:01:00.0 reload action fw_activate +The new firmware is activated by issuing a device specific Embedded +Management Processor reset which requests the device to reset and reload the +EMP firmware image. + +The driver does not currently support reloading the driver via +``DEVLINK_RELOAD_ACTION_DRIVER_REINIT``. diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c index a662a3cba33a..b0396141eb9b 100644 --- a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c +++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c @@ -390,6 +390,9 @@ static int ixgbe_devlink_info_get(struct devlink *devlink, if (!ctx) return -ENOMEM; + if (hw->mac.type == ixgbe_mac_e610) + ixgbe_refresh_fw_version(adapter); + ixgbe_info_get_dsn(adapter, ctx); err = devlink_info_serial_number_put(req, ctx->buf); if (err) @@ -432,11 +435,122 @@ static int ixgbe_devlink_info_get(struct devlink *devlink, return err; } +/** + * ixgbe_devlink_reload_empr_start - Start EMP reset to activate new firmware + * @devlink: pointer to the devlink instance to reload + * @netns_change: if true, the network namespace is changing + * @action: the action to perform. Must be DEVLINK_RELOAD_ACTION_FW_ACTIVATE + * @limit: limits on what reload should do, such as not resetting + * @extack: netlink extended ACK structure + * + * Allow user to activate new Embedded Management Processor firmware by + * issuing device specific EMP reset. Called in response to + * a DEVLINK_CMD_RELOAD with the DEVLINK_RELOAD_ACTION_FW_ACTIVATE. + * + * Note that teardown and rebuild of the driver state happens automatically as + * part of an interrupt and watchdog task. This is because all physical + * functions on the device must be able to reset when an EMP reset occurs from + * any source. + * + * Return: the exit code of the operation. + */ +static int ixgbe_devlink_reload_empr_start(struct devlink *devlink, + bool netns_change, + enum devlink_reload_action action, + enum devlink_reload_limit limit, + struct netlink_ext_ack *extack) +{ + struct ixgbe_devlink_priv *devlink_private = devlink_priv(devlink); + struct ixgbe_adapter *adapter = devlink_private->adapter; + struct ixgbe_hw *hw = &adapter->hw; + u8 pending; + int err; + + if (hw->mac.type != ixgbe_mac_e610) + return -EOPNOTSUPP; + + err = ixgbe_get_pending_updates(adapter, &pending, extack); + if (err) + return err; + + /* Pending is a bitmask of which flash banks h
[Intel-wired-lan] [PATCH iwl-next v2 07/13] ixgbe: add E610 functions getting PBA and FW ver info
Introduce 2 E610 specific callbacks implementations: -ixgbe_start_hw_e610() which expands the regular .start_hw callback with getting FW version information -ixgbe_read_pba_string_e610() which gets Product Board Assembly string Extend EEPROM ops with new .read_pba_string in order to distinguish generic one and the E610 one. Reviewed-by: Mateusz Polchlopek Co-developed-by: Stefan Wegrzyn Signed-off-by: Stefan Wegrzyn Signed-off-by: Jedrzej Jagielski --- .../ethernet/intel/ixgbe/devlink/devlink.c| 5 +- .../net/ethernet/intel/ixgbe/ixgbe_82598.c| 1 + .../net/ethernet/intel/ixgbe/ixgbe_82599.c| 1 + .../net/ethernet/intel/ixgbe/ixgbe_common.c | 1 + drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 181 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 2 + .../ethernet/intel/ixgbe/ixgbe_type_e610.h| 1 + drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c | 1 + drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 1 + 10 files changed, 190 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c index 7bc57f3f8e8e..69d878fe5986 100644 --- a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c +++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c @@ -236,10 +236,7 @@ static int ixgbe_devlink_info_get(struct devlink *devlink, if (err) goto free_ctx; - err = ixgbe_read_pba_string_generic(hw, ctx->buf, sizeof(ctx->buf)); - if (err) - goto free_ctx; - + hw->eeprom.ops.read_pba_string(hw, ctx->buf, sizeof(ctx->buf)); err = ixgbe_devlink_info_put(req, IXGBE_DL_VERSION_FIXED, DEVLINK_INFO_VERSION_GENERIC_BOARD_ID, ctx->buf); diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c index 4aaaea3b5f8f..444da982593f 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c @@ -1169,6 +1169,7 @@ static const struct ixgbe_eeprom_operations eeprom_ops_82598 = { .calc_checksum = &ixgbe_calc_eeprom_checksum_generic, .validate_checksum = &ixgbe_validate_eeprom_checksum_generic, .update_checksum= &ixgbe_update_eeprom_checksum_generic, + .read_pba_string= &ixgbe_read_pba_string_generic, }; static const struct ixgbe_phy_operations phy_ops_82598 = { diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c index 964988b4d58b..d5b1b974b4a3 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c @@ -2230,6 +2230,7 @@ static const struct ixgbe_eeprom_operations eeprom_ops_82599 = { .calc_checksum = &ixgbe_calc_eeprom_checksum_generic, .validate_checksum = &ixgbe_validate_eeprom_checksum_generic, .update_checksum= &ixgbe_update_eeprom_checksum_generic, + .read_pba_string= &ixgbe_read_pba_string_generic, }; static const struct ixgbe_phy_operations phy_ops_82599 = { diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c index 7beaf6ea57f9..5784d5d1896e 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c @@ -332,6 +332,7 @@ int ixgbe_start_hw_generic(struct ixgbe_hw *hw) * Devices in the second generation: * 82599 * X540 + * E610 **/ int ixgbe_start_hw_gen2(struct ixgbe_hw *hw) { diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c index e49e699fb141..da20071eb938 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c @@ -342,6 +342,41 @@ void ixgbe_fill_dflt_direct_cmd_desc(struct ixgbe_aci_desc *desc, u16 opcode) desc->flags = cpu_to_le16(IXGBE_ACI_FLAG_SI); } +/** + * ixgbe_aci_get_fw_ver - Get the firmware version + * @hw: pointer to the HW struct + * + * Get the firmware version using ACI command (0x0001). + * + * Return: the exit code of the operation. + */ +static int ixgbe_aci_get_fw_ver(struct ixgbe_hw *hw) +{ + struct ixgbe_aci_cmd_get_ver *resp; + struct ixgbe_aci_desc desc; + int err; + + resp = &desc.params.get_ver; + + ixgbe_fill_dflt_direct_cmd_desc(&desc, ixgbe_aci_opc_get_ver); + + err = ixgbe_aci_send_cmd(hw, &desc, NULL, 0); + + if (!err) { + hw->fw_branch = resp->fw_branch; + hw->fw_maj_ver = resp->fw_major; + hw->fw_min_ver = resp->fw_minor; + hw->fw_patch = resp->fw_patch; + hw->fw_build = le32_to_cpu(resp->fw_build); + hw->api_branch = resp->api_branch; +
[Intel-wired-lan] [PATCH iwl-next v2 09/13] ixgbe: add device flash update via devlink
Use the pldmfw library to implement device flash update for the Intel ixgbe networking device driver specifically for E610 devices. This support uses the devlink flash update interface. Using the pldmfw library, the provided firmware file will be scanned for the three major components, "fw.undi" for the Option ROM, "fw.mgmt" for the main NVM module containing the primary device firmware, and "fw.netlist" containing the netlist module. The flash is separated into two banks, the active bank containing the running firmware, and the inactive bank which we use for update. Each module is updated in a staged process. First, the inactive bank is erased, preparing the device for update. Second, the contents of the component are copied to the inactive portion of the flash. After all components are updated, the driver signals the device to switch the active bank during the next EMP reset. With this implementation, basic flash update for the E610 hardware is supported. Reviewed-by: Jacob Keller Co-developed-by: Slawomir Mrozowicz Signed-off-by: Slawomir Mrozowicz Co-developed-by: Piotr Kwapulinski Signed-off-by: Piotr Kwapulinski Co-developed-by: Stefan Wegrzyn Signed-off-by: Stefan Wegrzyn Signed-off-by: Jedrzej Jagielski --- Documentation/networking/devlink/ixgbe.rst| 24 + drivers/net/ethernet/intel/Kconfig| 1 + drivers/net/ethernet/intel/ixgbe/Makefile | 2 +- .../ethernet/intel/ixgbe/devlink/devlink.c| 4 + drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 208 ++ drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 11 + .../ethernet/intel/ixgbe/ixgbe_fw_update.c| 669 ++ .../ethernet/intel/ixgbe/ixgbe_fw_update.h| 12 + .../ethernet/intel/ixgbe/ixgbe_type_e610.h| 64 ++ 9 files changed, 994 insertions(+), 1 deletion(-) create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_fw_update.c create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_fw_update.h diff --git a/Documentation/networking/devlink/ixgbe.rst b/Documentation/networking/devlink/ixgbe.rst index a41073a62776..41aedf4b8017 100644 --- a/Documentation/networking/devlink/ixgbe.rst +++ b/Documentation/networking/devlink/ixgbe.rst @@ -64,3 +64,27 @@ The ``ixgbe`` driver reports the following versions - running - 0xee16ced7 - The first 4 bytes of the hash of the netlist module contents. + +Flash Update + +The ``ixgbe`` driver implements support for flash update using the +``devlink-flash`` interface. It supports updating the device flash using a +combined flash image that contains the ``fw.mgmt``, ``fw.undi``, and +``fw.netlist`` components. +.. list-table:: List of supported overwrite modes + :widths: 5 95 + * - Bits + - Behavior + * - ``DEVLINK_FLASH_OVERWRITE_SETTINGS`` + - Do not preserve settings stored in the flash components being + updated. This includes overwriting the port configuration that + determines the number of physical functions the device will + initialize with. + * - ``DEVLINK_FLASH_OVERWRITE_SETTINGS`` and ``DEVLINK_FLASH_OVERWRITE_IDENTIFIERS`` + - Do not preserve either settings or identifiers. Overwrite everything + in the flash with the contents from the provided image, without + performing any preservation. This includes overwriting device + identifying fields such as the MAC address, Vital product Data (VPD) area, + and device serial number. It is expected that this combination be used with an + image customized for the specific device. + diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig index 56ee58c9df21..b2adb40a5921 100644 --- a/drivers/net/ethernet/intel/Kconfig +++ b/drivers/net/ethernet/intel/Kconfig @@ -148,6 +148,7 @@ config IXGBE depends on PTP_1588_CLOCK_OPTIONAL select MDIO select NET_DEVLINK + select PLDMFW select PHYLIB help This driver supports Intel(R) 10GbE PCI Express family of diff --git a/drivers/net/ethernet/intel/ixgbe/Makefile b/drivers/net/ethernet/intel/ixgbe/Makefile index 11f37140c0a3..ce447540d146 100644 --- a/drivers/net/ethernet/intel/ixgbe/Makefile +++ b/drivers/net/ethernet/intel/ixgbe/Makefile @@ -10,7 +10,7 @@ obj-$(CONFIG_IXGBE) += ixgbe.o ixgbe-y := ixgbe_main.o ixgbe_common.o ixgbe_ethtool.o \ ixgbe_82599.o ixgbe_82598.o ixgbe_phy.o ixgbe_sriov.o \ ixgbe_mbx.o ixgbe_x540.o ixgbe_x550.o ixgbe_lib.o ixgbe_ptp.o \ - ixgbe_xsk.o ixgbe_e610.o devlink/devlink.o + ixgbe_xsk.o ixgbe_e610.o devlink/devlink.o ixgbe_fw_update.o ixgbe-$(CONFIG_IXGBE_DCB) += ixgbe_dcb.o ixgbe_dcb_82598.o \ ixgbe_dcb_82599.o ixgbe_dcb_nl.o diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c index bd27de229ae1..a662a3cba33a 100644 --- a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c +++ b/drivers/net/ethe
[Intel-wired-lan] [PATCH iwl-next v2 12/13] ixgbe: add E610 implementation of FW recovery mode
Add E610 implementation of fw_recovery_mode MAC operation. In case of E610 information about recovery mode is obtained from FW_MODES field in IXGBE_GL_MNG_FWSM register (0x000B6134). Introduce recovery specific probing flow and init only vital features. User should be able to perform NVM update using devlink once recovery mode is detected in order to load a healthy img. Reviewed-by: Mateusz Polchlopek Co-developed-by: Stefan Wegrzyn Signed-off-by: Stefan Wegrzyn Signed-off-by: Jedrzej Jagielski --- drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 17 .../ethernet/intel/ixgbe/ixgbe_fw_update.c| 15 +++- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 88 +-- .../ethernet/intel/ixgbe/ixgbe_type_e610.h| 3 + 4 files changed, 113 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c index f25cf0e7582b..c263e4e08179 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c @@ -1810,6 +1810,22 @@ void ixgbe_disable_rx_e610(struct ixgbe_hw *hw) } } +/** + * ixgbe_fw_recovery_mode_e610 - Check FW NVM recovery mode + * @hw: pointer to hardware structure + * + * Check FW NVM recovery mode by reading the value of + * the dedicated register. + * + * Return: true if FW is in recovery mode, otherwise false. + */ +static bool ixgbe_fw_recovery_mode_e610(struct ixgbe_hw *hw) +{ + u32 fwsm = IXGBE_READ_REG(hw, IXGBE_GL_MNG_FWSM); + + return !!(fwsm & IXGBE_GL_MNG_FWSM_RECOVERY_M); +} + /** * ixgbe_init_phy_ops_e610 - PHY specific init * @hw: pointer to hardware structure @@ -3872,6 +3888,7 @@ static const struct ixgbe_mac_operations mac_ops_e610 = { .reset_hw = ixgbe_reset_hw_e610, .get_media_type = ixgbe_get_media_type_e610, .setup_link = ixgbe_setup_link_e610, + .fw_recovery_mode = ixgbe_fw_recovery_mode_e610, .get_link_capabilities = ixgbe_get_link_capabilities_e610, .get_bus_info = ixgbe_get_bus_info_generic, .acquire_swfw_sync = ixgbe_acquire_swfw_sync_X540, diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fw_update.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fw_update.c index 844e40c1d747..1a542a6cab1a 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fw_update.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fw_update.c @@ -73,6 +73,8 @@ static int ixgbe_check_component_response(struct ixgbe_adapter *adapter, u8 response, u8 code, struct netlink_ext_ack *extack) { + struct ixgbe_hw *hw = &adapter->hw; + switch (response) { case IXGBE_ACI_NVM_PASS_COMP_CAN_BE_UPDATED: /* Firmware indicated this update is good to proceed. */ @@ -84,6 +86,11 @@ static int ixgbe_check_component_response(struct ixgbe_adapter *adapter, case IXGBE_ACI_NVM_PASS_COMP_CAN_NOT_BE_UPDATED: NL_SET_ERR_MSG_MOD(extack, "Firmware has rejected updating."); break; + case IXGBE_ACI_NVM_PASS_COMP_PARTIAL_CHECK: + if (hw->mac.ops.fw_recovery_mode && + hw->mac.ops.fw_recovery_mode(hw)) + return 0; + break; } switch (code) { @@ -654,7 +661,13 @@ int ixgbe_flash_pldm_image(struct devlink *devlink, return -EOPNOTSUPP; } - if (!hw->dev_caps.common_cap.nvm_unified_update) { + /* +* Cannot get caps in recovery mode, so lack of nvm_unified_update bit +* cannot lead to error +*/ + if (!hw->dev_caps.common_cap.nvm_unified_update && + (hw->mac.ops.fw_recovery_mode && +!hw->mac.ops.fw_recovery_mode(hw))) { NL_SET_ERR_MSG_MOD(extack, "Current firmware does not support unified update"); return -EOPNOTSUPP; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 0a4922e4e9cf..265770ea32bd 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -8420,6 +8420,18 @@ static bool ixgbe_check_fw_error(struct ixgbe_adapter *adapter) return false; } +static void ixgbe_recovery_service_task(struct work_struct *work) +{ + struct ixgbe_adapter *adapter = container_of(work, +struct ixgbe_adapter, +service_task); + + ixgbe_handle_fw_event(adapter); + ixgbe_service_event_complete(adapter); + + mod_timer(&adapter->service_timer, jiffies + msecs_to_jiffies(100)); +} + /** * ixgbe_service_task - manages and runs subtasks * @work: pointer to work_struct co
[Intel-wired-lan] [PATCH iwl-next v2 06/13] ixgbe: add .info_get extension specific for E610 devices
E610 devices give possibility to show more detailed info than the previous boards. Extend reporting NVM info with following pieces: fw.mgmt.api -> version number of the API fw.mgmt.build -> identifier of the source for the FW fw.psid.api -> version defining the format of the flash contents fw.netlist -> version of the netlist module fw.netlist.build -> first 4 bytes of the netlist hash Reviewed-by: Mateusz Polchlopek Co-developed-by: Slawomir Mrozowicz Signed-off-by: Slawomir Mrozowicz Co-developed-by: Piotr Kwapulinski Signed-off-by: Piotr Kwapulinski Signed-off-by: Jedrzej Jagielski --- Documentation/networking/devlink/ixgbe.rst| 26 .../ethernet/intel/ixgbe/devlink/devlink.c| 133 +- 2 files changed, 156 insertions(+), 3 deletions(-) diff --git a/Documentation/networking/devlink/ixgbe.rst b/Documentation/networking/devlink/ixgbe.rst index b63645de37e8..a41073a62776 100644 --- a/Documentation/networking/devlink/ixgbe.rst +++ b/Documentation/networking/devlink/ixgbe.rst @@ -38,3 +38,29 @@ The ``ixgbe`` driver reports the following versions - 0x8d0d - Unique identifier of the firmware image file that was loaded onto the device. Also referred to as the EETRACK identifier of the NVM. +* - ``fw.mgmt.api`` + - running + - 1.5.1 + - 3-digit version number (major.minor.patch) of the API exported over +the AdminQ by the management firmware. Used by the driver to +identify what commands are supported. Historical versions of the +kernel only displayed a 2-digit version number (major.minor). +* - ``fw.mgmt.build`` + - running + - 0x305d955f + - Unique identifier of the source for the management firmware. +* - ``fw.psid.api`` + - running + - 0.80 + - Version defining the format of the flash contents. +* - ``fw.netlist`` + - running + - 1.1.2000-6.7.0 + - The version of the netlist module. This module defines the device's +Ethernet capabilities and default settings, and is used by the +management firmware as part of managing link and device +connectivity. +* - ``fw.netlist.build`` + - running + - 0xee16ced7 + - The first 4 bytes of the hash of the netlist module contents. diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c index 9afdbfa45e67..7bc57f3f8e8e 100644 --- a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c +++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c @@ -41,14 +41,22 @@ static void ixgbe_info_get_dsn(struct ixgbe_adapter *adapter, snprintf(ctx->buf, sizeof(ctx->buf), "%8phD", dsn); } -static void ixgbe_info_nvm_ver(struct ixgbe_adapter *adapter, - struct ixgbe_info_ctx *ctx) +static void ixgbe_info_orom_ver(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) { struct ixgbe_hw *hw = &adapter->hw; struct ixgbe_nvm_version nvm_ver; ctx->buf[0] = '\0'; + if (hw->mac.type == ixgbe_mac_e610) { + struct ixgbe_orom_info *orom = &adapter->hw.flash.orom; + + snprintf(ctx->buf, sizeof(ctx->buf), "%d.%d.%d", +orom->major, orom->build, orom->patch); + return; + } + ixgbe_get_oem_prod_version(hw, &nvm_ver); if (nvm_ver.oem_valid) { snprintf(ctx->buf, sizeof(ctx->buf), "%x.%x.%x", @@ -70,6 +78,12 @@ static void ixgbe_info_eetrack(struct ixgbe_adapter *adapter, struct ixgbe_hw *hw = &adapter->hw; struct ixgbe_nvm_version nvm_ver; + if (hw->mac.type == ixgbe_mac_e610) { + snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", +hw->flash.nvm.eetrack); + return; + } + ixgbe_get_oem_prod_version(hw, &nvm_ver); /* No ETRACK version for OEM */ @@ -82,6 +96,113 @@ static void ixgbe_info_eetrack(struct ixgbe_adapter *adapter, snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", nvm_ver.etk_id); } +static void ixgbe_info_fw_api(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) +{ + struct ixgbe_hw *hw = &adapter->hw; + + snprintf(ctx->buf, sizeof(ctx->buf), "%u.%u.%u", +hw->api_maj_ver, hw->api_min_ver, hw->api_patch); +} + +static void ixgbe_info_fw_build(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) +{ + struct ixgbe_hw *hw = &adapter->hw; + + snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", hw->fw_build); +} + +static void ixgbe_info_fw_srev(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) +{ + struct ixgbe_nvm_info *nvm = &adapter->hw.flash.nvm; + + snprintf(ctx->buf, sizeof(ctx->buf), "%u", nvm->srev); +} + +static void ixgbe_info_orom_srev(struct ixgbe_ad
[Intel-wired-lan] [PATCH iwl-next v2 04/13] ixgbe: read the OROM version information
From: Slawomir Mrozowicz Add functions reading the OROM version info and use them as a part of the setting NVM info procedure. Reviewed-by: Mateusz Polchlopek Reviewed-by: Przemek Kitszel Signed-off-by: Slawomir Mrozowicz Co-developed-by: Piotr Kwapulinski Signed-off-by: Piotr Kwapulinski Signed-off-by: Jedrzej Jagielski --- drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 172 ++ .../ethernet/intel/ixgbe/ixgbe_type_e610.h| 15 ++ 2 files changed, 187 insertions(+) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c index e2121eec4f36..236c804f03c6 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c @@ -2579,6 +2579,35 @@ static int ixgbe_read_nvm_module(struct ixgbe_hw *hw, return err; } +/** + * ixgbe_read_orom_module - Read from the active Option ROM module + * @hw: pointer to the HW structure + * @bank: whether to read from active or inactive OROM module + * @offset: offset into the OROM module to read, in words + * @data: storage for returned word value + * + * Read the specified word from the active Option ROM module of the flash. + * Note that unlike the NVM module, the CSS data is stored at the end of the + * module instead of at the beginning. + * + * Return: the exit code of the operation. + */ +static int ixgbe_read_orom_module(struct ixgbe_hw *hw, + enum ixgbe_bank_select bank, + u32 offset, u16 *data) +{ + __le16 data_local; + int err; + + err = ixgbe_read_flash_module(hw, bank, IXGBE_E610_SR_1ST_OROM_BANK_PTR, + offset * sizeof(data_local), + (u8 *)&data_local, sizeof(data_local)); + if (!err) + *data = le16_to_cpu(data_local); + + return err; +} + /** * ixgbe_get_nvm_css_hdr_len - Read the CSS header length * @hw: pointer to the HW struct @@ -2675,6 +2704,143 @@ static int ixgbe_get_nvm_srev(struct ixgbe_hw *hw, return 0; } +/** + * ixgbe_get_orom_civd_data - Get the combo version information from Option ROM + * @hw: pointer to the HW struct + * @bank: whether to read from the active or inactive flash module + * @civd: storage for the Option ROM CIVD data. + * + * Searches through the Option ROM flash contents to locate the CIVD data for + * the image. + * + * Return: the exit code of the operation. + */ +static int +ixgbe_get_orom_civd_data(struct ixgbe_hw *hw, enum ixgbe_bank_select bank, +struct ixgbe_orom_civd_info *civd) +{ + struct ixgbe_orom_civd_info tmp; + u32 offset; + int err; + + /* The CIVD section is located in the Option ROM aligned to 512 bytes. +* The first 4 bytes must contain the ASCII characters "$CIV". +* A simple modulo 256 sum of all of the bytes of the structure must +* equal 0. +*/ + for (offset = 0; (offset + SZ_512) <= hw->flash.banks.orom_size; +offset += SZ_512) { + u8 sum = 0; + u32 i; + + err = ixgbe_read_flash_module(hw, bank, + IXGBE_E610_SR_1ST_OROM_BANK_PTR, + offset, + (u8 *)&tmp, sizeof(tmp)); + if (err) + return err; + + /* Skip forward until we find a matching signature */ + if (memcmp(IXGBE_OROM_CIV_SIGNATURE, tmp.signature, + sizeof(tmp.signature))) + continue; + + /* Verify that the simple checksum is zero */ + for (i = 0; i < sizeof(tmp); i++) + sum += ((u8 *)&tmp)[i]; + + if (sum) + return -EDOM; + + *civd = tmp; + return 0; + } + + return -ENODATA; +} + +/** + * ixgbe_get_orom_srev - Read the security revision from the OROM CSS header + * @hw: pointer to the HW struct + * @bank: whether to read from active or inactive flash module + * @srev: storage for security revision + * + * Read the security revision out of the CSS header of the active OROM module + * bank. + * + * Return: the exit code of the operation. + */ +static int ixgbe_get_orom_srev(struct ixgbe_hw *hw, + enum ixgbe_bank_select bank, + u32 *srev) +{ + u32 orom_size_word = hw->flash.banks.orom_size / 2; + u32 css_start, hdr_len; + u16 srev_l, srev_h; + int err; + + err = ixgbe_get_nvm_css_hdr_len(hw, bank, &hdr_len); + if (err) + return err; + + if (orom_size_word < hdr_len) + return -EINVAL; + + /* Calculate how far into the Option ROM the CSS header starts. Note +* that ixgbe_read_orom_module takes
[Intel-wired-lan] [PATCH iwl-next v2 01/13] ixgbe: add initial devlink support
Add an initial support for devlink interface to ixgbe driver. Similarly to i40e driver the implementation doesn't enable devlink to manage device-wide configuration. Devlink instance is created for each physical function of PCIe device. Create separate directory for devlink related ixgbe files and use naming scheme similar to the one used in the ice driver. Add a stub for Documentation, to be extended by further patches. Reviewed-by: Mateusz Polchlopek Signed-off-by: Jedrzej Jagielski --- v2: fix error patch in probe; minor tweaks --- Documentation/networking/devlink/index.rst| 1 + Documentation/networking/devlink/ixgbe.rst| 8 ++ drivers/net/ethernet/intel/Kconfig| 1 + drivers/net/ethernet/intel/ixgbe/Makefile | 3 +- .../ethernet/intel/ixgbe/devlink/devlink.c| 80 +++ .../ethernet/intel/ixgbe/devlink/devlink.h| 10 +++ drivers/net/ethernet/intel/ixgbe/ixgbe.h | 8 ++ drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 22 + 8 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 Documentation/networking/devlink/ixgbe.rst create mode 100644 drivers/net/ethernet/intel/ixgbe/devlink/devlink.c create mode 100644 drivers/net/ethernet/intel/ixgbe/devlink/devlink.h diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst index 948c8c44e233..8319f43b5933 100644 --- a/Documentation/networking/devlink/index.rst +++ b/Documentation/networking/devlink/index.rst @@ -84,6 +84,7 @@ parameters, info versions, and other features it supports. i40e ionic ice + ixgbe mlx4 mlx5 mlxsw diff --git a/Documentation/networking/devlink/ixgbe.rst b/Documentation/networking/devlink/ixgbe.rst new file mode 100644 index ..c04ac51c6d85 --- /dev/null +++ b/Documentation/networking/devlink/ixgbe.rst @@ -0,0 +1,8 @@ +.. SPDX-License-Identifier: GPL-2.0 + += +ixgbe devlink support += + +This document describes the devlink features implemented by the ``ixgbe`` +device driver. diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig index 1640d2f27833..56ee58c9df21 100644 --- a/drivers/net/ethernet/intel/Kconfig +++ b/drivers/net/ethernet/intel/Kconfig @@ -147,6 +147,7 @@ config IXGBE depends on PCI depends on PTP_1588_CLOCK_OPTIONAL select MDIO + select NET_DEVLINK select PHYLIB help This driver supports Intel(R) 10GbE PCI Express family of diff --git a/drivers/net/ethernet/intel/ixgbe/Makefile b/drivers/net/ethernet/intel/ixgbe/Makefile index b456d102655a..11f37140c0a3 100644 --- a/drivers/net/ethernet/intel/ixgbe/Makefile +++ b/drivers/net/ethernet/intel/ixgbe/Makefile @@ -4,12 +4,13 @@ # Makefile for the Intel(R) 10GbE PCI Express ethernet driver # +subdir-ccflags-y += -I$(src) obj-$(CONFIG_IXGBE) += ixgbe.o ixgbe-y := ixgbe_main.o ixgbe_common.o ixgbe_ethtool.o \ ixgbe_82599.o ixgbe_82598.o ixgbe_phy.o ixgbe_sriov.o \ ixgbe_mbx.o ixgbe_x540.o ixgbe_x550.o ixgbe_lib.o ixgbe_ptp.o \ - ixgbe_xsk.o ixgbe_e610.o + ixgbe_xsk.o ixgbe_e610.o devlink/devlink.o ixgbe-$(CONFIG_IXGBE_DCB) += ixgbe_dcb.o ixgbe_dcb_82598.o \ ixgbe_dcb_82599.o ixgbe_dcb_nl.o diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c new file mode 100644 index ..c052e95c9496 --- /dev/null +++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2025, Intel Corporation. */ + +#include "ixgbe.h" +#include "devlink.h" + +static const struct devlink_ops ixgbe_devlink_ops = { +}; + +/** + * ixgbe_allocate_devlink - Allocate devlink instance + * @adapter: pointer to the device adapter structure + * + * Allocate a devlink instance for this physical function. + * + * Return: 0 on success, -ENOMEM when allocation failed. + */ +int ixgbe_allocate_devlink(struct ixgbe_adapter *adapter) +{ + struct ixgbe_devlink_priv *devlink_private; + struct device *dev = &adapter->pdev->dev; + struct devlink *devlink; + + devlink = devlink_alloc(&ixgbe_devlink_ops, + sizeof(*devlink_private), dev); + if (!devlink) + return -ENOMEM; + + devlink_private = devlink_priv(devlink); + devlink_private->adapter = adapter; + + adapter->devlink = devlink; + + return 0; +} + +/** + * ixgbe_devlink_set_switch_id - Set unique switch ID based on PCI DSN + * @adapter: pointer to the device adapter structure + * @ppid: struct with switch id information + */ +static void ixgbe_devlink_set_switch_id(struct ixgbe_adapter *adapter, + struct netdev_phys_item_id *ppid) +{ + u64 id = pci_get_dsn(adapter->pdev); + + ppid->id_len = sizeof(id); + put_unalig
[Intel-wired-lan] [PATCH iwl-next v2 13/13] ixgbe: add support for FW rollback mode
From: Andrii Staikov The driver should detect whether the device entered FW rollback mode and then notify user with the dedicated message including FW and NVM versions. Even if the driver detected rollback mode, this should not result in an probe error and the normal flow proceeds. FW tries to rollback to “old” operational FW located in the inactive NVM bank in cases when newly loaded FW exhibits faulty behavior. If something goes wrong during boot the FW may switch into rollback mode in an attempt to avoid recovery mode and stay operational. After rollback is successful, the banks are swapped, and the “rollback” bank becomes the active bank for the next reset. Reviewed-by: Mateusz Polchlopek Signed-off-by: Andrii Staikov Signed-off-by: Jedrzej Jagielski --- .../ethernet/intel/ixgbe/devlink/devlink.c| 3 +- drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 + drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 34 +++ drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 26 ++ drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 3 ++ .../ethernet/intel/ixgbe/ixgbe_type_e610.h| 1 + 6 files changed, 67 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c index 4ba72ec5422b..885971447ea9 100644 --- a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c +++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c @@ -540,7 +540,8 @@ static int ixgbe_devlink_reload_empr_finish(struct devlink *devlink, *actions_performed = BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE); - adapter->flags2 &= ~IXGBE_FLAG2_API_MISMATCH; + adapter->flags2 &= ~(IXGBE_FLAG2_API_MISMATCH | +IXGBE_FLAG2_FW_ROLLBACK); return 0; } diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h index 59dceb96e76a..68f77d7b4cad 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h @@ -672,6 +672,7 @@ struct ixgbe_adapter { #define IXGBE_FLAG2_NO_MEDIA BIT(21) #define IXGBE_FLAG2_MOD_POWER_UNSUPPORTED BIT(22) #define IXGBE_FLAG2_API_MISMATCH BIT(23) +#define IXGBE_FLAG2_FW_ROLLBACKBIT(24) /* Tx fast path data */ int num_tx_queues; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c index c263e4e08179..e46696baf4a4 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c @@ -1826,6 +1826,22 @@ static bool ixgbe_fw_recovery_mode_e610(struct ixgbe_hw *hw) return !!(fwsm & IXGBE_GL_MNG_FWSM_RECOVERY_M); } +/** + * ixgbe_fw_rollback_mode_e610 - Check FW NVM rollback mode + * @hw: pointer to hardware structure + * + * Check FW NVM rollback mode by reading the value of + * the dedicated register. + * + * Return: true if FW is in rollback mode, otherwise false. + */ +static bool ixgbe_fw_rollback_mode_e610(struct ixgbe_hw *hw) +{ + u32 fwsm = IXGBE_READ_REG(hw, IXGBE_GL_MNG_FWSM); + + return !!(fwsm & IXGBE_GL_MNG_FWSM_ROLLBACK_M); +} + /** * ixgbe_init_phy_ops_e610 - PHY specific init * @hw: pointer to hardware structure @@ -3158,6 +3174,21 @@ int ixgbe_get_inactive_nvm_ver(struct ixgbe_hw *hw, struct ixgbe_nvm_info *nvm) return ixgbe_get_nvm_ver_info(hw, IXGBE_INACTIVE_FLASH_BANK, nvm); } +/** + * ixgbe_get_active_nvm_ver - Read Option ROM version from the active bank + * @hw: pointer to the HW structure + * @nvm: storage for Option ROM version information + * + * Reads the NVM EETRACK ID, Map version, and security revision of the + * active NVM bank. + * + * Return: the exit code of the operation. + */ +static int ixgbe_get_active_nvm_ver(struct ixgbe_hw *hw, struct ixgbe_nvm_info *nvm) +{ + return ixgbe_get_nvm_ver_info(hw, IXGBE_ACTIVE_FLASH_BANK, nvm); +} + /** * ixgbe_get_netlist_info - Read the netlist version information * @hw: pointer to the HW struct @@ -3889,6 +3920,9 @@ static const struct ixgbe_mac_operations mac_ops_e610 = { .get_media_type = ixgbe_get_media_type_e610, .setup_link = ixgbe_setup_link_e610, .fw_recovery_mode = ixgbe_fw_recovery_mode_e610, + .fw_rollback_mode = ixgbe_fw_rollback_mode_e610, + .get_fw_ver = ixgbe_aci_get_fw_ver, + .get_nvm_ver= ixgbe_get_active_nvm_ver, .get_link_capabilities = ixgbe_get_link_capabilities_e610, .get_bus_info = ixgbe_get_bus_info_generic, .acquire_swfw_sync = ixgbe_acquire_swfw_sync_X540, diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 265770ea32bd..dfb4c85f7a02 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +
[Intel-wired-lan] [PATCH iwl-next v2 02/13] ixgbe: add handler for devlink .info_get()
Provide devlink .info_get() callback implementation to allow the driver to report detailed version information. The following info is reported: "serial_number" -> The PCI DSN of the adapter "fw.bundle_id" -> Unique identifier for the combined flash image "fw.undi" -> Version of the Option ROM containing the UEFI driver "board.id" -> The PBA ID string Reviewed-by: Mateusz Polchlopek Signed-off-by: Jedrzej Jagielski --- v2: zero the ctx buff when chance it won't be filled out --- Documentation/networking/devlink/ixgbe.rst| 32 + .../ethernet/intel/ixgbe/devlink/devlink.c| 124 ++ 2 files changed, 156 insertions(+) diff --git a/Documentation/networking/devlink/ixgbe.rst b/Documentation/networking/devlink/ixgbe.rst index c04ac51c6d85..b63645de37e8 100644 --- a/Documentation/networking/devlink/ixgbe.rst +++ b/Documentation/networking/devlink/ixgbe.rst @@ -6,3 +6,35 @@ ixgbe devlink support This document describes the devlink features implemented by the ``ixgbe`` device driver. + +Info versions += + +The ``ixgbe`` driver reports the following versions + +.. list-table:: devlink info versions implemented +:widths: 5 5 5 90 + +* - Name + - Type + - Example + - Description +* - ``board.id`` + - fixed + - H49289-000 + - The Product Board Assembly (PBA) identifier of the board. +* - ``fw.undi`` + - running + - 1.1937.0 + - Version of the Option ROM containing the UEFI driver. The version is +reported in ``major.minor.patch`` format. The major version is +incremented whenever a major breaking change occurs, or when the +minor version would overflow. The minor version is incremented for +non-breaking changes and reset to 1 when the major version is +incremented. The patch version is normally 0 but is incremented when +a fix is delivered as a patch against an older base Option ROM. +* - ``fw.bundle_id`` + - running + - 0x8d0d + - Unique identifier of the firmware image file that was loaded onto +the device. Also referred to as the EETRACK identifier of the NVM. diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c index c052e95c9496..9afdbfa45e67 100644 --- a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c +++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c @@ -4,7 +4,131 @@ #include "ixgbe.h" #include "devlink.h" +struct ixgbe_info_ctx { + char buf[128]; +}; + +enum ixgbe_devlink_version_type { + IXGBE_DL_VERSION_FIXED, + IXGBE_DL_VERSION_RUNNING, +}; + +static int ixgbe_devlink_info_put(struct devlink_info_req *req, + enum ixgbe_devlink_version_type type, + const char *key, const char *value) +{ + if (!*value) + return 0; + + switch (type) { + case IXGBE_DL_VERSION_FIXED: + return devlink_info_version_fixed_put(req, key, value); + case IXGBE_DL_VERSION_RUNNING: + return devlink_info_version_running_put(req, key, value); + } + + return 0; +} + +static void ixgbe_info_get_dsn(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) +{ + u8 dsn[8]; + + /* Copy the DSN into an array in Big Endian format */ + put_unaligned_be64(pci_get_dsn(adapter->pdev), dsn); + + snprintf(ctx->buf, sizeof(ctx->buf), "%8phD", dsn); +} + +static void ixgbe_info_nvm_ver(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) +{ + struct ixgbe_hw *hw = &adapter->hw; + struct ixgbe_nvm_version nvm_ver; + + ctx->buf[0] = '\0'; + + ixgbe_get_oem_prod_version(hw, &nvm_ver); + if (nvm_ver.oem_valid) { + snprintf(ctx->buf, sizeof(ctx->buf), "%x.%x.%x", +nvm_ver.oem_major, nvm_ver.oem_minor, +nvm_ver.oem_release); + + return; + } + + ixgbe_get_orom_version(hw, &nvm_ver); + if (nvm_ver.or_valid) + snprintf(ctx->buf, sizeof(ctx->buf), "%d.%d.%d", +nvm_ver.or_major, nvm_ver.or_build, nvm_ver.or_patch); +} + +static void ixgbe_info_eetrack(struct ixgbe_adapter *adapter, + struct ixgbe_info_ctx *ctx) +{ + struct ixgbe_hw *hw = &adapter->hw; + struct ixgbe_nvm_version nvm_ver; + + ixgbe_get_oem_prod_version(hw, &nvm_ver); + + /* No ETRACK version for OEM */ + if (nvm_ver.oem_valid) { + ctx->buf[0] = '\0'; + return; + } + + ixgbe_get_etk_id(hw, &nvm_ver); + snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", nvm_ver.etk_id); +} + +static int ixgbe_devlink_info_get(struct devlink *devlink, + struct devlink_info_req *req, +
[Intel-wired-lan] [PATCH iwl-next v2 05/13] ixgbe: read the netlist version information
From: Slawomir Mrozowicz Add functions reading the netlist version info and use them as a part of the setting NVM info procedure. Reviewed-by: Mateusz Polchlopek Signed-off-by: Slawomir Mrozowicz Co-developed-by: Piotr Kwapulinski Signed-off-by: Piotr Kwapulinski Signed-off-by: Jedrzej Jagielski --- drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 112 ++ .../ethernet/intel/ixgbe/ixgbe_type_e610.h| 33 ++ 2 files changed, 145 insertions(+) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c index 236c804f03c6..e49e699fb141 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c @@ -2579,6 +2579,33 @@ static int ixgbe_read_nvm_module(struct ixgbe_hw *hw, return err; } +/** + * ixgbe_read_netlist_module - Read data from the netlist module area + * @hw: pointer to the HW structure + * @bank: whether to read from the active or inactive module + * @offset: offset into the netlist to read from + * @data: storage for returned word value + * + * Read a word from the specified netlist bank. + * + * Return: the exit code of the operation. + */ +static int ixgbe_read_netlist_module(struct ixgbe_hw *hw, +enum ixgbe_bank_select bank, +u32 offset, u16 *data) +{ + __le16 data_local; + int err; + + err = ixgbe_read_flash_module(hw, bank, IXGBE_E610_SR_NETLIST_BANK_PTR, + offset * sizeof(data_local), + (u8 *)&data_local, sizeof(data_local)); + if (!err) + *data = le16_to_cpu(data_local); + + return err; +} + /** * ixgbe_read_orom_module - Read from the active Option ROM module * @hw: pointer to the HW structure @@ -2884,6 +2911,86 @@ static int ixgbe_get_nvm_ver_info(struct ixgbe_hw *hw, return 0; } +/** + * ixgbe_get_netlist_info - Read the netlist version information + * @hw: pointer to the HW struct + * @bank: whether to read from the active or inactive flash bank + * @netlist: pointer to netlist version info structure + * + * Get the netlist version information from the requested bank. Reads the Link + * Topology section to find the Netlist ID block and extract the relevant + * information into the netlist version structure. + * + * Return: the exit code of the operation. + */ +static int ixgbe_get_netlist_info(struct ixgbe_hw *hw, + enum ixgbe_bank_select bank, + struct ixgbe_netlist_info *netlist) +{ + u16 module_id, length, node_count, i; + u16 *id_blk; + int err; + + err = ixgbe_read_netlist_module(hw, bank, IXGBE_NETLIST_TYPE_OFFSET, + &module_id); + if (err) + return err; + + if (module_id != IXGBE_NETLIST_LINK_TOPO_MOD_ID) + return -EIO; + + err = ixgbe_read_netlist_module(hw, bank, IXGBE_LINK_TOPO_MODULE_LEN, + &length); + if (err) + return err; + + /* Sanity check that we have at least enough words to store the +* netlist ID block. +*/ + if (length < IXGBE_NETLIST_ID_BLK_SIZE) + return -EIO; + + err = ixgbe_read_netlist_module(hw, bank, IXGBE_LINK_TOPO_NODE_COUNT, + &node_count); + if (err) + return err; + + node_count &= IXGBE_LINK_TOPO_NODE_COUNT_M; + + id_blk = kcalloc(IXGBE_NETLIST_ID_BLK_SIZE, sizeof(*id_blk), GFP_KERNEL); + if (!id_blk) + return -ENOMEM; + + /* Read out the entire Netlist ID Block at once. */ + err = ixgbe_read_flash_module(hw, bank, IXGBE_E610_SR_NETLIST_BANK_PTR, + IXGBE_NETLIST_ID_BLK_OFFSET(node_count) * + sizeof(*id_blk), (u8 *)id_blk, + IXGBE_NETLIST_ID_BLK_SIZE * + sizeof(*id_blk)); + if (err) + goto free_id_blk; + + for (i = 0; i < IXGBE_NETLIST_ID_BLK_SIZE; i++) + id_blk[i] = le16_to_cpu(((__le16 *)id_blk)[i]); + + netlist->major = id_blk[IXGBE_NETLIST_ID_BLK_MAJOR_VER_HIGH] << 16 | +id_blk[IXGBE_NETLIST_ID_BLK_MAJOR_VER_LOW]; + netlist->minor = id_blk[IXGBE_NETLIST_ID_BLK_MINOR_VER_HIGH] << 16 | +id_blk[IXGBE_NETLIST_ID_BLK_MINOR_VER_LOW]; + netlist->type = id_blk[IXGBE_NETLIST_ID_BLK_TYPE_HIGH] << 16 | + id_blk[IXGBE_NETLIST_ID_BLK_TYPE_LOW]; + netlist->rev = id_blk[IXGBE_NETLIST_ID_BLK_REV_HIGH] << 16 | + id_blk[IXGBE_NETLIST_ID_BLK_REV_LOW]; + netlist->cust_ver = id_blk[IXGBE_NETLIST_ID_BLK_CUST_VER]; + /* Read the left most 4 bytes of SHA */
[Intel-wired-lan] [PATCH iwl-next v2 00/13] ixgbe: Add basic devlink support
Create devlink specific directory for more convenient future feature development. Flashing and reloading are supported only by E610 devices. Introduce basic FW/NVM validation since devlink reload introduces possibility of runtime NVM update. Check FW API version, FW recovery mode and FW rollback mode. Introduce minimal recovery probe to let user to reload the faulty FW when recovery mode is detected. This series is based on the series introducing initial E610 device support: https://lore.kernel.org/intel-wired-lan/20241205084450.4651-1-piotr.kwapulin...@intel.com/ Andrii Staikov (1): ixgbe: add support for FW rollback mode Jedrzej Jagielski (9): ixgbe: add initial devlink support ixgbe: add handler for devlink .info_get() ixgbe: add .info_get extension specific for E610 devices ixgbe: add E610 functions getting PBA and FW ver info ixgbe: extend .info_get with stored versions ixgbe: add device flash update via devlink ixgbe: add support for devlink reload ixgbe: add FW API version check ixgbe: add E610 implementation of FW recovery mode Slawomir Mrozowicz (3): ixgbe: add E610 functions for acquiring flash data ixgbe: read the OROM version information ixgbe: read the netlist version information Documentation/networking/devlink/index.rst|1 + Documentation/networking/devlink/ixgbe.rst| 105 ++ drivers/net/ethernet/intel/Kconfig|2 + drivers/net/ethernet/intel/ixgbe/Makefile |3 +- .../ethernet/intel/ixgbe/devlink/devlink.c| 629 +++ .../ethernet/intel/ixgbe/devlink/devlink.h| 10 + drivers/net/ethernet/intel/ixgbe/ixgbe.h | 14 + .../net/ethernet/intel/ixgbe/ixgbe_82598.c|1 + .../net/ethernet/intel/ixgbe/ixgbe_82599.c|1 + .../net/ethernet/intel/ixgbe/ixgbe_common.c |1 + drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 1510 +++-- drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 16 + .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 12 + .../ethernet/intel/ixgbe/ixgbe_fw_update.c| 709 .../ethernet/intel/ixgbe/ixgbe_fw_update.h| 12 + drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 178 +- drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |5 + .../ethernet/intel/ixgbe/ixgbe_type_e610.h| 161 +- drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c |1 + drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c |1 + 20 files changed, 3256 insertions(+), 116 deletions(-) create mode 100644 Documentation/networking/devlink/ixgbe.rst create mode 100644 drivers/net/ethernet/intel/ixgbe/devlink/devlink.c create mode 100644 drivers/net/ethernet/intel/ixgbe/devlink/devlink.h create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_fw_update.c create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_fw_update.h base-commit: 09a7ccb316bce8347fefad05809426526b6699f3 -- 2.31.1
[Intel-wired-lan] [PATCH iwl-next v2 1/3] ice: rename ice_ptp_init_phc_eth56g function
From: Karol Kolacinski Refactor the code by changing ice_ptp_init_phc_eth56g function name to ice_ptp_init_phc_e825, to be consistent with the naming pattern for other devices. Signed-off-by: Karol Kolacinski Signed-off-by: Grzegorz Nitka --- drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c index 3e824f7b30c0..fbaf2819e40e 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c @@ -2650,18 +2650,17 @@ static void ice_sb_access_ena_eth56g(struct ice_hw *hw, bool enable) } /** - * ice_ptp_init_phc_eth56g - Perform E82X specific PHC initialization + * ice_ptp_init_phc_e825 - Perform E825 specific PHC initialization * @hw: pointer to HW struct * - * Perform PHC initialization steps specific to E82X devices. + * Perform E825-specific PTP hardware clock initialization steps. * - * Return: - * * %0 - success - * * %other - failed to initialize CGU + * Return: 0 on success, negative error code otherwise. */ -static int ice_ptp_init_phc_eth56g(struct ice_hw *hw) +static int ice_ptp_init_phc_e825(struct ice_hw *hw) { ice_sb_access_ena_eth56g(hw, true); + /* Initialize the Clock Generation Unit */ return ice_init_cgu_e82x(hw); } @@ -6123,7 +6122,7 @@ int ice_ptp_init_phc(struct ice_hw *hw) case ICE_MAC_GENERIC: return ice_ptp_init_phc_e82x(hw); case ICE_MAC_GENERIC_3K_E825: - return ice_ptp_init_phc_eth56g(hw); + return ice_ptp_init_phc_e825(hw); default: return -EOPNOTSUPP; } -- 2.39.3
[Intel-wired-lan] [PATCH iwl-next v2 0/3] E825C PTP cleanup
This patch series simplifies PTP code related to E825C products by simplifying PHY register info definition. Cleanup the code by removing unused register definitions. v1->v2: * remove sync delay adding from the series (patch 1/3). To be submitted as separate patch. * fix kdoc (patch 2/3) in ice_phy_reg_info_eth56g struct Karol Kolacinski (3): ice: rename ice_ptp_init_phc_eth56g function ice: Refactor E825C PHY registers info struct ice: E825C PHY register cleanup .../net/ethernet/intel/ice/ice_ptp_consts.h | 75 --- drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 19 +++-- drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 35 - 3 files changed, 40 insertions(+), 89 deletions(-) base-commit: 820e145d30facd90981914efefddb82c9786c963 -- 2.39.3
[Intel-wired-lan] [PATCH iwl-next v2 2/3] ice: Refactor E825C PHY registers info struct
From: Karol Kolacinski Simplify ice_phy_reg_info_eth56g struct definition to include base address for the very first quad. Use base address info and 'step' value to determine address for specific PHY quad. Reviewed-by: Przemek Kitszel Signed-off-by: Karol Kolacinski Signed-off-by: Grzegorz Nitka --- .../net/ethernet/intel/ice/ice_ptp_consts.h | 75 --- drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 6 +- drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 4 +- 3 files changed, 20 insertions(+), 65 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h index ac46d1183300..003cdfada3ca 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h @@ -10,70 +10,25 @@ /* Constants defined for the PTP 1588 clock hardware. */ const struct ice_phy_reg_info_eth56g eth56g_phy_res[NUM_ETH56G_PHY_RES] = { - /* ETH56G_PHY_REG_PTP */ - { - /* base_addr */ - { - 0x092000, - 0x126000, - 0x1BA000, - 0x24E000, - 0x2E2000, - }, - /* step */ - 0x98, + [ETH56G_PHY_REG_PTP] = { + .base_addr = 0x092000, + .step = 0x98, }, - /* ETH56G_PHY_MEM_PTP */ - { - /* base_addr */ - { - 0x093000, - 0x127000, - 0x1BB000, - 0x24F000, - 0x2E3000, - }, - /* step */ - 0x200, + [ETH56G_PHY_MEM_PTP] = { + .base_addr = 0x093000, + .step = 0x200, }, - /* ETH56G_PHY_REG_XPCS */ - { - /* base_addr */ - { - 0x00, - 0x009400, - 0x128000, - 0x1BC000, - 0x25, - }, - /* step */ - 0x21000, + [ETH56G_PHY_REG_XPCS] = { + .base_addr = 0x00, + .step = 0x21000, }, - /* ETH56G_PHY_REG_MAC */ - { - /* base_addr */ - { - 0x085000, - 0x119000, - 0x1AD000, - 0x241000, - 0x2D5000, - }, - /* step */ - 0x1000, + [ETH56G_PHY_REG_MAC] = { + .base_addr = 0x085000, + .step = 0x1000, }, - /* ETH56G_PHY_REG_GPCS */ - { - /* base_addr */ - { - 0x084000, - 0x118000, - 0x1AC000, - 0x24, - 0x2D4000, - }, - /* step */ - 0x400, + [ETH56G_PHY_REG_GPCS] = { + .base_addr = 0x084000, + .step = 0x400, }, }; diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c index fbaf2819e40e..89bb8461284a 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c @@ -1010,7 +1010,7 @@ static int ice_phy_res_address_eth56g(struct ice_hw *hw, u8 lane, /* Lanes 4..7 are in fact 0..3 on a second PHY */ lane %= hw->ptp.ports_per_phy; - *addr = eth56g_phy_res[res_type].base[0] + + *addr = eth56g_phy_res[res_type].base_addr + lane * eth56g_phy_res[res_type].step + offset; return 0; @@ -1240,7 +1240,7 @@ static int ice_write_quad_ptp_reg_eth56g(struct ice_hw *hw, u8 port, if (port >= hw->ptp.num_lports) return -EIO; - addr = eth56g_phy_res[ETH56G_PHY_REG_PTP].base[0] + offset; + addr = eth56g_phy_res[ETH56G_PHY_REG_PTP].base_addr + offset; return ice_write_phy_eth56g(hw, port, addr, val); } @@ -1265,7 +1265,7 @@ static int ice_read_quad_ptp_reg_eth56g(struct ice_hw *hw, u8 port, if (port >= hw->ptp.num_lports) return -EIO; - addr = eth56g_phy_res[ETH56G_PHY_REG_PTP].base[0] + offset; + addr = eth56g_phy_res[ETH56G_PHY_REG_PTP].base_addr + offset; return ice_read_phy_eth56g(hw, port, addr, val); } diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h index 8442d1d60351..cca81391b6ad 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h @@ -65,14 +65,14 @@ enum ice_eth56g_link_spd { /** * struct ice_phy_reg_info_eth56g - ETH56G PHY register parameters - * @base: base address for each PHY block + * @base_addr: base address for each PHY
[Intel-wired-lan] [PATCH iwl-next v2 3/3] ice: E825C PHY register cleanup
From: Karol Kolacinski Minor PTP register refactor, including logical grouping E825C 1-step timestamping registers. Remove unused register definitions (PHY_REG_GPCS_BITSLIP, PHY_REG_REVISION). Also, apply preferred GENMASK macro (instead of ICE_M) for register fields definition affected by this patch. Reviewed-by: Simon Horman Reviewed-by: Przemek Kitszel Signed-off-by: Karol Kolacinski Signed-off-by: Grzegorz Nitka --- drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 31 ++--- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h index cca81391b6ad..e5925ccc2613 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h @@ -780,36 +780,19 @@ static inline bool ice_is_dual(struct ice_hw *hw) #define PHY_MAC_XIF_TS_SFD_ENA_M ICE_M(0x1, 20) #define PHY_MAC_XIF_GMII_TS_SEL_M ICE_M(0x1, 21) -/* GPCS config register */ -#define PHY_GPCS_CONFIG_REG0 0x268 -#define PHY_GPCS_CONFIG_REG0_TX_THR_M ICE_M(0xF, 24) -#define PHY_GPCS_BITSLIP 0x5C - #define PHY_TS_INT_CONFIG_THRESHOLD_M ICE_M(0x3F, 0) #define PHY_TS_INT_CONFIG_ENA_MBIT(6) -/* 1-step PTP config */ -#define PHY_PTP_1STEP_CONFIG 0x270 -#define PHY_PTP_1STEP_T1S_UP64_M ICE_M(0xF, 4) -#define PHY_PTP_1STEP_T1S_DELTA_M ICE_M(0xF, 8) -#define PHY_PTP_1STEP_PEER_DELAY(_port)(0x274 + 4 * (_port)) -#define PHY_PTP_1STEP_PD_ADD_PD_M ICE_M(0x1, 0) -#define PHY_PTP_1STEP_PD_DELAY_M ICE_M(0x3fff, 1) -#define PHY_PTP_1STEP_PD_DLY_V_M ICE_M(0x1, 31) - /* Macros to derive offsets for TimeStampLow and TimeStampHigh */ #define PHY_TSTAMP_L(x) (((x) * 8) + 0) #define PHY_TSTAMP_U(x) (((x) * 8) + 4) -#define PHY_REG_REVISION 0x85000 - #define PHY_REG_DESKEW_0 0x94 #define PHY_REG_DESKEW_0_RLEVELGENMASK(6, 0) #define PHY_REG_DESKEW_0_RLEVEL_FRAC GENMASK(9, 7) #define PHY_REG_DESKEW_0_RLEVEL_FRAC_W 3 #define PHY_REG_DESKEW_0_VALID GENMASK(10, 10) -#define PHY_REG_GPCS_BITSLIP 0x5C #define PHY_REG_SD_BIT_SLIP(_port_offset) (0x29C + 4 * (_port_offset)) #define PHY_REVISION_ETH56G0x10200 #define PHY_VENDOR_TXLANE_THRESH 0x2000C @@ -829,7 +812,21 @@ static inline bool ice_is_dual(struct ice_hw *hw) #define PHY_MAC_BLOCKTIME 0x50 #define PHY_MAC_MARKERTIME 0x54 #define PHY_MAC_TX_OFFSET 0x58 +#define PHY_GPCS_BITSLIP 0x5C #define PHY_PTP_INT_STATUS 0x7FD140 +/* ETH56G registers shared per quad */ +/* GPCS config register */ +#define PHY_GPCS_CONFIG_REG0 0x268 +#define PHY_GPCS_CONFIG_REG0_TX_THR_M GENMASK(27, 24) +/* 1-step PTP config */ +#define PHY_PTP_1STEP_CONFIG 0x270 +#define PHY_PTP_1STEP_T1S_UP64_M GENMASK(7, 4) +#define PHY_PTP_1STEP_T1S_DELTA_M GENMASK(11, 8) +#define PHY_PTP_1STEP_PEER_DELAY(_quad_lane) (0x274 + 4 * (_quad_lane)) +#define PHY_PTP_1STEP_PD_ADD_PD_M BIT(0) +#define PHY_PTP_1STEP_PD_DELAY_M GENMASK(30, 1) +#define PHY_PTP_1STEP_PD_DLY_V_M BIT(31) + #endif /* _ICE_PTP_HW_H_ */ -- 2.39.3
Re: [Intel-wired-lan] [PATCH iwl-next v3] ixgbe: add support for thermal sensor event reception
> > > > Then driver > > > > logs appropriate message and closes the adapter instance. > > > > The card remains in that state until the platform is rebooted. > > > > > > As a user I’d be interested what the threshold is, and what the measured > > > temperature is. Currently, the log seems to be just generic? > > > > These details are FW internals. > > Driver just gets info that such event has happened. > > There's no additional information. > > > > In that case driver's job is just to inform user that such scenario > > has happened and tell what should be the next steps. > > From a user perspective that is a suboptimal behavior, and shows another > time that the Linux kernel should have all the control, and stuff like this > should be moved *out* of the firmware and not into the firmware. The older generations of hardware driven by this driver actually have HWMON support for the temperature sensor. I can understand the hardware protecting itself, and shutting down, but i agree with you, all the infrastructure already exists to report the temperature so why drop it? That actually seems like more work, and makes the device less friendly. Andrew
Re: [Intel-wired-lan] [PATCH v2] net: e1000e: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
On Sat, 8 Feb 2025 16:43:49 +0100 Piotr Wejman wrote: > - if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) > + if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP)) { > + NL_SET_ERR_MSG(extack, "No HW timestamp support\n"); No new lines at the end of extack messages, please. User space adds its own line breaks. -- pw-bot: cr
Re: [Intel-wired-lan] [PATCH iwl-net v2] idpf: call set_real_num_queues in idpf_open
From: Joshua Hay Date: Tue, 4 Feb 2025 18:08:11 -0800 > On initial driver load, alloc_etherdev_mqs is called with whatever max > queue values are provided by the control plane. However, if the driver > is loaded on a system where num_online_cpus() returns less than the max > queues, the netdev will think there are more queues than are actually > available. Only num_online_cpus() will be allocated, but > skb_get_queue_mapping(skb) could possibly return an index beyond the > range of allocated queues. Consequently, the packet is silently dropped > and it appears as if TX is broken. > > Set the real number of queues during open so the netdev knows how many > queues will be allocated. > > v2: > - call set_real_num_queues in idpf_open. Previous change called > set_real_num_queues function in idpf_up_complete, but it is possible > for up_complete to be called without holding the RTNL lock. If user > brings up interface, then issues a reset, the init_task will call > idpf_vport_open->idpf_up_complete. Since this is initiated by the > driver, the RTNL lock is not taken. > - adjust title to reflect new changes. > > Signed-off-by: Joshua Hay > Fixes: 1c325aac10a8 ("idpf: configure resources for TX queues") > Reviewed-by: Madhu Chittim > --- > drivers/net/ethernet/intel/idpf/idpf_lib.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c > b/drivers/net/ethernet/intel/idpf/idpf_lib.c > index 6df7f125ebde..9dc806411002 100644 > --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c > +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c > @@ -2159,8 +2159,13 @@ static int idpf_open(struct net_device *netdev) > idpf_vport_ctrl_lock(netdev); > vport = idpf_netdev_to_vport(netdev); > > + err = idpf_set_real_num_queues(vport); > + if (err) > + goto unlock; Why wasn't it removed from the soft reset flow now, although you did that in v1? > + > err = idpf_vport_open(vport); > > +unlock: > idpf_vport_ctrl_unlock(netdev); > > return err; Thanks, Olek
Re: [Intel-wired-lan] [PATCH 1/3] igb: Link IRQs to NAPI instances
On Mon, Feb 10, 2025 at 10:19:35AM +0100, Kurt Kanzenbach wrote: > Link IRQs to NAPI instances via netdev-genl API. This allows users to query > that information via netlink: > > |$ ./tools/net/ynl/pyynl/cli.py --spec > Documentation/netlink/specs/netdev.yaml \ > | --dump napi-get --json='{"ifindex": 2}' > |[{'defer-hard-irqs': 0, > | 'gro-flush-timeout': 0, > | 'id': 8204, > | 'ifindex': 2, > | 'irq': 127, > | 'irq-suspend-timeout': 0}, > | {'defer-hard-irqs': 0, > | 'gro-flush-timeout': 0, > | 'id': 8203, > | 'ifindex': 2, > | 'irq': 126, > | 'irq-suspend-timeout': 0}, > | {'defer-hard-irqs': 0, > | 'gro-flush-timeout': 0, > | 'id': 8202, > | 'ifindex': 2, > | 'irq': 125, > | 'irq-suspend-timeout': 0}, > | {'defer-hard-irqs': 0, > | 'gro-flush-timeout': 0, > | 'id': 8201, > | 'ifindex': 2, > | 'irq': 124, > | 'irq-suspend-timeout': 0}] > |$ cat /proc/interrupts | grep enp2s0 > |123: 0 1 IR-PCI-MSIX-:02:00.0 0-edge enp2s0 > |124: 0 7 IR-PCI-MSIX-:02:00.0 1-edge > enp2s0-TxRx-0 > |125: 0 0 IR-PCI-MSIX-:02:00.0 2-edge > enp2s0-TxRx-1 > |126: 0 5 IR-PCI-MSIX-:02:00.0 3-edge > enp2s0-TxRx-2 > |127: 0 0 IR-PCI-MSIX-:02:00.0 4-edge > enp2s0-TxRx-3 > > Signed-off-by: Kurt Kanzenbach > --- > drivers/net/ethernet/intel/igb/igb_main.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c > b/drivers/net/ethernet/intel/igb/igb_main.c > index > d368b753a4675d01b5dfa50dee4cd218e6a5e14b..d4128d19cc08f62f95682069bb5ed9b8bbbf10cb > 100644 > --- a/drivers/net/ethernet/intel/igb/igb_main.c > +++ b/drivers/net/ethernet/intel/igb/igb_main.c > @@ -947,6 +947,9 @@ static int igb_request_msix(struct igb_adapter *adapter) > q_vector); > if (err) > goto err_free; > + > + netif_napi_set_irq(&q_vector->napi, > +adapter->msix_entries[vector].vector); > } As far as I can tell, all paths that lead here hold RTNL: - power management (__igb_resume) - ethtool set_channels (igb_reinit_queues) - and regular ndo_open So: Reviewed-by: Joe Damato
Re: [Intel-wired-lan] [PATCH 2/3] igb: Link queues to NAPI instances
On Mon, Feb 10, 2025 at 10:19:36AM +0100, Kurt Kanzenbach wrote: > Link queues to NAPI instances via netdev-genl API. This is required to use > XDP/ZC busy polling. See commit 5ef44b3cb43b ("xsk: Bring back busy polling > support") for details. > > This also allows users to query the info with netlink: > > |$ ./tools/net/ynl/pyynl/cli.py --spec > Documentation/netlink/specs/netdev.yaml \ > | --dump queue-get --json='{"ifindex": 2}' > |[{'id': 0, 'ifindex': 2, 'napi-id': 8201, 'type': 'rx'}, > | {'id': 1, 'ifindex': 2, 'napi-id': 8202, 'type': 'rx'}, > | {'id': 2, 'ifindex': 2, 'napi-id': 8203, 'type': 'rx'}, > | {'id': 3, 'ifindex': 2, 'napi-id': 8204, 'type': 'rx'}, > | {'id': 0, 'ifindex': 2, 'napi-id': 8201, 'type': 'tx'}, > | {'id': 1, 'ifindex': 2, 'napi-id': 8202, 'type': 'tx'}, > | {'id': 2, 'ifindex': 2, 'napi-id': 8203, 'type': 'tx'}, > | {'id': 3, 'ifindex': 2, 'napi-id': 8204, 'type': 'tx'}] > > While at __igb_open() use RCT coding style. > > Signed-off-by: Kurt Kanzenbach > --- > drivers/net/ethernet/intel/igb/igb.h | 2 ++ > drivers/net/ethernet/intel/igb/igb_main.c | 35 > ++- > drivers/net/ethernet/intel/igb/igb_xsk.c | 2 ++ > 3 files changed, 34 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igb/igb.h > b/drivers/net/ethernet/intel/igb/igb.h > index > 02f340280d20a6f7e32bbd3dfcbb9c1c7b4c6662..79eca385a751bfdafdf384928b6cc1b350b22560 > 100644 > --- a/drivers/net/ethernet/intel/igb/igb.h > +++ b/drivers/net/ethernet/intel/igb/igb.h > @@ -722,6 +722,8 @@ enum igb_boards { > > extern char igb_driver_name[]; > > +void igb_set_queue_napi(struct igb_adapter *adapter, int q_idx, > + struct napi_struct *napi); > int igb_xmit_xdp_ring(struct igb_adapter *adapter, > struct igb_ring *ring, > struct xdp_frame *xdpf); > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c > b/drivers/net/ethernet/intel/igb/igb_main.c > index > d4128d19cc08f62f95682069bb5ed9b8bbbf10cb..8e964484f4c9854e4e3e0b4f3e8785fe93bd1207 > 100644 > --- a/drivers/net/ethernet/intel/igb/igb_main.c > +++ b/drivers/net/ethernet/intel/igb/igb_main.c > @@ -2099,6 +2099,22 @@ static void igb_check_swap_media(struct igb_adapter > *adapter) > wr32(E1000_CTRL_EXT, ctrl_ext); > } > > +void igb_set_queue_napi(struct igb_adapter *adapter, int vector, > + struct napi_struct *napi) > +{ > + struct igb_q_vector *q_vector = adapter->q_vector[vector]; > + > + if (q_vector->rx.ring) > + netif_queue_set_napi(adapter->netdev, > + q_vector->rx.ring->queue_index, > + NETDEV_QUEUE_TYPE_RX, napi); > + > + if (q_vector->tx.ring) > + netif_queue_set_napi(adapter->netdev, > + q_vector->tx.ring->queue_index, > + NETDEV_QUEUE_TYPE_TX, napi); > +} > + > /** > * igb_up - Open the interface and prepare it to handle traffic > * @adapter: board private structure > @@ -2106,6 +2122,7 @@ static void igb_check_swap_media(struct igb_adapter > *adapter) > int igb_up(struct igb_adapter *adapter) > { > struct e1000_hw *hw = &adapter->hw; > + struct napi_struct *napi; > int i; > > /* hardware has been reset, we need to reload some things */ > @@ -2113,8 +2130,11 @@ int igb_up(struct igb_adapter *adapter) > > clear_bit(__IGB_DOWN, &adapter->state); > > - for (i = 0; i < adapter->num_q_vectors; i++) > - napi_enable(&(adapter->q_vector[i]->napi)); > + for (i = 0; i < adapter->num_q_vectors; i++) { > + napi = &adapter->q_vector[i]->napi; > + napi_enable(napi); > + igb_set_queue_napi(adapter, i, napi); > + } It looks like igb_ub is called from igb_io_resume (struct pci_error_handlers). I don't know if RTNL is held in that path. If its not, this could trip the ASSERT_RTNL in netif_queue_set_napi. Can you check and see if this is an issue for that path? igb_reinit_locked looks OK (as the name implies). > > if (adapter->flags & IGB_FLAG_HAS_MSIX) > igb_configure_msix(adapter); > @@ -2184,6 +2204,7 @@ void igb_down(struct igb_adapter *adapter) > for (i = 0; i < adapter->num_q_vectors; i++) { > if (adapter->q_vector[i]) { > napi_synchronize(&adapter->q_vector[i]->napi); > + igb_set_queue_napi(adapter, i, NULL); > napi_disable(&adapter->q_vector[i]->napi); Same question as above. It looks like igb_down is called from igb_io_error_detected. I don't know if RTNL is held in that path. If its not, it'll trip the ASSERT_RTNL in netif_queue_set_napi. Can you check if that's an issue for this path, as well? > } > } > @@ -4116,8 +4137,9 @@ static int igb_sw_init(struct igb_adapter *adapter) >
[Intel-wired-lan] [PATCH iwl-net] idpf: check error for register_netdev() on init
Current init logic ignores the error code from register_netdev(), which will cause WARN_ON() on attempt to unregister it, if there was one, and there is no info for the user that the creation of the netdev failed. WARNING: CPU: 89 PID: 6902 at net/core/dev.c:11512 unregister_netdevice_many_notify+0x211/0x1a10 ... [ 3707.563641] unregister_netdev+0x1c/0x30 [ 3707.563656] idpf_vport_dealloc+0x5cf/0xce0 [idpf] [ 3707.563684] idpf_deinit_task+0xef/0x160 [idpf] [ 3707.563712] idpf_vc_core_deinit+0x84/0x320 [idpf] [ 3707.563739] idpf_remove+0xbf/0x780 [idpf] [ 3707.563769] pci_device_remove+0xab/0x1e0 [ 3707.563786] device_release_driver_internal+0x371/0x530 [ 3707.563803] driver_detach+0xbf/0x180 [ 3707.563816] bus_remove_driver+0x11b/0x2a0 [ 3707.563829] pci_unregister_driver+0x2a/0x250 Introduce an error check and log the vport number and error code. On removal make sure to check VPORT_REG_NETDEV flag prior to calling unregister and free on the netdev. Add local variables for idx, vport_config and netdev for readability. Fixes: 0fe45467a104 ("idpf: add create vport and netdev configuration") Reviewed-by: Madhu Chittim Suggested-by: Tony Nguyen Signed-off-by: Emil Tantilov --- drivers/net/ethernet/intel/idpf/idpf_lib.c | 27 ++ 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c index a3d6b8f198a8..a322a8ac771e 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c @@ -927,15 +927,19 @@ static int idpf_stop(struct net_device *netdev) static void idpf_decfg_netdev(struct idpf_vport *vport) { struct idpf_adapter *adapter = vport->adapter; + u16 idx = vport->idx; kfree(vport->rx_ptype_lkup); vport->rx_ptype_lkup = NULL; - unregister_netdev(vport->netdev); - free_netdev(vport->netdev); + if (test_and_clear_bit(IDPF_VPORT_REG_NETDEV, + adapter->vport_config[idx]->flags)) { + unregister_netdev(vport->netdev); + free_netdev(vport->netdev); + } vport->netdev = NULL; - adapter->netdevs[vport->idx] = NULL; + adapter->netdevs[idx] = NULL; } /** @@ -1536,12 +1540,17 @@ void idpf_init_task(struct work_struct *work) } for (index = 0; index < adapter->max_vports; index++) { - if (adapter->netdevs[index] && - !test_bit(IDPF_VPORT_REG_NETDEV, - adapter->vport_config[index]->flags)) { - register_netdev(adapter->netdevs[index]); - set_bit(IDPF_VPORT_REG_NETDEV, - adapter->vport_config[index]->flags); + struct idpf_vport_config *vport_config = adapter->vport_config[index]; + struct net_device *netdev = adapter->netdevs[index]; + + if (netdev && !test_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags)) { + err = register_netdev(netdev); + if (err) { + dev_err(&pdev->dev, "failed to register netdev for vport %d: %pe\n", + index, ERR_PTR(err)); + continue; + } + set_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags); } } -- 2.17.2
Re: [Intel-wired-lan] [PATCH 0/3] igb: XDP/ZC follow up
On Mon, Feb 10, 2025 at 10:19:34AM +0100, Kurt Kanzenbach wrote: > This is a follow up for the igb XDP/ZC implementation. The first two > patches link the IRQs and queues to NAPI instances. This is required to > bring back the XDP/ZC busy polling support. The last patch removes > undesired IRQs (injected via igb watchdog) while busy polling with > napi_defer_hard_irqs and gro_flush_timeout set. > > Signed-off-by: Kurt Kanzenbach > --- > Kurt Kanzenbach (3): > igb: Link IRQs to NAPI instances > igb: Link queues to NAPI instances > igb: Get rid of spurious interrupts > > drivers/net/ethernet/intel/igb/igb.h | 5 ++- > drivers/net/ethernet/intel/igb/igb_main.c | 67 > ++- > drivers/net/ethernet/intel/igb/igb_xsk.c | 3 ++ > 3 files changed, 65 insertions(+), 10 deletions(-) > --- > base-commit: acdefab0dcbc3833b5a734ab80d792bb778517a0 > change-id: 20250206-igb_irq-f5a4d4deb207 Overall wanted to note that Stanislav is working on some locking changes to remove the RTNL dependency [1]. My previous attempt at adding this API to virtio_net is on hold until the locking stuff Stanislav is doing is done. I am not sure if the maintainers will also ask to hold your series back, as well. [1]: https://lore.kernel.org/netdev/20250204230057.1270362-1-...@fomichev.me/
Re: [Intel-wired-lan] [PATCH 3/3] igb: Get rid of spurious interrupts
On Mon, Feb 10, 2025 at 10:19:37AM +0100, Kurt Kanzenbach wrote: > When running the igc with XDP/ZC in busy polling mode with deferral of hard > interrupts, interrupts still happen from time to time. That is caused by > the igc task watchdog which triggers Rx interrupts periodically. > > That mechanism has been introduced to overcome skb/memory allocation > failures [1]. So the Rx clean functions stop processing the Rx ring in case > of such failure. The task watchdog triggers Rx interrupts periodically in > the hope that memory became available in the mean time. > > The current behavior is undesirable for real time applications, because the > driver induced Rx interrupts trigger also the softirq processing. However, > all real time packets should be processed by the application which uses the > busy polling method. > > Therefore, only trigger the Rx interrupts in case of real allocation > failures. Introduce a new flag for signaling that condition. > > Follow the same logic as in commit 8dcf2c212078 ("igc: Get rid of spurious > interrupts"). > > [1] - > https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=3be507547e6177e5c808544bd6a2efa2c7f1d436 > > Signed-off-by: Kurt Kanzenbach > --- > drivers/net/ethernet/intel/igb/igb.h | 3 ++- > drivers/net/ethernet/intel/igb/igb_main.c | 29 + > drivers/net/ethernet/intel/igb/igb_xsk.c | 1 + > 3 files changed, 28 insertions(+), 5 deletions(-) I am not an igb expert (nor do I have such a device), but after reading the source a bit this seems reasonable. I suppose perhaps a better direction in the future would be to convert the driver to the page pool, but in the meantime the proposed change seems reasonable. Reviewed-by: Joe Damato
Re: [Intel-wired-lan] [PATCH 3/3] igb: Get rid of spurious interrupts
On 10.02.25 10:19, Kurt Kanzenbach wrote: When running the igc with XDP/ZC in busy polling mode with deferral of hard interrupts, interrupts still happen from time to time. That is caused by the igc task watchdog which triggers Rx interrupts periodically. igc or igb?
Re: [Intel-wired-lan] [PATCH net-next v7 1/5] net: move ARFS rmap management to core
On Mon, 10 Feb 2025 08:04:43 -0700 Ahmed Zaki wrote: > On 2025-02-06 7:29 p.m., Jakub Kicinski wrote: > > > Speaking of which, why do the auto-removal in napi_disable() > > rather than netif_napi_del() ? We don't reinstall on napi_enable() > > and doing a disable() + enable() is fairly common during driver > > reconfig. > > > > The patch does not re-install the notifiers in napi_add either, they are > installed in set_irq() : > > napi_add_config() -> napi_set_irq() -> napi_enable() > > so napi_disable or napi_del seemed both OK to me. > > However, I moved notifier auto-removal to npi_del() and did some testing > on ice but it seems the driver does not delete napi on "ip link down" > and that generates warnings on free_irq(). It only disables the napis. > > So is this a bug? Do we need to ask drivers to disable __and__ delete > napis before freeing the IRQs? > > If not, then we have to keep notifier aut-removal in napi_diasable(). If the driver releases the IRQ but keeps the NAPI instance I would have expected it to call: napi_set_irq(napi, -1); before freeing the IRQ. Otherwise the NAPI instance will "point" to a freed IRQ.
Re: [Intel-wired-lan] [PATCH 1/3] igb: Link IRQs to NAPI instances
On Mon, Feb 10, 2025 at 10:25:47AM -0800, Joe Damato wrote: > On Mon, Feb 10, 2025 at 10:19:35AM +0100, Kurt Kanzenbach wrote: > > Link IRQs to NAPI instances via netdev-genl API. This allows users to query > > that information via netlink: > > > > |$ ./tools/net/ynl/pyynl/cli.py --spec > > Documentation/netlink/specs/netdev.yaml \ > > | --dump napi-get --json='{"ifindex": 2}' > > |[{'defer-hard-irqs': 0, > > | 'gro-flush-timeout': 0, > > | 'id': 8204, > > | 'ifindex': 2, > > | 'irq': 127, > > | 'irq-suspend-timeout': 0}, > > | {'defer-hard-irqs': 0, > > | 'gro-flush-timeout': 0, > > | 'id': 8203, > > | 'ifindex': 2, > > | 'irq': 126, > > | 'irq-suspend-timeout': 0}, > > | {'defer-hard-irqs': 0, > > | 'gro-flush-timeout': 0, > > | 'id': 8202, > > | 'ifindex': 2, > > | 'irq': 125, > > | 'irq-suspend-timeout': 0}, > > | {'defer-hard-irqs': 0, > > | 'gro-flush-timeout': 0, > > | 'id': 8201, > > | 'ifindex': 2, > > | 'irq': 124, > > | 'irq-suspend-timeout': 0}] > > |$ cat /proc/interrupts | grep enp2s0 > > |123: 0 1 IR-PCI-MSIX-:02:00.0 0-edge enp2s0 > > |124: 0 7 IR-PCI-MSIX-:02:00.0 1-edge > > enp2s0-TxRx-0 > > |125: 0 0 IR-PCI-MSIX-:02:00.0 2-edge > > enp2s0-TxRx-1 > > |126: 0 5 IR-PCI-MSIX-:02:00.0 3-edge > > enp2s0-TxRx-2 > > |127: 0 0 IR-PCI-MSIX-:02:00.0 4-edge > > enp2s0-TxRx-3 > > > > Signed-off-by: Kurt Kanzenbach > > --- > > drivers/net/ethernet/intel/igb/igb_main.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c > > b/drivers/net/ethernet/intel/igb/igb_main.c > > index > > d368b753a4675d01b5dfa50dee4cd218e6a5e14b..d4128d19cc08f62f95682069bb5ed9b8bbbf10cb > > 100644 > > --- a/drivers/net/ethernet/intel/igb/igb_main.c > > +++ b/drivers/net/ethernet/intel/igb/igb_main.c > > @@ -947,6 +947,9 @@ static int igb_request_msix(struct igb_adapter *adapter) > > q_vector); > > if (err) > > goto err_free; > > + > > + netif_napi_set_irq(&q_vector->napi, > > + adapter->msix_entries[vector].vector); > > } > > As far as I can tell, all paths that lead here hold RTNL: A nit on my own comment, netif_napi_set_irq doesn't ASSERT_RTNL (but does hold net_device->lock); its the other functions in patch 2 that ASSERT_RTNL. My reviewed-by stands; just wanted to correct myself.
Re: [Intel-wired-lan] [PATCH 0/3] igb: XDP/ZC follow up
On Mon, Feb 10, 2025 at 10:19:34AM +0100, Kurt Kanzenbach wrote: > This is a follow up for the igb XDP/ZC implementation. The first two > patches link the IRQs and queues to NAPI instances. This is required to > bring back the XDP/ZC busy polling support. The last patch removes > undesired IRQs (injected via igb watchdog) while busy polling with > napi_defer_hard_irqs and gro_flush_timeout set. You may want to use netif_napi_add_config to enable persistent NAPI config, btw. This makes writing userland programs based on SO_INCOMING_NAPI_ID much easier. See also: https://lore.kernel.org/netdev/20250208012822.34327-1-jdam...@fastly.com/
Re: [Intel-wired-lan] [PATCH 3/3] igb: Get rid of spurious interrupts
On Mon Feb 10 2025, Gerhard Engleder wrote: > On 10.02.25 10:19, Kurt Kanzenbach wrote: >> When running the igc with XDP/ZC in busy polling mode with deferral of hard >> interrupts, interrupts still happen from time to time. That is caused by >> the igc task watchdog which triggers Rx interrupts periodically. > > igc or igb? igb of course. Thanks. signature.asc Description: PGP signature
Re: [Intel-wired-lan] [PATCH 2/3] igb: Link queues to NAPI instances
On Mon Feb 10 2025, Joe Damato wrote: > On Mon, Feb 10, 2025 at 10:19:36AM +0100, Kurt Kanzenbach wrote: >> Link queues to NAPI instances via netdev-genl API. This is required to use >> XDP/ZC busy polling. See commit 5ef44b3cb43b ("xsk: Bring back busy polling >> support") for details. >> >> This also allows users to query the info with netlink: >> >> |$ ./tools/net/ynl/pyynl/cli.py --spec >> Documentation/netlink/specs/netdev.yaml \ >> | --dump queue-get --json='{"ifindex": 2}' >> |[{'id': 0, 'ifindex': 2, 'napi-id': 8201, 'type': 'rx'}, >> | {'id': 1, 'ifindex': 2, 'napi-id': 8202, 'type': 'rx'}, >> | {'id': 2, 'ifindex': 2, 'napi-id': 8203, 'type': 'rx'}, >> | {'id': 3, 'ifindex': 2, 'napi-id': 8204, 'type': 'rx'}, >> | {'id': 0, 'ifindex': 2, 'napi-id': 8201, 'type': 'tx'}, >> | {'id': 1, 'ifindex': 2, 'napi-id': 8202, 'type': 'tx'}, >> | {'id': 2, 'ifindex': 2, 'napi-id': 8203, 'type': 'tx'}, >> | {'id': 3, 'ifindex': 2, 'napi-id': 8204, 'type': 'tx'}] >> >> While at __igb_open() use RCT coding style. >> >> Signed-off-by: Kurt Kanzenbach >> --- >> drivers/net/ethernet/intel/igb/igb.h | 2 ++ >> drivers/net/ethernet/intel/igb/igb_main.c | 35 >> ++- >> drivers/net/ethernet/intel/igb/igb_xsk.c | 2 ++ >> 3 files changed, 34 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/igb/igb.h >> b/drivers/net/ethernet/intel/igb/igb.h >> index >> 02f340280d20a6f7e32bbd3dfcbb9c1c7b4c6662..79eca385a751bfdafdf384928b6cc1b350b22560 >> 100644 >> --- a/drivers/net/ethernet/intel/igb/igb.h >> +++ b/drivers/net/ethernet/intel/igb/igb.h >> @@ -722,6 +722,8 @@ enum igb_boards { >> >> extern char igb_driver_name[]; >> >> +void igb_set_queue_napi(struct igb_adapter *adapter, int q_idx, >> +struct napi_struct *napi); >> int igb_xmit_xdp_ring(struct igb_adapter *adapter, >>struct igb_ring *ring, >>struct xdp_frame *xdpf); >> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c >> b/drivers/net/ethernet/intel/igb/igb_main.c >> index >> d4128d19cc08f62f95682069bb5ed9b8bbbf10cb..8e964484f4c9854e4e3e0b4f3e8785fe93bd1207 >> 100644 >> --- a/drivers/net/ethernet/intel/igb/igb_main.c >> +++ b/drivers/net/ethernet/intel/igb/igb_main.c >> @@ -2099,6 +2099,22 @@ static void igb_check_swap_media(struct igb_adapter >> *adapter) >> wr32(E1000_CTRL_EXT, ctrl_ext); >> } >> >> +void igb_set_queue_napi(struct igb_adapter *adapter, int vector, >> +struct napi_struct *napi) >> +{ >> +struct igb_q_vector *q_vector = adapter->q_vector[vector]; >> + >> +if (q_vector->rx.ring) >> +netif_queue_set_napi(adapter->netdev, >> + q_vector->rx.ring->queue_index, >> + NETDEV_QUEUE_TYPE_RX, napi); >> + >> +if (q_vector->tx.ring) >> +netif_queue_set_napi(adapter->netdev, >> + q_vector->tx.ring->queue_index, >> + NETDEV_QUEUE_TYPE_TX, napi); >> +} >> + >> /** >> * igb_up - Open the interface and prepare it to handle traffic >> * @adapter: board private structure >> @@ -2106,6 +2122,7 @@ static void igb_check_swap_media(struct igb_adapter >> *adapter) >> int igb_up(struct igb_adapter *adapter) >> { >> struct e1000_hw *hw = &adapter->hw; >> +struct napi_struct *napi; >> int i; >> >> /* hardware has been reset, we need to reload some things */ >> @@ -2113,8 +2130,11 @@ int igb_up(struct igb_adapter *adapter) >> >> clear_bit(__IGB_DOWN, &adapter->state); >> >> -for (i = 0; i < adapter->num_q_vectors; i++) >> -napi_enable(&(adapter->q_vector[i]->napi)); >> +for (i = 0; i < adapter->num_q_vectors; i++) { >> +napi = &adapter->q_vector[i]->napi; >> +napi_enable(napi); >> +igb_set_queue_napi(adapter, i, napi); >> +} > > It looks like igb_ub is called from igb_io_resume (struct > pci_error_handlers). I don't know if RTNL is held in that path. If > its not, this could trip the ASSERT_RTNL in netif_queue_set_napi. > > Can you check and see if this is an issue for that path? AFAICS the PCI error handlers are called in drivers/pci/pcie/err.c without RTNL lock held. These function take only the device_lock(). I'll add the missing rtnl_lock()/unlock() calls to igb_io_resume() and igb_io_error_detected(). Thanks. signature.asc Description: PGP signature
Re: [Intel-wired-lan] [PATCH iwl-net v2] idpf: call set_real_num_queues in idpf_open
> -Original Message- > From: Lobakin, Aleksander > Sent: Monday, February 10, 2025 7:02 AM > To: Hay, Joshua A > Cc: intel-wired-...@lists.osuosl.org; Samudrala, Sridhar > ; Chittim, Madhu > Subject: Re: [Intel-wired-lan] [PATCH iwl-net v2] idpf: call > set_real_num_queues in idpf_open > > From: Joshua Hay > Date: Tue, 4 Feb 2025 18:08:11 -0800 > > > On initial driver load, alloc_etherdev_mqs is called with whatever max > > queue values are provided by the control plane. However, if the driver > > is loaded on a system where num_online_cpus() returns less than the max > > queues, the netdev will think there are more queues than are actually > > available. Only num_online_cpus() will be allocated, but > > skb_get_queue_mapping(skb) could possibly return an index beyond the > > range of allocated queues. Consequently, the packet is silently dropped > > and it appears as if TX is broken. > > > > Set the real number of queues during open so the netdev knows how many > > queues will be allocated. > > > > v2: > > - call set_real_num_queues in idpf_open. Previous change called > > set_real_num_queues function in idpf_up_complete, but it is possible > > for up_complete to be called without holding the RTNL lock. If user > > brings up interface, then issues a reset, the init_task will call > > idpf_vport_open->idpf_up_complete. Since this is initiated by the > > driver, the RTNL lock is not taken. > > - adjust title to reflect new changes. > > > > Signed-off-by: Joshua Hay > > Fixes: 1c325aac10a8 ("idpf: configure resources for TX queues") > > Reviewed-by: Madhu Chittim > > --- > > drivers/net/ethernet/intel/idpf/idpf_lib.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c > b/drivers/net/ethernet/intel/idpf/idpf_lib.c > > index 6df7f125ebde..9dc806411002 100644 > > --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c > > +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c > > @@ -2159,8 +2159,13 @@ static int idpf_open(struct net_device *netdev) > > idpf_vport_ctrl_lock(netdev); > > vport = idpf_netdev_to_vport(netdev); > > > > + err = idpf_set_real_num_queues(vport); > > + if (err) > > + goto unlock; > > Why wasn't it removed from the soft reset flow now, although you did > that in v1? It wasn't _really_ removed from the soft reset flow in v1, just consolidated. The soft reset flow still called idpf_set_real_num_queues through idpf_up_complete. The soft reset flow stills need to call set_real_num_queues in this version, since idpf_open is not guaranteed to be called again (e.g. interface is already up and user calls ethtool -L) > > > + > > err = idpf_vport_open(vport); > > > > +unlock: > > idpf_vport_ctrl_unlock(netdev); > > > > return err; > > Thanks, > Olek Thanks, Josh