From 0b7fc525e143fbe282466c0ca12360321854088e Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 21 Mar 2020 09:33:58 +0200 Subject: [PATCH] Interrupts: Simplify IRQ disabling & enabling in IRQController(s) 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. --- Kernel/Interrupts/IOAPIC.cpp | 13 +++++++++++-- Kernel/Interrupts/IOAPIC.h | 1 + Kernel/Interrupts/IRQController.h | 3 +-- Kernel/Interrupts/PIC.cpp | 31 ++++++++++++++++--------------- Kernel/Interrupts/PIC.h | 2 ++ 5 files changed, 31 insertions(+), 19 deletions(-) diff --git a/Kernel/Interrupts/IOAPIC.cpp b/Kernel/Interrupts/IOAPIC.cpp index b6f4e3513b..e10509be78 100644 --- a/Kernel/Interrupts/IOAPIC.cpp +++ b/Kernel/Interrupts/IOAPIC.cpp @@ -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; @@ -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 @@ -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)); } diff --git a/Kernel/Interrupts/IOAPIC.h b/Kernel/Interrupts/IOAPIC.h index 36acf4e25e..9786d85146 100644 --- a/Kernel/Interrupts/IOAPIC.h +++ b/Kernel/Interrupts/IOAPIC.h @@ -48,6 +48,7 @@ public: 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; } diff --git a/Kernel/Interrupts/IRQController.h b/Kernel/Interrupts/IRQController.h index 2a99df4ab4..b04613a282 100644 --- a/Kernel/Interrupts/IRQController.h +++ b/Kernel/Interrupts/IRQController.h @@ -45,7 +45,7 @@ public: 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; @@ -59,7 +59,6 @@ public: protected: IRQController() {} virtual void initialize() = 0; - bool m_enabled { false }; bool m_hard_disabled { false }; }; } diff --git a/Kernel/Interrupts/PIC.cpp b/Kernel/Interrupts/PIC.cpp index a471f4a699..7b4ca9f411 100644 --- a/Kernel/Interrupts/PIC.cpp +++ b/Kernel/Interrupts/PIC.cpp @@ -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) @@ -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); @@ -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() @@ -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) @@ -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); @@ -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 @@ -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(); } @@ -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); diff --git a/Kernel/Interrupts/PIC.h b/Kernel/Interrupts/PIC.h index 95b3cc7286..81a1c650d5 100644 --- a/Kernel/Interrupts/PIC.h +++ b/Kernel/Interrupts/PIC.h @@ -38,6 +38,7 @@ public: 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; @@ -47,6 +48,7 @@ public: 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);