Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kernel: Use IOAPIC and LAPIC instead of the PIC #304

Merged
merged 4 commits into from
Jun 13, 2019

Conversation

roblabla
Copy link
Member

PIC is so 1980s, IOAPIC is so 2000s.

Moving to the APIC provides us with two immediate and one long term benefit:

  1. It allows us to use MSI for PCI interruptions, which should cleanly resolve the problem that level triggered interrupts don't work well in the context of a microkernel.

  2. It provides us with a nice speed boost compared to the PIC, what with the IO-APIC being a lot cleaner to use.

  3. It will eventually allow SMP.

The IO-APIC is mostly wired in the same way the PIC is. However, under QEmu, there's a bug that prevents using IRQ line 0, so we have to move the timer to another IRQ line (we use 2 since that's the recommended line for timers).

@roblabla roblabla added the project-kernel Related to the kernel label May 23, 2019
@todo
Copy link

todo bot commented May 23, 2019

Report that IOAPIC IRQ0 is broken under qemu.

Ideally, we'd use IRQ0 for the timer, in order to match what we have with the PIC. Unfortunately, qemu unconditionally redirects irqs on pin0 to pin2.
We should report this upstream bug, and move back to IRQ0 once it is fixed.


// TODO: Report that IOAPIC IRQ0 is broken under qemu.
// BODY: Ideally, we'd use IRQ0 for the timer, in order to match what we have
// BODY: with the PIC. Unfortunately, qemu [unconditionally redirects irqs on
// BODY: pin0 to pin2](https://github.com/qemu/qemu/blob/37560c259d7a0d6aceb96e9d6903ee002f4e5e0c/hw/intc/ioapic.c#L152).
// BODY:
// BODY: We should report this upstream bug, and move back to IRQ0 once it is
// BODY: fixed.
main_timer.set_interrupt_route(2);
// Clear the interrupt state
hpet_instance.enable();


This comment was generated by todo based on a TODO comment in 92d3d7a in #304. cc @roblabla.

@todo
Copy link

todo bot commented May 23, 2019

Support bitflags. That'd be nice.

// TODO: Support bitflags. That'd be nice.
type Value = T;
fn read(&self) -> Self::Value {
unsafe { (&self.0 as *const u128 as *const T).read_volatile() }
}
fn write(&mut self, value: Self::Value) {
unsafe { (&mut self.0 as *mut u128 as *mut T).write_volatile(value) }
}
}


This comment was generated by todo based on a TODO comment in 163e4d4 in #304. cc @roblabla.

@todo
Copy link

todo bot commented May 23, 2019

See chapter

/// TODO: See chapter
#[repr(transparent)]
#[derive(Clone, Copy)]
pub struct LocalApicVersion(u32);
impl Debug;
/// The version numbers of the local APIC:
///
/// - 00H - 0FH: 82489DX discrete APIC.
/// - 10H - 15H: Integrated APIC.
version, _: 7, 0;
/// Shows the number of LVT entries minus 1.


This comment was generated by todo based on a TODO comment in 163e4d4 in #304. cc @roblabla.

@todo
Copy link

todo bot commented May 23, 2019

This is wrong. It is not Send.

