Is someone else going to take over? Is this still something worthwhile to pursue? It seems like it based on the latest comments.

How did you find this? Do you have something automated that checks for this?

I committed this in rS361992.

This description looks better; thanks.

Before I commit this, I'd like to have a better description of the changes that are going into this patch; I see there are changes around Recovery Mode support, but I'd like things like significant bugs or feature additions to be called out since this is pretty large.

This was committed in rS361541.

Any other comments before I commit this?

• ice: Update MAINTAINERS file
• ice: Remove strange characters in ice_ddp README

• ice: Add entries to arm64 NOTES file

• ice: Add support for building on arm64 targets

In D21959#546203, @kbowling wrote:

Can we enable this in an unsupported state for other 64-bit archs (arm64. ppc64)? Just would like to see the files entry correct for such usage.

In D21959#546203, @kbowling wrote:

Can we enable this in an unsupported state for other 64-bit archs (arm64. ppc64)? Just would like to see the files entry correct for such usage.

• ice-ddp: Use "SRCTOP" instead of "S" in module Makefile

• Add em_if_needs_restart to igb's iflib device method list

• Use iflib_init_locked()

Fix incorrect type usage

• Follow Drew's suggestion and create an iflib method for this

This doesn't apply anymore, at least in part because of rS359438

I'll add @adrian to see if I'm using the firmware(9) functionality correctly enough; it's similar to how it's used in the Wi-Fi drivers I think. Add other reviewers if you think it might be helpful.

• Add ddp to kernel and build process
• ice: Use fancier ice module Makefile from OOT component
• Move ice files.amd64 entries to appropriate place
• ice: Update description in NOTES

• Remove extraneous line breaks from files.amd64
• ice: Fix spelling mistake caught by arcanist

• ice: Add missing includes to ice_iflib.h
• ice: Correct filename used in Makefile
• ice: Add files.amd64 entries for ice
• ice: Add "device ice" as a kernel config option; add it to GENERIC
• ice: Add ice module to sys/modules Makefile
• ice: Remove SR-IOV files/option from Makefile
• ice: Update ice driver code to a current version.

I'm generally okay with this, but with the caveat that we can't test to make sure any future changes we make will work on anything other than amd64.

The driver is only intended to work on amd64; I can accept PowerPC64 but we don't want to support this driver on 32-bit architectures.

Thanks for fixing the issue with IXL_DEBUG! The CPU_TO_LE16 changes look acceptable to me, but we can't guarantee we'll be vigilant about adding them to new code in the future since we only officially support amd64 for ixl.

So it sounds like you should keep the patch as-is, but then remove the limit sysctls and the arbitrary limit in a follow-on patch, possibly intended to be MFC'd in to 12.2?

One driver that is subject to the above scenario is the ixl driver.

I think this looks okay, but I don't think it'll apply to the Intel drivers, right?

Is there a reason that these syctls would need "CTLFLAG_NEEDSGIANT", or does this patch default to that flag because it's a safe default if the contents of the sysctl are unknown? I'm aware of the previous review that added these flags, but I didn't get to look at it too closely.

Can this get regenerated? It no longer applies cleanly.

It looks like this change was part of the recently committed D22912 / rS356107

In D22870#500744, @markj wrote:

IMO it makes slightly more sense to have the check in the ixl ioctl handler: it serves as a signal to anyone extending the ioctl handler that they should be careful, and doing so maintains the invariant that internal driver functions need not check for privileges. I am not the maintainer though so I don't insist on any particular approach.

In D22523#495057, @jacob.e.keller_intel.com wrote:

Hmm. I think I understand the change.. but it seems less intuitive.

Do you have any example of how much this improves? I.e. a C program with a large enough test case to show measurable improvement?

Builds okay now.

Don't remove the extra whitespace in if.c

Update patch to make it build on HEAD

I get a build error with this revision:

Adding @jpaetzel since he seemed to be the last person making updates to oce(4); is this change ok to make there? There's no functional change.