e1000: Add support for Ice Lake and Cannon Lake
ClosedPublic

Authored by kbowling on Dec 28 2017, 4:57 AM.

Details

Summary

This adds initial support for Ice Lake and Cannon Lake ethernet.

It also addresses 1.5.4.4 in https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/i218-i219-ethernet-connection-spec-update.pdf?asset=9561 for Sky Lake and Kabby Lake.

Test Plan

Untested, no access to HW yet

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kbowling created this revision.Dec 28 2017, 4:57 AM
kbowling updated this revision to Diff 37123.Dec 28 2017, 4:59 AM
sbruno accepted this revision.Dec 28 2017, 5:10 PM
This revision is now accepted and ready to land.Dec 28 2017, 5:10 PM
This revision was automatically updated to reflect the committed changes.
erj added a subscriber: erj.Jan 2 2018, 8:38 PM

We don't have anything for the Ice Lake hardware, but the Cannon Lake stuff looks like it's good.

head/sys/dev/e1000/if_em.c
3077

The line above already sets this bit, along with another. In fact, the line above clears everything but bit 28 and 29, and possibly those two as well.

Anyway, how does this fix the errata in 1.5.4.4? The description doesn't mention doing anything to the TARC0 register.

kbowling added inline comments.Jan 3 2018, 9:15 AM
head/sys/dev/e1000/if_em.c
3077

@erj this was taken from the Linux team's shared code https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/intel/e1000e?id=c0f4b163a03e73055dd734eaca64b9580e72e7fb

I'm not sure whether they intended or not to toggle all the bits but it made it past a bunch of people :/

The root cause for the fix seems to be attempting to slow down DMA https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/intel/e1000e?id=b10effb92e272051dd1ec0d7be56bf9ca85ab927

We can throw this at CNL and CFL ( Cannon Lake/Coffee Lake).

erj added inline comments.Jan 4 2018, 7:12 PM
head/sys/dev/e1000/if_em.c
3077

The linked patch includes the proper "~" before the constant in the block of code in netdev.c, so I think that was just missed here.

And my confusion in the second part was due to a typo in the comment -- this fix is for 1.5.4.5, not 1.5.4.4. Modifying TARC0 (it looks like it's a register for enabling/disabling hardware workarounds) wouldn't be unexpected for something like that issue.

erj added inline comments.Jan 4 2018, 7:18 PM
head/sys/dev/e1000/if_em.c
3077

And to nitpick a little bit -- netdev.c isn't shared code; that's Linux-specific.

It appears the client team is committing important fixes directly to the Linux code without notifying us. :(

kbowling added inline comments.
head/sys/dev/e1000/if_em.c
3077

@erj ok, I can't directly commit but I suggest you or @sbruno either backing out this errata part of my patch or just fixing it, these were both typos and my fault. @cramerj_intel.com can you settle up with the client team to figure out better communication with the shared teams?