[SOLVED] PIT interrupt somehow sent as IRQ10

[peachsmith]

Well this is a bit of a weird situation, and I'm not even sure which parts of my code to post.

After enabling the local APIC and mapping the legacy IRQs in the I/O APIC's redirection entries, the PIT interrupt appears to be coming in as IRQ10.
I checked the MADT for ISOs, and found one with bus-relative IRQ 0 and GSI of 2 (based on the HPET page of the wiki I assume this to be the timer ISO).

I've mapped the PIC IRQs to 0x20 through 0x2F and masked them.
I'm trying to map the I/O APIC IRQs starting at 0x30.

Here are some weird things I've noticed:

• The IRQ10 handler gets called regardless of whether or not I apply the ISO in my I/O APIC redirects.
• The PS2 emulation keyboard IRQ seems to be one less than the timer IRQ

So clearly I'm setting IRQ redirects incorrectly, and if I'm doing that wrong, then I'm most certainly fumbling the ISOs.
Where did I go wrong?


Here's how I set a redirection entry in the I/O APIC (based on code found at https://ethv.net/workshops/osdev/notes/notes-3.html):

Code: Select all

void ioapic_set_irq(uint8_t irq, uint8_t isr)
{
  uint32_t lo_index = irq * 2;
  uint32_t hi_index = irq * 2 + 1;

  // Get the current APIC ID
  uint32_t lapic_id = k_lapic_get_id();

  // Currently, the I/O APIC registers only have 4 bits for the
  // local APIC ID when in "physical" destination mode.
  // In theory, some chips can have up to 255 processors using "logical" destination mode.
  // TODO: figure out how to handle this scenario.
  if (lapic_id > 0xF)
  {
    fprintf(stddbg, "[ERROR] local APIC ID is more than 4 bits\n");
    for (;;);
  }


  // Put the local APIC ID in bits [59:56].
  // This is bits [31:24] of the high register.
  uint32_t hi_contents = ioapic_read(hi_index);

  // Clear bits [31:24] and set the local APIC address.
  hi_contents &= ~0xFF000000;
  hi_contents |= (lapic_id << 24);

  ioapic_write(hi_index, hi_contents);


  // Most of the IRQ description goes in the low register.
  uint32_t lo_contents = ioapic_read(lo_index);

  // Clear bits [10:8] to indicate fixed delivery mode.
  // TODO: add doc comments for delivery mode.
  lo_contents &= ~(0x700);

  // Clear bit 11 to indicate physical destination mode.
  lo_contents &= ~(0x800);

  // Bits [15:12] are ignored for now.
  // Bit 13 is the polarity. (1 for low, 0 for high)
  // Bit 15 is the trigger mode. (1 for level, 0 for edge)

  // Clear bit 16 to unmask the IRQ.
  lo_contents &= ~(0x10000);

  // Clear bits [7:0] in preparation for the new ISR index.
  lo_contents &= ~0xFF;

  // Set the index of the ISR in bits [7:0].
  lo_contents |= isr;

  ioapic_write(lo_index, lo_contents);
}

This is how I handle ISO entries in the MADT:

Code: Select all

uint8_t irq_src = *((uint8_t*)(entry + 3));
uint32_t gsi = *((uint32_t*)(entry + 4));
uint16_t flags = *((uint16_t*)(entry + 8));

// get the polarity and trigger mode from the flags
uint16_t pol = (flags & 0x3);
uint16_t trig = (flags & 0xC) >> 2;

// Add the redirection entry to the IRQ.
ioapic_set_iso(irq_src, 0x30 + gsi, pol, trig);

[Octocontrabass]

Where did I go wrong?

I see one potential problem here:

Code: Select all

ioapic_set_iso(irq_src, 0x30 + gsi, pol, trig);

It looks like your code will set redirection table entry 0 to trigger interrupt vector 0x32, when you actually want redirection table entry 2 to trigger interrupt vector 0x30. Since you're messing with the wrong redirection table entry, you don't see any changes when you remove the override. (But why not just change the IDT? If you're worried about interrupt priority, the local APIC assigns higher priority to higher vectors, so you'd need to completely rearrange them to match the legacy PIC priorities anyway.)

