From df666331116c26f9d6d2c6007afc015293df9802 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 10 Jun 2024 21:00:07 +0200 Subject: [PATCH] pcid: Store decoded MsixInfo instead of full capability in PciFeatureInfo --- net/rtl8139d/src/main.rs | 22 +++++++++++----------- net/rtl8168d/src/main.rs | 22 +++++++++++----------- pcid/src/driver_handler.rs | 8 +++++++- pcid/src/driver_interface/mod.rs | 2 +- pcid/src/pci/msi.rs | 31 +++++++++++++++++++++---------- storage/nvmed/src/main.rs | 16 ++++++++-------- storage/nvmed/src/nvme/mod.rs | 8 ++++---- virtio-core/src/arch/x86_64.rs | 14 +++++++------- virtio-core/src/probe.rs | 10 +++++----- xhcid/src/main.rs | 12 ++++++------ xhcid/src/xhci/mod.rs | 16 ++++++++-------- 11 files changed, 89 insertions(+), 72 deletions(-) diff --git a/net/rtl8139d/src/main.rs b/net/rtl8139d/src/main.rs index fc51f4653b..a949d46e18 100644 --- a/net/rtl8139d/src/main.rs +++ b/net/rtl8139d/src/main.rs @@ -13,7 +13,7 @@ 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::{MsixCapability, MsixTableEntry}; +use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use pcid_interface::{ MsiSetFeatureInfo, PciFeature, PciFeatureInfo, PcidServerHandle, SetFeatureInfo, SubdriverArguments, @@ -79,17 +79,17 @@ where } } -pub struct MsixInfo { +pub struct MappedMsixRegs { pub virt_table_base: NonNull, - pub capability: MsixCapability, + pub info: MsixInfo, } -impl 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.capability.table_size() as usize); + assert!(k < self.info.table_size as usize); unsafe { self.table_entry_pointer_unchecked(k) } } } @@ -129,20 +129,20 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { interrupt_handle } else if has_msix { - let capability = match pcid_handle.feature_info(PciFeature::MsiX).expect("rtl8139d: failed to retrieve the MSI-X capability structure from pcid") { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX).expect("rtl8139d: failed to retrieve the MSI-X capability structure from pcid") { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - capability.validate(pci_config.func.bars); + msix_info.validate(pci_config.func.bars); - let bar = &pci_config.func.bars[capability.table_bir() as usize]; + let bar = &pci_config.func.bars[msix_info.table_bar as usize]; let bar_address = unsafe { bar.physmap_mem("rtl8139d") } as usize; - let virt_table_base = (bar_address + capability.table_offset() as usize) as *mut MsixTableEntry; + let virt_table_base = (bar_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - let mut info = MsixInfo { + let mut info = MappedMsixRegs { virt_table_base: NonNull::new(virt_table_base).unwrap(), - capability, + info: msix_info, }; // Allocate one msi vector. diff --git a/net/rtl8168d/src/main.rs b/net/rtl8168d/src/main.rs index d13cc10d80..c797c2e059 100644 --- a/net/rtl8168d/src/main.rs +++ b/net/rtl8168d/src/main.rs @@ -12,7 +12,7 @@ use pcid_interface::{MsiSetFeatureInfo, PcidServerHandle, PciFeature, PciFeature #[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::{MsixCapability, MsixTableEntry}; +use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use redox_log::{RedoxLogger, OutputBuilder}; use syscall::EventFlags; @@ -74,17 +74,17 @@ where } } -pub struct MsixInfo { +pub struct MappedMsixRegs { pub virt_table_base: NonNull, - pub capability: MsixCapability, + pub info: MsixInfo, } -impl 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.capability.table_size() as usize); + assert!(k < self.info.table_size as usize); unsafe { self.table_entry_pointer_unchecked(k) } } } @@ -124,20 +124,20 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { interrupt_handle } else if has_msix { - let capability = match pcid_handle.feature_info(PciFeature::MsiX).expect("rtl8168d: failed to retrieve the MSI-X capability structure from pcid") { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX).expect("rtl8168d: failed to retrieve the MSI-X capability structure from pcid") { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - capability.validate(pci_config.func.bars); + msix_info.validate(pci_config.func.bars); - let bar = &pci_config.func.bars[capability.table_bir() as usize]; + let bar = &pci_config.func.bars[msix_info.table_bar as usize]; let bar_address = unsafe { bar.physmap_mem("rtl8168d") } as usize; - let virt_table_base = (bar_address + capability.table_offset() as usize) as *mut MsixTableEntry; + let virt_table_base = (bar_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - let mut info = MsixInfo { + let mut info = MappedMsixRegs { virt_table_base: NonNull::new(virt_table_base).unwrap(), - capability, + info: msix_info, }; // Allocate one msi vector. diff --git a/pcid/src/driver_handler.rs b/pcid/src/driver_handler.rs index c809184d9d..3358a501fc 100644 --- a/pcid/src/driver_handler.rs +++ b/pcid/src/driver_handler.rs @@ -212,7 +212,13 @@ impl DriverHandler { .iter() .find_map(|capability| capability.as_msix()) { - PciFeatureInfo::MsiX(*info) + PciFeatureInfo::MsiX(msi::MsixInfo { + table_bar: info.table_bir(), + table_offset: info.table_offset(), + table_size: info.table_size(), + pba_bar: info.pba_bir(), + pba_offset: info.pba_offset(), + }) } else { return PcidClientResponse::Error( PcidServerResponseError::NonexistentFeature(feature), diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index d7c1cdf16a..483da35602 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -133,7 +133,7 @@ impl PciFeature { #[derive(Debug, Serialize, Deserialize)] pub enum PciFeatureInfo { Msi(msi::MsiCapability), - MsiX(msi::MsixCapability), + MsiX(msi::MsixInfo), } #[derive(Debug, Error)] diff --git a/pcid/src/pci/msi.rs b/pcid/src/pci/msi.rs index ac042985c9..179ae638f2 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -231,24 +231,33 @@ impl MsiCapability { } } -impl MsixCapability { +#[derive(Debug, Serialize, Deserialize)] +pub struct MsixInfo { + pub table_bar: u8, + pub table_offset: u32, + pub table_size: u16, + pub pba_bar: u8, + pub pba_offset: u32, +} + +impl MsixInfo { pub fn validate(&self, bars: [PciBar; 6]) { - if self.table_bir() > 5 { - panic!("MSI-X Table BIR contained a reserved enum value: {}", self.table_bir()); + if self.table_bar > 5 { + panic!("MSI-X Table BIR contained a reserved enum value: {}", self.table_bar); } - if self.pba_bir() > 5 { - panic!("MSI-X PBA BIR contained a reserved enum value: {}", self.pba_bir()); + if self.pba_bar > 5 { + panic!("MSI-X PBA BIR contained a reserved enum value: {}", self.pba_bar); } - let table_size = self.table_size(); - let table_offset = self.table_offset() as usize; + let table_size = self.table_size; + let table_offset = self.table_offset as usize; let table_min_length = table_size * 16; - let pba_offset = self.pba_offset() as usize; + let pba_offset = self.pba_offset as usize; let pba_min_length = table_size.div_ceil(8); - let (_, table_bar_size) = bars[self.table_bir() as usize].expect_mem(); - let (_, pba_bar_size) = bars[self.pba_bir() as usize].expect_mem(); + let (_, table_bar_size) = bars[self.table_bar as usize].expect_mem(); + let (_, pba_bar_size) = bars[self.pba_bar as usize].expect_mem(); // Ensure that the table and PBA are within the BAR. @@ -270,7 +279,9 @@ impl MsixCapability { ); } } +} +impl MsixCapability { const MC_MSIX_ENABLED_BIT: u16 = 1 << 15; const MC_MSIX_ENABLED_SHIFT: u8 = 15; const MC_FUNCTION_MASK_BIT: u16 = 1 << 14; diff --git a/storage/nvmed/src/main.rs b/storage/nvmed/src/main.rs index 30d0b2788a..501c1b74e8 100644 --- a/storage/nvmed/src/main.rs +++ b/storage/nvmed/src/main.rs @@ -82,14 +82,14 @@ fn get_int_method( // TODO: Allocate more than one vector when possible and useful. if has_msix { // Extended message signaled interrupts. - use self::nvme::MsixCfg; + use self::nvme::MappedMsixRegs; use pcid_interface::msi::MsixTableEntry; - let mut capability_struct = match pcid_handle.feature_info(PciFeature::MsiX).unwrap() { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX).unwrap() { PciFeatureInfo::MsiX(msix) => msix, _ => unreachable!(), }; - capability_struct.validate(function.bars); + msix_info.validate(function.bars); fn bar_base( allocated_bars: &AllocatedBars, function: &PciFunction, @@ -109,11 +109,11 @@ fn get_int_method( } } let table_bar_base: *mut u8 = - bar_base(allocated_bars, function, capability_struct.table_bir())?.as_ptr(); + bar_base(allocated_bars, function, msix_info.table_bar)?.as_ptr(); let table_base = - unsafe { table_bar_base.offset(capability_struct.table_offset() as isize) }; + unsafe { table_bar_base.offset(msix_info.table_offset as isize) }; - let vector_count = capability_struct.table_size(); + 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) }; @@ -139,8 +139,8 @@ fn get_int_method( (0, irq_handle) }; - let interrupt_method = InterruptMethod::MsiX(MsixCfg { - cap: capability_struct, + let interrupt_method = InterruptMethod::MsiX(MappedMsixRegs { + info: msix_info, table: table_entries, }); let interrupt_sources = diff --git a/storage/nvmed/src/nvme/mod.rs b/storage/nvmed/src/nvme/mod.rs index 6378fa2fce..7781f477bc 100644 --- a/storage/nvmed/src/nvme/mod.rs +++ b/storage/nvmed/src/nvme/mod.rs @@ -22,7 +22,7 @@ pub mod queues; use self::cq_reactor::NotifReq; pub use self::queues::{NvmeCmd, NvmeCmdQueue, NvmeComp, NvmeCompQueue}; -use pcid_interface::msi::{MsiCapability, MsixCapability, MsixTableEntry}; +use pcid_interface::msi::{MsiCapability, MsixInfo, MsixTableEntry}; use pcid_interface::PcidServerHandle; #[cfg(target_arch = "aarch64")] @@ -93,7 +93,7 @@ pub enum InterruptMethod { /// Message signaled interrupts Msi(MsiCapability), /// Extended message signaled interrupts - MsiX(MsixCfg), + MsiX(MappedMsixRegs), } impl InterruptMethod { fn is_intx(&self) -> bool { @@ -119,8 +119,8 @@ impl InterruptMethod { } } -pub struct MsixCfg { - pub cap: MsixCapability, +pub struct MappedMsixRegs { + pub info: MsixInfo, pub table: &'static mut [MsixTableEntry], } diff --git a/virtio-core/src/arch/x86_64.rs b/virtio-core/src/arch/x86_64.rs index 7b95c4fe84..e1b25fbbb5 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -4,7 +4,7 @@ use pcid_interface::irq_helpers::{allocate_single_interrupt_vector_for_msi, read use pcid_interface::msi::MsixTableEntry; use std::{fs::File, ptr::NonNull}; -use crate::{probe::MsixInfo, MSIX_PRIMARY_VECTOR}; +use crate::{probe::MappedMsixRegs, MSIX_PRIMARY_VECTOR}; use pcid_interface::*; @@ -12,19 +12,19 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { let pci_config = pcid_handle.fetch_config()?; // Extended message signaled interrupts. - let capability = match pcid_handle.feature_info(PciFeature::MsiX)? { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX)? { PciFeatureInfo::MsiX(capability) => capability, _ => unreachable!(), }; - capability.validate(pci_config.func.bars); + msix_info.validate(pci_config.func.bars); - let bar = &pci_config.func.bars[capability.table_bir() as usize]; + let bar = &pci_config.func.bars[msix_info.table_bar as usize]; let bar_address = unsafe { bar.physmap_mem("virtio-core") } as usize; - let virt_table_base = (bar_address + capability.table_offset() as usize) as *mut MsixTableEntry; + let virt_table_base = (bar_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - let mut info = MsixInfo { + let mut info = MappedMsixRegs { virt_table_base: NonNull::new(virt_table_base).unwrap(), - capability, + info: msix_info, }; // Allocate the primary MSI vector. diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index e61898f848..7650736993 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -2,7 +2,7 @@ use std::fs::File; use std::ptr::NonNull; use std::sync::Arc; -use pcid_interface::msi::{MsixCapability, MsixTableEntry}; +use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use pcid_interface::*; use crate::spec::*; @@ -20,18 +20,18 @@ pub struct Device { unsafe impl Send for Device {} unsafe impl Sync for Device {} -pub struct MsixInfo { +pub struct MappedMsixRegs { pub virt_table_base: NonNull, - pub capability: MsixCapability, + pub info: MsixInfo, } -impl 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.capability.table_size() as usize); + assert!(k < self.info.table_size as usize); unsafe { self.table_entry_pointer_unchecked(k) } } } diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs index 047e16e148..ef5b122beb 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -119,18 +119,18 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, bar0_address: usize) -> (O (Some(interrupt_handle), InterruptMethod::Msi) } else if has_msix { - let capability = match pcid_handle.feature_info(PciFeature::MsiX).expect("xhcid: failed to retrieve the MSI-X capability structure from pcid") { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX).expect("xhcid: failed to retrieve the MSI-X capability structure from pcid") { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - capability.validate(pci_config.func.bars); + msix_info.validate(pci_config.func.bars); - assert_eq!(capability.table_bir(), 0); - let virt_table_base = (bar0_address + capability.table_offset() as usize) as *mut MsixTableEntry; + 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::MsixInfo { + let mut info = xhci::MappedMsixRegs { virt_table_base: NonNull::new(virt_table_base).unwrap(), - capability, + info: msix_info, }; // Allocate one msi vector. diff --git a/xhcid/src/xhci/mod.rs b/xhcid/src/xhci/mod.rs index d88a87c7be..32eb7f7dd4 100644 --- a/xhcid/src/xhci/mod.rs +++ b/xhcid/src/xhci/mod.rs @@ -21,7 +21,7 @@ use serde::Deserialize; use crate::usb; -use pcid_interface::msi::{MsixTableEntry, MsixCapability}; +use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use pcid_interface::{PcidServerHandle, PciFeature}; mod capability; @@ -64,19 +64,19 @@ pub enum InterruptMethod { Msi, /// Extended message signaled interrupts. - MsiX(Mutex), + MsiX(Mutex), } -pub struct MsixInfo { +pub struct MappedMsixRegs { pub virt_table_base: NonNull, - pub capability: MsixCapability, + pub info: MsixInfo, } -impl 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.capability.table_size() as usize); + assert!(k < self.info.table_size as usize); unsafe { self.table_entry_pointer_unchecked(k) } } } @@ -774,13 +774,13 @@ impl Xhci { if let InterruptMethod::MsiX(_) = self.interrupt_method { true } else { false } } // TODO: Perhaps use an rwlock? - pub fn msix_info(&self) -> Option> { + pub fn msix_info(&self) -> Option> { match self.interrupt_method { InterruptMethod::MsiX(ref info) => Some(info.lock().unwrap()), _ => None, } } - pub fn msix_info_mut(&self) -> Option> { + pub fn msix_info_mut(&self) -> Option> { match self.interrupt_method { InterruptMethod::MsiX(ref info) => Some(info.lock().unwrap()), _ => None,