Segment base registers are at 8-byte intervals, while the register
write helper takes a byte-aligned offset. This fixes
DEV_TAB_HARDWARE_ERROR events and associated peripheral I/O failures
on an Epyc-based system with 8-segment device tables.
Details
- Reviewers
kib - Commits
- rG5035db222e8f: amdiommu: Fix device table segment base register offsets
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
sys/x86/iommu/amd_drv.c | ||
---|---|---|
280 | Do we have an appropriate struct type to allow a more (human) friendly way to write this? e.g. (i - 1) * sizeof(struct some_struct) instead of (i - 1) << magic_number? |
sys/x86/iommu/amd_drv.c | ||
---|---|---|
280 | And if we don't have a struct that matches the bitfields inside reg would (i - 1) * sizeof(reg) make sense? |
sys/x86/iommu/amd_drv.c | ||
---|---|---|
280 | Is this better? reg = i == 0 ? AMDIOMMU_DEVTAB_BASE : AMDIOMMU_DEVTAB_S1_BASE + (i - 1) * (AMDIOMMU_DEVTAB_S2_BASE - AMDIOMMU_DEVTAB_S1_BASE)); |
sys/x86/iommu/amd_drv.c | ||
---|---|---|
280 | If we're that concerned about readability here, it seems as though it would be better to just have a small static array with the relevant DEVTAB offset in each entry. That would avoid the special "i == 0" case and would also allow us to add a KASSERT and to easily add entries if AMD should ever decide to add support for more than 8 segments (which would also necessitate placing the new segment base registers in some area not contiguous with the current S1_BASE-S7_BASE) |