Page MenuHomeFreeBSD

Split-out DMAR busdma backend
AbandonedPublic

Authored by br on Jun 1 2020, 1:43 PM.

Details

Reviewers
kib
Group Reviewers
x86
Summary

Split-out DMAR backend to some generic place so it could be used by ARM SMMU.

This patch carries no functional change, but it involves a name space changing:
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_LOCK -> IOMMU_LOCK
DMAR_UNLOCK -> IOMMU_UNLOCK
ctx_tag -> device_tag

(some wrappers are added to the end of sys/x86/iommu/busdma_dmar.c)

any suggestions welcome

Test Plan

Tested booting on X1 carbon with DMAR device attached.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

br requested review of this revision.Jun 1 2020, 1:43 PM
br created this revision.

o Don't declare iommu_gaddr_t since it is only a single occurance in the busdma_iommu.c, use bus_size_t instead and then cast it to dmar_gaddr_t.
o Fix style

Can you split this further ? For instance, rename dmar->iommu perhaps can be done before moving files to a new location.

sys/dev/iommu/busdma_iommu.c
67

This code is not amd64 only, it i for i386 as well.

90

Do you care to provide compatible command "dmar" ?

sys/dev/iommu/busdma_iommu.h
75

This lines can be used more. That said, why do you need these prototypes for simple move around of code ?

80

rmrr is something very x86 specific, BTW.

85

Style: no space after star.

sys/x86/iommu/busdma_dmar.c
70–71

I am confused, why this function is copied and the original still left around ?

o Fix style
o Remove duplicate dmar_get_requester()
o Add 'dmar' hw.busdma.default compatibility string

br marked 2 inline comments as done.Jul 6 2020, 2:26 PM
In D25094#565471, @kib wrote:

Can you split this further ? For instance, rename dmar->iommu perhaps can be done before moving files to a new location.

OK I can try

sys/dev/iommu/busdma_iommu.h
75

Style fixed.
Because busdma backend moved, but implementation of iommu_map() for x86 is not moved

80

So what to do with that? Rename to "bool arg" ?

85

Fixed!

sys/x86/iommu/busdma_dmar.c
70–71

Sorry, left by mistake. Removed!

sys/dev/iommu/busdma_iommu.h
80

I am not sure. Do you plan to use this interface (iommu_get_device) as is for SMMU ? Does SMMU classifiy TLPs based on originator rids ? Are there some kind of pre-established mappings that secure OS or hypervisor require from normal OS to maintain ?

RMRR is something that x86 ACPI requires from OS to keep in setting up DMAR translations. For instance, it could be display aperture used by SMM, or USB transfers used to emulate PC keyboard controller. Handling of RMRR is some significant (but not too large) part of the DMAR code. I make sure that specific requestor never get DMAR translation enabled without setting up RMRR mappings, if ACPI requires RMRR.

br marked 2 inline comments as done.Jul 7 2020, 12:14 PM
br added inline comments.
sys/dev/iommu/busdma_iommu.h
80

Yes, iommu_get_device() is used for SMMU.
SMMU has support for PASID TLP for stage 1 translation, but it is optional and we don't use it.
So the stage 1 traffic is associated with a (RequesterID, PASID) tuple. That is, the PASID differentiates between different stage 1 translation contexts.
Yes, we need to pre-establish a single GICv3 mapping (a page, write-only) so a device could send MSI interrupt by writing to the GICv3 register. We don't need other mappings.

In D25094#565471, @kib wrote:

Can you split this further ? For instance, rename dmar->iommu perhaps can be done before moving files to a new location.

Here is most of renamings that we can start with. Can you look ?
https://reviews.freebsd.org/D25574