Manage MSI iommu pages

[br]

Summary

This allows the interrupt controller driver only need a small change to create a map for the page the device will write to raise an interrupt.

o Split-out GAS macroses to a separate header iommu_gas.h
o Add iommu_msi.h

[kib]

Convention for var.h is not suitable for your case IMO. We have signal.h and signalvar.h, where signal.h is usable by userspace and signalvar.h is for kernel private stuff.

Can you provide some enumeration of stuff you want to keep in iommu.h, and an explanation of why ?

[br]

In D26705#595130, @kib wrote:

Convention for var.h is not suitable for your case IMO. We have signal.h and signalvar.h, where signal.h is usable by userspace and signalvar.h is for kernel private stuff.

Can you provide some enumeration of stuff you want to keep in iommu.h, and an explanation of why ?

I'm not insisting on this, Andrew wanted to include iommu.h from subr_intr.c, but he does not want to include pci_reg.h and pci_var.h as dependencies for iommu.h, so a split requested.

There are a few new prototypes needed from iommu.h to manage MSI iommu pages:
int iommu_map_msi(struct iommu_ctx *ctx, iommu_gaddr_t size, int offset, u_int eflags, u_int flags, vm_page_t *ma);
void iommu_translate_msi(struct iommu_domain *domain, uint64_t *addr);
struct iommu_domain *iommu_get_ctx_domain(struct iommu_ctx *ctx);
struct iommu_ctx *iommu_get_dev_ctx(device_t dev);

See for details https://reviews.freebsd.org/D24618

[kib]

So would adding a header iommu_msi.h with just msi-related prototypes enough ? I suspect that x86/msi.c can also get rid of iommu.h then.

[andrew]

I'd like to use some of the iommu functions in device code, e.g. non-pci devices on arm64 can be behind an iommu. I'd prefer to not have to teach these devices about PCI just to configure the iommu.

[br]

In D26705#595261, @kib wrote:

So would adding a header iommu_msi.h with just msi-related prototypes enough ? I suspect that x86/msi.c can also get rid of iommu.h then.

I will try and see if it works

[br]

Manage MSI iommu pages

[br]

In D26705#595414, @br wrote:

In D26705#595261, @kib wrote:

So would adding a header iommu_msi.h with just msi-related prototypes enough ? I suspect that x86/msi.c can also get rid of iommu.h then.

I will try and see if it works

It works but not sure what to do with typedef iommu_haddr_t ? I put it to sys/types.h since it is needed by iommu_msi.h and iommu.h

[br]

context added

[kib]

sys/dev/iommu/busdma_iommu.c

275 ↗

(On Diff #78015)

I suggest to commit iommu_get_dev_ctx() addition right away.

893 ↗

(On Diff #78015)

I think there should be per-arch define like IOMMU_DOMAIN_UNLOAD_SLEEP, instead of listing arches directly.

sys/sys/types.h

443 ↗

(On Diff #78015)

This contamination of standard header is forbidden . May be add some header in dev/iommu, for instance iommu_types.h.

[br]

Address kib's notes:

• add iommu_types.h
• add md macro IOMMU_UNLOAD_SLEEP

[kib]

sys/dev/iommu/busdma_iommu.c

64 ↗

(On Diff #78161)

What the gas.h header is need ? busdma_iommu.h includes it.

893 ↗

(On Diff #78161)

#ifndef

sys/dev/iommu/iommu.h

35 ↗

(On Diff #78161)

Please commit this separately.

sys/dev/iommu/iommu_gas.c

214 ↗

(On Diff #78161)

Commit this separately.

sys/dev/iommu/iommu_types.h

42 ↗

(On Diff #78161)

Can you extract iommu_types.h addition into a separate patch ?

[br]

Address kibs's notes

[br]

Regenerate

[br]

sys/dev/iommu/busdma_iommu.c

64 ↗

(On Diff #78161)

Not needed, thanks!

sys/dev/iommu/iommu_gas.c

214 ↗

(On Diff #78161)

Committed!

sys/dev/iommu/iommu_types.h

42 ↗

(On Diff #78161)

Yes, extracted and committed.

[kib]

I am curious. There is no arch restriction on the value of MSI base address ? System can map MSI page anywhere, and just programming the correct address into MSI address register would make it work ?

This is very non-x86ish.

sys/dev/iommu/iommu.h

116 ↗

(On Diff #78260)

So these three fields are in fact MD ?

sys/dev/iommu/iommu_msi.h

7 ↗

(On Diff #78260)

Really ?

46 ↗

(On Diff #78260)

I do not think that this is an appropriate header for get_ctx_domain() and get_dev_cxt().

sys/kern/msi_if.m

81 ↗

(On Diff #78260)

This #ifdef makes the header correct usage critically depend on the previous include of opt_iommu.h. I do not see any problem to declare the method unconditionally.

[br]

o Put Andrew's copyright
o Remove ifdef IOMMU

[br]

In D26705#598296, @kib wrote:

I am curious. There is no arch restriction on the value of MSI base address ? System can map MSI page anywhere, and just programming the correct address into MSI address register would make it work ?

This is very non-x86ish.

No restrictions. MSI address could be anywhere

sys/dev/iommu/iommu.h

116 ↗

(On Diff #78260)

Since you don't have these on x86, then yes

sys/dev/iommu/iommu_msi.h

7 ↗

(On Diff #78260)

I put Andrew's copyright since he wrote this part

46 ↗

(On Diff #78260)

Agree

sys/kern/msi_if.m

81 ↗

(On Diff #78260)

Ok, removed

[br]

o Move iommu_get_ctx_domain(), iommu_get_dev_ctx() to iommu.h

[br]

Regenerate

[kib]

sys/dev/iommu/iommu.h

116 ↗

(On Diff #78260)

Then mark them so in comments.

sys/dev/iommu/iommu_gas.c

782 ↗

(On Diff #78396)

Don't you want to assert that address is below the msi_entry end ? In fact, address + 8.

[br]

o Add a KASSERT
o Mark MSI fields in the iommu_domain as arch-specific

[br]

sys/dev/iommu/iommu.h

116 ↗

(On Diff #78260)

Maked

sys/dev/iommu/iommu_gas.c

782 ↗

(On Diff #78396)

Added

[kib]

sys/dev/iommu/busdma_iommu.c

63 ↗

(On Diff #78417)

Why this header is needed for busdma_iommu.c ?

sys/dev/iommu/iommu.h

114 ↗

(On Diff #78417)

Look at x86/iommu/intel_ctx.c:dmaor_domain_alloc(), the line under the
'Disable local apic region access'. I suggest to assign the resulting entry to msi_entry, as a follow-up commit.

[br]

sys/dev/iommu/busdma_iommu.c

63 ↗

(On Diff #78417)

Not needed, thanks