rename dmar->iommu

[br]

Summary

As part of splitting-out DMAR busdma backend, rename:

DMAR_LOCK -> IOMMU_LOCK
DMAR_ASSERT_LOCKED -> IOMMU_ASSERT_LOCKED
dmar_unit -> iommu_unit
dmar_domain -> iommu_domain
dmar_ctx -> iommu_device
dmar_map_entry->iommu_map_entry
DMAR_MAP_ENTRY*->IOMMU_MAP_ENTRY*
dmar_test_boundary -> iommu_test_boundary
ctx_tag -> device_tag
DMAR_GM -> IOMMU_MF
DMAR_DOMAIN -> IOMMU_DOMAIN
dmar_get_requester -> iommu_get_requester
dmar_bus_dma_is_dev_disabled -> iommu_bus_dma_is_dev_disabled
dmar_instantiate_ctx -> iommu_instantiate_device
dmar_bus_* -> iommu_bus_*
bus_dma_tag_dmar -> bus_dma_tag_iommu
bus_dmamap_dmar -> bus_dmamap_iommu
BUS_DMAMAP_DMAR* -> BUS_DMAMAP_IOMMU*
dmar_init_busdma -> iommu_init_busdma
dmar_fini_busdma -> iommu_fini_busdma
dmar_map_entries_tailq -> iommu_map_entries_tailq
bus_dma_dmar_impl -> bus_dma_iommu_impl

Test Plan

amd64 build and run with dmar enabled on X1 Carbon

[kib]

sys/x86/iommu/intel_dmar.h

177 ↗

(On Diff #74125)

I am not sure this would work out. You intent is to provide the arch-specific iommu_unit on armv8, right ?

I do not request you to do this, just discuss. IMO iommu_unit should be MI, and this x86 structure still being called dmar_unit. dmar_unit would embed iommu_unit as the first member, so you can trivially convert iommu_unit to dmar_unit. iommu_unit would contain something common to all implementations, perhaps lock, unit number, might be device.

Significant number of chunks in this patch then would be not needed, when passing dmar_unit to x86 DMAR code.

This is actually useful on x86 too, since there is AMD with its IOMMU which is not DMAR, but currently not supported.

[br]

sys/x86/iommu/intel_dmar.h

177 ↗

(On Diff #74125)

Yes, the intent is to provide the arch-specific iommu_unit, iommu_domain and iommu_device.

There is a small iommu framework/interface in arm64/iommu/ that defines struct iommu_unit, struct iommu_domain and struct iommu_device. The framework has no idea about SMMU. That allows us to keep SMMU driver small and simple, and to not overload if with unnecessary matters like busdma, etc.
The original idea was to use the framework on all platforms, but apparently everything is different on amd64, so I left it on arm64 only. It is used for VA space, devices and policies management on arm64, as well as busdma interactions.
So with that iommu framework/interface, the SMMU driver does not require to know about iommu_unit at all.

We already do the similar way that you proposed with smmu_domain. So smmu_domain embeds iommu_domain as a first member (see 'struct smmu_domain {' in https://reviews.freebsd.org/D24618).

So I think the idea you described does not fit well arm64 because of the framework, i.e. we already have another layer of abstraction on arm64 that keeps MMU drivers small. So renaming iommu_unit to smmu_unit may not be possible, unless we remove the framework.

For x86 it could be possible, let me check

[br]

Make the struct iommu_unit to be the first member of struct dmar_unit

[br]

Move IOMMU_LOCK macroses to busdma_dmar.h

[br]

sys/x86/iommu/intel_dmar.h

177 ↗

(On Diff #74125)

So here is an updated patch with struct iommu_unit as the first member of dmar_unit.
I moved a few members from dmar_unit to iommu_unit as needed by dmar_busdma.c.
It looks good I think (although it is the same size).
What do you think?

[kib]

sys/x86/iommu/intel_dmar.h

99 ↗

(On Diff #74169)

I do not think that the whole content of dmar_domain is relevant to other hw. For instance, mgaw/agaw/pglvl/awlvl are definitely dmar-specific.

125 ↗

(On Diff #74169)

Why this rename ? It is very strange. I would never call dmar context (this is hw-defined term) an iommu_device. In fact context is dmar-specific thing, it is some laziness on my part that currently it maps 1:1 to requestor, but I have unpublished patches that make context multi-device, which allows to use dmar driver from x86/iommu in bhyve.

sys/x86/iommu/intel_reg.h

54 ↗

(On Diff #74169)

This is hw-specific register, I see no point of renaming anything in intel_reg.h.

[br]

Rename: iommu_device_entry_t -> dmar_ctx_entry_t

[br]

sys/x86/iommu/intel_dmar.h

125 ↗

(On Diff #74169)

I can try to do similar change with dmar_ctx and dmar_domain as we did with dmar_unit.
(I.e. using iommu_device, iommu_domain as the first member)

sys/x86/iommu/intel_reg.h

54 ↗

(On Diff #74169)

Agree, fixed.

[br]

dmar_ctx and dmar_domain restored:
iommu_device and iommu_domain are first members of structs

[br]

restore M_DMAR_DOMAIN malloc define after sed

[br]

instead of renaming dmar functions, add wrappers for dmar_find(), dmar_domain_unload(), dmar_get_ctx_for_dev()

[kib]

sys/x86/iommu/busdma_dmar.h

78 ↗

(On Diff #74227)

You moved structures' definitions together with the locking annotations, but did not copied the legend.

Also I do not like the move of iommu_unit/devce (still device ?) into bsudma_dmar.h. I think this part deserves a new header in sys/sys.

85 ↗

(On Diff #74227)

I t is not a device, I already commented on this. It is still iommu translation context.

sys/x86/iommu/intel_dmar.h

49 ↗

(On Diff #74227)

iommu_map_entry is a good candidate to move together with iommu_unit etc. Together with flags definitions.

82 ↗

(On Diff #74227)

This is the locking annotations I mentioned earlier.

[br]

Restore dmar_unit member in struct dmar_domain. This makes patch smaller

[br]

Rename struct iommu_device to struct iommu_ctx

[br]

Move iommu ctx/domain/unit to sys/iommu.h
Move iommu_map_entry to sys/iommu.h

[br]

sys/x86/iommu/busdma_dmar.h

78 ↗

(On Diff #74227)

I had to change struct bus_dma_tag_iommu to struct bus_dma_tag_iommu * in sys/iommu.h, otherwise it wont build

85 ↗

(On Diff #74227)

Agree, device renamed to ctx!

sys/x86/iommu/intel_dmar.h

49 ↗

(On Diff #74227)

Agree, moved!

[br]

Move some prototypes to sys/iommu.h

[br]

add some iommu wrappers for gas functions: dmar_gas_map(), dmar_gas_map_region(), dmar_gas_alloc_entry, etc

[br]

o Check if a device is disabled using "iommu" string
o Rename busdma taskqueue
o Use both "hw.iommu.dma" and "hw.dmar.dma" tunables

[br]

Regenerate

[br]

Rename DMAR_CTX_ flags to IOMMU_CTX_
Move to sys/iommu.h

[br]

@kib what do you think ?

[kib]

sys/sys/iommu.h

36 ↗

(On Diff #74397)

I suggest to include <sys/queue.h> explicitly instead of relying on the namespace pollution, if you include from includes anyway. Also you should add sys/types.h.