// TODO: This is wrong. It is **not** Send.
unsafe impl Send for LocalApic {}
unsafe impl Sync for LocalApic {}
impl LocalApic {
/// Create a new LocalApic at the specified address.
///
/// # Panics
///
/// Panics if address is not page-aligned.
///


This comment was generated by todo based on a TODO comment in 163e4d4 in #304. cc @roblabla.

@todo
Copy link

todo bot commented May 23, 2019

Mask everything.

// TODO: Mask everything.
}
/// 10.4.3 Enabling or Disabling the Local APIC
///
/// The local APIC can be enabled or disabled in either of two ways:
///
/// - Using the APIC global enable/disable flag in the IA32_APIC_BASE MSR (MSR address 1BH; see Figure 10-5)
/// - Using the APIC software enable/disable flag in the spurious-interrupt vector register (see Figure 10-23)
pub fn enable(&self) {
// Enable the LAPIC. The MSR should be set by the BIOS, so we're only


This comment was generated by todo based on a TODO comment in 163e4d4 in #304. cc @roblabla.

@todo
Copy link

todo bot commented May 23, 2019

IoApic should not be Sync!

IoApic manually implements Sync to allow it to be stored in a static. This is, however, wildly unsafe. It "works" today because we only have a single CPU and no preemption.


// TODO: IoApic should not be Sync!
// BODY: IoApic manually implements Sync to allow it to be stored in a static.
// BODY: This is, however, wildly unsafe. It "works" today because we only have
// BODY: a single CPU and no preemption.
unsafe impl Send for IoApic {}
unsafe impl Sync for IoApic {}
bitfield! {
/// Description of a Redirection Entry in the IO-APIC. Unlike IRQ pins of the
/// 8259A, the notion of interrupt priority is completely unrelated to the
/// position of the physical interrupt input signal on the APIC. Instead,


This comment was generated by todo based on a TODO comment in 163e4d4 in #304. cc @roblabla.

@todo
Copy link

todo bot commented May 23, 2019

Avoid mapping the same MMIO pages multiple times.

Currently, if we need to map distinct MMIO regions sharing the same page, we do multiple mapping. This is wasteful of address space, which is a relatively scarce resource.
It might be a good idea to make an MMIO manager that hands out references to the same mapping (with different offsets) when a single page is shared.


// TODO: Avoid mapping the same MMIO pages multiple times.
// BODY: Currently, if we need to map distinct MMIO regions sharing the
// BODY: same page, we do multiple mapping. This is wasteful of address
// BODY: space, which is a relatively scarce resource.
// BODY:
// BODY: It might be a good idea to make an MMIO manager that hands out
// BODY: references to the same mapping (with different offsets) when
// BODY: a single page is shared.
if address.floor() != (address + 8).floor() {
panic!("Weird MMIO.")
}


This comment was generated by todo based on a TODO comment in 163e4d4 in #304. cc @roblabla.

@todo
Copy link

todo bot commented May 23, 2019

LocalAPIC should not be Send/Sync.

LocalApic should be stored in a cpu_local, removing the need for Send/ Sync bounds. Problem is, we don't really have a way to create CPU Locals yet.


// TODO: LocalAPIC should not be Send/Sync.
// BODY: LocalApic should be stored in a cpu_local, removing the need for Send/
// BODY: Sync bounds. Problem is, we don't really have a way to create CPU Locals
// BODY: yet.
unsafe impl Send for LocalApic {}
unsafe impl Sync for LocalApic {}
impl LocalApic {
/// Create a new LocalApic at the specified address.
///
/// # Panics


This comment was generated by todo based on a TODO comment in d5048d1 in #304. cc @roblabla.

@roblabla roblabla requested review from marysaka and Orycterope May 27, 2019 15:46
Copy link
Member

@Orycterope Orycterope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it took so long

kernel/src/devices/apic.rs Outdated Show resolved Hide resolved
kernel/src/devices/apic.rs Outdated Show resolved Hide resolved
kernel/src/devices/ioapic.rs Outdated Show resolved Hide resolved
roblabla added 4 commits June 12, 2019 13:12
PIC is so 1980s, IOAPIC is so 200s.

Moving to the APIC provides us with two immediate and one long term
benefit:

1. It allows us to use MSI for PCI interruptions, which should cleanly
   resolve the problem that level triggered interrupts don't work well
   in the context of a microkernel.

2. It provides us with a nice speed boost compared to the PIC, what with
   the IO-APIC being a lot cleaner to use.

3. It will eventually allow SMP.

The IO-APIC is mostly wired in the same way the PIC is. However, under
QEmu, there's a bug that prevents using IRQ line 0, so we have to move
the timer to another IRQ line (we use 2 since that's the recommended
line for timers).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project-kernel Related to the kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants