ioat(4): Just the cleanups / changes to users/jimarris/ioat

[cem]

Summary

Handle errors; use common error numbers instead of -1.

Add PCIIDs for newer QD/IOAT hardware (Haswell, Ivybridge, Broadwell).

Don't panic if a copy cannot be satisfied in one request, just fail it.

Use sysctls with CTL_*TUN instead of TUNABLE_FETCH in various places.

Not tested, just compiled it.

ioat(4): Drop LOGGING ifdef entirely

ioat(4): Style-cleanup tests / test code

Conditionalize /dev/ioat_test presense on hw.ioat.enable_ioat_test tunable.

But, compile it into the module unconditionally.

Update README to match reality.

Fix ioatcontrol Makefile to work with OBJDIR.

Test Plan

This is just the cleanups from jimharris's ioat branch to https://reviews.freebsd.org/D3456 .

[jimharris]

sys/dev/ioat/ioat.c

85

I don't think this works as a sysctl. This value is needed when the driver loads, to determine whether or not to force legacy interrupt usage. By time the sysctl is available to force legacy interrupt usage, it will be too late.

187

I agree with imp@ that this should go back to including both device and vendor ID in the table.

243

I missed an explicit pci_enable_busmaster() call here (with matching pci_disable_busmaster() in detach).

412

Suggest moving the ioat_read_4 and ioat_log_message be moved to your new ioat3_selftest function.

446

This function needs style updates too.

sys/dev/ioat/ioat_sysctl.h

32 ↗

(On Diff #8120)

Maybe ioat_logger.h and ioat_sysctl.h contents should just be transferred to ioat.h, since there's not much here.

sys/dev/ioat/ioat_test.c

60

The same comment applies here as the force_legacy_interrupts SYSCTL above. I think this needs to be a tunable. (But then I would argue if someone does not want the /dev/ioat_test device enabled, they should just not load the test driver.)

[cem]

sys/dev/ioat/ioat.c

85

Sure. I will change it to RDTUN.

412

Sure. Will do.

sys/dev/ioat/ioat_sysctl.h

32 ↗

(On Diff #8120)

You mean ioat_internal.h? Then ioat_logger.c would have to include all of the headers to define the types that ioat_internal.h depends on. We could also just merge ioat_logger.c into ioat.c — not much there either.

sys/dev/ioat/ioat_test.c

60

It is a tunable — it's just readable as a sysctl (RDTUN).

We could also split the /dev/ioat_test device out into a separate kld, but I figure it wasn't worth bothering. I don't imagine it adds much in the way of bloat to the size of the .ko.

(I don't like editing the Makefile to enable/disable the test device.)

[jimharris]

One more note - sys/conf/files.amd64 and sys/amd64/conf/NOTES should also be updated for ioat, so that it gets integrated into normal kernel builds.

You could add it to i386 too - the driver should work fine there too although I've never tested it. I think amd64 only is fine for now though.

sys/dev/ioat/ioat_sysctl.h

32 ↗

(On Diff #8120)

I agree on merging ioat_logger.c as well.

sys/dev/ioat/ioat_test.c

60

Sorry - I forgot oat_test was not a separate kld on my original branch. I think what you have here is fine.

One alternative would be to change enable_ioat_test to a SYSCTL_PROC, so that the cdev can be created/deleted dynamically (and the sysctl could even be invoked within the ioatcontrol utility).

[cem]

Will adjust files.amd64 and NOTES, thanks!

sys/dev/ioat/ioat_test.c

60

Hah, sure.

[cem]

Address CR feedback from D3456 and D3457.

[cem]

Add ioat to sys/modules/Makefile, cond'l on amd64

[cem]

This will stay here for historical interest, but it gets harder and harder to regenerate this diff because I'm touching files that have changed since user/jimharris/ioat branched from CURRENT. New changes will only land in D3456.