From 7dbce70d9551ace0fc74f1e6f31c14fb48412048 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 1 Mar 2025 17:09:52 +0100 Subject: [PATCH] Get rid of the State struct and the Arc wrapper around Pcie --- pcid/src/driver_handler.rs | 36 +++++++++++++------------- pcid/src/main.rs | 52 ++++++++++++++++---------------------- pcid/src/scheme.rs | 17 ++++++------- 3 files changed, 47 insertions(+), 58 deletions(-) diff --git a/pcid/src/driver_handler.rs b/pcid/src/driver_handler.rs index 8537be5c65..435c225937 100644 --- a/pcid/src/driver_handler.rs +++ b/pcid/src/driver_handler.rs @@ -1,17 +1,15 @@ -use std::sync::Arc; - use pci_types::capability::{MultipleMessageSupport, PciCapability}; use pci_types::{ConfigRegionAccess, EndpointHeader}; use pcid_interface::PciFunction; -use crate::State; +use crate::cfg_access::Pcie; pub struct DriverHandler<'a> { func: PciFunction, endpoint_header: &'a mut EndpointHeader, capabilities: Vec, - state: Arc, + pcie: &'a Pcie, } impl<'a> DriverHandler<'a> { @@ -19,13 +17,13 @@ impl<'a> DriverHandler<'a> { func: PciFunction, endpoint_header: &'a mut EndpointHeader, capabilities: Vec, - state: Arc, + pcie: &'a Pcie, ) -> Self { DriverHandler { func, endpoint_header, capabilities, - state, + pcie, } } @@ -39,7 +37,7 @@ impl<'a> DriverHandler<'a> { match request { PcidClientRequest::EnableDevice => { self.func.legacy_interrupt_line = - crate::enable_function(&self.state, &mut self.endpoint_header); + crate::enable_function(&self.pcie, &mut self.endpoint_header); PcidClientResponse::EnabledDevice } @@ -48,7 +46,7 @@ impl<'a> DriverHandler<'a> { .iter() .filter_map(|capability| match capability { PciCapability::Vendor(addr) => unsafe { - Some(VendorSpecificCapability::parse(*addr, &self.state.pcie)) + Some(VendorSpecificCapability::parse(*addr, self.pcie)) }, _ => None, }) @@ -80,7 +78,7 @@ impl<'a> DriverHandler<'a> { { // If MSI-X is supported disable it before enabling MSI as they can't be // active at the same time. - msix_capability.set_enabled(false, &self.state.pcie); + msix_capability.set_enabled(false, self.pcie); } let capability = match self.capabilities.iter_mut().find_map(|capability| { @@ -96,7 +94,7 @@ impl<'a> DriverHandler<'a> { ) } }; - capability.set_enabled(true, &self.state.pcie); + capability.set_enabled(true, self.pcie); PcidClientResponse::FeatureEnabled(feature) } PciFeature::MsiX => { @@ -110,7 +108,7 @@ impl<'a> DriverHandler<'a> { { // If MSI is supported disable it before enabling MSI-X as they can't be // active at the same time. - msi_capability.set_enabled(false, &self.state.pcie); + msi_capability.set_enabled(false, self.pcie); } let capability = match self.capabilities.iter_mut().find_map(|capability| { @@ -126,7 +124,7 @@ impl<'a> DriverHandler<'a> { ) } }; - capability.set_enabled(true, &self.state.pcie); + capability.set_enabled(true, self.pcie); PcidClientResponse::FeatureEnabled(feature) } } @@ -209,7 +207,7 @@ impl<'a> DriverHandler<'a> { ) } }, - &self.state.pcie, + self.pcie, ); } if let Some(message_addr_and_data) = info_to_set.message_address_and_data { @@ -220,7 +218,7 @@ impl<'a> DriverHandler<'a> { ); } if message_addr_and_data.data - & ((1 << info.multiple_message_enable(&self.state.pcie) as u8) - 1) + & ((1 << info.multiple_message_enable(self.pcie) as u8) - 1) != 0 { return PcidClientResponse::Error( @@ -233,11 +231,11 @@ impl<'a> DriverHandler<'a> { .data .try_into() .expect("pcid: MSI message data too big"), - &self.state.pcie, + self.pcie, ); } if let Some(mask_bits) = info_to_set.mask_bits { - info.set_message_mask(mask_bits, &self.state.pcie); + info.set_message_mask(mask_bits, self.pcie); } PcidClientResponse::SetFeatureInfo(PciFeature::Msi) } else { @@ -256,7 +254,7 @@ impl<'a> DriverHandler<'a> { }) { if let Some(mask) = function_mask { - info.set_function_mask(mask, &self.state.pcie); + info.set_function_mask(mask, self.pcie); } PcidClientResponse::SetFeatureInfo(PciFeature::MsiX) } else { @@ -268,12 +266,12 @@ impl<'a> DriverHandler<'a> { _ => unreachable!(), }, PcidClientRequest::ReadConfig(offset) => { - let value = unsafe { self.state.pcie.read(self.func.addr, offset) }; + let value = unsafe { self.pcie.read(self.func.addr, offset) }; return PcidClientResponse::ReadConfig(value); } PcidClientRequest::WriteConfig(offset, value) => { unsafe { - self.state.pcie.write(self.func.addr, offset, value); + self.pcie.write(self.func.addr, offset, value); } return PcidClientResponse::WriteConfig; } diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 8723cac4b5..3545c800f9 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -4,7 +4,6 @@ #![feature(if_let_guard)] use std::collections::BTreeMap; -use std::sync::Arc; use log::{debug, info, trace, warn}; use pci_types::capability::PciCapability; @@ -21,10 +20,6 @@ mod cfg_access; mod driver_handler; mod scheme; -pub struct State { - pcie: Pcie, -} - pub struct Func { inner: PciFunction, @@ -34,7 +29,7 @@ pub struct Func { } fn handle_parsed_header( - state: &State, + pcie: &Pcie, tree: &mut BTreeMap, endpoint_header: EndpointHeader, full_device_id: FullDeviceId, @@ -46,7 +41,7 @@ fn handle_parsed_header( skip = false; continue; } - match endpoint_header.bar(i, &state.pcie) { + match endpoint_header.bar(i, pcie) { Some(TyBar::Io { port }) => bars[i as usize] = PciBar::Port(port.try_into().unwrap()), Some(TyBar::Memory32 { address, @@ -84,10 +79,8 @@ fn handle_parsed_header( info!(" BAR{}", string); } - let capabilities = if endpoint_header.status(&state.pcie).has_capability_list() { - endpoint_header - .capabilities(&state.pcie) - .collect::>() + let capabilities = if endpoint_header.status(pcie).has_capability_list() { + endpoint_header.capabilities(pcie).collect::>() } else { Vec::new() }; @@ -114,11 +107,11 @@ fn handle_parsed_header( } fn enable_function( - state: &State, + pcie: &Pcie, endpoint_header: &mut EndpointHeader, ) -> Option { // Enable bus mastering, memory space, and I/O space - endpoint_header.update_command(&state.pcie, |cmd| { + endpoint_header.update_command(pcie, |cmd| { cmd | CommandRegister::BUS_MASTER_ENABLE | CommandRegister::MEMORY_ENABLE | CommandRegister::IO_ENABLE @@ -128,7 +121,7 @@ fn enable_function( let mut irq = 0xFF; let mut interrupt_pin = 0xFF; - endpoint_header.update_interrupt(&state.pcie, |(pin, mut line)| { + endpoint_header.update_interrupt(pcie, |(pin, mut line)| { if line == 0xFF { line = 9; } @@ -153,13 +146,12 @@ fn enable_function( | ((pci_address.device() as u32) << 11) | ((pci_address.function() as u32) << 8); let addr = [ - dt_address & state.pcie.interrupt_map_mask[0], + dt_address & pcie.interrupt_map_mask[0], 0u32, 0u32, - interrupt_pin as u32 & state.pcie.interrupt_map_mask[3], + interrupt_pin as u32 & pcie.interrupt_map_mask[3], ]; - let mapping = state - .pcie + let mapping = pcie .interrupt_map .iter() .find(|x| x.addr == addr[0..3] && x.interrupt == addr[3]); @@ -197,7 +189,7 @@ fn main() { } fn main_inner(daemon: redox_daemon::Daemon) -> ! { - let state = Arc::new(State { pcie: Pcie::new() }); + let pcie = Pcie::new(); let mut tree = BTreeMap::new(); info!("PCI SG-BS:DV.F VEND:DEVI CL.SC.IN.RV"); @@ -212,12 +204,12 @@ fn main_inner(daemon: redox_daemon::Daemon) -> ! { bus_i += 1; for dev_num in 0..32 { - scan_device(&mut tree, &state, &mut bus_nums, bus_num, dev_num); + scan_device(&mut tree, &pcie, &mut bus_nums, bus_num, dev_num); } } info!("Enumeration complete, now starting pci scheme"); - let mut scheme = scheme::PciScheme::new(state, tree); + let mut scheme = scheme::PciScheme::new(pcie, tree); let socket = redox_scheme::Socket::create("pci").expect("failed to open pci scheme socket"); let _ = daemon.ready(); @@ -250,7 +242,7 @@ fn main_inner(daemon: redox_daemon::Daemon) -> ! { fn scan_device( tree: &mut BTreeMap, - state: &State, + pcie: &Pcie, bus_nums: &mut Vec, bus_num: u8, dev_num: u8, @@ -258,7 +250,7 @@ fn scan_device( for func_num in 0..8 { let header = TyPciHeader::new(PciAddress::new(0, bus_num, dev_num, func_num)); - let (vendor_id, device_id) = header.id(&state.pcie); + let (vendor_id, device_id) = header.id(pcie); if vendor_id == 0xffff && device_id == 0xffff { if func_num == 0 { trace!("PCI {:>02X}:{:>02X}: no dev", bus_num, dev_num); @@ -268,7 +260,7 @@ fn scan_device( continue; } - let (revision, class, subclass, interface) = header.revision_and_class(&state.pcie); + let (revision, class, subclass, interface) = header.revision_and_class(pcie); let full_device_id = FullDeviceId { vendor_id, device_id, @@ -280,20 +272,20 @@ fn scan_device( info!("PCI {} {}", header.address(), full_device_id.display()); - let has_multiple_functions = header.has_multiple_functions(&state.pcie); + let has_multiple_functions = header.has_multiple_functions(pcie); - match header.header_type(&state.pcie) { + match header.header_type(pcie) { HeaderType::Endpoint => { handle_parsed_header( - state, + pcie, tree, - EndpointHeader::from_header(header, &state.pcie).unwrap(), + EndpointHeader::from_header(header, pcie).unwrap(), full_device_id, ); } HeaderType::PciPciBridge => { - let bridge_header = PciPciBridgeHeader::from_header(header, &state.pcie).unwrap(); - bus_nums.push(bridge_header.secondary_bus_number(&state.pcie)); + let bridge_header = PciPciBridgeHeader::from_header(header, pcie).unwrap(); + bus_nums.push(bridge_header.secondary_bus_number(pcie)); } ty => { warn!("pcid: unknown header type: {ty:?}"); diff --git a/pcid/src/scheme.rs b/pcid/src/scheme.rs index d1c35166d7..cee1fc7a11 100644 --- a/pcid/src/scheme.rs +++ b/pcid/src/scheme.rs @@ -1,5 +1,4 @@ use std::collections::{BTreeMap, VecDeque}; -use std::sync::Arc; use pci_types::PciAddress; use redox_scheme::{CallerCtx, OpenResult, Scheme}; @@ -9,12 +8,12 @@ use syscall::flag::{MODE_CHR, MODE_DIR, O_DIRECTORY, O_STAT}; use syscall::schemev2::NewFdFlags; use syscall::ENOLCK; -use crate::State; +use crate::cfg_access::Pcie; pub struct PciScheme { handles: BTreeMap, next_id: usize, - state: Arc, + pcie: Pcie, tree: BTreeMap, } enum Handle { @@ -184,7 +183,7 @@ impl Scheme for PciScheme { match handle.inner { Handle::Channel { addr, ref mut st } => { - Self::write_channel(&self.state, &mut self.tree, addr, st, buf) + Self::write_channel(&self.pcie, &mut self.tree, addr, st, buf) } _ => Err(Error::new(EBADF)), @@ -210,11 +209,11 @@ impl PciScheme { } impl PciScheme { - pub fn new(state: Arc, tree: BTreeMap) -> Self { + pub fn new(pcie: Pcie, tree: BTreeMap) -> Self { Self { handles: BTreeMap::new(), next_id: 0, - state, + pcie, tree, } } @@ -235,7 +234,7 @@ impl PciScheme { return Err(Error::new(ENOLCK)); } func.inner.legacy_interrupt_line = - crate::enable_function(&self.state, &mut func.endpoint_header); + crate::enable_function(&self.pcie, &mut func.endpoint_header); func.enabled = true; Handle::Channel { addr, @@ -264,7 +263,7 @@ impl PciScheme { } } fn write_channel( - pci_state: &Arc, + pci_state: &Pcie, tree: &mut BTreeMap, addr: PciAddress, state: &mut ChannelState, @@ -280,7 +279,7 @@ impl PciScheme { func.inner.clone(), &mut func.endpoint_header, func.capabilities.clone(), - Arc::clone(pci_state), + &*pci_state, ) .respond(request);