commit 442b29735720936f85cc5372113e101b00cfba4e
author Christopher Anderson <cja@google.com> Fri Jul 09 22:50:49 2021 +0000
committer CQ Bot <commit-bot@chromium.org> Fri Jul 09 22:50:49 2021 +0000
tree 6a75394fb3f292da9213edf30481938cee1973e1
parent 2190fd5fe3a0f72026261b55ed3b597c70a2babc

[x64][interrupts] Fix locking inversion in InterruptManager

During interrupt handler registration the InterruptManager
acquires its spinlock before calling InterruptTableEntry::SetHandler,
which acquires the table entry's spinlock.

InterruptManager::RegisterInterruptHandler (acquires IM::lock_)
  InterruptTableEntry::SetHandler (acquires ITE::lock_)

However, if an interrupt is configured as non-permanent with the option
INTERRUPT_MASK_POSTWAIT then during the handler dispatch path the
InterruptTableEntry lock will be acquired before the
InterruptEventDispatcher's IrqHandler calls into
InterruptManager::MaskInterrupt, which acquires the InterruptManager
lock.

platform_irq
  InterruptManager::InvokeX86Vector
    InterruptTableEntry::InvokeIfPresent (acquires ITE::lock_)
      InterruptEventDispatcher::IrqHandler
        InterruptManager::MaskInterrupt (acquires IM::lock_)

Fortunately, this is an easy fix because the locking in MaskInterrupt
and UnmaskInterrupt is unnecessary. The call stack looks like:

InterruptDispatcher::IrqHandler
  InterruptManager::MaskInterrupt (acquires lock_)
    IoApic::MaskIrq
      apic_io_mask_irq (acquires apic singleton lock)

InterruptManager will not go out of scope and those methods do nothing
but call into the IoApic with a vector number, so no state needs to be
protected in the InterruptManager or InterruptTableEntry. IoApic
maintains its own locking via a singleton lock acquired by
apic_io_mask_irq.

Change-Id: Ib8b1be10adf7a87f8a360db5269b763b9324ee54
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/550048
Commit-Queue: Christopher Anderson <cja@google.com>
Reviewed-by: Nick Maniscalco <maniscalco@google.com>
Reviewed-by: Adrian Danis <adanis@google.com>