fix: kernel concurrency, interrupt context, and user VA safety
This commit is contained in:
@@ -15,6 +15,7 @@
|
||||
#include <CppLib/Stream.hpp>
|
||||
#include <CppLib/Spinlock.hpp>
|
||||
#include <Libraries/Memory.hpp>
|
||||
#include <atomic>
|
||||
|
||||
namespace Drivers::Audio::IntelHda {
|
||||
|
||||
@@ -26,10 +27,33 @@ namespace Drivers::Audio::IntelHda {
|
||||
|
||||
static bool g_initialized = false;
|
||||
static kcp::Spinlock g_codecLock;
|
||||
// Serializes stream lifecycle, DMA write-pointer updates, and IRQ-side
|
||||
// stream inspection. The IRQ releases it before calling the mixer to keep
|
||||
// the lock order consistently Mixer -> HDA stream.
|
||||
// Serializes stream lifecycle and DMA write-pointer updates. Keep logging
|
||||
// out of this lock: Spinlock disables local interrupts, while the terminal
|
||||
// logger may wait for a lock owner that needs the local timer to run.
|
||||
static kcp::Spinlock g_streamLock;
|
||||
// The hard IRQ only acknowledges and masks stream completion sources.
|
||||
// Mixing/refilling is substantially too expensive for interrupt context.
|
||||
static std::atomic<bool> g_streamWorkPending{false};
|
||||
static uint32_t g_streamInterruptMask = 0;
|
||||
|
||||
// Runtime diagnostics are emitted by ProcessDeferredWork() in idle
|
||||
// context. Some HDA entry points run below the mixer's interrupt-disabling
|
||||
// spinlock, so even logging after releasing g_streamLock is not safe.
|
||||
enum DeferredDiagnostic : uint32_t {
|
||||
DIAG_STREAM_OPENED = 1u << 0,
|
||||
DIAG_STREAM_CLOSED = 1u << 1,
|
||||
DIAG_HEADPHONES = 1u << 2,
|
||||
DIAG_SPEAKERS = 1u << 3,
|
||||
};
|
||||
static std::atomic<uint32_t> g_diagnosticsPending{0};
|
||||
static std::atomic<uint32_t> g_openSampleRate{0};
|
||||
static std::atomic<uint32_t> g_openBitsPerSample{0};
|
||||
static std::atomic<uint32_t> g_openChannels{0};
|
||||
static std::atomic<uint32_t> g_codecTimeoutsPending{0};
|
||||
static std::atomic<uint32_t> g_codecTimeoutReports{0};
|
||||
static std::atomic<uint32_t> g_lastTimeoutCodec{0};
|
||||
static std::atomic<uint32_t> g_lastTimeoutNid{0};
|
||||
static std::atomic<uint32_t> g_lastTimeoutVerb{0};
|
||||
|
||||
static volatile uint8_t* g_mmioBase = nullptr;
|
||||
static uint8_t g_bus, g_dev, g_func;
|
||||
@@ -213,9 +237,10 @@ namespace Drivers::Audio::IntelHda {
|
||||
uint32_t response = 0;
|
||||
if (!ReadResponse(&response, nullptr)) {
|
||||
g_codecLock.Release();
|
||||
KernelLogStream(WARNING, "HDA") << "Verb timeout: codec=" << base::dec
|
||||
<< (uint64_t)codec << " nid=" << (uint64_t)nid
|
||||
<< " verb=" << base::hex << (uint64_t)verb;
|
||||
g_lastTimeoutCodec.store(codec, std::memory_order_relaxed);
|
||||
g_lastTimeoutNid.store(nid, std::memory_order_relaxed);
|
||||
g_lastTimeoutVerb.store(verb, std::memory_order_relaxed);
|
||||
g_codecTimeoutsPending.fetch_add(1, std::memory_order_release);
|
||||
return 0;
|
||||
}
|
||||
g_codecLock.Release();
|
||||
@@ -640,13 +665,15 @@ namespace Drivers::Audio::IntelHda {
|
||||
DisablePin(g_speakerNid);
|
||||
EnablePin(g_hpNid);
|
||||
g_pinNid = g_hpNid;
|
||||
KernelLogStream(INFO, "HDA") << "Switched to headphone output";
|
||||
g_diagnosticsPending.fetch_or(DIAG_HEADPHONES,
|
||||
std::memory_order_release);
|
||||
} else {
|
||||
// Mute HP, enable speaker
|
||||
DisablePin(g_hpNid);
|
||||
EnablePin(g_speakerNid);
|
||||
g_pinNid = g_speakerNid;
|
||||
KernelLogStream(INFO, "HDA") << "Switched to speaker output";
|
||||
g_diagnosticsPending.fetch_or(DIAG_SPEAKERS,
|
||||
std::memory_order_release);
|
||||
}
|
||||
// Re-apply volume on the now-active pin
|
||||
SetOutputVolume(g_volume);
|
||||
@@ -810,7 +837,7 @@ namespace Drivers::Audio::IntelHda {
|
||||
// MSI setup
|
||||
// =========================================================================
|
||||
|
||||
static void HandleInterrupt(uint8_t irq);
|
||||
static void HandleInterrupt(uint8_t irq, bool fromUser);
|
||||
|
||||
static bool SetupMsi(uint8_t bus, uint8_t dev, uint8_t func) {
|
||||
uint8_t cap = Pci::FindCapability(bus, dev, func, Pci::PCI_CAP_MSI);
|
||||
@@ -819,6 +846,8 @@ namespace Drivers::Audio::IntelHda {
|
||||
return false;
|
||||
}
|
||||
|
||||
Pci::DisableInterruptDelivery(bus, dev, func);
|
||||
|
||||
uint16_t msgCtrl = Pci::LegacyRead16(bus, dev, func, cap + 2);
|
||||
bool is64bit = (msgCtrl & (1 << 7)) != 0;
|
||||
|
||||
@@ -851,21 +880,21 @@ namespace Drivers::Audio::IntelHda {
|
||||
// Interrupt handler
|
||||
// =========================================================================
|
||||
|
||||
static void HandleInterrupt(uint8_t /*irq*/) {
|
||||
static void HandleInterrupt(uint8_t /*irq*/, bool /*fromUser*/) {
|
||||
uint32_t intsts = Read32(REG_INTSTS);
|
||||
bool bufferCompleted = false;
|
||||
|
||||
// Handle stream interrupts (bits 0-29 correspond to stream descriptors)
|
||||
g_streamLock.Acquire();
|
||||
if (g_stream.Active) {
|
||||
uint8_t si = g_stream.StreamIndex;
|
||||
if (intsts & (1u << si)) {
|
||||
uint8_t sts = ReadSD8(si, SD_STS);
|
||||
if (sts & SD_STS_BCIS) bufferCompleted = true;
|
||||
WriteSD8(si, SD_STS, sts);
|
||||
}
|
||||
// Handle stream interrupts (bits 0-29 correspond to stream
|
||||
// descriptors) without taking g_streamLock. Inspecting only the
|
||||
// hardware status registers is race-safe against stream lifecycle and
|
||||
// avoids spinning in an IRQ if another CPU is opening/closing audio.
|
||||
uint32_t streamBits = intsts & 0x3FFFFFFFu;
|
||||
for (uint8_t si = 0; si < 30; si++) {
|
||||
if (!(streamBits & (1u << si))) continue;
|
||||
uint8_t sts = ReadSD8(si, SD_STS);
|
||||
if (sts & SD_STS_BCIS) bufferCompleted = true;
|
||||
WriteSD8(si, SD_STS, sts);
|
||||
}
|
||||
g_streamLock.Release();
|
||||
|
||||
// Handle RIRB interrupt (controller interrupt enable bit 30)
|
||||
// Do NOT advance g_rirbReadPtr here — ReadResponse() owns it.
|
||||
@@ -876,10 +905,12 @@ namespace Drivers::Audio::IntelHda {
|
||||
Write8(REG_RIRBSTS, rirbSts);
|
||||
}
|
||||
|
||||
// Notify the mixer so it can refill the DMA ring with the next mix
|
||||
// window. Done after clearing status bits so re-entry can't latch.
|
||||
// A mixer pass can resample up to 4096 frames across every active
|
||||
// stream and must never run inside this high-priority IRQ. Mask stream
|
||||
// completion delivery and queue a process-safe bottom half instead.
|
||||
if (bufferCompleted) {
|
||||
Mixer::OnHdaBufferComplete();
|
||||
Write32(REG_INTCTL, Read32(REG_INTCTL) & ~g_streamInterruptMask);
|
||||
g_streamWorkPending.store(true, std::memory_order_release);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -987,7 +1018,13 @@ namespace Drivers::Audio::IntelHda {
|
||||
uint8_t irqLine = Pci::LegacyRead8(g_bus, g_dev, g_func, (uint8_t)Pci::PCI_REG_INTERRUPT);
|
||||
if (irqLine != 0xFF) {
|
||||
KernelLogStream(INFO, "HDA") << "Falling back to legacy IRQ " << base::dec << (uint64_t)irqLine;
|
||||
Pci::DisableInterruptDelivery(g_bus, g_dev, g_func);
|
||||
uint16_t command = Pci::LegacyRead16(g_bus, g_dev, g_func,
|
||||
(uint8_t)Pci::PCI_REG_COMMAND);
|
||||
Pci::LegacyWrite16(g_bus, g_dev, g_func,
|
||||
(uint8_t)Pci::PCI_REG_COMMAND, command & ~Pci::PCI_CMD_INTX_DISABLE);
|
||||
Hal::RegisterIrqHandler(irqLine, HandleInterrupt);
|
||||
Hal::IoApic::UnmaskIrq(Hal::IoApic::GetGsiForIrq(irqLine));
|
||||
} else {
|
||||
KernelLogStream(WARNING, "HDA") << "No interrupt available, polling only";
|
||||
}
|
||||
@@ -997,8 +1034,9 @@ namespace Drivers::Audio::IntelHda {
|
||||
uint32_t intctl = INTCTL_GIE | INTCTL_CIE;
|
||||
// Enable interrupt for all output streams
|
||||
for (uint8_t i = 0; i < g_numOutputStreams; i++) {
|
||||
intctl |= (1u << (g_numInputStreams + i));
|
||||
g_streamInterruptMask |= (1u << (g_numInputStreams + i));
|
||||
}
|
||||
intctl |= g_streamInterruptMask;
|
||||
Write32(REG_INTCTL, intctl);
|
||||
|
||||
// Discover codecs
|
||||
@@ -1028,6 +1066,63 @@ namespace Drivers::Audio::IntelHda {
|
||||
return g_initialized;
|
||||
}
|
||||
|
||||
bool HasDeferredWork() {
|
||||
return (g_initialized &&
|
||||
g_streamWorkPending.load(std::memory_order_acquire)) ||
|
||||
g_diagnosticsPending.load(std::memory_order_acquire) != 0 ||
|
||||
g_codecTimeoutsPending.load(std::memory_order_acquire) != 0;
|
||||
}
|
||||
|
||||
void ProcessDeferredWork() {
|
||||
if (g_initialized &&
|
||||
g_streamWorkPending.exchange(false, std::memory_order_acq_rel)) {
|
||||
Mixer::OnHdaBufferComplete();
|
||||
|
||||
// Clear-before-unmask closes the handoff race: if another completion
|
||||
// arrived while masked its SD_STS/INTSTS state is still asserted and
|
||||
// enabling the stream bit produces a fresh MSI/INTx interrupt.
|
||||
Write32(REG_INTCTL, Read32(REG_INTCTL) | g_streamInterruptMask |
|
||||
INTCTL_GIE | INTCTL_CIE);
|
||||
}
|
||||
|
||||
uint32_t diagnostics =
|
||||
g_diagnosticsPending.exchange(0, std::memory_order_acq_rel);
|
||||
if (diagnostics & DIAG_STREAM_OPENED) {
|
||||
KernelLogStream(OK, "HDA") << "Stream opened: " << base::dec
|
||||
<< (uint64_t)g_openSampleRate.load(std::memory_order_relaxed)
|
||||
<< "Hz "
|
||||
<< (uint64_t)g_openBitsPerSample.load(std::memory_order_relaxed)
|
||||
<< "-bit "
|
||||
<< (uint64_t)g_openChannels.load(std::memory_order_relaxed)
|
||||
<< "ch";
|
||||
}
|
||||
if (diagnostics & DIAG_STREAM_CLOSED)
|
||||
KernelLogStream(OK, "HDA") << "Stream closed";
|
||||
if (diagnostics & DIAG_HEADPHONES)
|
||||
KernelLogStream(INFO, "HDA") << "Switched to headphone output";
|
||||
if (diagnostics & DIAG_SPEAKERS)
|
||||
KernelLogStream(INFO, "HDA") << "Switched to speaker output";
|
||||
|
||||
uint32_t timeouts =
|
||||
g_codecTimeoutsPending.exchange(0, std::memory_order_acq_rel);
|
||||
if (timeouts != 0) {
|
||||
uint32_t report =
|
||||
g_codecTimeoutReports.fetch_add(1, std::memory_order_relaxed);
|
||||
if (report < 8) {
|
||||
KernelLogStream(WARNING, "HDA") << base::dec
|
||||
<< (uint64_t)timeouts << " codec verb timeout(s), last: codec="
|
||||
<< (uint64_t)g_lastTimeoutCodec.load(std::memory_order_relaxed)
|
||||
<< " nid="
|
||||
<< (uint64_t)g_lastTimeoutNid.load(std::memory_order_relaxed)
|
||||
<< " verb=" << base::hex
|
||||
<< (uint64_t)g_lastTimeoutVerb.load(std::memory_order_relaxed);
|
||||
} else if (report == 8) {
|
||||
KernelLogStream(WARNING, "HDA")
|
||||
<< "Further codec verb timeout reports suppressed";
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
uint32_t GetCodecVendorId() {
|
||||
return g_codecVendorId;
|
||||
}
|
||||
@@ -1075,11 +1170,12 @@ namespace Drivers::Audio::IntelHda {
|
||||
// Start the stream
|
||||
StartStream(streamIndex);
|
||||
|
||||
KernelLogStream(OK, "HDA") << "Stream opened: " << base::dec
|
||||
<< (uint64_t)sampleRate << "Hz " << (uint64_t)bitsPerSample << "-bit "
|
||||
<< (uint64_t)channels << "ch";
|
||||
|
||||
g_streamLock.Release();
|
||||
g_openSampleRate.store(sampleRate, std::memory_order_relaxed);
|
||||
g_openBitsPerSample.store(bitsPerSample, std::memory_order_relaxed);
|
||||
g_openChannels.store(channels, std::memory_order_relaxed);
|
||||
g_diagnosticsPending.fetch_or(DIAG_STREAM_OPENED,
|
||||
std::memory_order_release);
|
||||
return 0; // Handle 0
|
||||
}
|
||||
|
||||
@@ -1099,8 +1195,9 @@ namespace Drivers::Audio::IntelHda {
|
||||
|
||||
g_stream.Active = false;
|
||||
|
||||
KernelLogStream(OK, "HDA") << "Stream closed";
|
||||
g_streamLock.Release();
|
||||
g_diagnosticsPending.fetch_or(DIAG_STREAM_CLOSED,
|
||||
std::memory_order_release);
|
||||
}
|
||||
|
||||
uint32_t GetWriteSpace(int handle) {
|
||||
|
||||
Reference in New Issue
Block a user