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

On 8/6/26 15:19, Tomasz Lichwala wrote:
> 
> 
> 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 <[email protected]>
>>> Signed-off-by: Tomasz Lichwala <[email protected]>
>>> ---
>>
>> 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. 

oh yea

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.

yes, please do 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
>