From 92428c535bb7f50e4361cda04cc4fc79644ed170 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 21 Jan 2024 11:35:06 +0100 Subject: [PATCH 1/7] Only enable redox scheme logger when compiling for redox This allows running pcid tests on the host. --- pcid/src/main.rs | 66 +++++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/pcid/src/main.rs b/pcid/src/main.rs index e579a6d4ab..048b9cf03d 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -195,9 +195,9 @@ impl DriverHandler { let mut pcid_to_client = unsafe { File::from_raw_fd(pcid_to_client_write as RawFd) }; let mut pcid_from_client = unsafe { File::from_raw_fd(pcid_from_client_read as RawFd) }; - while let Ok(msg) = recv(&mut pcid_from_client) { - let response = self.respond(msg, &args); - send(&mut pcid_to_client, &response).unwrap(); + while let Ok(msg) = recv(&mut pcid_from_client) { + let response = self.respond(msg, &args); + send(&mut pcid_to_client, &response).unwrap(); } } } @@ -420,22 +420,22 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he info!("PCID SPAWN {:?}", command); - // TODO: libc wrapper? - let [fds1, fds2] = unsafe { - let mut fds1 = [0 as libc::c_int; 2]; - let mut fds2 = [0 as libc::c_int; 2]; + // TODO: libc wrapper? + let [fds1, fds2] = unsafe { + let mut fds1 = [0 as libc::c_int; 2]; + let mut fds2 = [0 as libc::c_int; 2]; - assert_eq!(libc::pipe(fds1.as_mut_ptr()), 0, "pcid: failed to create pcid->client pipe"); - assert_eq!(libc::pipe(fds2.as_mut_ptr()), 0, "pcid: failed to create client->pcid pipe"); + assert_eq!(libc::pipe(fds1.as_mut_ptr()), 0, "pcid: failed to create pcid->client pipe"); + assert_eq!(libc::pipe(fds2.as_mut_ptr()), 0, "pcid: failed to create client->pcid pipe"); - [ - fds1.map(|c| c as usize), - fds2.map(|c| c as usize), - ] - }; + [ + fds1.map(|c| c as usize), + fds2.map(|c| c as usize), + ] + }; - let [pcid_to_client_read, pcid_to_client_write] = fds1; - let [pcid_from_client_read, pcid_from_client_write] = fds2; + let [pcid_to_client_read, pcid_to_client_write] = fds1; + let [pcid_from_client_read, pcid_from_client_write] = fds2; let envs = vec![ ("PCID_TO_CLIENT_FD", format!("{}", pcid_to_client_read)), @@ -483,22 +483,24 @@ fn setup_logging(verbosity: u8) -> Option<&'static RedoxLogger> { .build() ); - match OutputBuilder::in_redox_logging_scheme("bus", "pci", "pcid.log") { - Ok(b) => logger = logger.with_output( - b.with_filter(log::LevelFilter::Trace) - .flush_on_newline(true) - .build() - ), - Err(error) => eprintln!("pcid: failed to open pcid.log"), - } - match OutputBuilder::in_redox_logging_scheme("bus", "pci", "pcid.ansi.log") { - Ok(b) => logger = logger.with_output( - b.with_filter(log::LevelFilter::Trace) - .with_ansi_escape_codes() - .flush_on_newline(true) - .build() - ), - Err(error) => eprintln!("pcid: failed to open pcid.ansi.log"), + #[cfg(target_os = "redox")] { + match OutputBuilder::in_redox_logging_scheme("bus", "pci", "pcid.log") { + Ok(b) => logger = logger.with_output( + b.with_filter(log::LevelFilter::Trace) + .flush_on_newline(true) + .build() + ), + Err(error) => eprintln!("pcid: failed to open pcid.log"), + } + match OutputBuilder::in_redox_logging_scheme("bus", "pci", "pcid.ansi.log") { + Ok(b) => logger = logger.with_output( + b.with_filter(log::LevelFilter::Trace) + .with_ansi_escape_codes() + .flush_on_newline(true) + .build() + ), + Err(error) => eprintln!("pcid: failed to open pcid.ansi.log"), + } } match logger.enable() { From b9c4c61dccabb22041afba6579925164aec4555a Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 21 Jan 2024 13:02:47 +0100 Subject: [PATCH 2/7] Move driver matching to config.rs --- pcid/src/config.rs | 68 ++++++++++++++++++++++++++++++++++++++++++++++ pcid/src/main.rs | 45 ++---------------------------- 2 files changed, 71 insertions(+), 42 deletions(-) diff --git a/pcid/src/config.rs b/pcid/src/config.rs index 83faddd586..6c79e7eb63 100644 --- a/pcid/src/config.rs +++ b/pcid/src/config.rs @@ -3,6 +3,8 @@ use std::ops::Range; use serde::Deserialize; +use crate::pci::PciHeader; + #[derive(Clone, Debug, Default, Deserialize)] pub struct Config { pub drivers: Vec, @@ -20,3 +22,69 @@ pub struct DriverConfig { pub device_id_range: Option>, pub command: Option>, } + +impl DriverConfig { + pub fn match_function(&self, header: &PciHeader) -> bool { + if let Some(class) = self.class { + if class != header.class().into() { + return false; + } + } + + if let Some(subclass) = self.subclass { + if subclass != header.subclass() { + return false; + } + } + + if let Some(interface) = self.interface { + if interface != header.interface() { + return false; + } + } + + if let Some(ref ids) = self.ids { + let mut device_found = false; + for (vendor, devices) in ids { + let vendor_without_prefix = vendor.trim_start_matches("0x"); + let vendor = i64::from_str_radix(vendor_without_prefix, 16).unwrap() as u16; + + if vendor != header.vendor_id() { + continue; + } + + for device in devices { + if *device == header.device_id() { + device_found = true; + break; + } + } + } + if !device_found { + return false; + } + } else { + if let Some(vendor) = self.vendor { + if vendor != header.vendor_id() { + return false; + } + } + + if let Some(device) = self.device { + if device != header.device_id() { + return false; + } + } + } + + if let Some(ref device_id_range) = self.device_id_range { + if header.device_id() < device_id_range.start + || device_id_range.end <= header.device_id() + { + return false; + } + } + + true + } +} diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 048b9cf03d..8e762dca09 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -2,8 +2,8 @@ use std::fs::{File, metadata, read_dir}; use std::io::prelude::*; use std::os::unix::io::{FromRawFd, RawFd}; use std::process::Command; +use std::thread; use std::sync::{Arc, Mutex}; -use std::{i64, thread}; use structopt::StructOpt; use log::{debug, error, info, warn, trace}; @@ -265,47 +265,8 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he info!("{}", string); for driver in config.drivers.iter() { - if let Some(class) = driver.class { - if class != raw_class { continue; } - } - - if let Some(subclass) = driver.subclass { - if subclass != header.subclass() { continue; } - } - - if let Some(interface) = driver.interface { - if interface != header.interface() { continue; } - } - - if let Some(ref ids) = driver.ids { - let mut device_found = false; - for (vendor, devices) in ids { - let vendor_without_prefix = vendor.trim_start_matches("0x"); - let vendor = i64::from_str_radix(vendor_without_prefix, 16).unwrap() as u16; - - if vendor != header.vendor_id() { continue; } - - for device in devices { - if *device == header.device_id() { - device_found = true; - break; - } - } - } - if !device_found { continue; } - } else { - if let Some(vendor) = driver.vendor { - if vendor != header.vendor_id() { continue; } - } - - if let Some(device) = driver.device { - if device != header.device_id() { continue; } - } - } - - if let Some(ref device_id_range) = driver.device_id_range { - if header.device_id() < device_id_range.start || - device_id_range.end <= header.device_id() { continue; } + if !driver.match_function(&header) { + continue; } if let Some(ref args) = driver.command { From 92914e808c43091d24bfa7b9fe60888562168cf6 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 21 Jan 2024 14:03:33 +0100 Subject: [PATCH 3/7] Reduce code indentation one level --- pcid/src/main.rs | 292 ++++++++++++++++++++++++----------------------- 1 file changed, 147 insertions(+), 145 deletions(-) diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 8e762dca09..8db92b7cbf 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -269,161 +269,163 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he continue; } - if let Some(ref args) = driver.command { - // Enable bus mastering, memory space, and I/O space - unsafe { - let mut data = pci.read(addr, 0x04); - data |= 7; - pci.write(addr, 0x04, data); + let Some(ref args) = driver.command else { + continue; + }; + + // Enable bus mastering, memory space, and I/O space + unsafe { + let mut data = pci.read(addr, 0x04); + data |= 7; + pci.write(addr, 0x04, data); + } + + // Set IRQ line to 9 if not set + let mut irq; + let interrupt_pin; + + unsafe { + let mut data = pci.read(addr, 0x3C); + irq = (data & 0xFF) as u8; + interrupt_pin = ((data & 0x0000_FF00) >> 8) as u8; + if irq == 0xFF { + irq = 9; + } + data = (data & 0xFFFFFF00) | irq as u32; + pci.write(addr, 0x3C, data); + }; + + // Find BAR sizes + //TODO: support 64-bit BAR sizes? + let mut bars = [PciBar::None; 6]; + let mut bar_sizes = [0; 6]; + unsafe { + let count = match header.header_type() { + PciHeaderType::GENERAL => 6, + PciHeaderType::PCITOPCI => 2, + _ => 0, + }; + + for i in 0..count { + bars[i] = header.get_bar(i); + + let offset = 0x10 + (i as u8) * 4; + + let original = pci.read(addr, offset.into()); + pci.write(addr, offset.into(), 0xFFFFFFFF); + + let new = pci.read(addr, offset.into()); + pci.write(addr, offset.into(), original); + + let masked = if new & 1 == 1 { + new & 0xFFFFFFFC + } else { + new & 0xFFFFFFF0 + }; + + let size = (!masked).wrapping_add(1); + bar_sizes[i] = if size <= 1 { + 0 + } else { + size + }; + } + } + + let capabilities = if header.status() & (1 << 4) != 0 { + let func = PciFunc { + pci: state.preferred_cfg_access(), + addr + }; + crate::pci::cap::CapabilitiesIter { inner: crate::pci::cap::CapabilityOffsetsIter::new(header.cap_pointer(), &func) }.collect::>() + } else { + Vec::new() + }; + debug!("PCI DEVICE CAPABILITIES for {}: {:?}", args.iter().map(|string| string.as_ref()).nth(0).unwrap_or("[unknown]"), capabilities); + + use driver_interface::LegacyInterruptPin; + + let legacy_interrupt_pin = match interrupt_pin { + 0 => None, + 1 => Some(LegacyInterruptPin::IntA), + 2 => Some(LegacyInterruptPin::IntB), + 3 => Some(LegacyInterruptPin::IntC), + 4 => Some(LegacyInterruptPin::IntD), + + other => { + warn!("pcid: invalid interrupt pin: {}", other); + None + } + }; + + let func = driver_interface::PciFunction { + bars, + bar_sizes, + addr, + devid: header.device_id(), + legacy_interrupt_line: irq, + legacy_interrupt_pin, + venid: header.vendor_id(), + }; + + let subdriver_args = driver_interface::SubdriverArguments { + func, + }; + + let mut args = args.iter(); + if let Some(program) = args.next() { + let mut command = Command::new(program); + for arg in args { + if arg.starts_with("$") { + panic!("support for $VARIABLE has been removed. use pcid_interface instead"); + } + command.arg(arg); } - // Set IRQ line to 9 if not set - let mut irq; - let interrupt_pin; + info!("PCID SPAWN {:?}", command); - unsafe { - let mut data = pci.read(addr, 0x3C); - irq = (data & 0xFF) as u8; - interrupt_pin = ((data & 0x0000_FF00) >> 8) as u8; - if irq == 0xFF { - irq = 9; - } - data = (data & 0xFFFFFF00) | irq as u32; - pci.write(addr, 0x3C, data); + // TODO: libc wrapper? + let [fds1, fds2] = unsafe { + let mut fds1 = [0 as libc::c_int; 2]; + let mut fds2 = [0 as libc::c_int; 2]; + + assert_eq!(libc::pipe(fds1.as_mut_ptr()), 0, "pcid: failed to create pcid->client pipe"); + assert_eq!(libc::pipe(fds2.as_mut_ptr()), 0, "pcid: failed to create client->pcid pipe"); + + [ + fds1.map(|c| c as usize), + fds2.map(|c| c as usize), + ] }; - // Find BAR sizes - //TODO: support 64-bit BAR sizes? - let mut bars = [PciBar::None; 6]; - let mut bar_sizes = [0; 6]; - unsafe { - let count = match header.header_type() { - PciHeaderType::GENERAL => 6, - PciHeaderType::PCITOPCI => 2, - _ => 0, - }; + let [pcid_to_client_read, pcid_to_client_write] = fds1; + let [pcid_from_client_read, pcid_from_client_write] = fds2; - for i in 0..count { - bars[i] = header.get_bar(i); + let envs = vec![ + ("PCID_TO_CLIENT_FD", format!("{}", pcid_to_client_read)), + ("PCID_FROM_CLIENT_FD", format!("{}", pcid_from_client_write)), + ]; - let offset = 0x10 + (i as u8) * 4; - - let original = pci.read(addr, offset.into()); - pci.write(addr, offset.into(), 0xFFFFFFFF); - - let new = pci.read(addr, offset.into()); - pci.write(addr, offset.into(), original); - - let masked = if new & 1 == 1 { - new & 0xFFFFFFFC - } else { - new & 0xFFFFFFF0 + match command.envs(envs).spawn() { + Ok(mut child) => { + let driver_handler = DriverHandler { + addr, + config: driver.clone(), + header, + state: Arc::clone(&state), + capabilities, }; - - let size = (!masked).wrapping_add(1); - bar_sizes[i] = if size <= 1 { - 0 - } else { - size - }; - } - } - - let capabilities = if header.status() & (1 << 4) != 0 { - let func = PciFunc { - pci: state.preferred_cfg_access(), - addr - }; - crate::pci::cap::CapabilitiesIter { inner: crate::pci::cap::CapabilityOffsetsIter::new(header.cap_pointer(), &func) }.collect::>() - } else { - Vec::new() - }; - debug!("PCI DEVICE CAPABILITIES for {}: {:?}", args.iter().map(|string| string.as_ref()).nth(0).unwrap_or("[unknown]"), capabilities); - - use driver_interface::LegacyInterruptPin; - - let legacy_interrupt_pin = match interrupt_pin { - 0 => None, - 1 => Some(LegacyInterruptPin::IntA), - 2 => Some(LegacyInterruptPin::IntB), - 3 => Some(LegacyInterruptPin::IntC), - 4 => Some(LegacyInterruptPin::IntD), - - other => { - warn!("pcid: invalid interrupt pin: {}", other); - None - } - }; - - let func = driver_interface::PciFunction { - bars, - bar_sizes, - addr, - devid: header.device_id(), - legacy_interrupt_line: irq, - legacy_interrupt_pin, - venid: header.vendor_id(), - }; - - let subdriver_args = driver_interface::SubdriverArguments { - func, - }; - - let mut args = args.iter(); - if let Some(program) = args.next() { - let mut command = Command::new(program); - for arg in args { - if arg.starts_with("$") { - panic!("support for $VARIABLE has been removed. use pcid_interface instead"); + let _handle = thread::spawn(move || { + driver_handler.handle_spawn(pcid_to_client_write, pcid_from_client_read, subdriver_args); + }); + // FIXME this currently deadlocks as pcid doesn't daemonize + //state.threads.lock().unwrap().push(handle); + match child.wait() { + Ok(_status) => (), + Err(err) => error!("pcid: failed to wait for {:?}: {}", command, err), } - command.arg(arg); - } - - info!("PCID SPAWN {:?}", command); - - // TODO: libc wrapper? - let [fds1, fds2] = unsafe { - let mut fds1 = [0 as libc::c_int; 2]; - let mut fds2 = [0 as libc::c_int; 2]; - - assert_eq!(libc::pipe(fds1.as_mut_ptr()), 0, "pcid: failed to create pcid->client pipe"); - assert_eq!(libc::pipe(fds2.as_mut_ptr()), 0, "pcid: failed to create client->pcid pipe"); - - [ - fds1.map(|c| c as usize), - fds2.map(|c| c as usize), - ] - }; - - let [pcid_to_client_read, pcid_to_client_write] = fds1; - let [pcid_from_client_read, pcid_from_client_write] = fds2; - - let envs = vec![ - ("PCID_TO_CLIENT_FD", format!("{}", pcid_to_client_read)), - ("PCID_FROM_CLIENT_FD", format!("{}", pcid_from_client_write)), - ]; - - match command.envs(envs).spawn() { - Ok(mut child) => { - let driver_handler = DriverHandler { - addr, - config: driver.clone(), - header, - state: Arc::clone(&state), - capabilities, - }; - let _handle = thread::spawn(move || { - driver_handler.handle_spawn(pcid_to_client_write, pcid_from_client_read, subdriver_args); - }); - // FIXME this currently deadlocks as pcid doesn't daemonize - //state.threads.lock().unwrap().push(handle); - match child.wait() { - Ok(_status) => (), - Err(err) => error!("pcid: failed to wait for {:?}: {}", command, err), - } - } - Err(err) => error!("pcid: failed to execute {:?}: {}", command, err) } + Err(err) => error!("pcid: failed to execute {:?}: {}", command, err) } } } From 1890293e86616c813e08155609cc3802b2c76c52 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 21 Jan 2024 15:18:56 +0100 Subject: [PATCH 4/7] Introduce a FullDeviceId type and pass it in pcid_interface --- ihdad/src/main.rs | 3 +- pcid/src/config.rs | 22 +++---- pcid/src/driver_interface/mod.rs | 8 +-- pcid/src/main.rs | 5 +- pcid/src/pci/header.rs | 108 +++++++++++-------------------- pcid/src/pci/id.rs | 12 ++++ pcid/src/pci/mod.rs | 2 + virtio-blkd/src/main.rs | 2 +- virtio-core/src/probe.rs | 2 +- virtio-gpud/src/main.rs | 2 +- virtio-netd/src/main.rs | 2 +- 11 files changed, 72 insertions(+), 96 deletions(-) create mode 100644 pcid/src/pci/id.rs diff --git a/ihdad/src/main.rs b/ihdad/src/main.rs index d91013cc47..16ab7ea4a9 100755 --- a/ihdad/src/main.rs +++ b/ihdad/src/main.rs @@ -185,7 +185,8 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let mut irq_file = get_int_method(&mut pcid_handle).expect("ihdad: no interrupt file"); { - let vend_prod:u32 = ((pci_config.func.venid as u32) << 16) | (pci_config.func.devid as u32); + let vend_prod: u32 = ((pci_config.func.full_device_id.vendor_id as u32) << 16) + | (pci_config.func.full_device_id.device_id as u32); let device = Arc::new(RefCell::new(unsafe { hda::IntelHDA::new(address, vend_prod).expect("ihdad: failed to allocate device") })); let socket_fd = syscall::open(":audiohw", syscall::O_RDWR | syscall::O_CREAT | syscall::O_NONBLOCK).expect("ihdad: failed to create hda scheme"); diff --git a/pcid/src/config.rs b/pcid/src/config.rs index 6c79e7eb63..df82e437d4 100644 --- a/pcid/src/config.rs +++ b/pcid/src/config.rs @@ -3,7 +3,7 @@ use std::ops::Range; use serde::Deserialize; -use crate::pci::PciHeader; +use crate::pci::FullDeviceId; #[derive(Clone, Debug, Default, Deserialize)] pub struct Config { @@ -24,21 +24,21 @@ pub struct DriverConfig { } impl DriverConfig { - pub fn match_function(&self, header: &PciHeader) -> bool { + pub fn match_function(&self, id: &FullDeviceId) -> bool { if let Some(class) = self.class { - if class != header.class().into() { + if class != id.class { return false; } } if let Some(subclass) = self.subclass { - if subclass != header.subclass() { + if subclass != id.subclass { return false; } } if let Some(interface) = self.interface { - if interface != header.interface() { + if interface != id.interface { return false; } } @@ -49,12 +49,12 @@ impl DriverConfig { let vendor_without_prefix = vendor.trim_start_matches("0x"); let vendor = i64::from_str_radix(vendor_without_prefix, 16).unwrap() as u16; - if vendor != header.vendor_id() { + if vendor != id.vendor_id { continue; } for device in devices { - if *device == header.device_id() { + if *device == id.device_id { device_found = true; break; } @@ -65,22 +65,20 @@ impl DriverConfig { } } else { if let Some(vendor) = self.vendor { - if vendor != header.vendor_id() { + if vendor != id.vendor_id { return false; } } if let Some(device) = self.device { - if device != header.device_id() { + if device != id.device_id { return false; } } } if let Some(ref device_id_range) = self.device_id_range { - if header.device_id() < device_id_range.start - || device_id_range.end <= header.device_id() - { + if id.device_id < device_id_range.start || device_id_range.end <= id.device_id { return false; } } diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index 1132423793..b7cb34a7f0 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -9,7 +9,7 @@ use thiserror::Error; pub use crate::pci::cap::Capability; pub use crate::pci::msi; -pub use crate::pci::{PciAddress, PciBar, PciHeader}; +pub use crate::pci::{FullDeviceId, PciAddress, PciBar, PciHeader}; pub mod irq_helpers; @@ -45,10 +45,8 @@ pub struct PciFunction { /// Legacy interrupt pin (INTx#), none if INTx# interrupts aren't supported at all. pub legacy_interrupt_pin: Option, - /// Vendor ID - pub venid: u16, - /// Device ID - pub devid: u16, + /// All identifying information of the PCI function. + pub full_device_id: FullDeviceId, } impl PciFunction { pub fn name(&self) -> String { diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 8db92b7cbf..f8725429e2 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -265,7 +265,7 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he info!("{}", string); for driver in config.drivers.iter() { - if !driver.match_function(&header) { + if !driver.match_function(header.full_device_id()) { continue; } @@ -362,10 +362,9 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he bars, bar_sizes, addr, - devid: header.device_id(), legacy_interrupt_line: irq, legacy_interrupt_pin, - venid: header.vendor_id(), + full_device_id: header.full_device_id().clone(), }; let subdriver_args = driver_interface::SubdriverArguments { diff --git a/pcid/src/pci/header.rs b/pcid/src/pci/header.rs index 2c7b0556ff..7f5a6adafa 100644 --- a/pcid/src/pci/header.rs +++ b/pcid/src/pci/header.rs @@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize}; use super::bar::PciBar; use super::class::PciClass; use super::func::ConfigReader; +use super::id::FullDeviceId; #[derive(Debug, PartialEq)] pub enum PciHeaderError { @@ -31,14 +32,9 @@ bitflags! { #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] pub struct SharedPciHeader { - vendor_id: u16, - device_id: u16, + full_device_id: FullDeviceId, command: u16, status: u16, - revision: u8, - interface: u8, - subclass: u8, - class: PciClass, cache_line_size: u8, latency_timer: u8, header_type: PciHeaderType, @@ -127,20 +123,22 @@ impl PciHeader { let revision = bytes[8]; let interface = bytes[9]; let subclass = bytes[10]; - let class = PciClass::from(bytes[11]); + let class = bytes[11]; let cache_line_size = bytes[12]; let latency_timer = bytes[13]; let header_type = PciHeaderType::from_bits_truncate(bytes[14]); let bist = bytes[15]; let shared = SharedPciHeader { + full_device_id: FullDeviceId { vendor_id, device_id, + class, + subclass, + interface, + revision, + }, command, status, - revision, - interface, - subclass, - class, cache_line_size, latency_timer, header_type, @@ -245,88 +243,56 @@ impl PciHeader { } } - /// Return the Vendor ID field. - pub fn vendor_id(&self) -> u16 { + /// Return all identifying information of the PCI function. + pub fn full_device_id(&self) -> &FullDeviceId { match self { - &PciHeader::General { - shared: SharedPciHeader { vendor_id, .. }, + PciHeader::General { + shared: + SharedPciHeader { + full_device_id: device_id, + .. + }, .. } - | &PciHeader::PciToPci { - shared: SharedPciHeader { vendor_id, .. }, - .. - } => vendor_id, - } - } - - /// Return the Device ID field. - pub fn device_id(&self) -> u16 { - match self { - &PciHeader::General { - shared: SharedPciHeader { device_id, .. }, - .. - } - | &PciHeader::PciToPci { - shared: SharedPciHeader { device_id, .. }, + | PciHeader::PciToPci { + shared: + SharedPciHeader { + full_device_id: device_id, + .. + }, .. } => device_id, } } + /// Return the Vendor ID field. + pub fn vendor_id(&self) -> u16 { + self.full_device_id().vendor_id + } + + /// Return the Device ID field. + pub fn device_id(&self) -> u16 { + self.full_device_id().device_id + } + /// Return the Revision field. pub fn revision(&self) -> u8 { - match self { - &PciHeader::General { - shared: SharedPciHeader { revision, .. }, - .. - } - | &PciHeader::PciToPci { - shared: SharedPciHeader { revision, .. }, - .. - } => revision, - } + self.full_device_id().revision } /// Return the Interface field. pub fn interface(&self) -> u8 { - match self { - &PciHeader::General { - shared: SharedPciHeader { interface, .. }, - .. - } - | &PciHeader::PciToPci { - shared: SharedPciHeader { interface, .. }, - .. - } => interface, - } + self.full_device_id().interface } /// Return the Subclass field. pub fn subclass(&self) -> u8 { - match self { - &PciHeader::General { - shared: SharedPciHeader { subclass, .. }, - .. - } - | &PciHeader::PciToPci { - shared: SharedPciHeader { subclass, .. }, - .. - } => subclass, - } + self.full_device_id().subclass } /// Return the Class field. pub fn class(&self) -> PciClass { - match self { - &PciHeader::General { - shared: SharedPciHeader { class, .. }, - .. - } - | &PciHeader::PciToPci { - shared: SharedPciHeader { class, .. }, - .. - } => class, - } + PciClass::from(self.full_device_id().class) } /// Return the Headers BARs. diff --git a/pcid/src/pci/id.rs b/pcid/src/pci/id.rs new file mode 100644 index 0000000000..9a59d660f6 --- /dev/null +++ b/pcid/src/pci/id.rs @@ -0,0 +1,12 @@ +use serde::{Deserialize, Serialize}; + +/// All identifying information of a PCI function. +#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] +pub struct FullDeviceId { + pub vendor_id: u16, + pub device_id: u16, + pub class: u8, + pub subclass: u8, + pub interface: u8, + pub revision: u8, +} diff --git a/pcid/src/pci/mod.rs b/pcid/src/pci/mod.rs index 6d39cbdb8f..264a3f35dd 100644 --- a/pcid/src/pci/mod.rs +++ b/pcid/src/pci/mod.rs @@ -7,12 +7,14 @@ pub use self::bar::PciBar; pub use self::class::PciClass; pub use self::func::PciFunc; pub use self::header::{PciHeader, PciHeaderError, PciHeaderType}; +pub use self::id::FullDeviceId; mod bar; pub mod cap; mod class; pub mod func; pub mod header; +mod id; pub mod msi; pub trait CfgAccess { diff --git a/virtio-blkd/src/main.rs b/virtio-blkd/src/main.rs index e280f62b6a..c6b3727a16 100644 --- a/virtio-blkd/src/main.rs +++ b/virtio-blkd/src/main.rs @@ -116,7 +116,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { // 0x1001 - virtio-blk let pci_config = pcid_handle.fetch_config()?; - assert_eq!(pci_config.func.devid, 0x1001); + assert_eq!(pci_config.func.full_device_id.device_id, 0x1001); log::info!("virtio-blk: initiating startup sequence :^)"); let device = virtio_core::probe_device(&mut pcid_handle)?; diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index 157ffa0f1f..c0c5ec3e1f 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -64,7 +64,7 @@ pub fn probe_device(pcid_handle: &mut PcidServerHandle) -> Result let pci_header = pcid_handle.fetch_header()?; assert_eq!( - pci_config.func.venid, 6900, + pci_config.func.full_device_id.vendor_id, 6900, "virtio_core::probe_device: not a virtio device" ); diff --git a/virtio-gpud/src/main.rs b/virtio-gpud/src/main.rs index 4d16d87a97..8a80510fbc 100644 --- a/virtio-gpud/src/main.rs +++ b/virtio-gpud/src/main.rs @@ -416,7 +416,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { // 0x1050 - virtio-gpu let pci_config = pcid_handle.fetch_config()?; - assert_eq!(pci_config.func.devid, 0x1050); + assert_eq!(pci_config.func.full_device_id.device_id, 0x1050); log::info!("virtio-gpu: initiating startup sequence :^)"); let device = DEVICE.try_call_once(|| virtio_core::probe_device(&mut pcid_handle))?; diff --git a/virtio-netd/src/main.rs b/virtio-netd/src/main.rs index c99b2c4577..c3113ed753 100644 --- a/virtio-netd/src/main.rs +++ b/virtio-netd/src/main.rs @@ -38,7 +38,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> Result<(), Error> { // 0x1000 - virtio-net let pci_config = pcid_handle.fetch_config()?; - assert_eq!(pci_config.func.devid, 0x1000); + assert_eq!(pci_config.func.full_device_id.device_id, 0x1000); log::info!("virtio-net: initiating startup sequence :^)"); let device = virtio_core::probe_device(&mut pcid_handle)?; From e5772c132bc514f7c2a7b5f09c6419b8b5737d28 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 21 Jan 2024 16:20:40 +0100 Subject: [PATCH 5/7] Remove a lot of fields from PciHeader which are unlikely to be used Some of these are removed with PCIe while many others only need to be used by the firmware to initialize the PCI bridges. If we ever need them anyway, we can always add them back, but for now it improves readability. --- pcid/src/pci/header.rs | 76 +++--------------------------------------- 1 file changed, 5 insertions(+), 71 deletions(-) diff --git a/pcid/src/pci/header.rs b/pcid/src/pci/header.rs index 7f5a6adafa..00de49ed30 100644 --- a/pcid/src/pci/header.rs +++ b/pcid/src/pci/header.rs @@ -33,12 +33,9 @@ bitflags! { #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] pub struct SharedPciHeader { full_device_id: FullDeviceId, - command: u16, - status: u16, - cache_line_size: u8, - latency_timer: u8, - header_type: PciHeaderType, - bist: u8, + command: u16, + status: u16, + header_type: PciHeaderType, } #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] @@ -46,36 +43,17 @@ pub enum PciHeader { General { shared: SharedPciHeader, bars: [PciBar; 6], - cardbus_cis_ptr: u32, subsystem_vendor_id: u16, subsystem_id: u16, - expansion_rom_bar: u32, cap_pointer: u8, interrupt_line: u8, interrupt_pin: u8, - min_grant: u8, - max_latency: u8, }, PciToPci { shared: SharedPciHeader, bars: [PciBar; 2], - primary_bus_num: u8, secondary_bus_num: u8, - subordinate_bus_num: u8, - secondary_latency_timer: u8, - io_base: u8, - io_limit: u8, - secondary_status: u16, - mem_base: u16, - mem_limit: u16, - prefetch_base: u16, - prefetch_limit: u16, - prefetch_base_upper: u32, - prefetch_limit_upper: u32, - io_base_upper: u16, - io_limit_upper: u16, cap_pointer: u8, - expansion_rom: u32, interrupt_line: u8, interrupt_pin: u8, bridge_control: u16, @@ -124,14 +102,11 @@ impl PciHeader { let interface = bytes[9]; let subclass = bytes[10]; let class = bytes[11]; - let cache_line_size = bytes[12]; - let latency_timer = bytes[13]; let header_type = PciHeaderType::from_bits_truncate(bytes[14]); - let bist = bytes[15]; let shared = SharedPciHeader { full_device_id: FullDeviceId { - vendor_id, - device_id, + vendor_id, + device_id, class, subclass, interface, @@ -139,10 +114,7 @@ impl PciHeader { }, command, status, - cache_line_size, - latency_timer, header_type, - bist, }; match header_type & PciHeaderType::HEADER_TYPE { @@ -150,73 +122,35 @@ impl PciHeader { let bytes = unsafe { reader.read_range(16, 48) }; let mut bars = [PciBar::None; 6]; Self::get_bars(&bytes, &mut bars); - let cardbus_cis_ptr = LittleEndian::read_u32(&bytes[24..28]); let subsystem_vendor_id = LittleEndian::read_u16(&bytes[28..30]); let subsystem_id = LittleEndian::read_u16(&bytes[30..32]); - let expansion_rom_bar = LittleEndian::read_u32(&bytes[32..36]); let cap_pointer = bytes[36]; let interrupt_line = bytes[44]; let interrupt_pin = bytes[45]; - let min_grant = bytes[46]; - let max_latency = bytes[47]; Ok(PciHeader::General { shared, bars, - cardbus_cis_ptr, subsystem_vendor_id, subsystem_id, - expansion_rom_bar, cap_pointer, interrupt_line, interrupt_pin, - min_grant, - max_latency, }) } PciHeaderType::PCITOPCI => { let bytes = unsafe { reader.read_range(16, 48) }; let mut bars = [PciBar::None; 2]; Self::get_bars(&bytes, &mut bars); - let primary_bus_num = bytes[8]; let secondary_bus_num = bytes[9]; - let subordinate_bus_num = bytes[10]; - let secondary_latency_timer = bytes[11]; - let io_base = bytes[12]; - let io_limit = bytes[13]; - let secondary_status = LittleEndian::read_u16(&bytes[14..16]); - let mem_base = LittleEndian::read_u16(&bytes[16..18]); - let mem_limit = LittleEndian::read_u16(&bytes[18..20]); - let prefetch_base = LittleEndian::read_u16(&bytes[20..22]); - let prefetch_limit = LittleEndian::read_u16(&bytes[22..24]); - let prefetch_base_upper = LittleEndian::read_u32(&bytes[24..28]); - let prefetch_limit_upper = LittleEndian::read_u32(&bytes[28..32]); - let io_base_upper = LittleEndian::read_u16(&bytes[32..34]); - let io_limit_upper = LittleEndian::read_u16(&bytes[34..36]); let cap_pointer = bytes[36]; - let expansion_rom = LittleEndian::read_u32(&bytes[40..44]); let interrupt_line = bytes[44]; let interrupt_pin = bytes[45]; let bridge_control = LittleEndian::read_u16(&bytes[46..48]); Ok(PciHeader::PciToPci { shared, bars, - primary_bus_num, secondary_bus_num, - subordinate_bus_num, - secondary_latency_timer, - io_base, - io_limit, - secondary_status, - mem_base, - mem_limit, - prefetch_base, - prefetch_limit, - prefetch_base_upper, - prefetch_limit_upper, - io_base_upper, - io_limit_upper, cap_pointer, - expansion_rom, interrupt_line, interrupt_pin, bridge_control, From d0e90f5ded685862fd16a1ab6a82cf2450e8d617 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 21 Jan 2024 19:35:39 +0100 Subject: [PATCH 6/7] Remove fetch_header from pcid_interface Most things are already stored in SubdriverArguments and the couple of things that aren't may change over time and thus should be read again each time they are accessed, while fetch_header would receive a fixed copy from pcid. --- ided/src/main.rs | 7 +++---- pcid/src/driver_interface/mod.rs | 11 +---------- pcid/src/main.rs | 7 ------- virtio-core/src/arch/x86_64.rs | 4 ++-- virtio-core/src/probe.rs | 5 ++--- 5 files changed, 8 insertions(+), 26 deletions(-) diff --git a/ided/src/main.rs b/ided/src/main.rs index f8e42c5deb..603a20baab 100644 --- a/ided/src/main.rs +++ b/ided/src/main.rs @@ -86,17 +86,16 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { info!("IDE PCI CONFIG: {:?}", pci_config); - let pci_header = pcid_handle.fetch_header().expect("ided: failed to fetch PCI header"); - let busmaster_base = match pci_header.get_bar(4) { + let busmaster_base = match pci_config.func.bars[4] { PciBar::Port(port) => port, other => panic!("TODO: IDE busmaster BAR {:#x?}", other), }; - let (primary, primary_irq) = if pci_header.interface() & 1 != 0 { + let (primary, primary_irq) = if pci_config.func.full_device_id.interface & 1 != 0 { panic!("TODO: IDE primary channel is PCI native"); } else { (Channel::primary_compat(busmaster_base).unwrap(), 14) }; - let (secondary, secondary_irq) = if pci_header.interface() & 1 != 0 { + let (secondary, secondary_irq) = if pci_config.func.full_device_id.interface & 1 != 0 { panic!("TODO: IDE secondary channel is PCI native"); } else { (Channel::secondary_compat(busmaster_base + 8).unwrap(), 15) diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index b7cb34a7f0..6357ec5a92 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -9,7 +9,7 @@ use thiserror::Error; pub use crate::pci::cap::Capability; pub use crate::pci::msi; -pub use crate::pci::{FullDeviceId, PciAddress, PciBar, PciHeader}; +pub use crate::pci::{FullDeviceId, PciAddress, PciBar}; pub mod irq_helpers; @@ -163,7 +163,6 @@ pub enum SetFeatureInfo { #[non_exhaustive] pub enum PcidClientRequest { RequestConfig, - RequestHeader, RequestFeatures, RequestCapabilities, EnableFeature(PciFeature), @@ -186,7 +185,6 @@ pub enum PcidServerResponseError { pub enum PcidClientResponse { Capabilities(Vec), Config(SubdriverArguments), - Header(PciHeader), AllFeatures(Vec<(PciFeature, FeatureStatus)>), FeatureEnabled(PciFeature), FeatureStatus(PciFeature, FeatureStatus), @@ -264,13 +262,6 @@ impl PcidServerHandle { } } - pub fn fetch_header(&mut self) -> Result { - self.send(&PcidClientRequest::RequestHeader)?; - match self.recv()? { - PcidClientResponse::Header(a) => Ok(a), - other => Err(PcidClientHandleError::InvalidResponse(other)), - } - } pub fn fetch_all_features(&mut self) -> Result> { self.send(&PcidClientRequest::RequestFeatures)?; match self.recv()? { diff --git a/pcid/src/main.rs b/pcid/src/main.rs index f8725429e2..74c3f80688 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -33,9 +33,7 @@ struct Args { } pub struct DriverHandler { - config: config::DriverConfig, addr: PciAddress, - header: PciHeader, capabilities: Vec<(u8, PciCapability)>, state: Arc, @@ -59,9 +57,6 @@ impl DriverHandler { PcidClientRequest::RequestConfig => { PcidClientResponse::Config(args.clone()) } - PcidClientRequest::RequestHeader => { - PcidClientResponse::Header(self.header.clone()) - } PcidClientRequest::RequestFeatures => { PcidClientResponse::AllFeatures(self.capabilities.iter().filter_map(|(_, capability)| match capability { PciCapability::Msi(msi) => Some((PciFeature::Msi, FeatureStatus::enabled(msi.enabled()))), @@ -409,8 +404,6 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he Ok(mut child) => { let driver_handler = DriverHandler { addr, - config: driver.clone(), - header, state: Arc::clone(&state), capabilities, }; diff --git a/virtio-core/src/arch/x86_64.rs b/virtio-core/src/arch/x86_64.rs index e8200da934..502bfcdafa 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -96,10 +96,10 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { } pub fn probe_legacy_port_transport( - pci_header: &PciHeader, + pci_config: &SubdriverArguments, pcid_handle: &mut PcidServerHandle, ) -> Result { - if let PciBar::Port(port) = pci_header.get_bar(0) { + if let PciBar::Port(port) = pci_config.func.bars[0] { unsafe { syscall::iopl(3).expect("virtio: failed to set I/O privilege level") }; log::warn!("virtio: using legacy transport"); diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index c0c5ec3e1f..292bf429a6 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -61,7 +61,6 @@ fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { /// This function panics if the device is not a virtio device. pub fn probe_device(pcid_handle: &mut PcidServerHandle) -> Result { let pci_config = pcid_handle.fetch_config()?; - let pci_header = pcid_handle.fetch_header()?; assert_eq!( pci_config.func.full_device_id.vendor_id, 6900, @@ -91,7 +90,7 @@ pub fn probe_device(pcid_handle: &mut PcidServerHandle) -> Result _ => continue, } - let bar = pci_header.get_bar(capability.bar as usize); + let bar = pci_config.func.bars[capability.bar as usize]; let addr = match bar { PciBar::Memory32(addr) => addr as usize, PciBar::Memory64(addr) => addr as usize, @@ -189,7 +188,7 @@ pub fn probe_device(pcid_handle: &mut PcidServerHandle) -> Result Ok(device) } else { - crate::arch::probe_legacy_port_transport(&pci_header, pcid_handle) + crate::arch::probe_legacy_port_transport(&pci_config, pcid_handle) } } From be0e6b9dd7e8ce6f05755a9976e3592c951ff31e Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 21 Jan 2024 20:46:00 +0100 Subject: [PATCH 7/7] Pass self instead of &self to PciFeature methods PciFeature is Copy, so passing it behind a reference is not necessary. --- pcid/src/driver_interface/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index 6357ec5a92..4f7fd64703 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -85,11 +85,11 @@ pub enum PciFeature { MsiX, } impl PciFeature { - pub fn is_msi(&self) -> bool { - if let &Self::Msi = self { true } else { false } + pub fn is_msi(self) -> bool { + if let Self::Msi = self { true } else { false } } - pub fn is_msix(&self) -> bool { - if let &Self::MsiX = self { true } else { false } + pub fn is_msix(self) -> bool { + if let Self::MsiX = self { true } else { false } } } #[derive(Debug, Serialize, Deserialize)]