Add support for new Intel Ethernet E610 family devices - Part 1

[Yogesh.Bhosale_intel.com]

Summary

Add support for new Intel Ethernet E610 family devices

Introduce new PCI device IDs:

• 57AE: Intel(R) E610 (Backplane)
• 57AF: Intel(R) E610 (SFP)
• 57B0: Intel(R) E610 (10 GbE)
• 57B1: Intel(R) E610 (2.5 GbE)
• 57B2: Intel(R) E610 (SGMII)

Key updates for E610 family:

• Firmware manages Link and PHY
• Implement new CSR-based Admin Command Interface (ACI) for SW-FW interaction
• Support exclusively for x64 operating systems; no 32-bit OS support
• Enable link speeds above 1G: 2.5G, 5G and 10G
• NVM Recovery Mode and Rollback support

Signed-off-by: Yogesh Bhosale [e-mail]
Co-developed-by: Krzysztof Galazka [e-mail]

[kgalazka]

@erj, @kbowling: We're still awaiting for feedback from validation team, but would you mind taking a look?

[erj]

In the description:

Support exclusively for x64 operating systems; no 32-bit OS support

Doesn't the in-kernel ixgbe driver still build for i386? Does this just mean no "official" support when it's running on non amd64 kernels?

Enable link speeds above 1G: 2.5G, 5G and 10G

These are already enabled -- do you mean just beyond the initial 1G E610 part? It's a little confusing to add this when support for those link speeds already exists.

Otherwise,
I think it mostly looks good -- but what's going to be included in part 2?

sys/dev/ixgbe/if_ix.c

1589

This should say "Link", right?

3054

It feels like you could combine this call with the one below for ixgbe_shutdown_aci() to reduce the number of if-statements here; mta isn't needed in these E610-only cleanup calls?

sys/dev/ixgbe/ixgbe_type_e610.h

805

Any 25G Base-T parts on the horizon for Linkville? :p

Or, are those only coming in maybe a future follow-on...

[kbowling]

To add to what @erj said, multi-arch build is a requirement the same as on other OS projects. We have to provide some advance notice to drop things and this should be MFC'd back to 13 and 14. See build(7), 'make universe' or 'make kernels' is enough. I don't see anything off hand that is problematic, and there is no expectation on Intel for validation for odd platforms. But it has to build everywhere it currently does.

[kgalazka]

In D50067#1149007, @erj wrote:

In the description:

Support exclusively for x64 operating systems; no 32-bit OS support

Doesn't the in-kernel ixgbe driver still build for i386? Does this just mean no "official" support when it's running on non amd64 kernels?

It does build on i386 and other architectures. The intention was only to make it clear that it was tested only on x86. I should have notice that the choice of words is not very fortunate. We'll fix that.

To be sure I run make kernels as suggested by Kevin and there was only one error on arm.TEGRA124, but unrelated to ixgbe:

src/sys/dev/drm2/ttm/ttm_bo_vm.c:238:2: error: call to undeclared function 'pctrie_iter_reset'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
  238 |         pctrie_iter_reset(&pages);

Enable link speeds above 1G: 2.5G, 5G and 10G

These are already enabled -- do you mean just beyond the initial 1G E610 part? It's a little confusing to add this when support for those link speeds already exists.

Yeah, that also needs better wording.

Otherwise,
I think it mostly looks good -- but what's going to be included in part 2?

In part 2 we will add support for debug dump and in part 3 FW logging. I thought it may be easier to review that way.

[kbowling]

Please reupload with context (git diff -U9999). It also seems to not be based on FreeBSD main.

sys/dev/ixgbe/if_ix.c

1414

I'm a little confused by this, is this diff against git main? @erj already pointed out we have support for this.

2441

this seems unnecessary if you rebase on main

sys/dev/ixgbe/ixgbe.h

200

this should not be necessary

sys/dev/ixgbe/ixgbe_vf.c

659

I wonder if this should all just be folded into the top level case, and the mac type and NON_STD chacks dropped.. isn't link speed just informational for a VF?

[kbowling]

sys/dev/ixgbe/ixgbe_osdep.h

203

is the aci accessed by multiple threads? the iflib admin task, media status, init, etc is protected internally by the CTX_LOCK

I wonder if you might be able to change ixgbe_acquire_lock to
#ifdef INVARIANTS
sx_assert(iflib_ctx_lock_get(((struct e1000_osdep *)hw->back)->ctx), SX_XLOCKED)
#endif

and make the other lock functions no-op and see what a WITNESS kernel says.

240

It seems this is main usage of this beyond the small HW buffer limits is to get the shadow ROM in a contiguous buffer which the checksum is then computed incrementally in word size chunks anyways. It could go away with a small refactoring to just move the 4k reads to the right place?

246

it seems like this is used for 4k IXGBE_ACI_MAX_BUFFER_SIZE operations, is there any reason not to do that on the stack?

[kbowling]

sys/dev/ixgbe/ixgbe_osdep.h

246

Otherwise since the use is all serialized, malloc one or more buffers you need on init and access them as needed. I don't think it makes sense to expose the runtime path to malloc and the associated error handling.