From f43aa8574cc632087b5146c5e21232fec4c6deb1 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 23 Jan 2024 14:24:26 +0100 Subject: [PATCH] Simplify MSI-X table pointer computation --- pcid/src/pci/msi.rs | 55 ++++++++++++++-------------------- rtl8139d/src/main.rs | 15 ++++------ rtl8168d/src/main.rs | 14 ++++----- virtio-core/src/arch/x86_64.rs | 14 +++------ xhcid/src/main.rs | 13 ++++---- 5 files changed, 41 insertions(+), 70 deletions(-) diff --git a/pcid/src/pci/msi.rs b/pcid/src/pci/msi.rs index a3c06d538a..a92351151a 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -204,37 +204,40 @@ impl MsiCapability { impl MsixCapability { 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.pba_bir() > 5 { + panic!("MSI-X PBA BIR contained a reserved enum value: {}", self.pba_bir()); + } + let table_size = self.table_size(); - let table_base = self.table_base_pointer(bars); + 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_min_length = table_size.div_ceil(8); - let pba_base = self.pba_base_pointer(bars); - - let bir = self.table_bir() as usize; - let bar = &bars[bir]; - let (bar_ptr, bar_size) = bar.expect_mem(); + let (_, table_bar_size) = bars[self.table_bir() as usize].expect_mem(); + let (_, pba_bar_size) = bars[self.pba_bir() as usize].expect_mem(); // Ensure that the table and PBA are within the BAR. - let bar_range = bar_ptr as u64..bar_ptr as u64 + bar_size as u64; - if !bar_range.contains(&(table_base as u64 + table_min_length as u64)) { + if !(0..table_bar_size as u64).contains(&(table_offset as u64 + table_min_length as u64)) { panic!( - "Table {:#x}{:#x} outside of BAR {:#x}:{:#x}", - table_base, - table_base + table_min_length as usize, - bar_ptr, - bar_ptr + bar_size + "Table {:#x}:{:#x} outside of BAR with length {:#x}", + table_offset, + table_offset + table_min_length as usize, + table_bar_size ); } - if !bar_range.contains(&(pba_base as u64 + pba_min_length as u64)) { + if !(0..pba_bar_size as u64).contains(&(pba_offset as u64 + pba_min_length as u64)) { panic!( - "PBA {:#x}{:#x} outside of BAR {:#x}:{:#X}", - pba_base, - pba_base + pba_min_length as usize, - bar_ptr, - bar_ptr + bar_size + "PBA {:#x}:{:#x} outside of BAR with length {:#x}", + pba_offset, + pba_offset + pba_min_length as usize, + pba_bar_size ); } } @@ -307,20 +310,6 @@ impl MsixCapability { } - pub fn table_base_pointer(&self, bars: [PciBar; 6]) -> usize { - if self.table_bir() > 5 { - panic!("MSI-X Table BIR contained a reserved enum value: {}", self.table_bir()); - } - bars[usize::from(self.table_bir())].expect_mem().0 + self.table_offset() as usize - } - - pub fn pba_base_pointer(&self, bars: [PciBar; 6]) -> usize { - if self.pba_bir() > 5 { - panic!("MSI-X PBA BIR contained a reserved enum value: {}", self.pba_bir()); - } - bars[usize::from(self.pba_bir())].expect_mem().0 + self.pba_offset() as usize - } - /// Write the first DWORD into configuration space (containing the partially modifiable Message /// Control field). pub unsafe fn write_a(&self, writer: &W, offset: u8) { diff --git a/rtl8139d/src/main.rs b/rtl8139d/src/main.rs index 24e5e916fe..b019c2c2ce 100644 --- a/rtl8139d/src/main.rs +++ b/rtl8139d/src/main.rs @@ -162,18 +162,13 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { PciFeatureInfo::MsiX(s) => s, }; capability.validate(pci_config.func.bars); - let table_base = capability.table_base_pointer(pci_config.func.bars); - let pba_base = capability.pba_base_pointer(pci_config.func.bars); + assert_eq!(capability.table_bir(), capability.pba_bir()); + let bar = &pci_config.func.bars[capability.table_bir() as usize]; + let bar_address = unsafe { bar.physmap_mem("rtl8139d") } as usize; - let bir = capability.table_bir() as usize; - let bar = &pci_config.func.bars[bir]; - let (bar_ptr, _) = bar.expect_mem(); - - let address = unsafe { bar.physmap_mem("rtl8139d") } as usize; - - let virt_table_base = ((table_base - bar_ptr) + address) as *mut MsixTableEntry; - let virt_pba_base = ((pba_base - bar_ptr) + address) as *mut u64; + let virt_table_base = (bar_address + capability.table_offset() as usize) as *mut MsixTableEntry; + let virt_pba_base = (bar_address + capability.pba_offset() as usize) as *mut u64; let mut info = MsixInfo { virt_table_base: NonNull::new(virt_table_base).unwrap(), diff --git a/rtl8168d/src/main.rs b/rtl8168d/src/main.rs index e93cf984bf..b099a389e1 100644 --- a/rtl8168d/src/main.rs +++ b/rtl8168d/src/main.rs @@ -160,17 +160,13 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { PciFeatureInfo::MsiX(s) => s, }; capability.validate(pci_config.func.bars); - let table_base = capability.table_base_pointer(pci_config.func.bars); - let pba_base = capability.pba_base_pointer(pci_config.func.bars); - let bir = capability.table_bir() as usize; - let bar = &pci_config.func.bars[bir]; - let (bar_ptr, _) = bar.expect_mem(); + assert_eq!(capability.table_bir(), capability.pba_bir()); + let bar = &pci_config.func.bars[capability.table_bir() as usize]; + let bar_address = unsafe { bar.physmap_mem("rtl8168d") } as usize; - let address = unsafe { bar.physmap_mem("rtl8168d") } as usize; - - let virt_table_base = ((table_base - bar_ptr) + address) as *mut MsixTableEntry; - let virt_pba_base = ((pba_base - bar_ptr) + address) as *mut u64; + let virt_table_base = (bar_address + capability.table_offset() as usize) as *mut MsixTableEntry; + let virt_pba_base = (bar_address + capability.pba_offset() as usize) as *mut u64; let mut info = MsixInfo { virt_table_base: NonNull::new(virt_table_base).unwrap(), diff --git a/virtio-core/src/arch/x86_64.rs b/virtio-core/src/arch/x86_64.rs index 0273423eb4..3ef5d171a3 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -20,16 +20,10 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { }; capability.validate(pci_config.func.bars); - let table_base = capability.table_base_pointer(pci_config.func.bars); - - let bir = capability.table_bir() as usize; - let bar = &pci_config.func.bars[bir]; - let (bar_ptr, _) = bar.expect_mem(); - - let address = unsafe { bar.physmap_mem("virtio-core") } as usize; - - - let virt_table_base = ((table_base - bar_ptr as usize) + address) as *mut MsixTableEntry; + assert_eq!(capability.table_bir(), capability.pba_bir()); + let bar = &pci_config.func.bars[capability.table_bir() 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 mut info = MsixInfo { virt_table_base: NonNull::new(virt_table_base).unwrap(), diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs index 96821f5f35..59290f4256 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -82,11 +82,9 @@ fn setup_logging(name: &str) -> Option<&'static RedoxLogger> { } #[cfg(target_arch = "x86_64")] -fn get_int_method(pcid_handle: &mut PcidServerHandle, address: usize) -> (Option, InterruptMethod) { +fn get_int_method(pcid_handle: &mut PcidServerHandle, bar0_address: usize) -> (Option, InterruptMethod) { let pci_config = pcid_handle.fetch_config().expect("xhcid: failed to fetch config"); - let (bar_ptr, bar_size) = pci_config.func.bars[0].expect_mem(); - let all_pci_features = pcid_handle.fetch_all_features().expect("xhcid: failed to fetch pci features"); log::debug!("XHCI PCI FEATURES: {:?}", all_pci_features); @@ -139,11 +137,10 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, address: usize) -> (Option }; capability.validate(pci_config.func.bars); - let table_base = capability.table_base_pointer(pci_config.func.bars); - let pba_base = capability.pba_base_pointer(pci_config.func.bars); - - let virt_table_base = ((table_base - bar_ptr as usize) + address) as *mut MsixTableEntry; - let virt_pba_base = ((pba_base - bar_ptr as usize) + address) as *mut u64; + assert_eq!(capability.table_bir(), 0); + assert_eq!(capability.pba_bir(), 0); + let virt_table_base = (bar0_address + capability.table_offset() as usize) as *mut MsixTableEntry; + let virt_pba_base = (bar0_address + capability.pba_offset() as usize) as *mut u64; let mut info = xhci::MsixInfo { virt_table_base: NonNull::new(virt_table_base).unwrap(),