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

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

On 6.08.2026 15:04, Przemek Kitszel wrote:
> On 8/6/26 12:46, 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.
>>
>> 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>
>> ---
> 
> next time please provide changelog section here
> 

Noted, will include changelog in subsequent versions.

>>   drivers/net/ethernet/intel/ixgbevf/vf.c | 77 +++++++++++++++++++++----
>>   1 file changed, 65 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
>> index f6df86d124b9..8ad06e28b5de 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"
>>   
> 
> [..]
> 
>> +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];
> 
> following Paul's suggestion, you could add:
>     if (IS_ENABLED(CONFIG_PCI_MMCONFIG)) {
>         dev_err_once(&adapter->pdev->dev, "cannot read link status, PCI_MMCONFIG is required for Hyper-V\n");
>         return -EOPNOTSUPP;
>     }
> 
> and this will give user more information
> 
> IS_ENABLED() is really nice macro :)
> 

I think you meant !IS_ENABLED() here. Without MMCONFIG, pci_read_config_byte() at offset 0x209 (extended config space) will return a PCIBIOS error anyway, and the caller handles that gracefully by reporting link down. I could add the explicit IS_ENABLED check for a friendlier diagnostic message, but the behavior is correct either way. Let me know if you'd prefer I add it in a v4.

>> +
>> +    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;
>> +}
>> +

Thanks,
Tomasz