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

On 30.07.2026 23:19, Tony Nguyen wrote:
> 
> 
> On 7/23/2026 7:51 AM, Tomasz Lichwala wrote:
> 
> ...
> 
>> +/**
>> + * 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)
>> +    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
>> +}
> 
> I ran Sashiko locally on this and it reported the following:
> Should a failed link read here return a fatal error to check_link, or
> report link down and return 0?
> When built without CONFIG_PCI_MMCONFIG this returns -EOPNOTSUPP on every
> call, and it also returns an error on any persistent
> pci_read_config_byte() failure.
> 

Good catch, you are right. Before this patch the Hyper-V check_link
callback always returned 0, so propagating the error is a regression
in the degraded case.

v2 will return 0 with *link_up = false and *speed =
IXGBE_LINK_SPEED_UNKNOWN on read failure. The dev_err_once() still
logs the root cause for diagnostics.

Thanks,
Tomasz

>>   /**
>>    * 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,32 @@ 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) {
>> +        s32 ret = ixgbevf_hv_read_links_e610(hw, &links_reg);
>> +
>> +        if (ret) {
>> +            *link_up = false;
>> +            *speed = IXGBE_LINK_SPEED_UNKNOWN;
>> +            return ret;
>> +        }
>> +        goto decode;
>> +    }
>> +
> Can this cause a repeating reset loop on the degraded configuration?
> The non-zero return propagates up through mac.ops.check_link() into
> ixgbevf_watchdog_update_link():
>     err = hw->mac.ops.check_link(hw, &link_speed, &link_up, false);
>     ...
>     if (err && time_after(jiffies, adapter->last_reset + (10 * HZ))) {
>         set_bit(__IXGBEVF_RESET_REQUESTED, &adapter->state);
>         link_up = false;
>     }
> A device reset cannot make CONFIG_PCI_MMCONFIG appear at compile time, nor
> fix a persistent config-space read failure, so wouldn't every watchdog
> cycle re-arm __IXGBEVF_RESET_REQUESTED roughly every 10 seconds, leaving
> the interface permanently down and continuously resetting?
> Before this patch the Hyper-V check_link callback always returned 0, so no
> reset request was triggered. Would returning 0 with link_up=false and
> speed IXGBE_LINK_SPEED_UNKNOWN for the unsupported/degraded case avoid the
> loop?
> One related note: the dev_err_once() reads as a benign one-time notice,
> but the underlying -EOPNOTSUPP is returned on every poll and drives the
> repeating reset behavior above.
> 
> Thanks,
> Tony