x86: locking when messing with the PIC
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 25 Oct 2013 21:43:13 +0000 (14:43 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 16 Jan 2014 19:16:49 +0000 (11:16 -0800)
This is ultra-paranoia.  In general, we only mess with the PIC from
core 0.  That's the only core we route PIC IRQs to.  But there's a
chance we have an MCP that triggers something like a mask or unmask of
the PIC, which could happen concurrently with an IRQ.  I don't know if
we have anything that can do that yet, but better safe than sorry.

As far as the extra perf overheads go, ideally we wouldn't even be using
the PIC, but we'd have the same set of issues with the IOAPIC, and will
need locking in those cases too.

kern/arch/x86/apic.c
kern/arch/x86/apic.h

index e8633e1..9b3abb9 100644 (file)
 
 system_timing_t RO system_timing = {0, 0, 0xffff, 0};
 bool core_id_ready = FALSE;
+spinlock_t piclock = SPINLOCK_INITIALIZER_IRQSAVE;
 
 /* * Remaps the Programmable Interrupt Controller to use IRQs 32-47
  * http://wiki.osdev.org/PIC
  * Check osdev for a more thorough explanation/implementation.
  * http://bochs.sourceforge.net/techspec/PORTS.LST  */
-void pic_remap(
+void pic_remap(void)
 {
+       spin_lock_irqsave(&piclock);
        /* start initialization (ICW1) */
        outb(PIC1_CMD, 0x11);
        outb(PIC2_CMD, 0x11);
@@ -43,38 +45,51 @@ void pic_remap()
        /* set masks, defaulting to all masked for now */
        outb(PIC1_DATA, 0xff);
        outb(PIC2_DATA, 0xff);
+       spin_unlock_irqsave(&piclock);
 }
 
 void pic_mask_irq(uint8_t irq)
 {
+       spin_lock_irqsave(&piclock);
        if (irq > 7)
                outb(PIC2_DATA, inb(PIC2_DATA) | (1 << (irq - 8)));
        else
                outb(PIC1_DATA, inb(PIC1_DATA) | (1 << irq));
+       spin_unlock_irqsave(&piclock);
 }
 
 void pic_unmask_irq(uint8_t irq)
 {
+       spin_lock_irqsave(&piclock);
        if (irq > 7) {
                outb(PIC2_DATA, inb(PIC2_DATA) & ~(1 << (irq - 8)));
                outb(PIC1_DATA, inb(PIC1_DATA) & 0xfb); // make sure irq2 is unmasked
        } else
                outb(PIC1_DATA, inb(PIC1_DATA) & ~(1 << irq));
+       spin_unlock_irqsave(&piclock);
 }
 
 /* Aka, the IMR.  Simply reading the data port are OCW1s. */
 uint16_t pic_get_mask(void)
 {
-       return (inb(PIC2_DATA) << 8) | inb(PIC1_DATA);
+       uint16_t ret;
+       spin_lock_irqsave(&piclock);
+       ret = (inb(PIC2_DATA) << 8) | inb(PIC1_DATA);
+       spin_unlock_irqsave(&piclock);
+       return ret;
 }
 
 static uint16_t __pic_get_irq_reg(int ocw3)
 {
+       uint16_t ret;
+       spin_lock_irqsave(&piclock);
        /* OCW3 to PIC CMD to get the register values.  PIC2 is chained, and
         * represents IRQs 8-15.  PIC1 is IRQs 0-7, with 2 being the chain */
        outb(PIC1_CMD, ocw3);
        outb(PIC2_CMD, ocw3);
-       return (inb(PIC2_CMD) << 8) | inb(PIC1_CMD);
+       ret = (inb(PIC2_CMD) << 8) | inb(PIC1_CMD);
+       spin_unlock_irqsave(&piclock);
+       return ret;
 }
 
 /* Returns the combined value of the cascaded PICs irq request register */
@@ -89,6 +104,16 @@ uint16_t pic_get_isr(void)
        return __pic_get_irq_reg(PIC_READ_ISR);
 }
 
+void pic_send_eoi(uint32_t irq)
+{
+       spin_lock_irqsave(&piclock);
+       // all irqs beyond the first seven need to be chained to the slave
+       if (irq > 7)
+               outb(PIC2_CMD, PIC_EOI);
+       outb(PIC1_CMD, PIC_EOI);
+       spin_unlock_irqsave(&piclock);
+}
+
 /* Debugging helper.  Note the ISR/IRR are 32 bits at a time, spaced every 16
  * bytes in the LAPIC address space. */
 void lapic_print_isr(void)
index 0c46a72..91aeaf7 100644 (file)
@@ -144,6 +144,7 @@ void pic_unmask_irq(uint8_t irq);
 uint16_t pic_get_mask(void);
 uint16_t pic_get_irr(void);
 uint16_t pic_get_isr(void);
+void pic_send_eoi(uint32_t irq);
 bool lapic_get_isr_bit(uint8_t vector);
 bool lapic_get_irr_bit(uint8_t vector);
 void lapic_print_isr(void);
@@ -159,7 +160,6 @@ void udelay_pit(uint64_t usec);
 uint64_t gettimer(void);
 uint64_t getfreq(void);
 
-static inline void pic_send_eoi(uint32_t irq);
 static inline void lapic_send_eoi(void);
 static inline uint32_t lapic_get_version(void);
 static inline uint32_t lapic_get_error(void);
@@ -185,14 +185,6 @@ static inline void __send_nmi(uint8_t hw_coreid);
 #define unmask_lapic_lvt(entry) \
        write_mmreg32(entry, read_mmreg32(entry) & ~LAPIC_LVT_MASK)
 
-static inline void pic_send_eoi(uint32_t irq)
-{
-       // all irqs beyond the first seven need to be chained to the slave
-       if (irq > 7)
-               outb(PIC2_CMD, PIC_EOI);
-       outb(PIC1_CMD, PIC_EOI);
-}
-
 static inline void lapic_send_eoi(void)
 {
        write_mmreg32(LAPIC_EOI, 0);