Avoid some out-of-range DMA accesses with PCI intel ethernet adapters.
ClosedPublic

Authored by kib on Oct 19 2016, 12:01 PM.

Details

Summary

This change sit in my local tree for very long time. It caused a conflict with recent netmap update, so I think it could be useful for me to get rid of it, either commit or revert locally.

The issue is that pre-PCIe, i.e. plain parallel PCI adapters from Intel, managed by the lem(4) drivers, apparently have very serious hardware bugs in the DMA engines. Both descriptor ring accesses, and packet buffer accesses, read past the end of the regions. I have a haswell machine with PCIe/PCI bridge, and 82541PI two-port card. If VT-d is enabled, card generates a fault for almost any access.

The patch in the review extends the descriptor rings by one element each, which fixes overruns there and rather cleanly demonstrates the bug. Unfortunately, I was not able to develop any form of sane fix for the packet memory accesses. So the patch is only partial.

Test Plan

Intel did not responded to the inquiries about the problem. This issue prevented me from having a working environment to integrate ACPI_DMAR and vmm.ko.

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.
kib updated this revision to Diff 21495.Oct 19 2016, 12:01 PM
kib retitled this revision from to Avoid some out-of-range DMA accesses with PCI intel ethernet adapters..
kib updated this object.
kib edited the test plan for this revision. (Show Details)
kib added reviewers: sbruno, jhb.
kib set the repository for this revision to rS FreeBSD src repository.
kib added a subscriber: emaste.
sbruno accepted this revision.Oct 19 2016, 5:28 PM
sbruno edited edge metadata.

Please commit this at your convenience.

This revision is now accepted and ready to land.Oct 19 2016, 5:28 PM
emaste added inline comments.Oct 19 2016, 5:34 PM
sys/dev/e1000/if_lem.c
546–547 ↗(On Diff #21495)

There should be a comment explaining the + 1 I think, it does not have to be overly detailed but should at least provide a "bread crumb" to suggest this is intentional.

erj requested changes to this revision.Oct 19 2016, 5:37 PM
erj added a reviewer: erj.
erj added a subscriber: erj.

I agree with @emaste's request for a comment.

This revision now requires changes to proceed.Oct 19 2016, 5:37 PM
kib updated this revision to Diff 21505.Oct 19 2016, 6:01 PM
kib edited edge metadata.

Add comment.

emaste accepted this revision.Oct 19 2016, 6:02 PM
emaste added a reviewer: emaste.
erj accepted this revision.Oct 19 2016, 6:05 PM
erj edited edge metadata.
This revision is now accepted and ready to land.Oct 19 2016, 6:05 PM
This revision was automatically updated to reflect the committed changes.