From b06f0534aaff2e3bd8ce8b8c1c4cd8a50f77742f Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Jan 2024 18:45:32 +0100 Subject: [PATCH] Remove interrupt status register probing It isn't used when MSI-X is used and we always use MSI-X interrupts. --- virtio-core/src/arch/x86.rs | 15 +++------------ virtio-core/src/arch/x86_64.rs | 22 +++++++--------------- virtio-core/src/probe.rs | 31 +++++++++---------------------- 3 files changed, 19 insertions(+), 49 deletions(-) diff --git a/virtio-core/src/arch/x86.rs b/virtio-core/src/arch/x86.rs index 08483e9346..56b33d8fd8 100644 --- a/virtio-core/src/arch/x86.rs +++ b/virtio-core/src/arch/x86.rs @@ -1,10 +1,4 @@ -use crate::{ - legacy_transport::LegacyTransport, - reinit, - transport::Error, - utils::VolatileCell, - Device, -}; +use crate::{legacy_transport::LegacyTransport, reinit, transport::Error, Device}; use std::fs::File; use pcid_interface::*; @@ -13,16 +7,14 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { panic!("virtio-core: x86 doesn't support enable_msix") } -pub fn probe_legacy_port_transport<'a>( +pub fn probe_legacy_port_transport( pci_header: &PciHeader, pcid_handle: &mut PcidServerHandle, -) -> Result, Error> { +) -> Result { if let PciBar::Port(port) = pci_header.get_bar(0) { unsafe { syscall::iopl(3).expect("virtio: failed to set I/O privilege level") }; log::warn!("virtio: using legacy transport"); - static SHIM: VolatileCell = VolatileCell::new(0); - let transport = LegacyTransport::new(port); // Setup interrupts. @@ -38,7 +30,6 @@ pub fn probe_legacy_port_transport<'a>( let device = Device { transport, irq_handle, - isr: &SHIM, device_space: core::ptr::null_mut(), }; diff --git a/virtio-core/src/arch/x86_64.rs b/virtio-core/src/arch/x86_64.rs index 1909c20aec..f13850f4c1 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -1,13 +1,8 @@ -use crate::{ - reinit, - transport::{Error}, - utils::VolatileCell, - Device, legacy_transport::LegacyTransport, -}; +use crate::{legacy_transport::LegacyTransport, reinit, transport::Error, Device}; -use pcid_interface::msi::{self, MsixTableEntry}; use pcid_interface::irq_helpers::{allocate_single_interrupt_vector, read_bsp_apic_id}; -use std::{ptr::NonNull, fs::File}; +use pcid_interface::msi::{self, MsixTableEntry}; +use std::{fs::File, ptr::NonNull}; use syscall::Io; @@ -16,7 +11,6 @@ use crate::{probe::MsixInfo, MSIX_PRIMARY_VECTOR}; use pcid_interface::*; pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { - let pci_config = pcid_handle.fetch_config()?; // Extended message signaled interrupts. @@ -80,7 +74,8 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { .unwrap() .expect("virtio_core: interrupt vector exhaustion"); - let msg_data = msi::x86_64::message_data_edge_triggered(msi::x86_64::DeliveryMode::Fixed, vector); + 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); @@ -98,16 +93,14 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { Ok(interrupt_handle) } -pub fn probe_legacy_port_transport<'a>( +pub fn probe_legacy_port_transport( pci_header: &PciHeader, pcid_handle: &mut PcidServerHandle, -) -> Result, Error> { +) -> Result { if let PciBar::Port(port) = pci_header.get_bar(0) { unsafe { syscall::iopl(3).expect("virtio: failed to set I/O privilege level") }; log::warn!("virtio: using legacy transport"); - static SHIM: VolatileCell = VolatileCell::new(0); - let transport = LegacyTransport::new(port); // Setup interrupts. @@ -123,7 +116,6 @@ pub fn probe_legacy_port_transport<'a>( let device = Device { transport, irq_handle, - isr: &SHIM, device_space: core::ptr::null_mut(), }; diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index 2a258bda28..3e31284799 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -7,19 +7,18 @@ use pcid_interface::*; use crate::spec::*; use crate::transport::{Error, StandardTransport, Transport}; -use crate::utils::{align_down, VolatileCell}; +use crate::utils::align_down; -pub struct Device<'a> { +pub struct Device { pub transport: Arc, pub device_space: *const u8, pub irq_handle: File, - pub isr: &'a VolatileCell, } // FIXME(andypython): `device_space` should not be `Send` nor `Sync`. Take // it out of `Device`. -unsafe impl Send for Device<'_> {} -unsafe impl Sync for Device<'_> {} +unsafe impl Send for Device {} +unsafe impl Sync for Device {} pub struct MsixInfo { pub virt_table_base: NonNull, @@ -60,7 +59,7 @@ fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { /// /// ## Panics /// This function panics if the device is not a virtio device. -pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result, Error> { +pub fn probe_device(pcid_handle: &mut PcidServerHandle) -> Result { let pci_config = pcid_handle.fetch_config()?; let pci_header = pcid_handle.fetch_header()?; @@ -71,7 +70,6 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result let mut common_addr = None; let mut notify_addr = None; - let mut isr_addr = None; let mut device_addr = None; for capability in pcid_handle @@ -89,7 +87,7 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result let capability = unsafe { &*(capability.data.as_ptr() as *const PciCapability) }; match capability.cfg_type { - CfgType::Common | CfgType::Notify | CfgType::Isr | CfgType::Device => {} + CfgType::Common | CfgType::Notify | CfgType::Device => {} _ => continue, } @@ -138,11 +136,6 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result notify_addr = Some((address, multiplier)); } - CfgType::Isr => { - debug_assert!(isr_addr.is_none()); - isr_addr = Some(address); - } - CfgType::Device => { debug_assert!(device_addr.is_none()); device_addr = Some(address); @@ -154,12 +147,8 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result log::trace!("virtio-core::device-probe: {capability:?}"); } - if let ( - Some(common_addr), - Some(isr_addr), - Some(device_addr), - Some((notify_addr, notify_multiplier)), - ) = (common_addr, isr_addr, device_addr, notify_addr) + if let (Some(common_addr), Some(device_addr), Some((notify_addr, notify_multiplier))) = + (common_addr, device_addr, notify_addr) { assert!( notify_multiplier != 0, @@ -168,7 +157,6 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result let common = unsafe { &mut *(common_addr as *mut CommonCfg) }; let device_space = unsafe { &mut *(device_addr as *mut u8) }; - let isr = unsafe { &*(isr_addr as *mut VolatileCell) }; let transport = StandardTransport::new( common, @@ -193,7 +181,6 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result transport, device_space, irq_handle, - isr, }; device.transport.reset(); @@ -205,7 +192,7 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result } } -pub fn reinit<'a>(device: &Device<'a>) -> Result<(), Error> { +pub fn reinit(device: &Device) -> Result<(), Error> { // XXX: According to the virtio specification v1.2, setting the ACKNOWLEDGE and DRIVER bits // in `device_status` is required to be done in two steps. device