Skip to content

Commit

Permalink
Interrupts: Simplify IRQ disabling & enabling in IRQController(s)
Browse files Browse the repository at this point in the history
Instead of blindly setting masks, if we want to disable an IRQ and it's
already masked, we just return. The same happens if we want to enable an
IRQ and it's unmasked.
  • Loading branch information
supercomputer7 authored and awesomekling committed Mar 24, 2020
1 parent 3f98a67 commit 0b7fc52
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 19 deletions.
13 changes: 11 additions & 2 deletions Kernel/Interrupts/IOAPIC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ void IOAPIC::map_pci_interrupts()
configure_redirection_entry(11, 11 + IRQ_VECTOR_BASE, DeliveryMode::Normal, false, false, true, true, 0);
}

bool IOAPIC::is_enabled() const
{
return !is_hard_disabled();
}

void IOAPIC::spurious_eoi(const GenericInterruptHandler& handler) const
{
InterruptDisabler disabler;
Expand Down Expand Up @@ -213,8 +218,10 @@ void IOAPIC::mask_all_redirection_entries() const
void IOAPIC::mask_redirection_entry(u8 index) const
{
ASSERT((u32)index < m_redirection_entries_count);
u32 redirection_entry = read_register((index << 1) + IOAPIC_REDIRECTION_ENTRY_OFFSET) | (1 << 16);
write_register((index << 1) + IOAPIC_REDIRECTION_ENTRY_OFFSET, redirection_entry);
u32 redirection_entry = read_register((index << 1) + IOAPIC_REDIRECTION_ENTRY_OFFSET);
if (redirection_entry & (1 << 16))
return;
write_register((index << 1) + IOAPIC_REDIRECTION_ENTRY_OFFSET, redirection_entry | (1 << 16));
}

bool IOAPIC::is_redirection_entry_masked(u8 index) const
Expand All @@ -227,6 +234,8 @@ void IOAPIC::unmask_redirection_entry(u8 index) const
{
ASSERT((u32)index < m_redirection_entries_count);
u32 redirection_entry = read_register((index << 1) + IOAPIC_REDIRECTION_ENTRY_OFFSET);
if (!(redirection_entry & (1 << 16)))
return;
write_register((index << 1) + IOAPIC_REDIRECTION_ENTRY_OFFSET, redirection_entry & ~(1 << 16));
}

Expand Down
1 change: 1 addition & 0 deletions Kernel/Interrupts/IOAPIC.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class IOAPIC final : public IRQController {
virtual void eoi(const GenericInterruptHandler&) const override;
virtual void spurious_eoi(const GenericInterruptHandler&) const override;
virtual bool is_vector_enabled(u8 number) const override;
virtual bool is_enabled() const override;
virtual u16 get_isr() const override;
virtual u16 get_irr() const override;
virtual u32 gsi_base() const override { return m_gsi_base; }
Expand Down
3 changes: 1 addition & 2 deletions Kernel/Interrupts/IRQController.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class IRQController : public RefCounted<IRQController> {
virtual void disable(const GenericInterruptHandler&) = 0;
virtual void hard_disable() { m_hard_disabled = true; }
virtual bool is_vector_enabled(u8 number) const = 0;
bool is_enabled() const { return m_enabled && !m_hard_disabled; }
virtual bool is_enabled() const = 0;
bool is_hard_disabled() const { return m_hard_disabled; }
virtual void eoi(const GenericInterruptHandler&) const = 0;
virtual void spurious_eoi(const GenericInterruptHandler&) const = 0;
Expand All @@ -59,7 +59,6 @@ class IRQController : public RefCounted<IRQController> {
protected:
IRQController() {}
virtual void initialize() = 0;
bool m_enabled { false };
bool m_hard_disabled { false };
};
}
31 changes: 16 additions & 15 deletions Kernel/Interrupts/PIC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,14 @@ namespace Kernel {
#define ICW4_BUF_MASTER 0x0C /* Buffered mode/master */
#define ICW4_SFNM 0x10 /* Special fully nested (not) */

bool inline static is_all_masked(u8 reg)
bool inline static is_all_masked(u16 reg)
{
return reg == 0xFF;
return reg == 0xFFFF;
}

bool PIC::is_enabled() const
{
return !is_all_masked(m_cached_irq_mask) && !is_hard_disabled();
}

void PIC::disable(const GenericInterruptHandler& handler)
Expand All @@ -65,6 +70,8 @@ void PIC::disable(const GenericInterruptHandler& handler)
ASSERT(!is_hard_disabled());
ASSERT(handler.interrupt_number() >= gsi_base() && handler.interrupt_number() < interrupt_vectors_count());
u8 irq = handler.interrupt_number();
if (m_cached_irq_mask & (1 << irq))
return;
u8 imr;
if (irq & 8) {
imr = IO::in8(PIC1_CMD);
Expand All @@ -75,9 +82,7 @@ void PIC::disable(const GenericInterruptHandler& handler)
imr |= 1 << irq;
IO::out8(PIC0_CMD, imr);
}

if (is_all_masked(imr))
m_enabled = false;
m_cached_irq_mask |= 1 << irq;
}

PIC::PIC()
Expand All @@ -98,15 +103,7 @@ void PIC::spurious_eoi(const GenericInterruptHandler& handler) const

bool PIC::is_vector_enabled(u8 irq) const
{
u8 imr;
if (irq & 8) {
imr = IO::in8(PIC1_CMD);
imr &= 1 << (irq & 7);
} else {
imr = IO::in8(PIC0_CMD);
imr &= 1 << irq;
}
return imr != 0;
return m_cached_irq_mask & (1 << irq);
}

void PIC::enable(const GenericInterruptHandler& handler)
Expand All @@ -121,6 +118,8 @@ void PIC::enable_vector(u8 irq)
{
InterruptDisabler disabler;
ASSERT(!is_hard_disabled());
if (!(m_cached_irq_mask & (1 << irq)))
return;
u8 imr;
if (irq & 8) {
imr = IO::in8(PIC1_CMD);
Expand All @@ -131,7 +130,7 @@ void PIC::enable_vector(u8 irq)
imr &= ~(1 << irq);
IO::out8(PIC0_CMD, imr);
}
m_enabled = true;
m_cached_irq_mask &= ~(1 << irq);
}

void PIC::eoi(const GenericInterruptHandler& handler) const
Expand Down Expand Up @@ -166,6 +165,7 @@ void PIC::hard_disable()
remap(0x20);
IO::out8(PIC0_CMD, 0xff);
IO::out8(PIC1_CMD, 0xff);
m_cached_irq_mask = 0xffff;
IRQController::hard_disable();
}

Expand All @@ -190,6 +190,7 @@ void PIC::remap(u8 offset)
// Mask -- start out with all IRQs disabled.
IO::out8(PIC0_CMD, 0xff);
IO::out8(PIC1_CMD, 0xff);
m_cached_irq_mask = 0xffff;

// ...except IRQ2, since that's needed for the master to let through slave interrupts.
enable_vector(2);
Expand Down
2 changes: 2 additions & 0 deletions Kernel/Interrupts/PIC.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class PIC final : public IRQController {
virtual void hard_disable() override;
virtual void eoi(const GenericInterruptHandler&) const override;
virtual bool is_vector_enabled(u8 number) const override;
virtual bool is_enabled() const override;
virtual void spurious_eoi(const GenericInterruptHandler&) const override;
virtual u16 get_isr() const override;
virtual u16 get_irr() const override;
Expand All @@ -47,6 +48,7 @@ class PIC final : public IRQController {
virtual IRQControllerType type() const override { return IRQControllerType::i8259; }

private:
u16 m_cached_irq_mask { 0xffff };
void eoi_interrupt(u8 irq) const;
void enable_vector(u8 number);
void remap(u8 offset);
Expand Down

0 comments on commit 0b7fc52

Please sign in to comment.