If that were the only problem, the PIT would call your IRQ2 handler and the keyboard would call your IRQ1 handler. Since it's off by 8, I suspect issues with your IDT setup.

[peachsmith]

I installed a temporary debug ISR for interrupt 0x30 in my IDT and triggered it with a INT $0x30 instruction, so I'm pretty sure the installation of interrupt handlers in the IDT is correct.

I've added a "correction" variable that will allow me to shift the range of APIC IRQ mappings.
After mapping the IRQs to different ranges (e.g. starting at 0x2F or starting at 0x31 ) IRQ10 still gets called, but the bit set in the local APIC's in-service register gets shifted by the amount of the correction variable (which is expected).

Here's how I set up the initial IRQ redirections before applying ISOs:

Code: Select all

uint8_t irq_base = 0x30; // start at index 48 in the IDT
int correction = 0;      // offset that can be changed to debug redirection

// Install the legacy IRQ handlers
k_install_isr(apic_irq_0, irq_base + correction + 0);
k_install_isr(apic_irq_1, irq_base + correction + 1);
k_install_isr(apic_irq_2, irq_base + correction + 2);
k_install_isr(apic_irq_3, irq_base + correction + 3);
k_install_isr(apic_irq_4, irq_base + correction + 4);
k_install_isr(apic_irq_5, irq_base + correction + 5);
k_install_isr(apic_irq_6, irq_base + correction + 6);
k_install_isr(apic_irq_7, irq_base + correction + 7);
k_install_isr(apic_irq_8, irq_base + correction + 8);
k_install_isr(apic_irq_9, irq_base + correction + 9);
k_install_isr(apic_irq_10, irq_base + correction + 10);
k_install_isr(apic_irq_11, irq_base + correction + 11);
k_install_isr(apic_irq_12, irq_base + correction + 12);
k_install_isr(apic_irq_13, irq_base + correction + 13);
k_install_isr(apic_irq_14, irq_base + correction + 14);
k_install_isr(apic_irq_15, irq_base + correction + 15);

// Set the initial IRQ redirects.
for (uint8_t i = 0; i < 16; i++)
{
  ioapic_set_irq(i, irq_base + correction + i);
}

And here's how I'm applying the ISO now that I switched the first two arguments:

Code: Select all

uint8_t bus_src = *((uint8_t*)(entry + 2));
uint8_t irq_src = *((uint8_t*)(entry + 3));
uint32_t gsi = *((uint32_t*)(entry + 4));
uint16_t flags = *((uint16_t*)(entry + 8));

// polarity and trigger mode
uint16_t pol = (flags & 0x3);
uint16_t trig = (flags & 0xC) >> 2;

ioapic_set_iso(gsi, irq_base + correction + irq_src, pol, trig);

The implementation of ioapic_set_irq and ioapic_set_iso are pretty much the same. The only difference is that ioapic_set_iso takes into account the polarity and trigger mode from the MADT.

[Octocontrabass]

I took another look and spotted something else that might be a problem. Here, you're calculating the index into the redirection table.

Code: Select all

  uint32_t lo_index = irq * 2;
  uint32_t hi_index = irq * 2 + 1;

But it looks like you're calling functions that expect the IOAPIC register number, not the redirection table index.

Code: Select all

  ioapic_write(hi_index, hi_contents);

[peachsmith]

I took another look and spotted something else that might be a problem. Here, you're calculating the index into the redirection table.

Code: Select all

  uint32_t lo_index = irq * 2;
  uint32_t hi_index = irq * 2 + 1;

But it looks like you're calling functions that expect the IOAPIC register number, not the redirection table index.

Code: Select all

  ioapic_write(hi_index, hi_contents);

Sure enough, I was forgetting to add 0x10 when calculating the IOREDTBL index.
Both the redirections and ISOs are now working as expected.
Thanks a bunch!