From 0b6b5e0d04422b8601c13926ed571920e5a4814f Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 30 Aug 2025 18:25:29 +0200 Subject: [PATCH] pcid: Move MappedMsixRegs to pcid_interface --- net/rtl8139d/src/main.rs | 35 ++--------------------- net/rtl8168d/src/main.rs | 35 ++--------------------- pcid/src/driver_interface/msi.rs | 48 +++++++++++++++++++++++++++++++- storage/nvmed/src/main.rs | 30 +++----------------- storage/nvmed/src/nvme/mod.rs | 14 ++-------- virtio-core/src/arch/x86.rs | 19 ++----------- virtio-core/src/probe.rs | 19 ------------- xhcid/src/main.rs | 26 +++-------------- xhcid/src/xhci/mod.rs | 17 +---------- 9 files changed, 66 insertions(+), 177 deletions(-) diff --git a/net/rtl8139d/src/main.rs b/net/rtl8139d/src/main.rs index c0272136c7..80b1091808 100644 --- a/net/rtl8139d/src/main.rs +++ b/net/rtl8139d/src/main.rs @@ -1,17 +1,14 @@ use std::fs::File; use std::io::{Read, Write}; use std::os::unix::io::AsRawFd; -use std::ptr::NonNull; -use std::rc::Rc; use driver_network::NetworkScheme; use event::{user_data, EventQueue}; #[cfg(target_arch = "x86_64")] use pcid_interface::irq_helpers::allocate_single_interrupt_vector_for_msi; use pcid_interface::irq_helpers::read_bsp_apic_id; -use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use pcid_interface::{ - MappedBar, MsiSetFeatureInfo, PciFeature, PciFeatureInfo, PciFunctionHandle, SetFeatureInfo, + MsiSetFeatureInfo, PciFeature, PciFeatureInfo, PciFunctionHandle, SetFeatureInfo, }; pub mod device; @@ -28,21 +25,6 @@ where } } -pub struct MappedMsixRegs { - pub virt_table_base: NonNull, - pub info: MsixInfo, -} - -impl MappedMsixRegs { - pub unsafe fn table_entry_pointer_unchecked(&mut self, k: usize) -> &mut MsixTableEntry { - &mut *self.virt_table_base.as_ptr().offset(k as isize) - } - pub fn table_entry_pointer(&mut self, k: usize) -> &mut MsixTableEntry { - assert!(k < self.info.table_size as usize); - unsafe { self.table_entry_pointer_unchecked(k) } - } -} - #[cfg(target_arch = "x86_64")] fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { let pci_config = pcid_handle.config(); @@ -83,19 +65,7 @@ fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - msix_info.validate(pci_config.func.bars); - - let bar_address = unsafe { pcid_handle.map_bar(msix_info.table_bar) } - .ptr - .as_ptr() as usize; - - let virt_table_base = - (bar_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - - let mut info = MappedMsixRegs { - virt_table_base: NonNull::new(virt_table_base).unwrap(), - info: msix_info, - }; + let mut info = unsafe { msix_info.map_and_mask_all(pcid_handle) }; // Allocate one msi vector. @@ -103,7 +73,6 @@ fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { // primary interrupter let k = 0; - assert_eq!(std::mem::size_of::(), 16); let table_entry_pointer = info.table_entry_pointer(k); let destination_id = read_bsp_apic_id().expect("rtl8139d: failed to read BSP apic id"); diff --git a/net/rtl8168d/src/main.rs b/net/rtl8168d/src/main.rs index 44150572f7..44c8f41859 100644 --- a/net/rtl8168d/src/main.rs +++ b/net/rtl8168d/src/main.rs @@ -1,17 +1,14 @@ use std::fs::File; use std::io::{Read, Write}; use std::os::unix::io::AsRawFd; -use std::ptr::NonNull; -use std::rc::Rc; use driver_network::NetworkScheme; use event::{user_data, EventQueue}; #[cfg(target_arch = "x86_64")] use pcid_interface::irq_helpers::allocate_single_interrupt_vector_for_msi; use pcid_interface::irq_helpers::read_bsp_apic_id; -use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use pcid_interface::{ - MappedBar, MsiSetFeatureInfo, PciFeature, PciFeatureInfo, PciFunctionHandle, SetFeatureInfo, + MsiSetFeatureInfo, PciFeature, PciFeatureInfo, PciFunctionHandle, SetFeatureInfo, }; pub mod device; @@ -28,21 +25,6 @@ where } } -pub struct MappedMsixRegs { - pub virt_table_base: NonNull, - pub info: MsixInfo, -} - -impl MappedMsixRegs { - pub unsafe fn table_entry_pointer_unchecked(&mut self, k: usize) -> &mut MsixTableEntry { - &mut *self.virt_table_base.as_ptr().offset(k as isize) - } - pub fn table_entry_pointer(&mut self, k: usize) -> &mut MsixTableEntry { - assert!(k < self.info.table_size as usize); - unsafe { self.table_entry_pointer_unchecked(k) } - } -} - #[cfg(target_arch = "x86_64")] fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { let pci_config = pcid_handle.config(); @@ -83,19 +65,7 @@ fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - msix_info.validate(pci_config.func.bars); - - let bar_address = unsafe { pcid_handle.map_bar(msix_info.table_bar) } - .ptr - .as_ptr() as usize; - - let virt_table_base = - (bar_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - - let mut info = MappedMsixRegs { - virt_table_base: NonNull::new(virt_table_base).unwrap(), - info: msix_info, - }; + let mut info = unsafe { msix_info.map_and_mask_all(pcid_handle) }; // Allocate one msi vector. @@ -103,7 +73,6 @@ fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { // primary interrupter let k = 0; - assert_eq!(std::mem::size_of::(), 16); let table_entry_pointer = info.table_entry_pointer(k); let destination_id = read_bsp_apic_id().expect("rtl8168d: failed to read BSP apic id"); diff --git a/pcid/src/driver_interface/msi.rs b/pcid/src/driver_interface/msi.rs index 822ac0976c..0ca68ec556 100644 --- a/pcid/src/driver_interface/msi.rs +++ b/pcid/src/driver_interface/msi.rs @@ -1,6 +1,8 @@ use std::fmt; +use std::ptr::NonNull; use crate::driver_interface::PciBar; +use crate::PciFunctionHandle; use common::io::{Io, Mmio}; use serde::{Deserialize, Serialize}; @@ -32,7 +34,32 @@ pub struct MsixInfo { } impl MsixInfo { - pub fn validate(&self, bars: [PciBar; 6]) { + pub unsafe fn map_and_mask_all(self, pcid_handle: &mut PciFunctionHandle) -> MappedMsixRegs { + self.validate(pcid_handle.config().func.bars); + + let virt_table_base = unsafe { + pcid_handle + .map_bar(self.table_bar) + .ptr + .as_ptr() + .byte_add(self.table_offset as usize) + }; + + let mut info = MappedMsixRegs { + virt_table_base: NonNull::new(virt_table_base.cast::()).unwrap(), + info: self, + }; + + // Mask all interrupts in case some earlier driver/os already unmasked them (according to + // the PCI Local Bus spec 3.0, they are masked after system reset). + for i in 0..info.info.table_size { + info.table_entry_pointer(i.into()).mask(); + } + + info + } + + fn validate(&self, bars: [PciBar; 6]) { if self.table_bar > 5 { panic!( "MSI-X Table BIR contained a reserved enum value: {}", @@ -78,6 +105,21 @@ impl MsixInfo { } } +pub struct MappedMsixRegs { + pub virt_table_base: NonNull, + pub info: MsixInfo, +} +impl MappedMsixRegs { + pub unsafe fn table_entry_pointer_unchecked(&mut self, k: usize) -> &mut MsixTableEntry { + &mut *self.virt_table_base.as_ptr().add(k) + } + + pub fn table_entry_pointer(&mut self, k: usize) -> &mut MsixTableEntry { + assert!(k < self.info.table_size as usize); + unsafe { self.table_entry_pointer_unchecked(k) } + } +} + #[repr(C, packed)] pub struct MsixTableEntry { pub addr_lo: Mmio, @@ -86,6 +128,10 @@ pub struct MsixTableEntry { pub vec_ctl: Mmio, } +const _: () = { + assert!(size_of::() == 16); +}; + #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] pub mod x86 { #[repr(u8)] diff --git a/storage/nvmed/src/main.rs b/storage/nvmed/src/main.rs index 6966570a52..342caf055e 100644 --- a/storage/nvmed/src/main.rs +++ b/storage/nvmed/src/main.rs @@ -5,10 +5,9 @@ use std::cell::RefCell; use std::fs::File; use std::io::{self, Read, Write}; use std::os::fd::AsRawFd; -use std::ptr::NonNull; use std::rc::Rc; use std::sync::Arc; -use std::{slice, usize}; +use std::usize; use driver_block::{Disk, DiskScheme}; use pcid_interface::{PciFeature, PciFeatureInfo, PciFunction, PciFunctionHandle}; @@ -39,35 +38,17 @@ fn get_int_method( // TODO: Allocate more than one vector when possible and useful. if has_msix { // Extended message signaled interrupts. - use self::nvme::MappedMsixRegs; - use pcid_interface::msi::MsixTableEntry; let msix_info = match pcid_handle.feature_info(PciFeature::MsiX) { PciFeatureInfo::MsiX(msix) => msix, _ => unreachable!(), }; - msix_info.validate(function.bars); - fn bar_base(pcid_handle: &mut PciFunctionHandle, bir: u8) -> Result> { - Ok(unsafe { pcid_handle.map_bar(bir) }.ptr) - } - let table_bar_base: *mut u8 = bar_base(pcid_handle, msix_info.table_bar)?.as_ptr(); - let table_base = unsafe { table_bar_base.offset(msix_info.table_offset as isize) }; - - let vector_count = msix_info.table_size; - let table_entries: &'static mut [MsixTableEntry] = unsafe { - slice::from_raw_parts_mut(table_base as *mut MsixTableEntry, vector_count as usize) - }; - - // Mask all interrupts in case some earlier driver/os already unmasked them (according to - // the PCI Local Bus spec 3.0, they are masked after system reset). - for table_entry in table_entries.iter_mut() { - table_entry.mask(); - } + let mut info = unsafe { msix_info.map_and_mask_all(pcid_handle) }; pcid_handle.enable_feature(PciFeature::MsiX); let (msix_vector_number, irq_handle) = { - let entry: &mut MsixTableEntry = &mut table_entries[0]; + let entry = info.table_entry_pointer(0); let bsp_cpu_id = irq_helpers::read_bsp_apic_id().expect("nvmed: failed to read APIC ID"); @@ -79,10 +60,7 @@ fn get_int_method( (0, irq_handle) }; - let interrupt_method = InterruptMethod::MsiX(MappedMsixRegs { - info: msix_info, - table: table_entries, - }); + let interrupt_method = InterruptMethod::MsiX(info); let interrupt_sources = InterruptSources::MsiX(std::iter::once((msix_vector_number, irq_handle)).collect()); diff --git a/storage/nvmed/src/nvme/mod.rs b/storage/nvmed/src/nvme/mod.rs index 513814c9d0..93dfab4ed2 100644 --- a/storage/nvmed/src/nvme/mod.rs +++ b/storage/nvmed/src/nvme/mod.rs @@ -21,7 +21,7 @@ pub mod queues; use self::executor::NvmeExecutor; pub use self::queues::{NvmeCmd, NvmeCmdQueue, NvmeComp, NvmeCompQueue}; -use pcid_interface::msi::{MsiInfo, MsixInfo, MsixTableEntry}; +use pcid_interface::msi::{MappedMsixRegs, MsiInfo}; use pcid_interface::PciFunctionHandle; #[cfg(target_arch = "aarch64")] @@ -133,11 +133,6 @@ impl InterruptMethod { } } -pub struct MappedMsixRegs { - pub info: MsixInfo, - pub table: &'static mut [MsixTableEntry], -} - #[repr(C, packed)] pub struct NvmeRegs { /// Controller Capabilities @@ -316,7 +311,7 @@ impl Nvme { self.regs.get_mut().intmc.write(0x0000_0001); } &mut InterruptMethod::MsiX(ref mut cfg) => { - cfg.table[0].unmask(); + cfg.table_entry_pointer(0).unmask(); } } @@ -442,10 +437,7 @@ impl Nvme { } &mut InterruptMethod::MsiX(ref mut cfg) => { for (vector, mask) in vectors { - cfg.table - .get_mut(vector as usize) - .expect("nvmed: internal error: MSI-X vector out of range") - .set_masked(mask); + cfg.table_entry_pointer(vector.into()).set_masked(mask); } } } diff --git a/virtio-core/src/arch/x86.rs b/virtio-core/src/arch/x86.rs index 7275c63e41..751f268e07 100644 --- a/virtio-core/src/arch/x86.rs +++ b/virtio-core/src/arch/x86.rs @@ -1,32 +1,19 @@ use crate::transport::Error; use pcid_interface::irq_helpers::{allocate_single_interrupt_vector_for_msi, read_bsp_apic_id}; -use pcid_interface::msi::MsixTableEntry; -use std::{fs::File, ptr::NonNull}; +use std::fs::File; -use crate::{probe::MappedMsixRegs, MSIX_PRIMARY_VECTOR}; +use crate::MSIX_PRIMARY_VECTOR; use pcid_interface::*; pub fn enable_msix(pcid_handle: &mut PciFunctionHandle) -> Result { - let pci_config = pcid_handle.config(); - // Extended message signaled interrupts. let msix_info = match pcid_handle.feature_info(PciFeature::MsiX) { PciFeatureInfo::MsiX(capability) => capability, _ => unreachable!(), }; - msix_info.validate(pci_config.func.bars); - - let bar_address = unsafe { pcid_handle.map_bar(msix_info.table_bar) } - .ptr - .as_ptr() as usize; - let virt_table_base = (bar_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - - let mut info = MappedMsixRegs { - virt_table_base: NonNull::new(virt_table_base).unwrap(), - info: msix_info, - }; + let mut info = unsafe { msix_info.map_and_mask_all(pcid_handle) }; // Allocate the primary MSI vector. // FIXME allow the driver to register multiple MSI-X vectors diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index be3977d96b..da95234636 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -1,8 +1,6 @@ use std::fs::File; -use std::ptr::NonNull; use std::sync::Arc; -use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use pcid_interface::*; use crate::spec::*; @@ -20,23 +18,6 @@ pub struct Device { unsafe impl Send for Device {} unsafe impl Sync for Device {} -pub struct MappedMsixRegs { - pub virt_table_base: NonNull, - pub info: MsixInfo, -} - -impl MappedMsixRegs { - pub unsafe fn table_entry_pointer_unchecked(&mut self, k: usize) -> &mut MsixTableEntry { - &mut *self.virt_table_base.as_ptr().add(k) - } - - pub fn table_entry_pointer(&mut self, k: usize) -> &mut MsixTableEntry { - assert!(k < self.info.table_size as usize); - unsafe { self.table_entry_pointer_unchecked(k) } - } -} - -static_assertions::const_assert_eq!(std::mem::size_of::(), 16); pub const MSIX_PRIMARY_VECTOR: u16 = 0; diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs index 800614e2e0..a7c6034105 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -26,13 +26,11 @@ extern crate bitflags; use std::fs::File; -use std::ptr::NonNull; use std::sync::{Arc, Mutex}; #[cfg(target_arch = "x86_64")] use pcid_interface::irq_helpers::allocate_single_interrupt_vector_for_msi; use pcid_interface::irq_helpers::read_bsp_apic_id; -use pcid_interface::msi::MsixTableEntry; use pcid_interface::{ MsiSetFeatureInfo, PciFeature, PciFeatureInfo, PciFunctionHandle, SetFeatureInfo, }; @@ -50,10 +48,7 @@ mod usb; mod xhci; #[cfg(target_arch = "x86_64")] -fn get_int_method( - pcid_handle: &mut PciFunctionHandle, - bar0_address: usize, -) -> (Option, InterruptMethod) { +fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> (Option, InterruptMethod) { let pci_config = pcid_handle.config(); let all_pci_features = pcid_handle.fetch_all_features(); @@ -92,16 +87,7 @@ fn get_int_method( PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - msix_info.validate(pci_config.func.bars); - - assert_eq!(msix_info.table_bar, 0); - let virt_table_base = - (bar0_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - - let mut info = xhci::MappedMsixRegs { - virt_table_base: NonNull::new(virt_table_base).unwrap(), - info: msix_info, - }; + let mut info = unsafe { msix_info.map_and_mask_all(pcid_handle) }; // Allocate one msi vector. @@ -109,7 +95,6 @@ fn get_int_method( // primary interrupter let k = 0; - assert_eq!(std::mem::size_of::(), 16); let table_entry_pointer = info.table_entry_pointer(k); let destination_id = read_bsp_apic_id().expect("xhcid: failed to read BSP apic id"); @@ -141,10 +126,7 @@ fn get_int_method( //TODO: MSI on non-x86_64? #[cfg(not(target_arch = "x86_64"))] -fn get_int_method( - pcid_handle: &mut PciFunctionHandle, - address: usize, -) -> (Option, InterruptMethod) { +fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> (Option, InterruptMethod) { let pci_config = pcid_handle.config(); if let Some(irq) = pci_config.func.legacy_interrupt_line { @@ -179,7 +161,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let address = unsafe { pcid_handle.map_bar(0) }.ptr.as_ptr() as usize; - let (irq_file, interrupt_method) = (None, InterruptMethod::Polling); //get_int_method(&mut pcid_handle, address); + let (irq_file, interrupt_method) = (None, InterruptMethod::Polling); //get_int_method(&mut pcid_handle); //TODO: Fix interrupts. println!(" + XHCI {}", pci_config.func.display()); diff --git a/xhcid/src/xhci/mod.rs b/xhcid/src/xhci/mod.rs index 3d3798a312..bbbfb9ffbd 100644 --- a/xhcid/src/xhci/mod.rs +++ b/xhcid/src/xhci/mod.rs @@ -12,11 +12,11 @@ use std::collections::BTreeMap; use std::convert::TryFrom; use std::fs::File; -use std::ptr::NonNull; use std::sync::atomic::AtomicUsize; use std::sync::{Arc, Mutex, MutexGuard}; use std::{mem, process, slice, thread}; +use pcid_interface::msi::MappedMsixRegs; use syscall::error::{Error, Result, EBADF, EBADMSG, EIO, ENOENT}; use syscall::{EAGAIN, PAGE_SIZE}; @@ -28,7 +28,6 @@ use serde::Deserialize; use crate::usb; -use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use pcid_interface::PciFunctionHandle; mod capability; @@ -78,20 +77,6 @@ pub enum InterruptMethod { MsiX(Mutex), } -pub struct MappedMsixRegs { - pub virt_table_base: NonNull, - pub info: MsixInfo, -} -impl MappedMsixRegs { - pub unsafe fn table_entry_pointer_unchecked(&mut self, k: usize) -> &mut MsixTableEntry { - &mut *self.virt_table_base.as_ptr().offset(k as isize) - } - pub fn table_entry_pointer(&mut self, k: usize) -> &mut MsixTableEntry { - assert!(k < self.info.table_size as usize); - unsafe { self.table_entry_pointer_unchecked(k) } - } -} - impl Xhci { /// Gets descriptors, before the port state is initiated. async fn get_desc_raw(