git: 450c3f8b3d25 - main - e1000: Re-arm link changes

[Kevin Bowling]

On Mon, Sep 27, 2021 at 9:29 AM Kevin Bowling <kbowling at freebsd.org> wrote:
>
> The branch main has been updated by kbowling (ports committer):
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=450c3f8b3d259c7eb82488319aff45f1f6554aaf
>
> commit 450c3f8b3d259c7eb82488319aff45f1f6554aaf
> Author:     Kevin Bowling <kbowling at FreeBSD.org>
> AuthorDate: 2021-09-27 16:17:48 +0000
> Commit:     Kevin Bowling <kbowling at FreeBSD.org>
> CommitDate: 2021-09-27 16:25:58 +0000
>
>     e1000: Re-arm link changes
>
>     A change to MSI-X link handler was somehow causing issues on
>     MSI-based em(4) NICs.

If anyone has any deeper insight here, I'd like to fill in my
knowledge.  A 'device_printf' in 'em_msix_link()' never fires on em(4)
using MSI so I do not see the relationship between this code and the
experienced issues.

>     Revert the change based on user reports and testing.
>
>     PR:             258551
>     Reported by:    Franco Fichtner <franco at opnsense.org>, t_uemura at macome.co.jp
>     Reviewed by:    markj, Franco Fichtner <franco at opnsense.org>
>     Tested by:      t_uemura at macome.co.jp
>     MFC after:      1 day
> ---
>  sys/dev/e1000/if_em.c | 22 ++++++----------------
>  1 file changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c
> index 47513c5d9e1e..34d7b8f5f87e 100644
> --- a/sys/dev/e1000/if_em.c
> +++ b/sys/dev/e1000/if_em.c
> @@ -1495,7 +1495,6 @@ em_msix_link(void *arg)
>  {
>         struct e1000_softc *sc = arg;
>         u32 reg_icr;
> -       bool notlink = false;
>
>         ++sc->link_irq;
>         MPASS(sc->hw.back != NULL);
> @@ -1506,17 +1505,14 @@ em_msix_link(void *arg)
>
>         if (reg_icr & (E1000_ICR_RXSEQ | E1000_ICR_LSC))
>                 em_handle_link(sc->ctx);
> -       else
> -               notlink = true;
>
> -       /* Re-arm for other/spurious interrupts */
> -       if (notlink && sc->hw.mac.type >= igb_mac_min) {
> +       /* Re-arm unconditionally */
> +       if (sc->hw.mac.type >= igb_mac_min) {
>                 E1000_WRITE_REG(&sc->hw, E1000_IMS, E1000_IMS_LSC);
>                 E1000_WRITE_REG(&sc->hw, E1000_EIMS, sc->link_mask);
>         } else if (sc->hw.mac.type == e1000_82574) {
> -               if (notlink)
> -                       E1000_WRITE_REG(&sc->hw, E1000_IMS, E1000_IMS_LSC |
> -                           E1000_IMS_OTHER);
> +               E1000_WRITE_REG(&sc->hw, E1000_IMS, E1000_IMS_LSC |
> +                   E1000_IMS_OTHER);
>                 /*
>                  * Because we must read the ICR for this interrupt it may
>                  * clear other causes using autoclear, for this reason we
> @@ -1524,7 +1520,8 @@ em_msix_link(void *arg)
>                  */
>                 if (reg_icr)
>                         E1000_WRITE_REG(&sc->hw, E1000_ICS, sc->ims);
> -       }
> +       } else
> +               E1000_WRITE_REG(&sc->hw, E1000_IMS, E1000_IMS_LSC);
>
>         return (FILTER_HANDLED);
>  }
> @@ -1873,13 +1870,6 @@ em_if_update_admin_status(if_ctx_t ctx)
>
>         if (hw->mac.type < em_mac_min)
>                 lem_smartspeed(sc);
> -       else if (hw->mac.type >= igb_mac_min &&
> -           sc->intr_type == IFLIB_INTR_MSIX) {
> -               E1000_WRITE_REG(&sc->hw, E1000_IMS, E1000_IMS_LSC);
> -               E1000_WRITE_REG(&sc->hw, E1000_EIMS, sc->link_mask);
> -       } else if (hw->mac.type == e1000_82574 &&
> -           sc->intr_type == IFLIB_INTR_MSIX)
> -               E1000_WRITE_REG(hw, E1000_IMS, E1000_IMS_LSC | E1000_IMS_OTHER);
>  }
>
>  static void
> _______________________________________________
> dev-commits-src-main at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main
> To unsubscribe, send any mail to "dev-commits-src-main-unsubscribe at freebsd.org"