iommu static analysis cleanup

[rlibby]

Summary

A static analyzer complained about a couple instances of checking a
variable against NULL after already having dereferenced it.

• dmar_gas_alloc_region: remove the tautological NULL checks
• dmar_release_resources / dmar_fini_fault_log: don't deref unit->regs unless initialized.

And while here, fix an inverted initialization check in dmar_fini_qi.

Test Plan

So far just make buildkernel, but I can put this through a quick
internal sanity check. The dmar_release_resources path only executes on
dmar_attach error, which is to say, rarely.

[kib]

sys/x86/iommu/intel_fault.c

294 ↗

(On Diff #57407)

Why this move is needed ? Disabling interrupts regardless of the init status seems to be innocent and might be even useful.

sys/x86/iommu/intel_gas.c

563 ↗

(On Diff #57407)

Both these lines are fine.

sys/x86/iommu/intel_qi.c

428 ↗

(On Diff #57407)

This is fine.

[rlibby]

sys/x86/iommu/intel_fault.c

294 ↗

(On Diff #57407)

What the static analyzer noticed is that this depends on unit->regs
being initialized, but there's a check for unit->regs == NULL after this
point in dmar_release_resources().

That part is actually okay though, since in the current code,
dmar_release_resources() is only called after unit->regs is initialized.
(But perhaps it was contemplated that unit->regs could be NULL, else why
have the check?)

However another issue is that this also uses the unit->lock (DMAR_LOCK()),
but there are call sites where the mutex is not yet initialized. We
could move up the mutex initialization.

I'm happy to take your suggestion here, or to leave it to you.

[tychon]

sys/x86/iommu/intel_fault.c

294 ↗

(On Diff #57407)

If you wanted to you ... you could move the if (unit->regs != NULL) { earlier in dmar_release_resources(). I believe that will address the analyzer's concern and make it clear that regs is assumed to be initialized very early on.

[rlibby]

sys/x86/iommu/intel_fault.c

294 ↗

(On Diff #57407)

If you wanted to you ... you could move the if (unit->regs != NULL) { earlier in dmar_release_resources(). I believe that will address the analyzer's concern and make it clear that regs is assumed to be initialized very early on.

Yes, that would quiet the analyzer, but it wouldn't address the mutex initialization issue.