Page MenuHomeFreeBSD

busdma_iommu: map without extra offset bytes
ClosedPublic

Authored by dougm on May 17 2022, 9:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 2, 5:29 PM
Unknown Object (File)
Thu, May 2, 5:29 PM
Unknown Object (File)
Thu, May 2, 5:29 PM
Unknown Object (File)
Thu, May 2, 5:29 PM
Unknown Object (File)
Thu, May 2, 5:28 PM
Unknown Object (File)
Thu, May 2, 5:28 PM
Unknown Object (File)
Sat, Apr 20, 1:45 PM
Unknown Object (File)
Sat, Apr 13, 11:39 PM
Subscribers

Details

Summary

iommu_map takes offset and size parameters, and promises to allocate a range that satisfies boundary constraints. iommu_bus_dmamap_load_something1 adds offset to size before calling iommu_map, and shrinks the result by offset, perhaps to work around a bug that was in iommu_map before commit 11fced21ccea1b80327d159a4c27046cb1f46952. This change removes that addition and subtraction of offset to avoid requiring a larger address range than necessary.

Test Plan

Peter, a run with DMAR turned on, please.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dougm requested review of this revision.May 17 2022, 9:21 AM
dougm created this revision.

To test, ask Peter Holm to do a run with DMAR turned on.

sys/dev/iommu/busdma_iommu.c
619

I agree with the removal of the asserts and the clip above this one, but IMO tag maxsegsz check should be kept, at least in the form of the assert.

sys/dev/iommu/busdma_iommu.c
619

The maxsegsz assert is still there, at line 639(before)/605(after).

dougm added a subscriber: pho.

iommu_map->iommu_gas_map->iommu_gas_find_space requires that the size parameter be a multiple of page size, so restore the round-up to respect that.

panic: segment too large: ctx 0xfffff800041ca180 start 0xfef01000 end 0xfef24000 buflen1 0x23000 maxsegsz 0x22400

https://people.freebsd.org/~pho/stress/log/log0316.txt

I guess rounding buflen1 up to a multiple of page size, when maxsegsz is not a multiple of page size, is a problem. So restore check to reduce buflen1 to maxsegsz.

Kernel page fault with the following non-sleepable locks held:
exclusive rw vm object (vm object) r = 0 (0xfffff80004837b58) locked @ x86/iommu/intel_idpgtbl.c:550
exclusive sleep mutex AHCI channel lock (AHCI channel lock) r = 0 (0xfffffe003ce28400) locked @ kern/kern_mutex.c:211
stack backtrace:
#0 0xffffffff80c85445 at witness_debugger+0x65
#1 0xffffffff80c8659a at witness_warn+0x3ea
#2 0xffffffff810fcce6 at trap_pfault+0x86
#3 0xffffffff810cdc18 at calltrap+0x8
#4 0xffffffff81078e7e at iommu_gas_map+0x15e
#5 0xffffffff81077339 at iommu_bus_dmamap_load_something+0x119
#6 0xffffffff81076995 at iommu_bus_dmamap_load_buffer+0x1c5
#7 0xffffffff80c55a3e at _bus_dmamap_load_ccb+0x20e
#8 0xffffffff80c557cc at bus_dmamap_load_ccb+0x8c
#9 0xffffffff803929d9 at xpt_run_devq+0x2f9
#10 0xffffffff80395de7 at xpt_release_simq+0x67
#11 0xffffffff80c3027a at softclock_call_cc+0x15a
#12 0xffffffff80c31b96 at softclock_thread+0xc6
#13 0xffffffff80bc9850 at fork_exit+0x80
#14 0xffffffff810cec8e at fork_trampoline+0xe

https://people.freebsd.org/~pho/stress/log/log0317.txt

Remove 'offset' in computing initial size, but not when when examining iommu_map product.

Recover missing "- offset".

Remove the material part of this patch, because a stealth reviewer finds a bug in it, and leave only the style bits which do simplify some obscure code.

alc added inline comments.
sys/dev/iommu/busdma_iommu.c
602–605

I would recommend adding a comment here to say that this is handling a split buffer.

This revision is now accepted and ready to land.Jun 2 2022, 5:05 PM
dougm removed a subscriber: alc.

Add recommended comment.

This revision now requires review to proceed.Jun 2 2022, 6:12 PM
This revision is now accepted and ready to land.Jun 2 2022, 8:01 PM
This revision was automatically updated to reflect the committed changes.