From df814340887dfd9707029f2a25271441ce94691b Mon Sep 17 00:00:00 2001 From: Yuke Peng Date: Thu, 10 Aug 2023 16:13:41 +0800 Subject: [PATCH] Fix pci framework bugs --- framework/jinux-frame/src/bus/pci/bus.rs | 10 ++- .../jinux-frame/src/bus/pci/capability/mod.rs | 6 +- .../src/bus/pci/capability/msix.rs | 90 ++++++++++++------- .../src/bus/pci/capability/vendor.rs | 2 +- .../jinux-frame/src/bus/pci/common_device.rs | 6 +- framework/jinux-frame/src/bus/pci/mod.rs | 6 +- 6 files changed, 77 insertions(+), 43 deletions(-) diff --git a/framework/jinux-frame/src/bus/pci/bus.rs b/framework/jinux-frame/src/bus/pci/bus.rs index 8b545de00..968e76755 100644 --- a/framework/jinux-frame/src/bus/pci/bus.rs +++ b/framework/jinux-frame/src/bus/pci/bus.rs @@ -9,7 +9,7 @@ pub trait PciDevice: Sync + Send + Debug { fn device_id(&self) -> PciDeviceId; } -#[derive(Debug)] +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] pub enum PciDriverProbeError { DeviceNotMatch, ConfigurationSpaceError, @@ -55,7 +55,9 @@ impl PciBus { continue; } Err((err, common_device)) => { - error!("PCI device construction failed, reason: {:?}", err); + if err != PciDriverProbeError::DeviceNotMatch { + error!("PCI device construction failed, reason: {:?}", err); + } debug_assert!(device_id == *common_device.device_id()); common_device } @@ -76,7 +78,9 @@ impl PciBus { return; } Err((err, common_device)) => { - error!("PCI device construction failed, reason: {:?}", err); + if err != PciDriverProbeError::DeviceNotMatch { + error!("PCI device construction failed, reason: {:?}", err); + } debug_assert!(device_id == *common_device.device_id()); common_device } diff --git a/framework/jinux-frame/src/bus/pci/capability/mod.rs b/framework/jinux-frame/src/bus/pci/capability/mod.rs index 92cb145af..63628e74f 100644 --- a/framework/jinux-frame/src/bus/pci/capability/mod.rs +++ b/framework/jinux-frame/src/bus/pci/capability/mod.rs @@ -73,8 +73,12 @@ impl Capability { /// 0xFC, the top of the capability position. const CAPABILITY_TOP: u16 = 0xFC; + pub fn capability_data(&self) -> &CapabilityData { + &self.cap_data + } + /// get the capabilities of one device - pub fn device_capabilities(dev: &mut PciCommonDevice) -> Vec { + pub(super) fn device_capabilities(dev: &mut PciCommonDevice) -> Vec { if !dev.status().contains(Status::CAPABILITIES_LIST) { return Vec::new(); } diff --git a/framework/jinux-frame/src/bus/pci/capability/msix.rs b/framework/jinux-frame/src/bus/pci/capability/msix.rs index 25e92de3c..6e61125fa 100644 --- a/framework/jinux-frame/src/bus/pci/capability/msix.rs +++ b/framework/jinux-frame/src/bus/pci/capability/msix.rs @@ -2,16 +2,17 @@ use alloc::{sync::Arc, vec::Vec}; use crate::{ bus::pci::{ - cfg_space::{Bar, MemoryBar}, + cfg_space::{Bar, Command, MemoryBar}, common_device::PciCommonDevice, device_info::PciDeviceLocation, }, + sync::{SpinLock, SpinLockGuard}, trap::IrqAllocateHandle, vm::VmIo, }; /// MSI-X capability. It will set the BAR space it uses to be hidden. -#[derive(Debug, Clone)] +#[derive(Debug)] #[repr(C)] pub struct CapabilityMsixData { loc: PciDeviceLocation, @@ -22,7 +23,26 @@ pub struct CapabilityMsixData { table_bar: Arc, /// Pending bits table. pending_table_bar: Arc, - irq_allocate_handles: Vec>>, + table_offset: usize, + pending_table_offset: usize, + irq_allocate_handles: SpinLock>>, +} + +impl Clone for CapabilityMsixData { + fn clone(&self) -> Self { + let handles = self.irq_allocate_handles.lock(); + let new_vec = handles.clone().to_vec(); + Self { + loc: self.loc.clone(), + ptr: self.ptr.clone(), + table_size: self.table_size.clone(), + table_bar: self.table_bar.clone(), + pending_table_bar: self.pending_table_bar.clone(), + irq_allocate_handles: SpinLock::new(new_vec), + table_offset: self.table_offset, + pending_table_offset: self.pending_table_offset, + } + } } impl CapabilityMsixData { @@ -60,28 +80,37 @@ impl CapabilityMsixData { } } + let pba_offset = (pba_info & !(0b111u32)) as usize; + let table_offset = (table_info & !(0b111u32)) as usize; + let table_size = (dev.location().read16(cap_ptr + 2) & 0b11_1111_1111) + 1; // TODO: Different architecture seems to have different, so we should set different address here. - let message_address = 0xFEE0_000 as u32; - let message_upper_address = 0 as u32; + let message_address = 0xFEE0_0000u32; + let message_upper_address = 0u32; // Set message address 0xFEE0_0000 for i in 0..table_size { // Set message address and disable this msix entry table_bar .io_mem() - .write_val((16 * i) as usize, &message_address) + .write_val((16 * i) as usize + table_offset, &message_address) .unwrap(); table_bar .io_mem() - .write_val((16 * i + 4) as usize, &message_upper_address) + .write_val((16 * i + 4) as usize + table_offset, &message_upper_address) .unwrap(); table_bar .io_mem() - .write_val((16 * i + 12) as usize, &(1 as u32)) + .write_val((16 * i + 12) as usize + table_offset, &(1 as u32)) .unwrap(); } + // enable MSI-X, bit15: MSI-X Enable + dev.location() + .write16(cap_ptr + 2, dev.location().read16(cap_ptr + 2) | 0x8000); + // disable INTx, enable Bus master. + dev.set_command(dev.command() | Command::INTERRUPT_DISABLE | Command::BUS_MASTER); + let mut irq_allocate_handles = Vec::with_capacity(table_size as usize); for i in 0..table_size { irq_allocate_handles.push(None); @@ -93,7 +122,9 @@ impl CapabilityMsixData { table_size: (dev.location().read16(cap_ptr + 2) & 0b11_1111_1111) + 1, table_bar, pending_table_bar: pba_bar, - irq_allocate_handles, + irq_allocate_handles: SpinLock::new(irq_allocate_handles), + table_offset: table_offset, + pending_table_offset: pba_offset, } } @@ -102,38 +133,31 @@ impl CapabilityMsixData { (self.loc.read16(self.ptr + 2) & 0b11_1111_1111) + 1 } - pub fn set_msix_enable(&self, enable: bool) { - // bit15: msix enable - let value = (enable as u16) << 15; - // message control - self.loc.write16( - self.ptr + 2, - set_bit(self.loc.read16(self.ptr + 2), 15, enable), - ) - } - - pub fn set_interrupt_enable(&self, enable: bool) { - // bit14: msix enable - let value = (enable as u16) << 14; - // message control - self.loc.write16( - self.ptr + 2, - set_bit(self.loc.read16(self.ptr + 2), 14, enable), - ) - } - - pub fn set_interrupt_vector(&mut self, vector: Arc, index: u16) { + pub fn set_interrupt_vector(&self, handle: IrqAllocateHandle, index: u16) { if index >= self.table_size { return; } - let old_handles = - core::mem::replace(&mut self.irq_allocate_handles[index as usize], Some(vector)); + self.table_bar + .io_mem() + .write_val( + (16 * index + 8) as usize + self.table_offset, + &(handle.num() as u32), + ) + .unwrap(); + let old_handles = core::mem::replace( + &mut self.irq_allocate_handles.lock()[index as usize], + Some(handle), + ); // Enable this msix vector self.table_bar .io_mem() - .write_val((16 * index + 12) as usize, &(0 as u32)) + .write_val((16 * index + 12) as usize + self.table_offset, &(0 as u32)) .unwrap(); } + + pub fn interrupt_handles(&self) -> SpinLockGuard<'_, Vec>> { + self.irq_allocate_handles.lock() + } } fn set_bit(origin_value: u16, offset: usize, set: bool) -> u16 { diff --git a/framework/jinux-frame/src/bus/pci/capability/vendor.rs b/framework/jinux-frame/src/bus/pci/capability/vendor.rs index b4bc2d1d9..a43ea829d 100644 --- a/framework/jinux-frame/src/bus/pci/capability/vendor.rs +++ b/framework/jinux-frame/src/bus/pci/capability/vendor.rs @@ -55,7 +55,7 @@ impl CapabilityVndrData { #[inline] fn check_range(&self, offset: u16) -> Result<()> { - if self.length > offset { + if self.length < offset { return Err(Error::InvalidArgs); } Ok(()) diff --git a/framework/jinux-frame/src/bus/pci/common_device.rs b/framework/jinux-frame/src/bus/pci/common_device.rs index 9b3f1e02d..5cd0d23f7 100644 --- a/framework/jinux-frame/src/bus/pci/common_device.rs +++ b/framework/jinux-frame/src/bus/pci/common_device.rs @@ -112,15 +112,17 @@ impl BarManager { while idx < max { match Bar::new(location, idx) { Ok(bar) => { + let mut idx_step = 0; match &bar { Bar::Memory(memory_bar) => { if memory_bar.address_length() == AddrLen::Bits64 { - idx += 1; + idx_step = 1; } } Bar::Io(_) => {} } - bars[idx as usize] = Some((bar, false)); + bars[idx as usize] = Some((bar, true)); + idx += idx_step; } // ignore for now Err(_) => {} diff --git a/framework/jinux-frame/src/bus/pci/mod.rs b/framework/jinux-frame/src/bus/pci/mod.rs index f81f3be20..d4497b547 100644 --- a/framework/jinux-frame/src/bus/pci/mod.rs +++ b/framework/jinux-frame/src/bus/pci/mod.rs @@ -49,9 +49,9 @@ //! ``` pub mod bus; -mod capability; -mod cfg_space; -mod common_device; +pub mod capability; +pub mod cfg_space; +pub mod common_device; mod device_info; use crate::sync::Mutex;