Re: [Intel-wired-lan] [PATCH iwl-net v2] ixgbevf: fix link speed reporting for Hyper-V E610 VFs

[Tomasz Lichwala <tomasz.lichwala@linux.intel.com>]

On 5.08.2026 07:56, Paul Menzel wrote:
> Dear Tomasz,
> 
> 
> Thank you for your patch.
> 
> Am 04.08.26 um 17:39 schrieb Tomasz Lichwala:
>> 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. Read and decode it when checking link on
>> E610 VFs.
> 
> Is that emulated space documented somewhere? It’d be great if you added a reference.
> 

The emulated PCI config space layout is defined by the Hyper-V NetVSC synthetic NIC interface. I've expanded the commit message to mention the Hyper-V synthetic NIC and added before/after ethtool output.

> Also, it’d be great if you documented the commands and output without and with your patch in the commit message, and mention the Hyper-V environment.
> 

Expanded the commit message to describe the problem — VFLINKS does not reflect the actual negotiated speed on E610 VFs under Hyper-V, and how the fix reads link status from emulated PCI config space instead. I did not include specific ethtool output because the reported speed varies depending on  the NIC and link configuration.

>> Fixes: 4c44b450c69b ("ixgbevf: Add support for Intel(R) E610 device")
>> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
>> Signed-off-by: Tomasz Lichwala <tomasz.lichwala@linux.intel.com>
>> ---
>>   drivers/net/ethernet/intel/ixgbevf/vf.c | 79 ++++++++++++++++++++++---
>>   1 file changed, 70 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
>> index f6df86d124b9..4c460ec62de3 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;
>> +
>> +#if IS_ENABLED(CONFIG_PCI_MMCONFIG)
> 
> Can the check be done in C code, and the linker will remove the unused stuff? This way everything would be seen by the compiler and compile checked.
> 

Good point. Dropped the #if IS_ENABLED(CONFIG_PCI_MMCONFIG) guard entirely. Offset 0x209 is in extended PCI config space, so pci_read_config_byte() will return a PCIBIOS error if ECAM is not available, and the caller already handles read failures by reporting link down. This way the compiler always sees the full code path.

>> +    u8 data[IXGBE_HV_LINK_STATUS_SIZE];
>> +
>> +    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;
>> +#else
>> +    dev_err_once(&adapter->pdev->dev, "cannot read link status, PCI_MMCONFIG is required for Hyper-V\n");
>> +    return -EOPNOTSUPP;
>> +#endif
>> +}
>> +
>>   /**
>>    * 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,
>> @@ -923,13 +961,30 @@ static s32 ixgbevf_hv_check_mac_link_vf(struct ixgbe_hw *hw,
>>       if (!mbx->ops.check_for_rst(hw) || !mbx->timeout)
>>           mac->get_link_status = true;
>>   +    /* E610 VFs always read link status from emulated PCI config space
>> +     * because VFLINKS does not carry valid speed for these devices.
>> +     * Skip get_link_status caching since PCI config reads are cheap.
>> +     */
>> +    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;
>> +        }
>> +        goto decode;
>> +    }
>> +
>>       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 (!(links_reg & IXGBE_LINKS_UP))
>> -        goto out;
>> +
>> +decode:
> 
> How would the implementation without goto look like?
> 

Replaced goto decode with an if/else structure - the E610 path and the VFLINKS register read are now in separate branches, falling through to the common link-down / speed decode logic below.

>> +    if (!(links_reg & IXGBE_LINKS_UP)) {
>> +        *link_up = false;
>> +        *speed = IXGBE_LINK_SPEED_UNKNOWN;
>> +        return 0;
>> +    }
>>         /* for SFP+ modules and DA cables on 82599 it can take up to 500usecs
>>        * before the link status is correct
>> @@ -941,8 +996,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 +1014,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
> 
> 
> Kind regards,
> 
> Paul


Kind regards,

Tomasz