drivers: update some interfaces related to the interrupt

drivers/src/irq/riscv_intc.rs

@@ -56,7 +56,7 @@ impl Scheme for Intc {
     fn handle_irq(&self, cause: usize) {
         self.with_handler(cause, |opt| {
             if let Some(h) = opt {
-                h(cause);
+                h();
             } else {
                 warn!("no registered handler for SCAUSE {}!", cause);
             }
@@ -67,30 +67,33 @@ impl Scheme for Intc {
 }
 
 impl IrqScheme for Intc {
-    fn mask(&self, cause: usize) {
+    fn is_valid_irq(&self, cause: usize) -> bool {
+        matches!(cause, S_SOFT | S_TIMER | S_EXT)
+    }
+
+    fn mask(&self, cause: usize) -> DeviceResult {
         unsafe {
-            match cause {
+            Ok(match cause {
                 S_SOFT => sie::clear_ssoft(),
                 S_TIMER => sie::clear_stimer(),
                 S_EXT => sie::clear_sext(),
-                _ => {}
+                _ => return Err(DeviceError::InvalidParam),
-            }
+            })
         }
     }
 
-    fn unmask(&self, cause: usize) {
+    fn unmask(&self, cause: usize) -> DeviceResult {
         unsafe {
-            match cause {
+            Ok(match cause {
                 S_SOFT => sie::set_ssoft(),
                 S_TIMER => sie::set_stimer(),
                 S_EXT => sie::set_sext(),
-                _ => {}
+                _ => return Err(DeviceError::InvalidParam),
-            }
+            })
         }
     }
 
     fn register_handler(&self, cause: usize, handler: IrqHandler) -> DeviceResult {
-        self.unmask(cause);
         self.with_handler(cause, |opt| {
             if opt.is_some() {
                 Err(DeviceError::AlreadyExists)
@@ -100,4 +103,15 @@ impl IrqScheme for Intc {
             }
         })
     }
+
+    fn unregister(&self, cause: usize) -> DeviceResult {
+        self.with_handler(cause, |opt| {
+            if opt.is_some() {
+                *opt = None;
+                Ok(())
+            } else {
+                Err(DeviceError::InvalidParam)
+            }
+        })
+    }
 }

drivers/src/irq/riscv_plic.rs

@@ -4,7 +4,7 @@ use spin::Mutex;
 
 use crate::io::{Io, Mmio};
 use crate::scheme::{IrqHandler, IrqScheme, Scheme};
-use crate::{utils::IrqManager, DeviceResult};
+use crate::{utils::IrqManager, DeviceError, DeviceResult};
 
 const IRQ_RANGE: Range<usize> = 1..1024;
 
@@ -98,19 +98,34 @@ impl Scheme for Plic {
 }
 
 impl IrqScheme for Plic {
-    fn mask(&self, irq_num: usize) {
+    fn is_valid_irq(&self, irq_num: usize) -> bool {
-        self.inner.lock().toggle(irq_num, false)
+        IRQ_RANGE.contains(&irq_num)
     }
 
-    fn unmask(&self, irq_num: usize) {
+    fn mask(&self, irq_num: usize) -> DeviceResult {
-        self.inner.lock().toggle(irq_num, true)
+        if self.is_valid_irq(irq_num) {
+            Ok(self.inner.lock().toggle(irq_num, false))
+        } else {
+            Err(DeviceError::InvalidParam)
+        }
+    }
+
+    fn unmask(&self, irq_num: usize) -> DeviceResult {
+        if self.is_valid_irq(irq_num) {
+            Ok(self.inner.lock().toggle(irq_num, true))
+        } else {
+            Err(DeviceError::InvalidParam)
+        }
     }
 
     fn register_handler(&self, irq_num: usize, handler: IrqHandler) -> DeviceResult {
         let mut inner = self.inner.lock();
-        inner.toggle(irq_num, true);
+        inner.manager.register_handler(irq_num, handler).map(|_| {
-        inner.set_priority(irq_num, 7);
+            inner.set_priority(irq_num, 7);
-        inner.manager.register_handler(irq_num, handler)?;
+        })
-        Ok(())
+    }
+
+    fn unregister(&self, irq_num: usize) -> DeviceResult {
+        self.inner.lock().manager.unregister_handler(irq_num)
     }
 }

drivers/src/irq/x86_apic/consts.rs

@@ -1,7 +1,6 @@
 // TODO: configurable
 
 pub const X86_INT_BASE: usize = 0x20;
-pub const X86_INT_MAX: usize = 0xff;
 
 pub const X86_INT_LOCAL_APIC_BASE: usize = 0xf0;
 pub const X86_INT_APIC_SPURIOUS: usize = X86_INT_LOCAL_APIC_BASE + 0x0;

drivers/src/irq/x86_apic/ioapic.rs

@@ -4,9 +4,9 @@ use core::{fmt, ptr::NonNull};
 use acpi::platform::interrupt::InterruptModel;
 use acpi::{AcpiHandler, AcpiTables, PhysicalMapping};
 use spin::Mutex;
-use x2apic::ioapic::IoApic as IoApicInner;
+use x2apic::ioapic::{IoApic as IoApicInner, IrqFlags, IrqMode};
 
-use super::Phys2VirtFn;
+use super::{IrqPolarity, IrqTriggerMode, Phys2VirtFn};
 
 const PAGE_SIZE: usize = 4096;
 
@@ -52,9 +52,9 @@ impl IoApic {
     pub fn new(id: u8, base_vaddr: usize, gsi_start: u32) -> Self {
         let mut inner = unsafe { IoApicInner::new(base_vaddr as u64) };
         let max_entry = unsafe { inner.max_table_entry() };
-        unsafe {
+        unsafe { assert_eq!(id, inner.id()) };
-            assert_eq!(id, inner.id());
+        for i in 0..max_entry + 1 {
-            inner.init(super::consts::X86_INT_BASE as _);
+            unsafe { inner.disable_irq(i) }
         }
         Self {
             id,
@@ -89,6 +89,26 @@ impl IoApic {
             inner.set_table_entry(idx, entry);
         }
     }
+
+    pub fn configure(&self, gsi: u32, tm: IrqTriggerMode, pol: IrqPolarity, dest: u8, vector: u8) {
+        let idx = (gsi - self.gsi_start) as u8;
+        let mut inner = self.inner.lock();
+        let mut entry = unsafe { inner.table_entry(idx) };
+        entry.set_vector(vector);
+        entry.set_mode(IrqMode::Fixed);
+        entry.set_dest(dest);
+
+        let mut flags = IrqFlags::MASKED; // destination mode: physical
+        if matches!(tm, IrqTriggerMode::Edge) {
+            flags |= IrqFlags::LEVEL_TRIGGERED;
+        }
+        if matches!(pol, IrqPolarity::ActiveLow) {
+            flags |= IrqFlags::LOW_ACTIVE;
+        }
+        entry.set_flags(flags);
+
+        unsafe { inner.set_table_entry(idx, entry) };
+    }
 }
 
 impl IoApicList {

drivers/src/irq/x86_apic/lapic.rs

@@ -3,38 +3,49 @@ use x2apic::lapic::{xapic_base, LocalApic as LocalApicInner, LocalApicBuilder};
 use super::{consts, Phys2VirtFn};
 
 static mut LOCAL_APIC: Option<LocalApic> = None;
+static mut BSP_ID: Option<u8> = None;
 
 pub struct LocalApic {
     inner: LocalApicInner,
 }
 
 impl LocalApic {
+    pub unsafe fn get<'a>() -> &'a mut LocalApic {
+        LOCAL_APIC
+            .as_mut()
+            .expect("Local APIC is not initialized by BSP")
+    }
+
+    pub unsafe fn init_bsp(phys_to_virt: Phys2VirtFn) {
+        let base_vaddr = phys_to_virt(xapic_base() as usize);
+        let mut inner = LocalApicBuilder::new()
+            .timer_vector(consts::X86_INT_APIC_TIMER)
+            .error_vector(consts::X86_INT_APIC_ERROR)
+            .spurious_vector(consts::X86_INT_APIC_SPURIOUS)
+            .set_xapic_base(base_vaddr as u64)
+            .build()
+            .unwrap_or_else(|err| panic!("{}", err));
+
+        assert!(inner.is_bsp());
+        BSP_ID = Some((inner.id() >> 24) as u8);
+
+        inner.enable();
+        LOCAL_APIC = Some(LocalApic { inner });
+    }
+
+    pub unsafe fn init_ap() {
+        Self::get().inner.enable()
+    }
+
+    pub fn bsp_id() -> u8 {
+        unsafe { BSP_ID.unwrap() }
+    }
+
+    pub fn id(&mut self) -> u8 {
+        unsafe { (self.inner.id() >> 24) as u8 }
+    }
+
     pub fn eoi(&mut self) {
         unsafe { self.inner.end_of_interrupt() }
     }
 }
-
-pub unsafe fn get_local_apic<'a>() -> &'a mut LocalApic {
-    LOCAL_APIC
-        .as_mut()
-        .expect("Local APIC is not initialized by BSP")
-}
-
-pub unsafe fn init_bsp(phys_to_virt: Phys2VirtFn) {
-    let base_vaddr = phys_to_virt(xapic_base() as usize);
-    let mut inner = LocalApicBuilder::new()
-        .timer_vector(consts::X86_INT_APIC_TIMER)
-        .error_vector(consts::X86_INT_APIC_ERROR)
-        .spurious_vector(consts::X86_INT_APIC_SPURIOUS)
-        .set_xapic_base(base_vaddr as u64)
-        .build()
-        .unwrap_or_else(|err| panic!("{}", err));
-    assert!(inner.is_bsp());
-
-    inner.enable();
-    LOCAL_APIC = Some(LocalApic { inner });
-}
-
-pub unsafe fn init_ap() {
-    get_local_apic().inner.enable()
-}

drivers/src/irq/x86_apic/mod.rs

@@ -6,26 +6,29 @@ use core::ops::Range;
 
 use spin::Mutex;
 
-use self::consts::{X86_INT_BASE, X86_INT_LOCAL_APIC_BASE, X86_INT_MAX};
+use self::consts::{X86_INT_BASE, X86_INT_LOCAL_APIC_BASE};
 use self::ioapic::{IoApic, IoApicList};
 use self::lapic::LocalApic;
-use crate::scheme::{IrqHandler, IrqScheme, Scheme};
+use crate::scheme::{IrqHandler, IrqPolarity, IrqScheme, IrqTriggerMode, Scheme};
 use crate::{utils::IrqManager, DeviceError, DeviceResult};
 
-const IRQ_RANGE: Range<usize> = X86_INT_BASE..X86_INT_MAX + 1;
+const IOAPIC_IRQ_RANGE: Range<usize> = X86_INT_BASE..X86_INT_LOCAL_APIC_BASE;
+const LAPIC_IRQ_RANGE: Range<usize> = 0..16;
 
 type Phys2VirtFn = fn(usize) -> usize;
 
 pub struct Apic {
     ioapic_list: IoApicList,
-    manager: Mutex<IrqManager<256>>,
+    manager_ioapic: Mutex<IrqManager<256>>,
+    manager_lapic: Mutex<IrqManager<16>>,
 }
 
 impl Apic {
     pub fn new(acpi_rsdp: usize, phys_to_virt: Phys2VirtFn) -> Self {
         Self {
-            manager: Mutex::new(IrqManager::new(IRQ_RANGE)),
             ioapic_list: IoApicList::new(acpi_rsdp, phys_to_virt),
+            manager_ioapic: Mutex::new(IrqManager::new(IOAPIC_IRQ_RANGE)),
+            manager_lapic: Mutex::new(IrqManager::new(LAPIC_IRQ_RANGE)),
         }
     }
 
@@ -45,20 +48,22 @@ impl Apic {
     }
 
     pub fn init_local_apic_bsp(phys_to_virt: Phys2VirtFn) {
-        unsafe { self::lapic::init_bsp(phys_to_virt) }
+        unsafe { LocalApic::init_bsp(phys_to_virt) }
     }
 
     pub fn init_local_apic_ap() {
-        unsafe { self::lapic::init_ap() }
+        unsafe { LocalApic::init_ap() }
     }
 
     pub fn local_apic<'a>() -> &'a mut LocalApic {
-        unsafe { self::lapic::get_local_apic() }
+        unsafe { LocalApic::get() }
     }
 
     pub fn register_local_apic_handler(&self, vector: usize, handler: IrqHandler) -> DeviceResult {
         if vector >= X86_INT_LOCAL_APIC_BASE {
-            self.manager.lock().register_handler(vector, handler)?;
+            self.manager_lapic
+                .lock()
+                .register_handler(vector - X86_INT_LOCAL_APIC_BASE, handler)?;
             Ok(())
         } else {
             error!("invalid local APIC interrupt vector {}", vector);
@@ -69,7 +74,14 @@ impl Apic {
 
 impl Scheme for Apic {
     fn handle_irq(&self, vector: usize) {
-        if self.manager.lock().handle(vector).is_err() {
+        let res = if vector >= X86_INT_LOCAL_APIC_BASE {
+            self.manager_lapic
+                .lock()
+                .handle(vector - X86_INT_LOCAL_APIC_BASE)
+        } else {
+            self.manager_ioapic.lock().handle(vector)
+        };
+        if res.is_err() {
             warn!("no registered handler for interrupt vector {}!", vector);
         }
         Self::local_apic().eoi();
@@ -77,24 +89,73 @@ impl Scheme for Apic {
 }
 
 impl IrqScheme for Apic {
-    fn mask(&self, gsi: usize) {
+    fn is_valid_irq(&self, gsi: usize) -> bool {
-        self.with_ioapic(gsi as _, |apic| Ok(apic.toggle(gsi as _, false)))
+        self.ioapic_list.find(gsi as _).is_some()
-            .ok();
     }
 
-    fn unmask(&self, gsi: usize) {
+    fn mask(&self, gsi: usize) -> DeviceResult {
+        self.with_ioapic(gsi as _, |apic| Ok(apic.toggle(gsi as _, false)))
+    }
+
+    fn unmask(&self, gsi: usize) -> DeviceResult {
         self.with_ioapic(gsi as _, |apic| Ok(apic.toggle(gsi as _, true)))
-            .ok();
+    }
+
+    fn configure(&self, gsi: usize, tm: IrqTriggerMode, pol: IrqPolarity) -> DeviceResult {
+        let gsi = gsi as u32;
+        self.with_ioapic(gsi, |apic| {
+            apic.configure(gsi, tm, pol, LocalApic::bsp_id(), 0);
+            Ok(())
+        })
     }
 
     fn register_handler(&self, gsi: usize, handler: IrqHandler) -> DeviceResult {
         let gsi = gsi as u32;
         self.with_ioapic(gsi, |apic| {
             let vector = apic.get_vector(gsi) as _; // if not mapped, allocate an available vector by `register_handler()`.
-            let vector = self.manager.lock().register_handler(vector, handler)? as u8;
+            let vector = self
+                .manager_ioapic
+                .lock()
+                .register_handler(vector, handler)? as u8;
             apic.map_vector(gsi, vector);
-            apic.toggle(gsi, true);
             Ok(())
         })
     }
+
+    fn unregister(&self, gsi: usize) -> DeviceResult {
+        let gsi = gsi as u32;
+        self.with_ioapic(gsi, |apic| {
+            let vector = apic.get_vector(gsi) as _;
+            self.manager_ioapic.lock().unregister_handler(vector)?;
+            apic.map_vector(gsi, 0);
+            Ok(())
+        })
+    }
+
+    fn msi_alloc_block(&self, requested_irqs: usize) -> DeviceResult<Range<usize>> {
+        let alloc_size = requested_irqs.next_power_of_two();
+        let start = self.manager_ioapic.lock().alloc_block(alloc_size)?;
+        Ok(start..start + alloc_size)
+    }
+
+    fn msi_free_block(&self, block: Range<usize>) -> DeviceResult {
+        self.manager_lapic
+            .lock()
+            .free_block(block.start, block.len())
+    }
+
+    fn msi_register_handler(
+        &self,
+        block: Range<usize>,
+        msi_id: usize,
+        handler: IrqHandler,
+    ) -> DeviceResult {
+        if msi_id < block.len() {
+            self.manager_ioapic
+                .lock()
+                .overwrite_handler(block.start + msi_id, handler)
+        } else {
+            Err(DeviceError::InvalidParam)
+        }
+    }
 }

drivers/src/scheme/event.rs

@@ -23,7 +23,7 @@ impl<T: Scheme> Scheme for EventListener<T> {
     fn handle_irq(&self, irq_num: usize) {
         self.inner.handle_irq(irq_num);
         self.events.lock().retain(|(f, once)| {
-            f(irq_num);
+            f();
             !once
         });
     }

drivers/src/scheme/irq.rs

@@ -1,16 +1,71 @@
 use alloc::boxed::Box;
+use core::ops::Range;
 
 use super::Scheme;
 use crate::DeviceResult;
 
-pub type IrqHandler = Box<dyn Fn(usize) + Send + Sync>;
+pub type IrqHandler = Box<dyn Fn() + Send + Sync>;
+
+#[derive(Debug)]
+pub enum IrqTriggerMode {
+    Edge,
+    Level,
+}
+
+#[derive(Debug)]
+pub enum IrqPolarity {
+    ActiveHigh,
+    ActiveLow,
+}
 
 pub trait IrqScheme: Scheme {
-    fn mask(&self, irq_num: usize);
+    /// Is a valid IRQ number.
-    fn unmask(&self, irq_num: usize);
+    fn is_valid_irq(&self, irq_num: usize) -> bool;
 
+    /// Enable IRQ.
+    fn mask(&self, irq_num: usize) -> DeviceResult;
+
+    /// Disable IRQ.
+    fn unmask(&self, irq_num: usize) -> DeviceResult;
+
+    /// Configure the specified interrupt vector. If it is invoked, it must be
+    /// invoked prior to interrupt registration.
+    fn configure(&self, _irq_num: usize, _tm: IrqTriggerMode, _pol: IrqPolarity) -> DeviceResult {
+        unimplemented!()
+    }
+
+    /// Add an interrupt handle to an IRQ.
     fn register_handler(&self, irq_num: usize, handler: IrqHandler) -> DeviceResult;
+
+    /// Register the device to delivery an IRQ.
     fn register_device(&self, irq_num: usize, dev: &'static dyn Scheme) -> DeviceResult {
-        self.register_handler(irq_num, Box::new(move |n| dev.handle_irq(n)))
+        self.register_handler(irq_num, Box::new(move || dev.handle_irq(irq_num)))
+    }
+
+    /// Remove the interrupt handle to an IRQ.
+    fn unregister(&self, irq_num: usize) -> DeviceResult;
+
+    /// Method used for platform allocation of blocks of MSI and MSI-X compatible
+    /// IRQ targets.
+    fn msi_alloc_block(&self, _requested_irqs: usize) -> DeviceResult<Range<usize>> {
+        unimplemented!()
+    }
+
+    /// Method used to free a block of MSI IRQs previously allocated by msi_alloc_block().
+    /// This does not unregister IRQ handlers.
+    fn msi_free_block(&self, _block: Range<usize>) -> DeviceResult {
+        unimplemented!()
+    }
+
+    /// Register a handler function for a given msi_id within an msi_block_t. Passing a
+    /// NULL handler will effectively unregister a handler for a given msi_id within the
+    /// block.
+    fn msi_register_handler(
+        &self,
+        _block: Range<usize>,
+        _msi_id: usize,
+        _handler: IrqHandler,
+    ) -> DeviceResult {
+        unimplemented!()
     }
 }

drivers/src/scheme/mod.rs

@@ -10,7 +10,7 @@ pub use block::BlockScheme;
 pub use display::DisplayScheme;
 pub use event::EventListener;
 pub use input::InputScheme;
-pub use irq::{IrqHandler, IrqScheme};
+pub use irq::{IrqHandler, IrqPolarity, IrqScheme, IrqTriggerMode};
 pub use net::NetScheme;
 pub use uart::UartScheme;
 

drivers/src/utils/id_allocator.rs

@@ -1,7 +1,7 @@
 use alloc::boxed::Box;
 use core::ops::{Deref, DerefMut, Range};
 
-use bitmap_allocator::{BitAlloc, BitAlloc256, BitAlloc4K, BitAlloc64K};
+use bitmap_allocator::{BitAlloc, BitAlloc16, BitAlloc256, BitAlloc4K, BitAlloc64K};
 
 use crate::{DeviceError, DeviceResult};
 
@@ -21,7 +21,8 @@ pub struct IdAllocator(Box<dyn IdAllocatorWrapper>);
 impl IdAllocator {
     pub fn new(range: Range<usize>) -> DeviceResult<Self> {
         Ok(match range.end {
-            0..=0x100 => Self(Box::new(IdAllocator256::new(range))),
+            0..=0x10 => Self(Box::new(IdAllocator16::new(range))),
+            0x11..=0x100 => Self(Box::new(IdAllocator256::new(range))),
             0x101..=0x1000 => Self(Box::new(IdAllocator4K::new(range))),
             0x1001..=0x10000 => Self(Box::new(IdAllocator64K::new(range))),
             _ => {
@@ -98,6 +99,7 @@ macro_rules! define_allocator {
     };
 }
 
+define_allocator!(IdAllocator16, BitAlloc16);
 define_allocator!(IdAllocator256, BitAlloc256);
 define_allocator!(IdAllocator4K, BitAlloc4K);
 define_allocator!(IdAllocator64K, BitAlloc64K);

drivers/src/utils/irq_manager.rs

@@ -24,19 +24,21 @@ impl<const IRQ_COUNT: usize> IrqManager<IRQ_COUNT> {
     }
 
     pub fn alloc_block(&mut self, count: usize) -> DeviceResult<usize> {
+        info!("IRQ alloc_block {}", count);
         debug_assert!(count.is_power_of_two());
-        let align_log2 = 31 - count.leading_zeros();
+        let align_log2 = 31 - (count as u32).leading_zeros();
         self.allocator.alloc_contiguous(count, align_log2 as _)
     }
 
     pub fn free_block(&mut self, start: usize, count: usize) -> DeviceResult {
+        info!("IRQ free_block {:#x?}", start..start + count);
         self.allocator.free(start, count)
     }
 
     /// Add a handler to IRQ table. if `irq_num == 0`, we need to allocate one.
     /// Returns the specified IRQ number or an allocated IRQ on success.
     pub fn register_handler(&mut self, irq_num: usize, handler: IrqHandler) -> DeviceResult<usize> {
-        info!("IRQ add handler {:#x?}", irq_num);
+        info!("IRQ register handler {:#x}", irq_num);
         let irq_num = if irq_num == 0 {
             // allocate a valid IRQ number
             self.allocator.alloc()?
@@ -51,7 +53,7 @@ impl<const IRQ_COUNT: usize> IrqManager<IRQ_COUNT> {
     }
 
     pub fn unregister_handler(&mut self, irq_num: usize) -> DeviceResult {
-        info!("IRQ remove handler {:#x?}", irq_num);
+        info!("IRQ unregister handler {:#x}", irq_num);
         if !self.allocator.is_alloced(irq_num) {
             Err(DeviceError::InvalidParam)
         } else {
@@ -62,7 +64,7 @@ impl<const IRQ_COUNT: usize> IrqManager<IRQ_COUNT> {
     }
 
     pub fn overwrite_handler(&mut self, irq_num: usize, handler: IrqHandler) -> DeviceResult {
-        info!("IRQ overwrite handle {:#x?}", irq_num);
+        info!("IRQ overwrite handle {:#x}", irq_num);
         if !self.allocator.is_alloced(irq_num) {
             Err(DeviceError::InvalidParam)
         } else {
@@ -73,7 +75,7 @@ impl<const IRQ_COUNT: usize> IrqManager<IRQ_COUNT> {
 
     pub fn handle(&self, irq_num: usize) -> DeviceResult {
         if let Some(f) = &self.table[irq_num] {
-            f(irq_num);
+            f();
             Ok(())
         } else {
             Err(DeviceError::InvalidParam)

kernel-hal/src/bare/arch/riscv/drivers.rs

@@ -20,16 +20,20 @@ pub(super) fn init() -> DeviceResult {
 
     PLIC.init_by(Plic::new(phys_to_virt(consts::PLIC_BASE)));
     PLIC.register_device(consts::UART0_INT_NUM, UART.as_scheme())?;
+    PLIC.unmask(consts::UART0_INT_NUM)?;
 
     IRQ.register_handler(
         ScauseIntCode::SupervisorSoft as _,
-        Box::new(|_| trap::super_soft()),
+        Box::new(|| trap::super_soft()),
     )?;
     IRQ.register_handler(
         ScauseIntCode::SupervisorTimer as _,
-        Box::new(|_| trap::super_timer()),
+        Box::new(|| trap::super_timer()),
     )?;
     IRQ.register_device(ScauseIntCode::SupervisorExternal as _, PLIC.as_scheme())?;
+    IRQ.unmask(ScauseIntCode::SupervisorSoft as _)?;
+    IRQ.unmask(ScauseIntCode::SupervisorTimer as _)?;
+    IRQ.unmask(ScauseIntCode::SupervisorExternal as _)?;
 
     Ok(())
 }

kernel-hal/src/bare/arch/riscv/interrupt.rs

@@ -1,12 +1,18 @@
-pub(super) fn init() {
+use riscv::{asm, register::sstatus};
-    unsafe { riscv::register::sstatus::set_sie() };
-    info!("+++ setup interrupt OK +++");
-}
 
 hal_fn_impl! {
     impl mod crate::hal_fn::interrupt {
-        fn handle_irq(cause: u32) {
+        fn wait_for_interrupt() {
-            crate::drivers::IRQ.handle_irq(cause as usize)
+            unsafe {
+                // enable interrupt and disable
+                sstatus::set_sie();
+                asm::wfi();
+                sstatus::clear_sie();
+            }
+        }
+
+        fn handle_irq(cause: usize) {
+            crate::drivers::IRQ.handle_irq(cause)
         }
     }
 }

kernel-hal/src/bare/arch/riscv/mod.rs

@@ -10,14 +10,12 @@ pub mod context;
 pub mod cpu;
 pub mod interrupt;
 pub mod mem;
-pub mod special;
 pub mod timer;
 pub mod vm;
 
 pub fn init() {
     vm::remap_the_kernel().unwrap();
     drivers::init().unwrap();
-    interrupt::init();
     timer::init();
 
     #[cfg(feature = "board_qemu")]

kernel-hal/src/bare/arch/riscv/special.rs

@@ -1,11 +0,0 @@
-pub fn wait_for_interrupt() {
-    unsafe {
-        // enable interrupt and disable
-        let sie = riscv::register::sstatus::read().sie();
-        riscv::register::sstatus::set_sie();
-        riscv::asm::wfi();
-        if !sie {
-            riscv::register::sstatus::clear_sie();
-        }
-    }
-}

kernel-hal/src/bare/arch/riscv/timer.rs

@@ -1,13 +1,14 @@
 use core::time::Duration;
 
+const CLOCK_FREQ: u64 = 12_500_000;
+const TICKS_PER_SEC: u64 = 100;
+
 fn get_cycle() -> u64 {
     riscv::register::time::read() as u64
 }
 
 pub(super) fn timer_set_next() {
-    //let TIMEBASE: u64 = 100000;
+    super::sbi::set_timer(get_cycle() + CLOCK_FREQ / TICKS_PER_SEC);
-    const TIMEBASE: u64 = 10_000_000;
-    super::sbi::set_timer(get_cycle() + TIMEBASE);
 }
 
 pub(super) fn init() {
@@ -15,7 +16,6 @@ pub(super) fn init() {
 }
 
 pub(crate) fn timer_now() -> Duration {
-    const FREQUENCY: u64 = 10_000_000; // ???
     let time = get_cycle();
-    Duration::from_nanos(time * 1_000_000_000 / FREQUENCY as u64)
+    Duration::from_nanos(time * 1_000_000_000 / CLOCK_FREQ as u64)
 }

kernel-hal/src/bare/arch/riscv/trap.rs

@@ -46,7 +46,7 @@ pub extern "C" fn trap_handler(tf: &mut TrapFrame) {
         Trap::Exception(Exception::LoadPageFault) => page_fault(MMUFlags::READ),
         Trap::Exception(Exception::StorePageFault) => page_fault(MMUFlags::WRITE),
         Trap::Exception(Exception::InstructionPageFault) => page_fault(MMUFlags::EXECUTE),
-        Trap::Interrupt(_) => crate::interrupt::handle_irq(scause.code() as u32),
+        Trap::Interrupt(_) => crate::interrupt::handle_irq(scause.code()),
         _ => panic!("Undefined Trap: {:?}", scause.cause()),
     }
 }

kernel-hal/src/bare/arch/x86_64/drivers.rs

@@ -17,9 +17,10 @@ pub(super) fn init() -> DeviceResult {
         crate::mem::phys_to_virt,
     ));
     irq.register_device(trap::X86_ISA_IRQ_COM1, UART.as_scheme())?;
+    irq.unmask(trap::X86_ISA_IRQ_COM1)?;
     irq.register_local_apic_handler(
         trap::X86_INT_APIC_TIMER,
-        Box::new(|_| crate::timer::timer_tick()),
+        Box::new(|| crate::timer::timer_tick()),
     )?;
     IRQ.init_by(irq);
 

kernel-hal/src/bare/arch/x86_64/interrupt.rs

@@ -1,10 +1,8 @@
-use alloc::boxed::Box;
 use core::ops::Range;
 
+use crate::drivers::IRQ;
 use crate::HalResult;
 
-type IrqHandler = Box<dyn Fn() + Send + Sync>;
-
 /*
 lazy_static! {
     static ref MOUSE: Mutex<Mouse> = Mutex::new(Mouse::new());
@@ -38,48 +36,54 @@ fn mouse() {
 
 hal_fn_impl! {
     impl mod crate::hal_fn::interrupt {
-        fn enable_irq(irq: u32) {
+        fn wait_for_interrupt() {
-            todo!()
+            use x86_64::instructions::interrupts;
+            interrupts::enable_and_hlt();
+            interrupts::disable();
         }
 
-        fn disable_irq(irq: u32) {
+        fn is_valid_irq(gsi: usize) -> bool {
-            todo!()
+            IRQ.is_valid_irq(gsi)
         }
 
-        fn is_valid_irq(irq: u32) -> bool {
+        fn mask_irq(gsi: usize) -> HalResult {
-            todo!()
+            Ok(IRQ.mask(gsi)?)
         }
 
-        fn configure_irq(vector: u32, trig_mode: bool, polarity: bool) -> HalResult {
+        fn unmask_irq(gsi: usize) -> HalResult {
-            todo!()
+            Ok(IRQ.unmask(gsi)?)
         }
 
-        fn register_irq_handler(global_irq: u32, handler: IrqHandler) -> HalResult<u32> {
+        fn configure_irq(gsi: usize, tm: IrqTriggerMode, pol: IrqPolarity) -> HalResult {
-            todo!()
+            Ok(IRQ.configure(gsi, tm, pol)?)
         }
 
-        fn unregister_irq_handler(global_irq: u32) -> HalResult {
+        fn register_irq_handler(gsi: usize, handler: IrqHandler) -> HalResult {
-            todo!()
+            Ok(IRQ.register_handler(gsi, handler)?)
         }
 
-        fn handle_irq(vector: u32) {
+        fn unregister_irq_handler(gsi: usize) -> HalResult {
-            crate::drivers::IRQ.handle_irq(vector as usize);
+            Ok(IRQ.unregister(gsi)?)
         }
 
-        fn msi_allocate_block(requested_irqs: u32) -> HalResult<Range<u32>> {
+        fn handle_irq(vector: usize) {
-            todo!()
+            IRQ.handle_irq(vector as usize);
         }
 
-        fn msi_free_block(block: Range<u32>) -> HalResult {
+        fn msi_alloc_block(requested_irqs: usize) -> HalResult<Range<usize>> {
-            todo!()
+            Ok(IRQ.msi_alloc_block(requested_irqs)?)
+        }
+
+        fn msi_free_block(block: Range<usize>) -> HalResult {
+            Ok(IRQ.msi_free_block(block)?)
         }
 
         fn msi_register_handler(
-            block: Range<u32>,
+            block: Range<usize>,
-            msi_id: u32,
+            msi_id: usize,
-            handler: Box<dyn Fn() + Send + Sync>,
+            handler: IrqHandler,
         ) -> HalResult {
-            todo!()
+            Ok(IRQ.msi_register_handler(block, msi_id, handler)?)
         }
     }
 }

kernel-hal/src/bare/arch/x86_64/trap.rs

@@ -59,12 +59,16 @@ fn page_fault(tf: &mut TrapFrame) {
 
 #[no_mangle]
 pub extern "C" fn trap_handler(tf: &mut TrapFrame) {
-    trace!("Interrupt: {:#x} @ CPU{}", tf.trap_num, 0); // TODO 0 should replace in multi-core case
+    trace!(
+        "Interrupt: {:#x} @ CPU{}",
+        tf.trap_num,
+        super::cpu::cpu_id()
+    );
     match tf.trap_num {
         BREAKPOINT => breakpoint(),
         DOUBLE_FAULT => double_fault(tf),
         PAGE_FAULT => page_fault(tf),
-        X86_INT_BASE..=X86_INT_MAX => crate::interrupt::handle_irq(tf.trap_num as u32),
+        X86_INT_BASE..=X86_INT_MAX => crate::interrupt::handle_irq(tf.trap_num),
         _ => panic!("Unhandled interrupt {:x} {:#x?}", tf.trap_num, tf),
     }
 }

kernel-hal/src/bare/mod.rs

@@ -6,7 +6,6 @@ cfg_if! {
     } else if #[cfg(any(target_arch = "riscv32", target_arch = "riscv64"))] {
         #[path = "arch/riscv/mod.rs"]
         mod arch;
-        pub use self::arch::special as riscv;
     }
 }
 

kernel-hal/src/common/defs.rs

@@ -2,6 +2,7 @@ use bitflags::bitflags;
 use numeric_enum_macro::numeric_enum;
 
 /// The error type which is returned from HAL functions.
+/// TODO: more error types.
 #[derive(Debug)]
 pub struct HalError;
 

kernel-hal/src/common/future.rs

@@ -79,7 +79,7 @@ impl Future for SerialFuture<'_> {
             return Poll::Ready(n);
         }
         let waker = cx.waker().clone();
-        UART.subscribe(Box::new(move |_| waker.wake_by_ref()), true);
+        UART.subscribe(Box::new(move || waker.wake_by_ref()), true);
         Poll::Pending
     }
 }

kernel-hal/src/common/interrupt.rs

@@ -0,0 +1,11 @@
+use crate::HalError;
+use core::convert::From;
+use zcore_drivers::DeviceError;
+
+pub use zcore_drivers::scheme::{IrqHandler, IrqPolarity, IrqTriggerMode};
+
+impl From<DeviceError> for HalError {
+    fn from(_err: DeviceError) -> Self {
+        Self
+    }
+}

kernel-hal/src/common/mod.rs

@@ -1,14 +1,15 @@
 mod future;
 
+pub(super) mod context;
 pub(super) mod defs;
 pub(super) mod fb;
+pub(super) mod interrupt;
+pub(super) mod mem;
+pub(super) mod thread;
+pub(super) mod vdso;
+pub(super) mod vm;
 
 pub mod addr;
-pub mod context;
 pub mod drivers;
-pub mod mem;
 pub mod serial;
-pub mod thread;
 pub mod user;
-pub mod vdso;
-pub mod vm;

kernel-hal/src/hal_fn.rs

@@ -46,41 +46,45 @@ hal_fn_def! {
         pub(crate) fn pt_clone_kernel_space(dst_pt_root: PhysAddr, src_pt_root: PhysAddr);
     }
 
-    pub mod interrupt {
+    pub mod interrupt: common::interrupt {
-        /// Enable IRQ.
+        /// Suspend the CPU (also enable interrupts) and wait for an interrupt
-        pub fn enable_irq(vector: u32);
+        /// to occurs, then disable interrupts.
-
+        pub fn wait_for_interrupt();
-        /// Disable IRQ.
-        pub fn disable_irq(vector: u32);
 
         /// Is a valid IRQ number.
-        pub fn is_valid_irq(vector: u32) -> bool;
+        pub fn is_valid_irq(vector: usize) -> bool;
+
+        /// Disable IRQ.
+        pub fn mask_irq(vector: usize) -> HalResult;
+
+        /// Enable IRQ.
+        pub fn unmask_irq(vector: usize) -> HalResult;
 
         /// Configure the specified interrupt vector. If it is invoked, it must be
         /// invoked prior to interrupt registration.
-        pub fn configure_irq(vector: u32, trig_mode: bool, polarity: bool) -> HalResult;
+        pub fn configure_irq(vector: usize, tm: IrqTriggerMode, pol: IrqPolarity) -> HalResult;
 
-        /// Add an interrupt handle to an IRQ
+        /// Add an interrupt handle to an IRQ.
-        pub fn register_irq_handler(vector: u32, handler: Box<dyn Fn() + Send + Sync>) -> HalResult<u32>;
+        pub fn register_irq_handler(vector: usize, handler: IrqHandler) -> HalResult;
 
-        /// Remove the interrupt handle to an IRQ
+        /// Remove the interrupt handle to an IRQ.
-        pub fn unregister_irq_handler(vector: u32) -> HalResult;
+        pub fn unregister_irq_handler(vector: usize) -> HalResult;
 
         /// Handle IRQ.
-        pub fn handle_irq(vector: u32);
+        pub fn handle_irq(vector: usize);
 
         /// Method used for platform allocation of blocks of MSI and MSI-X compatible
         /// IRQ targets.
-        pub fn msi_allocate_block(requested_irqs: u32) -> HalResult<Range<u32>>;
+        pub fn msi_alloc_block(requested_irqs: usize) -> HalResult<Range<usize>>;
 
         /// Method used to free a block of MSI IRQs previously allocated by msi_alloc_block().
         /// This does not unregister IRQ handlers.
-        pub fn msi_free_block(block: Range<u32>) -> HalResult;
+        pub fn msi_free_block(block: Range<usize>) -> HalResult;
 
         /// Register a handler function for a given msi_id within an msi_block_t. Passing a
         /// NULL handler will effectively unregister a handler for a given msi_id within the
         /// block.
-        pub fn msi_register_handler(block: Range<u32>, msi_id: u32, handler: Box<dyn Fn() + Send + Sync>) -> HalResult;
+        pub fn msi_register_handler(block: Range<usize>, msi_id: usize, handler: IrqHandler) -> HalResult;
     }
 
     pub mod context: common::context {

linux-loader/src/lib.rs

@@ -93,7 +93,7 @@ async fn new_thread(thread: CurrentThread) {
         match cx.trap_num {
             0x100 => handle_syscall(&thread, &mut cx.general).await,
             0x20..=0xff => {
-                kernel_hal::interrupt::handle_irq(cx.trap_num as u32);
+                kernel_hal::interrupt::handle_irq(cx.trap_num);
                 if cx.trap_num == 0x20 {
                     kernel_hal::thread::yield_now().await;
                 }
@@ -120,34 +120,41 @@ async fn new_thread(thread: CurrentThread) {
         {
             let trap_num = kernel_hal::context::fetch_trap_num(&cx);
             let is_interrupt = ((trap_num >> (core::mem::size_of::<usize>() * 8 - 1)) & 1) == 1;
-            assert!(!is_interrupt);
             let trap_num = trap_num & 0xfff;
             let pid = thread.proc().id();
-            match trap_num {
+            if is_interrupt {
-                // syscall
+                kernel_hal::interrupt::handle_irq(trap_num);
-                8 => handle_syscall(&thread, &mut cx).await,
+                // Timer
-                // PageFault
+                if trap_num == 5 {
-                12 | 13 | 15 => {
+                    kernel_hal::thread::yield_now().await;
-                    let (vaddr, flags) = kernel_hal::context::fetch_page_fault_info(trap_num);
+                }
-                    info!(
+            } else {
-                        "page fault from pid: {} user mode, vaddr:{:#x}, trap:{}",
+                match trap_num {
-                        pid, vaddr, trap_num
+                    // syscall
-                    );
+                    8 => handle_syscall(&thread, &mut cx).await,
-                    let vmar = thread.proc().vmar();
+                    // PageFault
-                    match vmar.handle_page_fault(vaddr, flags) {
+                    12 | 13 | 15 => {
-                        Ok(()) => {}
+                        let (vaddr, flags) = kernel_hal::context::fetch_page_fault_info(trap_num);
-                        Err(error) => {
+                        info!(
-                            panic!(
+                            "page fault from pid: {} user mode, vaddr:{:#x}, trap:{}",
-                                "Page Fault from user mode @ {:#x}({:?}): {:?}\n{:#x?}",
+                            pid, vaddr, trap_num
-                                vaddr, flags, error, cx
+                        );
-                            );
+                        let vmar = thread.proc().vmar();
+                        match vmar.handle_page_fault(vaddr, flags) {
+                            Ok(()) => {}
+                            Err(error) => {
+                                panic!(
+                                    "Page Fault from user mode @ {:#x}({:?}): {:?}\n{:#x?}",
+                                    vaddr, flags, error, cx
+                                );
+                            }
                         }
                     }
+                    _ => panic!(
+                        "not supported pid: {} exception {} from user mode. {:#x?}",
+                        pid, trap_num, cx
+                    ),
                 }
-                _ => panic!(
-                    "not supported pid: {} exception {} from user mode. {:#x?}",
-                    pid, trap_num, cx
-                ),
             }
         }
         thread.end_running(cx);

linux-loader/src/main.rs

@@ -24,7 +24,7 @@ async fn main() {
 
     use kernel_hal::drivers::UART;
     UART.subscribe(
-        Box::new(|_| {
+        Box::new(|| {
             while let Some(c) = UART.try_recv().unwrap() {
                 let c = if c == b'\r' { b'\n' } else { c };
                 STDIN.push(c as char);

zCore/Cargo.toml

@@ -40,11 +40,9 @@ rcore-fs = { git = "https://github.com/rcore-os/rcore-fs", rev = "6df6cd2" }
 fatfs = { git = "https://github.com/rafalh/rust-fatfs.git", rev = "26fa79", default-features = false }
 
 [target.'cfg(target_arch = "x86_64")'.dependencies]
-x86_64 = "0.14"
 rlibc-opt = { git = "https://github.com/rcore-os/rlibc-opt.git", rev = "fb1d4a9" }
 rboot = { path = "../rboot", default-features = false }
 rvm = { git = "https://github.com/rcore-os/RVM", rev = "e91d625", optional = true }
 
 [target.'cfg(target_arch = "riscv64")'.dependencies]
 rlibc = "1.0"
-riscv = { version = "0.7", features = ["inline-asm"] }

zCore/src/main.rs

@@ -171,7 +171,7 @@ fn main(ramfs_data: &'static mut [u8], cmdline: &str) -> ! {
     use linux_object::fs::STDIN;
 
     UART.subscribe(
-        Box::new(|_| {
+        Box::new(|| {
             while let Some(c) = UART.try_recv().unwrap() {
                 let c = if c == b'\r' { b'\n' } else { c };
                 STDIN.push(c as char);
@@ -194,13 +194,7 @@ fn main(ramfs_data: &'static mut [u8], cmdline: &str) -> ! {
 fn run() -> ! {
     loop {
         executor::run_until_idle();
-        #[cfg(target_arch = "x86_64")]
+        kernel_hal::interrupt::wait_for_interrupt();
-        {
-            x86_64::instructions::interrupts::enable_and_hlt();
-            x86_64::instructions::interrupts::disable();
-        }
-        #[cfg(target_arch = "riscv64")]
-        kernel_hal::riscv::wait_for_interrupt();
     }
 }
 

zircon-loader/src/lib.rs

@@ -217,7 +217,7 @@ async fn new_thread(thread: CurrentThread) {
         match trap_num {
             0x100 => handle_syscall(&thread).await,
             0x20..=0xff => {
-                kernel_hal::interrupt::handle_irq(trap_num as u32);
+                kernel_hal::interrupt::handle_irq(trap_num);
                 if trap_num == 0x20 {
                     EXCEPTIONS_TIMER.add(1);
                     kernel_hal::thread::yield_now().await;

zircon-object/src/dev/interrupt/event_interrupt.rs

@@ -2,7 +2,7 @@ use kernel_hal::interrupt;
 use {super::*, spin::Mutex};
 
 pub struct EventInterrupt {
-    vector: u32,
+    vector: usize,
     inner: Mutex<EventInterruptInner>,
 }
 
@@ -15,7 +15,7 @@ impl EventInterrupt {
     pub fn new(vector: usize) -> Box<Self> {
         // TODO check vector is a vaild IRQ number
         Box::new(EventInterrupt {
-            vector: vector as u32,
+            vector,
             inner: Default::default(),
         })
     }
@@ -25,14 +25,14 @@ impl InterruptTrait for EventInterrupt {
     fn mask(&self) {
         let inner = self.inner.lock();
         if inner.register {
-            interrupt::disable_irq(self.vector);
+            interrupt::mask_irq(self.vector).unwrap();
         }
     }
 
     fn unmask(&self) {
         let inner = self.inner.lock();
         if inner.register {
-            interrupt::enable_irq(self.vector);
+            interrupt::unmask_irq(self.vector).unwrap();
         }
     }
 

zircon-object/src/dev/pci/bus.rs

@@ -331,7 +331,7 @@ impl PCIeBusDriver {
     }
 
     /// Find the legacy IRQ handler.
-    pub fn find_legacy_irq_handler(&self, irq_id: u32) -> ZxResult<Arc<SharedLegacyIrqHandler>> {
+    pub fn find_legacy_irq_handler(&self, irq_id: usize) -> ZxResult<Arc<SharedLegacyIrqHandler>> {
         let mut list = self.legacy_irq_list.lock();
         for i in list.iter() {
             if irq_id == i.irq_id {
@@ -540,8 +540,8 @@ impl PcieDeviceKObject {
     }
 
     /// Set IRQ mode.
-    pub fn set_irq_mode(&self, mode: PcieIrqMode, irq_count: u32) -> ZxResult {
+    pub fn set_irq_mode(&self, mode: PcieIrqMode, requested_irqs: usize) -> ZxResult {
-        self.device.device().set_irq_mode(mode, irq_count)
+        self.device.device().set_irq_mode(mode, requested_irqs)
     }
 
     /// Read the device's config.

zircon-object/src/dev/pci/caps.rs

@@ -36,37 +36,36 @@ impl PciCapabilityStd {
 pub struct PciMsiBlock {
     pub target_addr: u64,
     pub allocated: bool,
-    pub base_irq: u32,
+    pub base_irq: usize,
-    pub num_irq: u32,
+    pub num_irq: usize,
     pub target_data: u32,
 }
 
 impl PciMsiBlock {
-    pub fn allocate(irq_num: u32) -> ZxResult<Self> {
+    pub fn allocate(irq_num: usize) -> ZxResult<Self> {
         if irq_num == 0 || irq_num > 32 {
             return Err(ZxError::INVALID_ARGS);
         }
-        let range = interrupt::msi_allocate_block(irq_num).map_err(|_| ZxError::NO_RESOURCES)?;
+        let range = interrupt::msi_alloc_block(irq_num).map_err(|_| ZxError::NO_RESOURCES)?;
         Ok(PciMsiBlock {
             target_addr: (0xFEE0_0000 | 0x08) & !0x4,
-            target_data: range.start,
+            target_data: range.start as u32,
             base_irq: range.start,
-            num_irq: range.len() as u32,
+            num_irq: range.len(),
             allocated: true,
         })
     }
     pub fn free(&self) {
         interrupt::msi_free_block(self.base_irq..self.base_irq + self.num_irq).ok();
     }
-    pub fn register_handler(&self, msi_id: u32, handle: Box<dyn Fn() + Send + Sync>) {
+    pub fn register_handler(&self, msi_id: usize, handle: Box<dyn Fn() + Send + Sync>) {
         assert!(self.allocated);
-        assert!(msi_id < self.num_irq);
         interrupt::msi_register_handler(
             self.base_irq..self.base_irq + self.num_irq,
             msi_id,
             handle,
         )
-        .ok();
+        .unwrap();
     }
 }
 

zircon-object/src/dev/pci/mod.rs

@@ -61,5 +61,5 @@ pub mod constants {
     pub const PCI_CFG_SPACE_TYPE_PIO: u8 = 0;
     pub const PCI_CFG_SPACE_TYPE_MMIO: u8 = 1;
     pub const PCIE_IRQRET_MASK: u32 = 0x1;
-    pub const PCIE_MAX_MSI_IRQS: u32 = 32;
+    pub const PCIE_MAX_MSI_IRQS: usize = 32;
 }

zircon-object/src/dev/pci/nodes.rs

@@ -184,30 +184,29 @@ pub struct PcieBarInfo {
 #[derive(Default)]
 pub struct SharedLegacyIrqHandler {
     /// The IRQ id.
-    pub irq_id: u32,
+    pub irq_id: usize,
     device_handler: Mutex<Vec<Arc<PcieDevice>>>,
 }
 
 impl SharedLegacyIrqHandler {
     /// Create a new SharedLegacyIrqHandler.
-    pub fn create(irq_id: u32) -> Option<Arc<SharedLegacyIrqHandler>> {
+    pub fn create(irq_id: usize) -> Option<Arc<SharedLegacyIrqHandler>> {
         info!("SharedLegacyIrqHandler created for {:#x?}", irq_id);
-        interrupt::disable_irq(irq_id);
+        interrupt::mask_irq(irq_id).unwrap();
         let handler = Arc::new(SharedLegacyIrqHandler {
             irq_id,
             device_handler: Mutex::new(Vec::new()),
         });
         let handler_copy = handler.clone();
-        interrupt::register_irq_handler(irq_id, Box::new(move || handler_copy.handle()))
+        interrupt::register_irq_handler(irq_id, Box::new(move || handler_copy.handle())).ok()?;
-            .map(|_| handler)
+        Some(handler)
-            .ok()
     }
 
     /// Handle the IRQ.
     pub fn handle(&self) {
         let device_handler = self.device_handler.lock();
         if device_handler.is_empty() {
-            interrupt::disable_irq(self.irq_id);
+            interrupt::mask_irq(self.irq_id).unwrap();
             return;
         }
         for dev in device_handler.iter() {
@@ -256,7 +255,7 @@ impl SharedLegacyIrqHandler {
         let is_first = device_handler.is_empty();
         device_handler.push(device);
         if is_first {
-            interrupt::enable_irq(self.irq_id);
+            interrupt::unmask_irq(self.irq_id).unwrap();
         }
     }
     pub fn remove_device(&self, device: Arc<PcieDevice>) {
@@ -268,7 +267,7 @@ impl SharedLegacyIrqHandler {
         let mut device_handler = self.device_handler.lock();
         device_handler.retain(|h| Arc::ptr_eq(h, &device));
         if device_handler.is_empty() {
-            interrupt::disable_irq(self.irq_id);
+            interrupt::mask_irq(self.irq_id).unwrap();
         }
     }
 }
@@ -338,7 +337,7 @@ impl Default for PcieIrqState {
 /// Class for managing shared legacy IRQ handlers.
 #[derive(Default)]
 pub struct PcieIrqHandlerState {
-    irq_id: u32,
+    irq_id: usize,
     masked: Mutex<bool>,
     enabled: Mutex<bool>,
     handler: Mutex<Option<Box<dyn Fn() -> u32 + Send + Sync>>>,
@@ -607,7 +606,7 @@ impl PcieDevice {
         if pin != 0 {
             inner.irq.legacy.id = self.map_pin_to_irq_locked(upstream, pin)?;
             inner.irq.legacy.shared_handler =
-                driver.find_legacy_irq_handler(inner.irq.legacy.id as u32)?;
+                driver.find_legacy_irq_handler(inner.irq.legacy.id)?;
         }
         Ok(())
     }
@@ -946,11 +945,11 @@ impl PcieDevice {
     fn allocate_irq_handler(
         &self,
         inner: &mut MutexGuard<PcieDeviceInner>,
-        irq_count: u32,
+        requested_irqs: usize,
         masked: bool,
     ) {
         assert!(inner.irq.handlers.is_empty());
-        for i in 0..irq_count {
+        for i in 0..requested_irqs {
             inner.irq.handlers.push(Arc::new(PcieIrqHandlerState {
                 irq_id: i,
                 enabled: Mutex::new(false),
@@ -959,7 +958,11 @@ impl PcieDevice {
             }))
         }
     }
-    fn enter_msi_irq_mode(&self, inner: &mut MutexGuard<PcieDeviceInner>, irq: u32) -> ZxResult {
+    fn enter_msi_irq_mode(
+        &self,
+        inner: &mut MutexGuard<PcieDeviceInner>,
+        requested_irqs: usize,
+    ) -> ZxResult {
         let (_std, msi) = inner.msi().ok_or(ZxError::NOT_SUPPORTED)?;
         let initially_masked = if msi.has_pvm {
             self.cfg
@@ -970,25 +973,25 @@ impl PcieDevice {
         } else {
             false
         };
-        match PciMsiBlock::allocate(irq) {
+        match PciMsiBlock::allocate(requested_irqs) {
             Ok(block) => *msi.irq_block.lock() = block,
             Err(ex) => {
                 self.leave_msi_irq_mode(inner);
                 return Err(ex);
             }
         };
-        self.allocate_irq_handler(inner, irq, initially_masked);
+        self.allocate_irq_handler(inner, requested_irqs, initially_masked);
         inner.irq.mode = PcieIrqMode::Msi;
         let (_std, msi) = inner.msi().ok_or(ZxError::NOT_SUPPORTED)?;
         let block = msi.irq_block.lock();
         let (target_addr, target_data) = (block.target_addr, block.target_data);
         self.set_msi_target(inner, target_addr, target_data);
-        self.set_msi_multi_message_enb(inner, irq);
+        self.set_msi_multi_message_enb(inner, requested_irqs);
         for (i, e) in inner.irq.handlers.iter().enumerate() {
             let arc_self = inner.arc_self();
             let handler_copy = e.clone();
             block.register_handler(
-                i as u32,
+                i,
                 Box::new(move || Self::msi_irq_handler(arc_self.clone(), handler_copy.clone())),
             );
         }
@@ -1031,9 +1034,13 @@ impl PcieDevice {
         }
         cfg.write16_(data_reg as usize, target_data as u16);
     }
-    fn set_msi_multi_message_enb(&self, inner: &MutexGuard<PcieDeviceInner>, irq_num: u32) {
+    fn set_msi_multi_message_enb(
-        assert!((1..=PCIE_MAX_MSI_IRQS).contains(&irq_num));
+        &self,
-        let log2 = u32::next_power_of_two(irq_num).trailing_zeros();
+        inner: &MutexGuard<PcieDeviceInner>,
+        requested_irqs: usize,
+    ) {
+        assert!((1..=PCIE_MAX_MSI_IRQS).contains(&requested_irqs));
+        let log2 = requested_irqs.next_power_of_two().trailing_zeros();
         assert!(log2 <= 5);
         let cfg = self.cfg.as_ref().unwrap();
         let (std, _msi) = inner.msi().unwrap();
@@ -1051,7 +1058,7 @@ impl PcieDevice {
     }
     fn mask_all_msi_vectors(&self, inner: &MutexGuard<PcieDeviceInner>) {
         for i in 0..inner.irq.handlers.len() {
-            self.mask_msi_irq(inner, i as u32, true);
+            self.mask_msi_irq(inner, i, true);
         }
         // just to be careful
         let cfg = self.cfg.as_ref().unwrap();
@@ -1060,7 +1067,7 @@ impl PcieDevice {
             cfg.write32_offset(msi.mask_bits_offset, u32::MAX);
         }
     }
-    fn mask_msi_irq(&self, inner: &MutexGuard<PcieDeviceInner>, irq: u32, mask: bool) -> bool {
+    fn mask_msi_irq(&self, inner: &MutexGuard<PcieDeviceInner>, irq: usize, mask: bool) -> bool {
         assert!(!inner.irq.handlers.is_empty());
         let cfg = self.cfg.as_ref().unwrap();
         let (_std, msi) = inner.msi().unwrap();
@@ -1098,13 +1105,14 @@ impl PcieDevice {
     }
 
     /// Set IRQ mode.
-    pub fn set_irq_mode(&self, mode: PcieIrqMode, mut irq_count: u32) -> ZxResult {
+    pub fn set_irq_mode(&self, mode: PcieIrqMode, requested_irqs: usize) -> ZxResult {
         let mut inner = self.inner.lock();
+        let mut requested_irqs = requested_irqs;
         if let PcieIrqMode::Disabled = mode {
-            irq_count = 0;
+            requested_irqs = 0;
         } else if !inner.plugged_in {
             return Err(ZxError::BAD_STATE);
-        } else if irq_count < 1 {
+        } else if requested_irqs < 1 {
             return Err(ZxError::INVALID_ARGS);
         }
         match inner.irq.mode {
@@ -1131,16 +1139,16 @@ impl PcieDevice {
         match mode {
             PcieIrqMode::Disabled => Ok(()),
             PcieIrqMode::Legacy => {
-                if inner.irq.legacy.pin == 0 || irq_count > 1 {
+                if inner.irq.legacy.pin == 0 || requested_irqs > 1 {
                     return Err(ZxError::NOT_SUPPORTED);
                 }
                 self.modify_cmd(0, PCIE_CFG_COMMAND_INT_DISABLE);
-                self.allocate_irq_handler(&mut inner, irq_count, true);
+                self.allocate_irq_handler(&mut inner, requested_irqs, true);
                 inner.irq.mode = PcieIrqMode::Legacy;
                 inner.irq.legacy.shared_handler.add_device(inner.arc_self());
                 Ok(())
             }
-            PcieIrqMode::Msi => self.enter_msi_irq_mode(&mut inner, irq_count),
+            PcieIrqMode::Msi => self.enter_msi_irq_mode(&mut inner, requested_irqs),
             PcieIrqMode::MsiX => Err(ZxError::NOT_SUPPORTED),
             _ => Err(ZxError::INVALID_ARGS),
         }

zircon-object/src/dev/pci/pci_init_args.rs

@@ -7,12 +7,13 @@ use super::constants::*;
 use crate::{ZxError, ZxResult};
 
 #[repr(transparent)]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, Debug)]
 pub struct PciIrqSwizzleLut(
     [[[u32; PCI_MAX_LEGACY_IRQ_PINS]; PCI_MAX_FUNCTIONS_PER_DEVICE]; PCI_MAX_DEVICES_PER_BUS],
 );
 
 #[repr(C)]
+#[derive(Debug)]
 pub struct PciInitArgsIrqs {
     pub global_irq: u32,
     pub level_triggered: bool,
@@ -21,6 +22,7 @@ pub struct PciInitArgsIrqs {
 }
 
 #[repr(C)]
+#[derive(Debug)]
 pub struct PciInitArgsHeader {
     pub dev_pin_to_global_irq: PciIrqSwizzleLut,
     pub num_irqs: u32,
@@ -52,16 +54,23 @@ impl PciInitArgsHeader {
         for i in 0..self.num_irqs as usize {
             let irq = &mut self.irqs[i];
             let global_irq = irq.global_irq;
-            if !interrupt::is_valid_irq(global_irq) {
+            if !interrupt::is_valid_irq(global_irq as usize) {
                 irq.global_irq = PCI_NO_IRQ_MAPPING;
                 self.dev_pin_to_global_irq.remove_irq(global_irq);
             } else {
-                interrupt::configure_irq(
+                use kernel_hal::interrupt::{IrqPolarity, IrqTriggerMode};
-                    global_irq,
+                let tm = if irq.level_triggered {
-                    irq.level_triggered, /* Trigger mode */
+                    IrqTriggerMode::Level
-                    irq.active_high,     /* Polarity */
+                } else {
-                )
+                    IrqTriggerMode::Edge
-                .map_err(|_| ZxError::INVALID_ARGS)?;
+                };
+                let pol = if irq.active_high {
+                    IrqPolarity::ActiveHigh
+                } else {
+                    IrqPolarity::ActiveLow
+                };
+                interrupt::configure_irq(global_irq as usize, tm, pol)
+                    .map_err(|_| ZxError::INVALID_ARGS)?;
             }
         }
         Ok(())

zircon-syscall/src/pci.rs

@@ -255,7 +255,7 @@ impl Syscall<'_> {
         );
         let proc = self.thread.proc();
         let device = proc.get_object_with_rights::<PcieDeviceKObject>(handle, Rights::WRITE)?;
-        device.set_irq_mode(mode, requested_irq_count)
+        device.set_irq_mode(mode, requested_irq_count as usize)
     }
 
     pub fn sys_pci_config_read(