This email covers the Sashiko review for this patch.
On 24.08.2026 13:08, Tomasz Lichwala wrote:
> When an E610 VF is running under Hyper-V, the VFLINKS register does not
> carry valid link speed. The existing code reads speed from VFLINKS, which
> does not reflect the actual negotiated speed. This results in ethtool
> reporting a stale or incorrect link speed.
>
> The Hyper-V synthetic NIC exposes the actual link status through
> emulated PCI config space at offset 0x209 in VFLINKS register format.
> Read and decode link status from there when checking link on E610 VFs.
>
> To avoid generating unnecessary VMBus transactions on every watchdog
> cycle, read the PCI config register on demand when ethtool or sysfs
> queries link speed, rather than polling it periodically. The cached
> value is used by the watchdog and link state notifications as before.
>
> Guard the PCI config space read with IS_ENABLED(CONFIG_PCI_MMCONFIG),
> since accessing offsets above 256 requires MMCONFIG support.
>
> Fixes: 4c44b450c69b ("ixgbevf: Add support for Intel(R) E610 device")
> Reviewed-by: Marcin Szycik <[email protected]>
> Signed-off-by: Tomasz Lichwala <[email protected]>
> ---
> v4:
> - Read link speed on demand from ethtool/sysfs query instead of polling
> every watchdog cycle to avoid unnecessary VMBus transactions (ethtool.c)
> - Restore get_link_status caching for E610 VFs in check_link
> - Add IS_ENABLED(CONFIG_PCI_MMCONFIG) guard for PCI config reads above
> offset 256
> - Add CC [email protected]
> v3:
> - Remove #if IS_ENABLED(CONFIG_PCI_MMCONFIG) preprocessor guard
> - Replace goto decode label with if/else control flow
> - Move get_link_status check into else branch (non-E610 path only)
> - Expand commit message with problem description
> v2:
> - Simplify error path: return 0 with link_up=false instead of
> propagating PCI read error code
> - Fix alignment in macro definitions
> - Update comment describing Hyper-V PCI config offsets
> drivers/net/ethernet/intel/ixgbevf/ethtool.c | 12 ++++
> drivers/net/ethernet/intel/ixgbevf/vf.c | 68 +++++++++++++++++---
> 2 files changed, 72 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
> b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
> index 537a60d5276f..2463fccffd4d 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
> @@ -83,6 +83,18 @@ static int ixgbevf_get_link_ksettings(struct net_device
> *netdev,
> struct ethtool_link_ksettings *cmd)
> {
> struct ixgbevf_adapter *adapter = netdev_priv(netdev);
> + struct ixgbe_hw *hw = &adapter->hw;
> +
> + /* E610 Hyper-V VFs: read current link speed from PCI config space
> + * on every link speed query.
> + */
> + if (hw->mac.type == ixgbe_mac_e610_vf) {
The mac type check matches both Hyper-V and bare-metal E610 VFs. On bare-metal,
forcing get_link_status=true and calling ixgbevf_check_mac_link_vf() would
indeed corrupt the link state — the CTS mailbox read fails (no pending CTS),
causing a false link-down report.
Fixed in v5 by checking the subsystem device ID (IXGBE_SUBDEV_ID_E610_VF_HV)
instead of the mac type. This is the same identifier used in the PCI ID table
to select Hyper-V mac_ops during probe, so the on-demand read now only triggers
for Hyper-V VFs where hw->mac.ops.check_link points to
ixgbevf_hv_check_mac_link_vf().
> + spin_lock_bh(&adapter->mbx_lock);
> + hw->mac.get_link_status = true;
> + hw->mac.ops.check_link(hw, &adapter->link_speed,
> + &adapter->link_up, false);
This is not a race introduced by this patch. The watchdog's pattern of caching
adapter->link_speed into a local variable before taking mbx_lock is
pre-existing for all Hyper-V VF types. The ethtool response is always correct
because it reads adapter->link_speed immediately after check_link returns
within the same function call. The mbx_lock serializes concurrent check_link
calls - ethtool and watchdog cannot execute check_link simultaneously.
> + spin_unlock_bh(&adapter->mbx_lock);
> + }
>
> ethtool_link_ksettings_zero_link_mode(cmd, supported);
> ethtool_link_ksettings_add_link_mode(cmd, supported, 10000baseT_Full);
> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c
> b/drivers/net/ethernet/intel/ixgbevf/vf.c
> index f6df86d124b9..a84b62641e9b 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
> @@ -1,14 +1,17 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Copyright(c) 1999 - 2024 Intel Corporation. */
>
> +#include <linux/unaligned.h>
> +
> #include "vf.h"
> #include "ixgbevf.h"
>
> -/* On Hyper-V, to reset, we need to read from this offset
> - * from the PCI config space. This is the mechanism used on
> - * Hyper-V to support PF/VF communication.
> +/* On Hyper-V the PF/VF communication is through emulated PCI config
> + * space. The reset and link status are exposed at the offsets below.
> */
> -#define IXGBE_HV_RESET_OFFSET 0x201
> +#define IXGBE_HV_RESET_OFFSET 0x201
> +#define IXGBE_HV_LINK_STATUS_OFFSET 0x209
> +#define IXGBE_HV_LINK_STATUS_SIZE 4
>
> static inline s32 ixgbevf_write_msg_read_ack(struct ixgbe_hw *hw, u32 *msg,
> u32 *retmsg, u16 size)
> @@ -901,6 +904,40 @@ static s32 ixgbevf_check_mac_link_vf(struct ixgbe_hw *hw,
> return ret_val;
> }
>
> +/**
> + * ixgbevf_hv_read_links_e610 - read link status from PCI config space
> + * @hw: pointer to hardware structure
> + * @links_reg: pointer to store read value
> + *
> + * On Hyper-V E610 VFs the VFLINKS register does not carry valid link speed.
> + * Instead, link status is exposed through emulated PCI config space at
> offset
> + * 0x209 in VFLINKS register format.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +static s32 ixgbevf_hv_read_links_e610(struct ixgbe_hw *hw, u32 *links_reg)
> +{
> + struct ixgbevf_adapter *adapter = hw->back;
> + u8 data[IXGBE_HV_LINK_STATUS_SIZE];
> +
> + if (!IS_ENABLED(CONFIG_PCI_MMCONFIG)) {
The IS_ENABLED(CONFIG_PCI_MMCONFIG) guard is intentionally kept for consistency
with xgbevf_hv_reset_hw_vf(), which uses the same pattern to access emulated
PCI config space at offset x201. Both functions read from extended config
space offsets (>256) that require MMCONFIG on x86. In practice, Hyper-V E610
VFs are only deployed on x86 hosts where CONFIG_PCI_MMCONFIG is always enabled.
If ARM64 Hyper-V support becomes relevant, both functions would need to be
updated together.
> + dev_err_once(&adapter->pdev->dev,
> + "cannot read link status, PCI_MMCONFIG is required
> for Hyper-V\n");
> + return -EOPNOTSUPP;
> + }
> +
> + for (int i = 0; i < IXGBE_HV_LINK_STATUS_SIZE; i++) {
> + int ret = pci_read_config_byte(adapter->pdev,
> + IXGBE_HV_LINK_STATUS_OFFSET + i,
> + &data[i]);
> + if (ret)
> + return pcibios_err_to_errno(ret);
> + }
> +
> + *links_reg = get_unaligned_le32(data);
> + return 0;
> +}
> +
> /**
> * ixgbevf_hv_check_mac_link_vf - check link
> * @hw: pointer to private hardware struct
> @@ -909,6 +946,7 @@ static s32 ixgbevf_check_mac_link_vf(struct ixgbe_hw *hw,
> * @autoneg_wait_to_complete: unused
> *
> * Hyper-V variant; there is no mailbox communication.
> + * For E610 VFs, link status is read from emulated PCI config space.
> */
> static s32 ixgbevf_hv_check_mac_link_vf(struct ixgbe_hw *hw,
> ixgbe_link_speed *speed,
> @@ -926,8 +964,16 @@ static s32 ixgbevf_hv_check_mac_link_vf(struct ixgbe_hw
> *hw,
> if (!mac->get_link_status)
> goto out;
>
> - /* if link status is down no point in checking to see if pf is up */
> - links_reg = IXGBE_READ_REG(hw, IXGBE_VFLINKS);
> + if (mac->type == ixgbe_mac_e610_vf) {
> + if (ixgbevf_hv_read_links_e610(hw, &links_reg)) {
> + *link_up = false;
> + *speed = IXGBE_LINK_SPEED_UNKNOWN;
> + return 0;
> + }
> + } else {
> + links_reg = IXGBE_READ_REG(hw, IXGBE_VFLINKS);
> + }
> +
> if (!(links_reg & IXGBE_LINKS_UP))
> goto out;
>
> @@ -941,8 +987,11 @@ static s32 ixgbevf_hv_check_mac_link_vf(struct ixgbe_hw
> *hw,
> udelay(100);
> links_reg = IXGBE_READ_REG(hw, IXGBE_VFLINKS);
>
> - if (!(links_reg & IXGBE_LINKS_UP))
> - goto out;
> + if (!(links_reg & IXGBE_LINKS_UP)) {
> + *link_up = false;
> + *speed = IXGBE_LINK_SPEED_UNKNOWN;
> + return 0;
> + }
> }
> }
>
> @@ -956,6 +1005,9 @@ static s32 ixgbevf_hv_check_mac_link_vf(struct ixgbe_hw
> *hw,
> case IXGBE_LINKS_SPEED_100_82599:
> *speed = IXGBE_LINK_SPEED_100_FULL;
> break;
> + default:
> + *speed = IXGBE_LINK_SPEED_UNKNOWN;
> + break;
> }
>
> /* if we passed all the tests above then the link is up and we no
