From 7dbf22331b7000ee0015f22a2231e0a41d8d3143 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 23 Jan 2024 13:38:31 +0100 Subject: [PATCH 1/5] Share msix config validation code --- nvmed/src/main.rs | 1 + pcid/src/pci/msi.rs | 36 ++++++++++++++++++++++++++++++++++ rtl8139d/src/main.rs | 14 ++----------- rtl8168d/src/main.rs | 15 ++------------ virtio-core/src/arch/x86_64.rs | 14 ++----------- xhcid/src/main.rs | 14 ++----------- 6 files changed, 45 insertions(+), 49 deletions(-) diff --git a/nvmed/src/main.rs b/nvmed/src/main.rs index 8e57eb4db0..92f5c14729 100644 --- a/nvmed/src/main.rs +++ b/nvmed/src/main.rs @@ -88,6 +88,7 @@ fn get_int_method( PciFeatureInfo::MsiX(msix) => msix, _ => unreachable!(), }; + capability_struct.validate(function.bars); fn bar_base( allocated_bars: &AllocatedBars, function: &PciFunction, diff --git a/pcid/src/pci/msi.rs b/pcid/src/pci/msi.rs index fe09768a91..a3c06d538a 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -203,6 +203,42 @@ impl MsiCapability { } impl MsixCapability { + pub fn validate(&self, bars: [PciBar; 6]) { + let table_size = self.table_size(); + let table_base = self.table_base_pointer(bars); + let table_min_length = table_size * 16; + 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(); + + // 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)) { + panic!( + "Table {:#x}{:#x} outside of BAR {:#x}:{:#x}", + table_base, + table_base + table_min_length as usize, + bar_ptr, + bar_ptr + bar_size + ); + } + + if !bar_range.contains(&(pba_base 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 + ); + } + } + 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/rtl8139d/src/main.rs b/rtl8139d/src/main.rs index e0bfbf2648..24e5e916fe 100644 --- a/rtl8139d/src/main.rs +++ b/rtl8139d/src/main.rs @@ -161,27 +161,17 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - let table_size = capability.table_size(); + capability.validate(pci_config.func.bars); let table_base = capability.table_base_pointer(pci_config.func.bars); - let table_min_length = table_size * 16; - let pba_min_length = div_round_up(table_size, 8); 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_size) = bar.expect_mem(); + let (bar_ptr, _) = bar.expect_mem(); let address = unsafe { bar.physmap_mem("rtl8139d") } as usize; - if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(table_base 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); - } - - if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(pba_base 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); - } - let virt_table_base = ((table_base - bar_ptr) + address) as *mut MsixTableEntry; let virt_pba_base = ((pba_base - bar_ptr) + address) as *mut u64; diff --git a/rtl8168d/src/main.rs b/rtl8168d/src/main.rs index d157ab8706..e93cf984bf 100644 --- a/rtl8168d/src/main.rs +++ b/rtl8168d/src/main.rs @@ -159,27 +159,16 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - let table_size = capability.table_size(); + capability.validate(pci_config.func.bars); let table_base = capability.table_base_pointer(pci_config.func.bars); - let table_min_length = table_size * 16; - let pba_min_length = div_round_up(table_size, 8); - 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_size) = bar.expect_mem(); + let (bar_ptr, _) = bar.expect_mem(); let address = unsafe { bar.physmap_mem("rtl8168d") } as usize; - if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(table_base 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); - } - - if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(pba_base 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); - } - let virt_table_base = ((table_base - bar_ptr) + address) as *mut MsixTableEntry; let virt_pba_base = ((pba_base - bar_ptr) + address) as *mut u64; diff --git a/virtio-core/src/arch/x86_64.rs b/virtio-core/src/arch/x86_64.rs index a160f508cd..0273423eb4 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -18,26 +18,16 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { PciFeatureInfo::MsiX(capability) => capability, _ => unreachable!(), }; + capability.validate(pci_config.func.bars); - let table_size = capability.table_size(); let table_base = capability.table_base_pointer(pci_config.func.bars); - let table_min_length = table_size * 16; - let pba_min_length = table_size.div_ceil(8); - - 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_size) = bar.expect_mem(); + let (bar_ptr, _) = bar.expect_mem(); let address = unsafe { bar.physmap_mem("virtio-core") } as usize; - // Ensure that the table and PBA are be within the BAR. - { - let bar_range = bar_ptr as u64..bar_ptr as u64 + bar_size as u64; - assert!(bar_range.contains(&(table_base as u64 + table_min_length as u64))); - assert!(bar_range.contains(&(pba_base as u64 + pba_min_length as u64))); - } let virt_table_base = ((table_base - bar_ptr as usize) + address) as *mut MsixTableEntry; diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs index c6dd1565a5..96821f5f35 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -137,21 +137,11 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, address: usize) -> (Option PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - let table_size = capability.table_size(); + capability.validate(pci_config.func.bars); + let table_base = capability.table_base_pointer(pci_config.func.bars); - let table_min_length = table_size * 16; - let pba_min_length = crate::xhci::scheme::div_round_up(table_size, 8); - let pba_base = capability.pba_base_pointer(pci_config.func.bars); - if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(table_base 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); - } - - if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(pba_base 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); - } - 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; 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 2/5] 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(), From ecdcefba69f8b073ecb9d2b26607a29894ae8932 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 23 Jan 2024 15:45:44 +0100 Subject: [PATCH 3/5] Clarify message_address argument names --- pcid/src/pci/msi.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pcid/src/pci/msi.rs b/pcid/src/pci/msi.rs index a92351151a..96ab83e915 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -352,11 +352,11 @@ pub mod x86_64 { } // TODO: should the reserved field be preserved? - pub const fn message_address(destination_id: u8, rh: bool, dm: bool) -> u32 { + pub const fn message_address(destination_id: u8, redirect_hint: bool, dest_mode_logical: bool) -> u32 { 0xFEE0_0000u32 | ((destination_id as u32) << 12) - | ((rh as u32) << 3) - | ((dm as u32) << 2) + | ((redirect_hint as u32) << 3) + | ((dest_mode_logical as u32) << 2) } pub const fn message_data(trigger_mode: TriggerMode, level_trigger_mode: LevelTriggerMode, delivery_mode: DeliveryMode, vector: u8) -> u32 { ((trigger_mode as u32) << 15) From 9b809312c20f74f4fb52d184de602cabed1ac718 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 23 Jan 2024 15:56:29 +0100 Subject: [PATCH 4/5] Ignore MSI-X PBA in all drivers It isn't used in any driver and even on Linux no driver seems to use it at all. The only times it seems to be useful are if you were to mask an interrupt and want to check if the interrupt fired without unmasking or if you want to make the device itself trigger an interrupt. --- nvmed/src/main.rs | 10 ---------- nvmed/src/nvme/mod.rs | 1 - rtl8139d/src/main.rs | 16 ---------------- rtl8168d/src/main.rs | 16 ---------------- virtio-core/src/arch/x86_64.rs | 1 - xhcid/src/main.rs | 3 --- xhcid/src/xhci/mod.rs | 13 ------------- 7 files changed, 60 deletions(-) diff --git a/nvmed/src/main.rs b/nvmed/src/main.rs index 92f5c14729..e38060bc1b 100644 --- a/nvmed/src/main.rs +++ b/nvmed/src/main.rs @@ -109,22 +109,13 @@ fn get_int_method( } let table_bar_base: *mut u8 = bar_base(allocated_bars, function, capability_struct.table_bir())?.as_ptr(); - let pba_bar_base: *mut u8 = - bar_base(allocated_bars, function, capability_struct.pba_bir())?.as_ptr(); let table_base = unsafe { table_bar_base.offset(capability_struct.table_offset() as isize) }; - let pba_base = unsafe { pba_bar_base.offset(capability_struct.pba_offset() as isize) }; let vector_count = capability_struct.table_size(); let table_entries: &'static mut [MsixTableEntry] = unsafe { slice::from_raw_parts_mut(table_base as *mut MsixTableEntry, vector_count as usize) }; - let pba_entries: &'static mut [Mmio] = unsafe { - slice::from_raw_parts_mut( - table_base as *mut Mmio, - (vector_count as usize + 63) / 64, - ) - }; // 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). @@ -162,7 +153,6 @@ fn get_int_method( let interrupt_method = InterruptMethod::MsiX(MsixCfg { cap: capability_struct, table: table_entries, - pba: pba_entries, }); let interrupt_sources = InterruptSources::MsiX(std::iter::once((msix_vector_number, irq_handle)).collect()); diff --git a/nvmed/src/nvme/mod.rs b/nvmed/src/nvme/mod.rs index 1b368c521d..6378fa2fce 100644 --- a/nvmed/src/nvme/mod.rs +++ b/nvmed/src/nvme/mod.rs @@ -122,7 +122,6 @@ impl InterruptMethod { pub struct MsixCfg { pub cap: MsixCapability, pub table: &'static mut [MsixTableEntry], - pub pba: &'static mut [Mmio], } #[repr(packed)] diff --git a/rtl8139d/src/main.rs b/rtl8139d/src/main.rs index b019c2c2ce..a8400063be 100644 --- a/rtl8139d/src/main.rs +++ b/rtl8139d/src/main.rs @@ -81,7 +81,6 @@ where pub struct MsixInfo { pub virt_table_base: NonNull, - pub virt_pba_base: NonNull, pub capability: MsixCapability, } @@ -93,18 +92,6 @@ impl MsixInfo { assert!(k < self.capability.table_size() as usize); unsafe { self.table_entry_pointer_unchecked(k) } } - pub unsafe fn pba_pointer_unchecked(&mut self, k: usize) -> &mut u64 { - &mut *self.virt_pba_base.as_ptr().offset(k as isize) - } - pub fn pba_pointer(&mut self, k: usize) -> &mut u64 { - assert!(k < self.capability.table_size() as usize); - unsafe { self.pba_pointer_unchecked(k) } - } - pub fn pba(&mut self, k: usize) -> bool { - let byte = k / 64; - let bit = k % 64; - *self.pba_pointer(byte) & (1 << bit) != 0 - } } #[cfg(target_arch = "x86_64")] @@ -163,16 +150,13 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { }; capability.validate(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 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(), - virt_pba_base: NonNull::new(virt_pba_base).unwrap(), capability, }; diff --git a/rtl8168d/src/main.rs b/rtl8168d/src/main.rs index b099a389e1..334554d12e 100644 --- a/rtl8168d/src/main.rs +++ b/rtl8168d/src/main.rs @@ -79,7 +79,6 @@ where pub struct MsixInfo { pub virt_table_base: NonNull, - pub virt_pba_base: NonNull, pub capability: MsixCapability, } @@ -91,18 +90,6 @@ impl MsixInfo { assert!(k < self.capability.table_size() as usize); unsafe { self.table_entry_pointer_unchecked(k) } } - pub unsafe fn pba_pointer_unchecked(&mut self, k: usize) -> &mut u64 { - &mut *self.virt_pba_base.as_ptr().offset(k as isize) - } - pub fn pba_pointer(&mut self, k: usize) -> &mut u64 { - assert!(k < self.capability.table_size() as usize); - unsafe { self.pba_pointer_unchecked(k) } - } - pub fn pba(&mut self, k: usize) -> bool { - let byte = k / 64; - let bit = k % 64; - *self.pba_pointer(byte) & (1 << bit) != 0 - } } #[cfg(target_arch = "x86_64")] @@ -161,16 +148,13 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { }; capability.validate(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("rtl8168d") } as usize; 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(), - virt_pba_base: NonNull::new(virt_pba_base).unwrap(), capability, }; diff --git a/virtio-core/src/arch/x86_64.rs b/virtio-core/src/arch/x86_64.rs index 3ef5d171a3..3965e08c39 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -20,7 +20,6 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { }; capability.validate(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("virtio-core") } as usize; let virt_table_base = (bar_address + capability.table_offset() as usize) as *mut MsixTableEntry; diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs index 59290f4256..2315c2a1dc 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -138,13 +138,10 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, bar0_address: usize) -> (O capability.validate(pci_config.func.bars); 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(), - virt_pba_base: NonNull::new(virt_pba_base).unwrap(), capability, }; diff --git a/xhcid/src/xhci/mod.rs b/xhcid/src/xhci/mod.rs index aaf8b453c1..32d9d1eb82 100644 --- a/xhcid/src/xhci/mod.rs +++ b/xhcid/src/xhci/mod.rs @@ -70,7 +70,6 @@ pub enum InterruptMethod { pub struct MsixInfo { pub virt_table_base: NonNull, - pub virt_pba_base: NonNull, pub capability: MsixCapability, } impl MsixInfo { @@ -81,18 +80,6 @@ impl MsixInfo { assert!(k < self.capability.table_size() as usize); unsafe { self.table_entry_pointer_unchecked(k) } } - pub unsafe fn pba_pointer_unchecked(&mut self, k: usize) -> &mut u64 { - &mut *self.virt_pba_base.as_ptr().offset(k as isize) - } - pub fn pba_pointer(&mut self, k: usize) -> &mut u64 { - assert!(k < self.capability.table_size() as usize); - unsafe { self.pba_pointer_unchecked(k) } - } - pub fn pba(&mut self, k: usize) -> bool { - let byte = k / 64; - let bit = k % 64; - *self.pba_pointer(byte) & (1 << bit) != 0 - } } impl Xhci { From 67d3015e2efea14b1e1d16d0762003d5869c5f1f Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 23 Jan 2024 19:22:32 +0100 Subject: [PATCH 5/5] Add helper for allocating a single MSI/MSI-X interrupt vector It doesn't actually configure the device to emit it though, but does make this easier to do. --- ihdad/src/main.rs | 15 ++-------- nvmed/src/main.rs | 37 +++++------------------- pcid/src/driver_interface/irq_helpers.rs | 21 ++++++++++++++ pcid/src/driver_interface/mod.rs | 18 +++--------- pcid/src/main.rs | 22 +++++++------- pcid/src/pci/msi.rs | 33 +++++++++++++++++---- rtl8139d/src/main.rs | 35 +++++----------------- rtl8168d/src/main.rs | 35 +++++----------------- virtio-core/src/arch/x86_64.rs | 29 ++++--------------- xhcid/src/main.rs | 33 +++++---------------- 10 files changed, 104 insertions(+), 174 deletions(-) diff --git a/ihdad/src/main.rs b/ihdad/src/main.rs index 7f87807b40..ba8ae837d8 100755 --- a/ihdad/src/main.rs +++ b/ihdad/src/main.rs @@ -6,7 +6,6 @@ extern crate spin; extern crate syscall; extern crate event; -use std::convert::TryFrom; use std::usize; use std::fs::File; use std::io::{ErrorKind, Read, Write, Result}; @@ -17,7 +16,7 @@ use std::sync::Arc; use event::EventQueue; use pcid_interface::{MsiSetFeatureInfo, PcidServerHandle, PciFeature, PciFeatureInfo, SetFeatureInfo}; -use pcid_interface::irq_helpers::{read_bsp_apic_id, allocate_single_interrupt_vector}; +use pcid_interface::irq_helpers::{read_bsp_apic_id, allocate_single_interrupt_vector_for_msi}; use redox_log::{OutputBuilder, RedoxLogger}; pub mod hda; @@ -91,8 +90,6 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { } if msi_enabled && !msix_enabled { - use pcid_interface::msi::x86_64::{DeliveryMode, self as x86_64_msix}; - let capability = match pcid_handle.feature_info(PciFeature::Msi).expect("ihdad: failed to retrieve the MSI capability structure from pcid") { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), @@ -103,17 +100,11 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { // pcid_interface, so that this can be shared between nvmed, xhcid, ixgebd, etc.. let destination_id = read_bsp_apic_id().expect("ihdad: failed to read BSP apic id"); - let lapic_id = u8::try_from(destination_id).expect("CPU id didn't fit inside u8"); - let msg_addr = x86_64_msix::message_address(lapic_id, false, false); - - let (vector, interrupt_handle) = allocate_single_interrupt_vector(destination_id).expect("ihdad: failed to allocate interrupt vector").expect("ihdad: no interrupt vectors left"); - let msg_data = x86_64_msix::message_data_edge_triggered(DeliveryMode::Fixed, vector); + let (msg_addr_and_data, interrupt_handle) = allocate_single_interrupt_vector_for_msi(destination_id); let set_feature_info = MsiSetFeatureInfo { multi_message_enable: Some(0), - message_address: Some(msg_addr), - message_upper_address: Some(0), - message_data: Some(msg_data as u16), + message_address_and_data: Some(msg_addr_and_data), mask_bits: None, }; pcid_handle.set_feature_info(SetFeatureInfo::Msi(set_feature_info)).expect("ihdad: failed to set feature info"); diff --git a/nvmed/src/main.rs b/nvmed/src/main.rs index e38060bc1b..7058e9f6ae 100644 --- a/nvmed/src/main.rs +++ b/nvmed/src/main.rs @@ -127,25 +127,14 @@ fn get_int_method( capability_struct.set_msix_enabled(true); // only affects our local mirror of the cap let (msix_vector_number, irq_handle) = { - use msi_x86_64::DeliveryMode; - use pcid_interface::msi::x86_64 as msi_x86_64; - let entry: &mut MsixTableEntry = &mut table_entries[0]; let bsp_cpu_id = irq_helpers::read_bsp_apic_id().expect("nvmed: failed to read APIC ID"); - let bsp_lapic_id = bsp_cpu_id - .try_into() - .expect("nvmed: BSP local apic ID couldn't fit inside u8"); - let (vector, irq_handle) = irq_helpers::allocate_single_interrupt_vector(bsp_cpu_id) - .expect("nvmed: failed to allocate single MSI-X interrupt vector") - .expect("nvmed: no interrupt vectors left on BSP"); - - let msg_addr = msi_x86_64::message_address(bsp_lapic_id, false, false); - let msg_data = msi_x86_64::message_data_edge_triggered(DeliveryMode::Fixed, vector); - - entry.set_addr_lo(msg_addr); - entry.set_msg_data(msg_data); + let (msg_addr_and_data, irq_handle) = + irq_helpers::allocate_single_interrupt_vector_for_msi(bsp_cpu_id); + entry.write_addr_and_data(msg_addr_and_data); + entry.unmask(); (0, irq_handle) }; @@ -166,27 +155,15 @@ fn get_int_method( }; let (msi_vector_number, irq_handle) = { - use msi_x86_64::DeliveryMode; - use pcid_interface::msi::x86_64 as msi_x86_64; use pcid_interface::{MsiSetFeatureInfo, SetFeatureInfo}; let bsp_cpu_id = irq_helpers::read_bsp_apic_id().expect("nvmed: failed to read BSP APIC ID"); - let bsp_lapic_id = bsp_cpu_id - .try_into() - .expect("nvmed: BSP local apic ID couldn't fit inside u8"); - let (vector, irq_handle) = irq_helpers::allocate_single_interrupt_vector(bsp_cpu_id) - .expect("nvmed: failed to allocate single MSI interrupt vector") - .expect("nvmed: no interrupt vectors left on BSP"); - - let msg_addr = msi_x86_64::message_address(bsp_lapic_id, false, false); - let msg_data = - msi_x86_64::message_data_edge_triggered(DeliveryMode::Fixed, vector) as u16; + let (msg_addr_and_data, irq_handle) = + irq_helpers::allocate_single_interrupt_vector_for_msi(bsp_cpu_id); pcid_handle.set_feature_info(SetFeatureInfo::Msi(MsiSetFeatureInfo { - message_address: Some(msg_addr), - message_upper_address: Some(0), - message_data: Some(msg_data), + message_address_and_data: Some(msg_addr_and_data), multi_message_enable: Some(0), // enable 2^0=1 vectors mask_bits: None, })).unwrap(); diff --git a/pcid/src/driver_interface/irq_helpers.rs b/pcid/src/driver_interface/irq_helpers.rs index e0d63164e5..d7c9b4b5ea 100644 --- a/pcid/src/driver_interface/irq_helpers.rs +++ b/pcid/src/driver_interface/irq_helpers.rs @@ -8,6 +8,8 @@ use std::fs::{self, File}; use std::io::{self, prelude::*}; use std::num::NonZeroU8; +use crate::pci::msi::MsiAddrAndData; + /// Read the local APIC ID of the bootstrap processor. pub fn read_bsp_apic_id() -> io::Result { let mut buffer = [0u8; 8]; @@ -153,3 +155,22 @@ pub fn allocate_single_interrupt_vector(cpu_id: usize) -> io::Result (MsiAddrAndData, File) { + use crate::pci::msi::x86_64 as x86_64_msix; + + // FIXME for cpu_id >255 we need to use the IOMMU to use IRQ remapping + let lapic_id = u8::try_from(cpu_id).expect("CPU id couldn't fit inside u8"); + let rh = false; + let dm = false; + let addr = x86_64_msix::message_address(lapic_id, rh, dm); + + let (vector, interrupt_handle) = allocate_single_interrupt_vector(cpu_id) + .expect("failed to allocate interrupt vector") + .expect("no interrupt vectors left"); + let msg_data = + x86_64_msix::message_data_edge_triggered(x86_64_msix::DeliveryMode::Fixed, vector); + + (MsiAddrAndData::new(addr, msg_data), interrupt_handle) +} diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index 88b7068915..0595fee914 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -163,22 +163,12 @@ pub struct MsiSetFeatureInfo { /// is the log2 of the interrupt vectors, minus one. Can only be 0b000..=0b101. pub multi_message_enable: Option, - /// The system-specific message address, must be DWORD aligned. + /// The system-specific message address and data. /// /// The message address contains things like the CPU that will be targeted, at least on - /// x86_64. - pub message_address: Option, - - /// The upper 32 bits of the 64-bit message address. Not guaranteed to exist, and is - /// reserved on x86_64 (currently). - pub message_upper_address: Option, - - /// The message data, containing the actual interrupt vector (lower 8 bits), etc. - /// - /// The spec mentions that the lower N bits can be modified, where N is the multi message - /// enable, which means that the vector set here has to be aligned to that number, and that - /// all vectors in that range have to be allocated. - pub message_data: Option, + /// x86_64. The message data contains the actual interrupt vector (lower 8 bits) and + /// the kind of interrupt, at least on x86_64. + pub message_address_and_data: Option, /// A bitmap of the vectors that are masked. This field is not guaranteed (and not likely, /// at least according to the feature flags I got from QEMU), to exist. diff --git a/pcid/src/main.rs b/pcid/src/main.rs index bbc46d0794..56f78191bf 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -14,7 +14,7 @@ use redox_log::{OutputBuilder, RedoxLogger}; use crate::cfg_access::Pcie; use crate::config::Config; use crate::driver_interface::LegacyInterruptLine; -use crate::pci::{PciBar, PciFunc}; +use crate::pci::PciFunc; use crate::pci::cap::Capability as PciCapability; use crate::pci::func::{ConfigReader, ConfigWriter}; use crate::pci_header::{PciEndpointHeader, PciHeader, PciHeaderError}; @@ -130,20 +130,22 @@ impl DriverHandler { info.set_multi_message_enable(mme); } - if let Some(message_addr) = info_to_set.message_address { + if let Some(message_addr_and_data) = info_to_set.message_address_and_data { + let message_addr = message_addr_and_data.addr; if message_addr & 0b11 != 0 { return PcidClientResponse::Error(PcidServerResponseError::InvalidBitPattern); } - info.set_message_address(message_addr); - } - if let Some(message_addr_upper) = info_to_set.message_upper_address { - info.set_message_upper_address(message_addr_upper); - } - if let Some(message_data) = info_to_set.message_data { - if message_data & ((1 << info.multi_message_enable()) - 1) != 0 { + info.set_message_address(message_addr as u32); + info.set_message_upper_address((message_addr >> 32) as u32); + if message_addr_and_data.data & ((1 << info.multi_message_enable()) - 1) != 0 { return PcidClientResponse::Error(PcidServerResponseError::InvalidBitPattern); } - info.set_message_data(message_data); + info.set_message_data( + message_addr_and_data + .data + .try_into() + .expect("pcid: MSI message data too big"), + ); } if let Some(mask_bits) = info_to_set.mask_bits { info.set_mask_bits(mask_bits); diff --git a/pcid/src/pci/msi.rs b/pcid/src/pci/msi.rs index 96ab83e915..ce176a1846 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -4,8 +4,25 @@ use super::bar::PciBar; pub use super::cap::{MsiCapability, MsixCapability}; use super::func::{ConfigReader, ConfigWriter}; +use serde::{Deserialize, Serialize}; use syscall::{Io, Mmio}; +/// The address and data to use for MSI and MSI-X. +/// +/// For MSI using this only works when you need a single interrupt vector. +/// For MSI-X you can have a single [MsiEntry] for each interrupt vector. +#[derive(Debug, Default, Serialize, Deserialize)] +pub struct MsiAddrAndData { + pub(crate) addr: u64, + pub(crate) data: u32, +} + +impl MsiAddrAndData { + pub fn new(addr: u64, data: u32) -> Self { + MsiAddrAndData { addr, data } + } +} + impl MsiCapability { pub const MC_PVT_CAPABLE_BIT: u16 = 1 << 8; pub const MC_64_BIT_ADDR_BIT: u16 = 1 << 7; @@ -352,11 +369,11 @@ pub mod x86_64 { } // TODO: should the reserved field be preserved? - pub const fn message_address(destination_id: u8, redirect_hint: bool, dest_mode_logical: bool) -> u32 { - 0xFEE0_0000u32 - | ((destination_id as u32) << 12) - | ((redirect_hint as u32) << 3) - | ((dest_mode_logical as u32) << 2) + pub const fn message_address(destination_id: u8, redirect_hint: bool, dest_mode_logical: bool) -> u64 { + 0x0000_0000_FEE0_0000u64 + | ((destination_id as u64) << 12) + | ((redirect_hint as u64) << 3) + | ((dest_mode_logical as u64) << 2) } pub const fn message_data(trigger_mode: TriggerMode, level_trigger_mode: LevelTriggerMode, delivery_mode: DeliveryMode, vector: u8) -> u32 { ((trigger_mode as u32) << 15) @@ -408,6 +425,12 @@ impl MsixTableEntry { pub fn unmask(&mut self) { self.set_masked(false); } + + pub fn write_addr_and_data(&mut self, entry: MsiAddrAndData) { + self.set_addr_lo(entry.addr as u32); + self.set_addr_hi((entry.addr >> 32) as u32); + self.set_msg_data(entry.data); + } } impl fmt::Debug for MsixTableEntry { diff --git a/rtl8139d/src/main.rs b/rtl8139d/src/main.rs index a8400063be..c61e3049ab 100644 --- a/rtl8139d/src/main.rs +++ b/rtl8139d/src/main.rs @@ -5,7 +5,7 @@ extern crate netutils; extern crate syscall; use std::cell::RefCell; -use std::convert::{TryFrom, TryInto}; +use std::convert::TryInto; use std::{env, process}; use std::fs::File; use std::io::{ErrorKind, Read, Result, Write}; @@ -15,11 +15,10 @@ use std::sync::Arc; use event::EventQueue; use pcid_interface::{MsiSetFeatureInfo, PcidServerHandle, PciFeature, PciFeatureInfo, SetFeatureInfo, SubdriverArguments}; -use pcid_interface::irq_helpers::{read_bsp_apic_id, allocate_single_interrupt_vector}; +use pcid_interface::irq_helpers::{read_bsp_apic_id, allocate_single_interrupt_vector_for_msi}; use pcid_interface::msi::{MsixCapability, MsixTableEntry}; use redox_log::{RedoxLogger, OutputBuilder}; use syscall::{EventFlags, Packet, SchemeBlockMut}; -use syscall::io::Io; pub mod device; @@ -112,8 +111,6 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { } if msi_enabled && !msix_enabled { - use pcid_interface::msi::x86_64::{DeliveryMode, self as x86_64_msix}; - let capability = match pcid_handle.feature_info(PciFeature::Msi).expect("rtl8139d: failed to retrieve the MSI capability structure from pcid") { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), @@ -124,17 +121,11 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { // pcid_interface, so that this can be shared between nvmed, xhcid, ixgebd, etc.. let destination_id = read_bsp_apic_id().expect("rtl8139d: failed to read BSP apic id"); - let lapic_id = u8::try_from(destination_id).expect("CPU id didn't fit inside u8"); - let msg_addr = x86_64_msix::message_address(lapic_id, false, false); - - let (vector, interrupt_handle) = allocate_single_interrupt_vector(destination_id).expect("rtl8139d: failed to allocate interrupt vector").expect("rtl8139d: no interrupt vectors left"); - let msg_data = x86_64_msix::message_data_edge_triggered(DeliveryMode::Fixed, vector); + let (msg_addr_and_data, interrupt_handle) = allocate_single_interrupt_vector_for_msi(destination_id); let set_feature_info = MsiSetFeatureInfo { multi_message_enable: Some(0), - message_address: Some(msg_addr), - message_upper_address: Some(0), - message_data: Some(msg_data as u16), + message_address_and_data: Some(msg_addr_and_data), mask_bits: None, }; pcid_handle.set_feature_info(SetFeatureInfo::Msi(set_feature_info)).expect("rtl8139d: failed to set feature info"); @@ -163,8 +154,6 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { // Allocate one msi vector. let method = { - use pcid_interface::msi::x86_64::{DeliveryMode, self as x86_64_msix}; - // primary interrupter let k = 0; @@ -172,18 +161,10 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { let table_entry_pointer = info.table_entry_pointer(k); let destination_id = read_bsp_apic_id().expect("rtl8139d: failed to read BSP apic id"); - let lapic_id = u8::try_from(destination_id).expect("rtl8139d: CPU id couldn't fit inside u8"); - let rh = false; - let dm = false; - let addr = x86_64_msix::message_address(lapic_id, rh, dm); - - let (vector, interrupt_handle) = allocate_single_interrupt_vector(destination_id).expect("rtl8139d: failed to allocate interrupt vector").expect("rtl8139d: no interrupt vectors left"); - let msg_data = x86_64_msix::message_data_edge_triggered(DeliveryMode::Fixed, vector); - - table_entry_pointer.addr_lo.write(addr); - table_entry_pointer.addr_hi.write(0); - table_entry_pointer.msg_data.write(msg_data); - table_entry_pointer.vec_ctl.writef(MsixTableEntry::VEC_CTL_MASK_BIT, false); + let (msg_addr_and_data, interrupt_handle) = + allocate_single_interrupt_vector_for_msi(destination_id); + table_entry_pointer.write_addr_and_data(msg_addr_and_data); + table_entry_pointer.unmask(); interrupt_handle }; diff --git a/rtl8168d/src/main.rs b/rtl8168d/src/main.rs index 334554d12e..f5452baff9 100644 --- a/rtl8168d/src/main.rs +++ b/rtl8168d/src/main.rs @@ -3,7 +3,7 @@ extern crate netutils; extern crate syscall; use std::cell::RefCell; -use std::convert::{TryFrom, TryInto}; +use std::convert::TryInto; use std::{env, process}; use std::fs::File; use std::io::{ErrorKind, Read, Result, Write}; @@ -13,11 +13,10 @@ use std::sync::Arc; use event::EventQueue; use pcid_interface::{MsiSetFeatureInfo, PcidServerHandle, PciFeature, PciFeatureInfo, SetFeatureInfo, SubdriverArguments}; -use pcid_interface::irq_helpers::{read_bsp_apic_id, allocate_single_interrupt_vector}; +use pcid_interface::irq_helpers::{read_bsp_apic_id, allocate_single_interrupt_vector_for_msi}; use pcid_interface::msi::{MsixCapability, MsixTableEntry}; use redox_log::{RedoxLogger, OutputBuilder}; use syscall::{EventFlags, Packet, SchemeBlockMut}; -use syscall::io::Io; pub mod device; @@ -110,8 +109,6 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { } if msi_enabled && !msix_enabled { - use pcid_interface::msi::x86_64::{DeliveryMode, self as x86_64_msix}; - let capability = match pcid_handle.feature_info(PciFeature::Msi).expect("rtl8168d: failed to retrieve the MSI capability structure from pcid") { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), @@ -122,17 +119,11 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { // pcid_interface, so that this can be shared between nvmed, xhcid, ixgebd, etc.. let destination_id = read_bsp_apic_id().expect("rtl8168d: failed to read BSP apic id"); - let lapic_id = u8::try_from(destination_id).expect("CPU id didn't fit inside u8"); - let msg_addr = x86_64_msix::message_address(lapic_id, false, false); - - let (vector, interrupt_handle) = allocate_single_interrupt_vector(destination_id).expect("rtl8168d: failed to allocate interrupt vector").expect("rtl8168d: no interrupt vectors left"); - let msg_data = x86_64_msix::message_data_edge_triggered(DeliveryMode::Fixed, vector); + let (msg_addr_and_data, interrupt_handle) = allocate_single_interrupt_vector_for_msi(destination_id); let set_feature_info = MsiSetFeatureInfo { multi_message_enable: Some(0), - message_address: Some(msg_addr), - message_upper_address: Some(0), - message_data: Some(msg_data as u16), + message_address_and_data: Some(msg_addr_and_data), mask_bits: None, }; pcid_handle.set_feature_info(SetFeatureInfo::Msi(set_feature_info)).expect("rtl8168d: failed to set feature info"); @@ -161,8 +152,6 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { // Allocate one msi vector. let method = { - use pcid_interface::msi::x86_64::{DeliveryMode, self as x86_64_msix}; - // primary interrupter let k = 0; @@ -170,18 +159,10 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { let table_entry_pointer = info.table_entry_pointer(k); let destination_id = read_bsp_apic_id().expect("rtl8168d: failed to read BSP apic id"); - let lapic_id = u8::try_from(destination_id).expect("rtl8168d: CPU id couldn't fit inside u8"); - let rh = false; - let dm = false; - let addr = x86_64_msix::message_address(lapic_id, rh, dm); - - let (vector, interrupt_handle) = allocate_single_interrupt_vector(destination_id).expect("rtl8168d: failed to allocate interrupt vector").expect("rtl8168d: no interrupt vectors left"); - let msg_data = x86_64_msix::message_data_edge_triggered(DeliveryMode::Fixed, vector); - - table_entry_pointer.addr_lo.write(addr); - table_entry_pointer.addr_hi.write(0); - table_entry_pointer.msg_data.write(msg_data); - table_entry_pointer.vec_ctl.writef(MsixTableEntry::VEC_CTL_MASK_BIT, false); + let (msg_addr_and_data, interrupt_handle) = + allocate_single_interrupt_vector_for_msi(destination_id); + table_entry_pointer.write_addr_and_data(msg_addr_and_data); + table_entry_pointer.unmask(); interrupt_handle }; diff --git a/virtio-core/src/arch/x86_64.rs b/virtio-core/src/arch/x86_64.rs index 3965e08c39..e67d8ab767 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -1,11 +1,9 @@ use crate::{legacy_transport::LegacyTransport, reinit, transport::Error, Device}; -use pcid_interface::irq_helpers::{allocate_single_interrupt_vector, read_bsp_apic_id}; -use pcid_interface::msi::{self, MsixTableEntry}; +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 syscall::Io; - use crate::{probe::MsixInfo, MSIX_PRIMARY_VECTOR}; use pcid_interface::*; @@ -36,25 +34,10 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { let table_entry_pointer = info.table_entry_pointer(MSIX_PRIMARY_VECTOR as usize); let destination_id = read_bsp_apic_id().expect("virtio_core: `read_bsp_apic_id()` failed"); - let lapic_id = u8::try_from(destination_id).unwrap(); - - let rh = false; - let dm = false; - let addr = msi::x86_64::message_address(lapic_id, rh, dm); - - let (vector, interrupt_handle) = allocate_single_interrupt_vector(destination_id) - .unwrap() - .expect("virtio_core: interrupt vector exhaustion"); - - let msg_data = - msi::x86_64::message_data_edge_triggered(msi::x86_64::DeliveryMode::Fixed, vector); - - table_entry_pointer.addr_lo.write(addr); - table_entry_pointer.addr_hi.write(0); - table_entry_pointer.msg_data.write(msg_data); - table_entry_pointer - .vec_ctl - .writef(MsixTableEntry::VEC_CTL_MASK_BIT, false); + let (msg_addr_and_data, interrupt_handle) = + allocate_single_interrupt_vector_for_msi(destination_id); + table_entry_pointer.write_addr_and_data(msg_addr_and_data); + table_entry_pointer.unmask(); interrupt_handle }; diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs index 2315c2a1dc..bbab755d9a 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -12,8 +12,8 @@ use std::sync::{Arc, Mutex}; use std::env; use pcid_interface::{MsiSetFeatureInfo, PcidServerHandle, PciFeature, PciFeatureInfo, SetFeatureInfo}; -use pcid_interface::irq_helpers::{read_bsp_apic_id, allocate_single_interrupt_vector}; -use pcid_interface::msi::{MsiCapability, MsixCapability, MsixTableEntry}; +use pcid_interface::irq_helpers::{read_bsp_apic_id, allocate_single_interrupt_vector_for_msi}; +use pcid_interface::msi::MsixTableEntry; use event::{Event, EventQueue}; use redox_log::{RedoxLogger, OutputBuilder}; @@ -99,8 +99,6 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, bar0_address: usize) -> (O } if msi_enabled && !msix_enabled { - use pcid_interface::msi::x86_64::{DeliveryMode, self as x86_64_msix}; - let mut capability = match pcid_handle.feature_info(PciFeature::Msi).expect("xhcid: failed to retrieve the MSI capability structure from pcid") { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), @@ -111,17 +109,11 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, bar0_address: usize) -> (O // pcid_interface, so that this can be shared between nvmed, xhcid, ixgebd, etc.. let destination_id = read_bsp_apic_id().expect("xhcid: failed to read BSP apic id"); - let lapic_id = u8::try_from(destination_id).expect("CPU id didn't fit inside u8"); - let msg_addr = x86_64_msix::message_address(lapic_id, false, false); - - let (vector, interrupt_handle) = allocate_single_interrupt_vector(destination_id).expect("xhcid: failed to allocate interrupt vector").expect("xhcid: no interrupt vectors left"); - let msg_data = x86_64_msix::message_data_edge_triggered(DeliveryMode::Fixed, vector); + let (msg_addr_and_data, interrupt_handle) = allocate_single_interrupt_vector_for_msi(destination_id); let set_feature_info = MsiSetFeatureInfo { multi_message_enable: Some(0), - message_address: Some(msg_addr), - message_upper_address: Some(0), - message_data: Some(msg_data as u16), + message_address_and_data: Some(msg_addr_and_data), mask_bits: None, }; pcid_handle.set_feature_info(SetFeatureInfo::Msi(set_feature_info)).expect("xhcid: failed to set feature info"); @@ -148,8 +140,6 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, bar0_address: usize) -> (O // Allocate one msi vector. let method = { - use pcid_interface::msi::x86_64::{DeliveryMode, self as x86_64_msix}; - // primary interrupter let k = 0; @@ -157,18 +147,9 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, bar0_address: usize) -> (O let table_entry_pointer = info.table_entry_pointer(k); let destination_id = read_bsp_apic_id().expect("xhcid: failed to read BSP apic id"); - let lapic_id = u8::try_from(destination_id).expect("xhcid: CPU id couldn't fit inside u8"); - let rh = false; - let dm = false; - let addr = x86_64_msix::message_address(lapic_id, rh, dm); - - let (vector, interrupt_handle) = allocate_single_interrupt_vector(destination_id).expect("xhcid: failed to allocate interrupt vector").expect("xhcid: no interrupt vectors left"); - let msg_data = x86_64_msix::message_data_edge_triggered(DeliveryMode::Fixed, vector); - - table_entry_pointer.addr_lo.write(addr); - table_entry_pointer.addr_hi.write(0); - table_entry_pointer.msg_data.write(msg_data); - table_entry_pointer.vec_ctl.writef(MsixTableEntry::VEC_CTL_MASK_BIT, false); + let (msg_addr_and_data, interrupt_handle) = allocate_single_interrupt_vector_for_msi(destination_id); + table_entry_pointer.write_addr_and_data(msg_addr_and_data); + table_entry_pointer.unmask(); (Some(interrupt_handle), InterruptMethod::MsiX(Mutex::new(info))) };