From ba6b91d561802ed7275ea825ba86718cb0f0a9d3 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 16:46:59 +0200 Subject: [PATCH 1/8] pcid: Move print_pci_function to a PciHeader method --- pcid/src/main.rs | 28 +--------------------------- pcid/src/pci_header.rs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 9e674fd437..08f8eb09a6 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -5,7 +5,6 @@ use std::process::Command; use std::thread; use std::sync::{Arc, Mutex}; -use pci_types::device_type::DeviceType; use pci_types::{CommandRegister, ConfigRegionAccess, PciAddress}; use structopt::StructOpt; use log::{debug, error, info, warn, trace}; @@ -209,31 +208,6 @@ pub struct State { pcie: Pcie, } -fn print_pci_function(header: &PciHeader) { - let mut string = format!("PCI {} {:>04X}:{:>04X} {:>02X}.{:>02X}.{:>02X}.{:>02X} {:?}", - header.address(), header.vendor_id(), header.device_id(), header.class(), - header.subclass(), header.interface(), header.revision(), header.class()); - let device_type = DeviceType::from((header.class(), header.subclass())); - match device_type { - DeviceType::LegacyVgaCompatible => string.push_str(" VGA CTL"), - DeviceType::IdeController => string.push_str(" IDE"), - DeviceType::SataController => match header.interface() { - 0 => string.push_str(" SATA VND"), - 1 => string.push_str(" SATA AHCI"), - _ => (), - }, - DeviceType::UsbController => match header.interface() { - 0x00 => string.push_str(" UHCI"), - 0x10 => string.push_str(" OHCI"), - 0x20 => string.push_str(" EHCI"), - 0x30 => string.push_str(" XHCI"), - _ => (), - }, - _ => (), - } - info!("{}", string); -} - fn handle_parsed_header(state: Arc, config: &Config, header: PciEndpointHeader) { for driver in config.drivers.iter() { if !driver.match_function(header.full_device_id()) { @@ -470,7 +444,7 @@ fn main(args: Args) { let func_addr = PciAddress::new(0, bus_num, dev_num, func_num); match PciHeader::from_reader(&state.pcie, func_addr) { Ok(header) => { - print_pci_function(&header); + info!("{}", header.display()); match header { PciHeader::General(endpoint_header) => { handle_parsed_header(Arc::clone(&state), &config, endpoint_header); diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index 1b884b7849..343d66b466 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -1,5 +1,6 @@ use std::convert::TryInto; +use pci_types::device_type::DeviceType; use pci_types::{ Bar as TyBar, ConfigRegionAccess, EndpointHeader, HeaderType, PciAddress, PciHeader as TyPciHeader, PciPciBridgeHeader, @@ -149,6 +150,40 @@ impl PciHeader { pub fn class(&self) -> u8 { self.full_device_id().class } + + /// Format a human readable string indicating the address and type of PCI device. + pub fn display(&self) -> String { + let mut string = format!( + "PCI {} {:>04X}:{:>04X} {:>02X}.{:>02X}.{:>02X}.{:>02X} {:?}", + self.address(), + self.vendor_id(), + self.device_id(), + self.class(), + self.subclass(), + self.interface(), + self.revision(), + self.class() + ); + let device_type = DeviceType::from((self.class(), self.subclass())); + match device_type { + DeviceType::LegacyVgaCompatible => string.push_str(" VGA CTL"), + DeviceType::IdeController => string.push_str(" IDE"), + DeviceType::SataController => match self.interface() { + 0 => string.push_str(" SATA VND"), + 1 => string.push_str(" SATA AHCI"), + _ => (), + }, + DeviceType::UsbController => match self.interface() { + 0x00 => string.push_str(" UHCI"), + 0x10 => string.push_str(" OHCI"), + 0x20 => string.push_str(" EHCI"), + 0x30 => string.push_str(" XHCI"), + _ => (), + }, + _ => (), + } + string + } } impl PciEndpointHeader { From 7f5a340c83e6453dae3673858ec05bc0ed4b5bf3 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 18:06:33 +0200 Subject: [PATCH 2/8] pcid: Remove cap_pointer from PciHeader::PciToPci It is never used. --- pcid/src/pci_header.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index 343d66b466..af5fcee75d 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -34,7 +34,6 @@ pub enum PciHeader { PciToPci { shared: SharedPciHeader, secondary_bus_num: u8, - cap_pointer: u8, }, } @@ -80,11 +79,9 @@ impl PciHeader { HeaderType::PciPciBridge => { let bridge_header = PciPciBridgeHeader::from_header(header, access).unwrap(); let secondary_bus_num = bridge_header.secondary_bus_number(access); - let cap_pointer = (unsafe { access.read(addr, 0x34) } & 0xff) as u8; Ok(PciHeader::PciToPci { shared, secondary_bus_num, - cap_pointer, }) } ty => Err(PciHeaderError::UnknownHeaderType(ty)), From 1593884938bf27b1a582104f3be66f393c0056a5 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 18:12:59 +0200 Subject: [PATCH 3/8] pcid: Use capability_pointer method instead of a raw read --- pcid/src/pci_header.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index af5fcee75d..471f74b27c 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -1,5 +1,3 @@ -use std::convert::TryInto; - use pci_types::device_type::DeviceType; use pci_types::{ Bar as TyBar, ConfigRegionAccess, EndpointHeader, HeaderType, PciAddress, @@ -68,7 +66,10 @@ impl PciHeader { HeaderType::Endpoint => { let endpoint_header = EndpointHeader::from_header(header, access).unwrap(); let (subsystem_id, subsystem_vendor_id) = endpoint_header.subsystem(access); - let cap_pointer = (unsafe { access.read(addr, 0x34) } & 0xff) as u8; + let cap_pointer = endpoint_header + .capability_pointer(access) + .try_into() + .unwrap(); Ok(PciHeader::General(PciEndpointHeader { shared, subsystem_vendor_id, From 9d37d15827e7ef1fe3ffdc847cdcd8ca2351be89 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 18:14:18 +0200 Subject: [PATCH 4/8] pcid: Avoid duplicate read of vendor and device id --- pcid/src/pci_header.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index 471f74b27c..95c960851a 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -42,12 +42,13 @@ impl PciHeader { access: &impl ConfigRegionAccess, addr: PciAddress, ) -> Result { - if unsafe { access.read(addr, 0) } == 0xffffffff { + let header = TyPciHeader::new(addr); + let (vendor_id, device_id) = header.id(access); + + if vendor_id == 0xffff && device_id == 0xffff { return Err(PciHeaderError::NoDevice); } - let header = TyPciHeader::new(addr); - let (vendor_id, device_id) = header.id(access); let (revision, class, subclass, interface) = header.revision_and_class(access); let header_type = header.header_type(access); let shared = SharedPciHeader { From 9aed7d560e8f760452f93141c217408dc0f4c682 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 18:21:50 +0200 Subject: [PATCH 5/8] pcid: Merge CapabilityOffsetsIter into CapabilitiesIter --- pcid/src/main.rs | 2 +- pcid/src/pci/cap.rs | 36 +++++++++++++----------------------- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 08f8eb09a6..7e852bb982 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -267,7 +267,7 @@ fn handle_parsed_header(state: Arc, config: &Config, header: PciEndpointH pci: &state.pcie, addr: header.address(), }; - crate::pci::cap::CapabilitiesIter { inner: crate::pci::cap::CapabilityOffsetsIter::new(header.cap_pointer(), &func) }.collect::>() + crate::pci::cap::CapabilitiesIter::new(header.cap_pointer(), &func).collect::>() } else { Vec::new() }; diff --git a/pcid/src/pci/cap.rs b/pcid/src/pci/cap.rs index 5e852a528f..41e480a71c 100644 --- a/pcid/src/pci/cap.rs +++ b/pcid/src/pci/cap.rs @@ -1,11 +1,11 @@ use super::func::ConfigReader; use serde::{Serialize, Deserialize}; -pub struct CapabilityOffsetsIter<'a, R> { +pub struct CapabilitiesIter<'a, R> { offset: u8, reader: &'a R, } -impl<'a, R> CapabilityOffsetsIter<'a, R> { +impl<'a, R> CapabilitiesIter<'a, R> { pub fn new(offset: u8, reader: &'a R) -> Self { Self { offset, @@ -13,14 +13,14 @@ impl<'a, R> CapabilityOffsetsIter<'a, R> { } } } -impl<'a, R> Iterator for CapabilityOffsetsIter<'a, R> +impl<'a, R> Iterator for CapabilitiesIter<'a, R> where R: ConfigReader { - type Item = u8; + type Item = (u8, Capability); fn next(&mut self) -> Option { - unsafe { + let offset = unsafe { // mask RsvdP bits self.offset = self.offset & 0xFC; @@ -32,8 +32,14 @@ where let offset = self.offset; self.offset = next; - Some(offset) - } + offset + }; + + let cap = unsafe { + Capability::parse(self.reader, offset) + }; + + Some((offset, cap)) } } @@ -229,19 +235,3 @@ impl Capability { } } } - -pub struct CapabilitiesIter<'a, R> { - pub inner: CapabilityOffsetsIter<'a, R>, -} - -impl<'a, R> Iterator for CapabilitiesIter<'a, R> -where - R: ConfigReader -{ - type Item = (u8, Capability); - - fn next(&mut self) -> Option { - let offset = self.inner.next()?; - Some((offset, unsafe { Capability::parse(self.inner.reader, offset) })) - } -} From d70cf080bb8e02fa867549083e73fd8e4a9fac6e Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 18:40:03 +0200 Subject: [PATCH 6/8] pcid: Remove with_pci_func_raw --- pcid/src/main.rs | 45 +++++++++++++++------------------------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 7e852bb982..18a64f2060 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -5,7 +5,7 @@ use std::process::Command; use std::thread; use std::sync::{Arc, Mutex}; -use pci_types::{CommandRegister, ConfigRegionAccess, PciAddress}; +use pci_types::{CommandRegister, PciAddress}; use structopt::StructOpt; use log::{debug, error, info, warn, trace}; use redox_log::{OutputBuilder, RedoxLogger}; @@ -42,18 +42,17 @@ pub struct DriverHandler { state: Arc, } -fn with_pci_func_raw T>(pci: &dyn ConfigRegionAccess, addr: PciAddress, function: F) -> T { - let func = PciFunc { - pci, - addr, - }; - function(&func) -} + impl DriverHandler { fn respond(&mut self, request: driver_interface::PcidClientRequest, args: &driver_interface::SubdriverArguments) -> driver_interface::PcidClientResponse { use driver_interface::*; use crate::pci::cap::{MsiCapability, MsixCapability}; + let func = PciFunc { + pci: &self.state.pcie, + addr: self.addr, + }; + match request { PcidClientRequest::RequestCapabilities => { PcidClientResponse::Capabilities(self.capabilities.iter().map(|(_, capability)| capability.clone()).collect::>()) @@ -75,10 +74,8 @@ impl DriverHandler { None => return PcidClientResponse::Error(PcidServerResponseError::NonexistentFeature(feature)), }; unsafe { - with_pci_func_raw(&self.state.pcie, self.addr, |func| { - capability.set_enabled(true); - capability.write_message_control(func, offset); - }); + capability.set_enabled(true); + capability.write_message_control(&func, offset); } PcidClientResponse::FeatureEnabled(feature) } @@ -88,10 +85,8 @@ impl DriverHandler { None => return PcidClientResponse::Error(PcidServerResponseError::NonexistentFeature(feature)), }; unsafe { - with_pci_func_raw(&self.state.pcie, self.addr, |func| { - capability.set_msix_enabled(true); - capability.write_a(func, offset); - }); + capability.set_msix_enabled(true); + capability.write_a(&func, offset); } PcidClientResponse::FeatureEnabled(feature) } @@ -150,9 +145,7 @@ impl DriverHandler { info.set_mask_bits(mask_bits); } unsafe { - with_pci_func_raw(&self.state.pcie, self.addr, |func| { - info.write_all(func, offset); - }); + info.write_all(&func, offset); } PcidClientResponse::SetFeatureInfo(PciFeature::Msi) } else { @@ -162,9 +155,7 @@ impl DriverHandler { if let Some(mask) = function_mask { info.set_function_mask(mask); unsafe { - with_pci_func_raw(&self.state.pcie, self.addr, |func| { - info.write_a(func, offset); - }); + info.write_a(&func, offset); } } PcidClientResponse::SetFeatureInfo(PciFeature::MsiX) @@ -173,18 +164,12 @@ impl DriverHandler { } } PcidClientRequest::ReadConfig(offset) => { - let value = unsafe { - with_pci_func_raw(&self.state.pcie, self.addr, |func| { - func.read_u32(offset) - }) - }; + let value = unsafe { func.read_u32(offset) }; return PcidClientResponse::ReadConfig(value); }, PcidClientRequest::WriteConfig(offset, value) => { unsafe { - with_pci_func_raw(&self.state.pcie, self.addr, |func| { - func.write_u32(offset, value); - }); + func.write_u32(offset, value); } return PcidClientResponse::WriteConfig; } From b04915673d957f9aaadf68b83ee60c88bb55231e Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 18:51:55 +0200 Subject: [PATCH 7/8] pcid: Remove ConfigReader and ConfigWriter traits Only PciFunc implements this trait. --- pcid/src/main.rs | 1 - pcid/src/pci/cap.rs | 96 ++++++++++++++++++++++---------------------- pcid/src/pci/func.rs | 29 +++++-------- pcid/src/pci/msi.rs | 78 +++++++++++++++++------------------ 4 files changed, 97 insertions(+), 107 deletions(-) diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 18a64f2060..f5d146632f 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -15,7 +15,6 @@ use crate::config::Config; use crate::driver_interface::LegacyInterruptLine; use crate::pci::PciFunc; use crate::pci::cap::Capability as PciCapability; -use crate::pci::func::{ConfigReader, ConfigWriter}; use crate::pci_header::{PciEndpointHeader, PciHeader, PciHeaderError}; mod cfg_access; diff --git a/pcid/src/pci/cap.rs b/pcid/src/pci/cap.rs index 41e480a71c..729845ef84 100644 --- a/pcid/src/pci/cap.rs +++ b/pcid/src/pci/cap.rs @@ -1,22 +1,20 @@ -use super::func::ConfigReader; +use super::func::PciFunc; + use serde::{Serialize, Deserialize}; -pub struct CapabilitiesIter<'a, R> { +pub struct CapabilitiesIter<'a> { offset: u8, - reader: &'a R, + func: &'a PciFunc<'a>, } -impl<'a, R> CapabilitiesIter<'a, R> { - pub fn new(offset: u8, reader: &'a R) -> Self { +impl<'a> CapabilitiesIter<'a> { + pub fn new(offset: u8, func: &'a PciFunc) -> Self { Self { offset, - reader, + func, } } } -impl<'a, R> Iterator for CapabilitiesIter<'a, R> -where - R: ConfigReader -{ +impl<'a> Iterator for CapabilitiesIter<'a> { type Item = (u8, Capability); fn next(&mut self) -> Option { @@ -26,7 +24,7 @@ where if self.offset == 0 { return None }; - let first_dword = self.reader.read_u32(u16::from(self.offset)); + let first_dword = self.func.read_u32(u16::from(self.offset)); let next = ((first_dword >> 8) & 0xFF) as u8; let offset = self.offset; @@ -36,7 +34,7 @@ where }; let cap = unsafe { - Capability::parse(self.reader, offset) + Capability::parse(self.func, offset) }; Some((offset, cap)) @@ -159,28 +157,28 @@ impl Capability { _ => None, } } - unsafe fn parse_msi(reader: &R, offset: u8) -> Self { - Self::Msi(MsiCapability::parse(reader, offset)) + unsafe fn parse_msi(func: &PciFunc, offset: u8) -> Self { + Self::Msi(MsiCapability::parse(func, offset)) } - unsafe fn parse_msix(reader: &R, offset: u8) -> Self { + unsafe fn parse_msix(func: &PciFunc, offset: u8) -> Self { Self::MsiX(MsixCapability { - a: reader.read_u32(u16::from(offset)), - b: reader.read_u32(u16::from(offset + 4)), - c: reader.read_u32(u16::from(offset + 8)), + a: func.read_u32(u16::from(offset)), + b: func.read_u32(u16::from(offset + 4)), + c: func.read_u32(u16::from(offset + 8)), }) } - unsafe fn parse_pwr(reader: &R, offset: u8) -> Self { + unsafe fn parse_pwr(func: &PciFunc, offset: u8) -> Self { Self::PwrMgmt(PwrMgmtCapability { - a: reader.read_u32(u16::from(offset)), - b: reader.read_u32(u16::from(offset + 4)), + a: func.read_u32(u16::from(offset)), + b: func.read_u32(u16::from(offset + 4)), }) } - unsafe fn parse_vendor(reader: &R, offset: u8) -> Self { - let next = reader.read_u8(u16::from(offset+1)); - let length = reader.read_u8(u16::from(offset+2)); + unsafe fn parse_vendor(func: &PciFunc, offset: u8) -> Self { + let next = func.read_u8(u16::from(offset+1)); + let length = func.read_u8(u16::from(offset+2)); log::info!("Vendor specific offset: {offset:#02x} next: {next:#02x} cap len: {length:#02x}"); let data = if length > 0 { - let mut raw_data = reader.read_range(offset.into(), length.into()); + let mut raw_data = func.read_range(offset.into(), length.into()); raw_data.drain(3..).collect() } else { log::warn!("Vendor specific capability is invalid"); @@ -190,45 +188,45 @@ impl Capability { data }) } - unsafe fn parse_pcie(reader: &R, offset: u8) -> Self { + unsafe fn parse_pcie(func: &PciFunc, offset: u8) -> Self { let offset = u16::from(offset); Self::Pcie(PcieCapability { - pcie_caps: reader.read_u32(offset), - dev_caps: reader.read_u32(offset + 0x04), - dev_sts_ctl: reader.read_u32(offset + 0x08), - link_caps: reader.read_u32(offset + 0x0C), - link_sts_ctl: reader.read_u32(offset + 0x10), - slot_caps: reader.read_u32(offset + 0x14), - slot_sts_ctl: reader.read_u32(offset + 0x18), - root_cap_ctl: reader.read_u32(offset + 0x1C), - root_sts: reader.read_u32(offset + 0x20), - dev_caps2: reader.read_u32(offset + 0x24), - dev_sts_ctl2: reader.read_u32(offset + 0x28), - link_caps2: reader.read_u32(offset + 0x2C), - link_sts_ctl2: reader.read_u32(offset + 0x30), - slot_caps2: reader.read_u32(offset + 0x34), - slot_sts_ctl2: reader.read_u32(offset + 0x38), + pcie_caps: func.read_u32(offset), + dev_caps: func.read_u32(offset + 0x04), + dev_sts_ctl: func.read_u32(offset + 0x08), + link_caps: func.read_u32(offset + 0x0C), + link_sts_ctl: func.read_u32(offset + 0x10), + slot_caps: func.read_u32(offset + 0x14), + slot_sts_ctl: func.read_u32(offset + 0x18), + root_cap_ctl: func.read_u32(offset + 0x1C), + root_sts: func.read_u32(offset + 0x20), + dev_caps2: func.read_u32(offset + 0x24), + dev_sts_ctl2: func.read_u32(offset + 0x28), + link_caps2: func.read_u32(offset + 0x2C), + link_sts_ctl2: func.read_u32(offset + 0x30), + slot_caps2: func.read_u32(offset + 0x34), + slot_sts_ctl2: func.read_u32(offset + 0x38), }) } - unsafe fn parse(reader: &R, offset: u8) -> Self { + unsafe fn parse(func: &PciFunc, offset: u8) -> Self { assert_eq!(offset & 0xFC, offset, "capability must be dword aligned"); - let dword = reader.read_u32(u16::from(offset)); + let dword = func.read_u32(u16::from(offset)); let capability_id = (dword & 0xFF) as u8; if capability_id == CapabilityId::Msi as u8 { - Self::parse_msi(reader, offset) + Self::parse_msi(func, offset) } else if capability_id == CapabilityId::MsiX as u8 { - Self::parse_msix(reader, offset) + Self::parse_msix(func, offset) } else if capability_id == CapabilityId::Pcie as u8 { - Self::parse_pcie(reader, offset) + Self::parse_pcie(func, offset) } else if capability_id == CapabilityId::PwrMgmt as u8{ - Self::parse_pwr(reader, offset) + Self::parse_pwr(func, offset) } else if capability_id == CapabilityId::Vendor as u8 { - Self::parse_vendor(reader, offset) + Self::parse_vendor(func, offset) } else if capability_id == CapabilityId::Sata as u8 { - Self::FunctionSpecific(capability_id, reader.read_range(offset.into(), 8)) + Self::FunctionSpecific(capability_id, func.read_range(offset.into(), 8)) } else { log::warn!("unimplemented or malformed capability id: {}", capability_id); Self::Other(capability_id) diff --git a/pcid/src/pci/func.rs b/pcid/src/pci/func.rs index 7bdabff43d..e94e40818d 100644 --- a/pcid/src/pci/func.rs +++ b/pcid/src/pci/func.rs @@ -1,8 +1,13 @@ use byteorder::{ByteOrder, LittleEndian}; use pci_types::{ConfigRegionAccess, PciAddress}; -pub trait ConfigReader { - unsafe fn read_range(&self, offset: u16, len: u16) -> Vec { +pub struct PciFunc<'pci> { + pub pci: &'pci dyn ConfigRegionAccess, + pub addr: PciAddress, +} + +impl<'pci> PciFunc<'pci> { + pub unsafe fn read_range(&self, offset: u16, len: u16) -> Vec { assert!(len > 3 && len % 4 == 0, "invalid range length: {}", len); let mut ret = Vec::with_capacity(len as usize); let results = (offset..offset + len) @@ -17,32 +22,20 @@ pub trait ConfigReader { ret } - unsafe fn read_u32(&self, offset: u16) -> u32; - - unsafe fn read_u8(&self, offset: u16) -> u8 { + pub unsafe fn read_u8(&self, offset: u16) -> u8 { let dword_offset = (offset / 4) * 4; let dword = self.read_u32(dword_offset); let shift = (offset % 4) * 8; ((dword >> shift) & 0xFF) as u8 } -} -pub trait ConfigWriter { - unsafe fn write_u32(&self, offset: u16, value: u32); -} -pub struct PciFunc<'pci> { - pub pci: &'pci dyn ConfigRegionAccess, - pub addr: PciAddress, -} - -impl<'pci> ConfigReader for PciFunc<'pci> { - unsafe fn read_u32(&self, offset: u16) -> u32 { + pub unsafe fn read_u32(&self, offset: u16) -> u32 { self.pci.read(self.addr, offset) } } -impl<'pci> ConfigWriter for PciFunc<'pci> { - unsafe fn write_u32(&self, offset: u16, value: u32) { +impl<'pci> PciFunc<'pci> { + pub unsafe fn write_u32(&self, offset: u16, value: u32) { self.pci.write(self.addr, offset, value); } } diff --git a/pcid/src/pci/msi.rs b/pcid/src/pci/msi.rs index ce176a1846..8d6e8f4f83 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -2,7 +2,7 @@ use std::fmt; use super::bar::PciBar; pub use super::cap::{MsiCapability, MsixCapability}; -use super::func::{ConfigReader, ConfigWriter}; +use super::func::PciFunc; use serde::{Deserialize, Serialize}; use syscall::{Io, Mmio}; @@ -35,8 +35,8 @@ impl MsiCapability { pub const MC_MSI_ENABLED_BIT: u16 = 1; - pub unsafe fn parse(reader: &R, offset: u8) -> Self { - let dword = reader.read_u32(u16::from(offset)); + pub unsafe fn parse(func: &PciFunc, offset: u8) -> Self { + let dword = func.read_u32(u16::from(offset)); let message_control = (dword >> 16) as u16; @@ -44,34 +44,34 @@ impl MsiCapability { if message_control & Self::MC_64_BIT_ADDR_BIT != 0 { Self::_64BitAddressWithPvm { message_control: dword, - message_address_lo: reader.read_u32(u16::from(offset + 4)), - message_address_hi: reader.read_u32(u16::from(offset + 8)), - message_data: reader.read_u32(u16::from(offset + 12)), - mask_bits: reader.read_u32(u16::from(offset + 16)), - pending_bits: reader.read_u32(u16::from(offset + 20)), + message_address_lo: func.read_u32(u16::from(offset + 4)), + message_address_hi: func.read_u32(u16::from(offset + 8)), + message_data: func.read_u32(u16::from(offset + 12)), + mask_bits: func.read_u32(u16::from(offset + 16)), + pending_bits: func.read_u32(u16::from(offset + 20)), } } else { Self::_32BitAddressWithPvm { message_control: dword, - message_address: reader.read_u32(u16::from(offset + 4)), - message_data: reader.read_u32(u16::from(offset + 8)), - mask_bits: reader.read_u32(u16::from(offset + 12)), - pending_bits: reader.read_u32(u16::from(offset + 16)), + message_address: func.read_u32(u16::from(offset + 4)), + message_data: func.read_u32(u16::from(offset + 8)), + mask_bits: func.read_u32(u16::from(offset + 12)), + pending_bits: func.read_u32(u16::from(offset + 16)), } } } else { if message_control & Self::MC_64_BIT_ADDR_BIT != 0 { Self::_64BitAddress { message_control: dword, - message_address_lo: reader.read_u32(u16::from(offset + 4)), - message_address_hi: reader.read_u32(u16::from(offset + 8)), - message_data: reader.read_u32(u16::from(offset + 12)) as u16, + message_address_lo: func.read_u32(u16::from(offset + 4)), + message_address_hi: func.read_u32(u16::from(offset + 8)), + message_data: func.read_u32(u16::from(offset + 12)) as u16, } } else { Self::_32BitAddress { message_control: dword, - message_address: reader.read_u32(u16::from(offset + 4)), - message_data: reader.read_u32(u16::from(offset + 8)) as u16, + message_address: func.read_u32(u16::from(offset + 4)), + message_data: func.read_u32(u16::from(offset + 8)) as u16, } } } @@ -97,8 +97,8 @@ impl MsiCapability { | Self::_64BitAddressWithPvm { ref mut message_control, .. } => *message_control = new_message_control, } } - pub unsafe fn write_message_control(&self, writer: &W, offset: u8) { - writer.write_u32(u16::from(offset), self.message_control_raw()); + pub unsafe fn write_message_control(&self, func: &PciFunc, offset: u8) { + func.write_u32(u16::from(offset), self.message_control_raw()); } pub fn is_pvt_capable(&self) -> bool { self.message_control() & Self::MC_PVT_CAPABLE_BIT != 0 @@ -186,36 +186,36 @@ impl MsiCapability { } Some(()) } - pub unsafe fn write_message_address(&self, writer: &W, offset: u8) { - writer.write_u32(u16::from(offset) + 4, self.message_address()) + pub unsafe fn write_message_address(&self, func: &PciFunc, offset: u8) { + func.write_u32(u16::from(offset) + 4, self.message_address()) } - pub unsafe fn write_message_upper_address(&self, writer: &W, offset: u8) -> Option<()> { + pub unsafe fn write_message_upper_address(&self, func: &PciFunc, offset: u8) -> Option<()> { let value = self.message_upper_address()?; - writer.write_u32(u16::from(offset + 8), value); + func.write_u32(u16::from(offset + 8), value); Some(()) } - pub unsafe fn write_message_data(&self, writer: &W, offset: u8) { + pub unsafe fn write_message_data(&self, func: &PciFunc, offset: u8) { match self { - &Self::_32BitAddress { message_data, .. } => writer.write_u32(u16::from(offset + 8), message_data.into()), - &Self::_32BitAddressWithPvm { message_data, .. } => writer.write_u32(u16::from(offset + 8), message_data), - &Self::_64BitAddress { message_data, .. } => writer.write_u32(u16::from(offset + 12), message_data.into()), - &Self::_64BitAddressWithPvm { message_data, .. } => writer.write_u32(u16::from(offset + 12), message_data), + &Self::_32BitAddress { message_data, .. } => func.write_u32(u16::from(offset + 8), message_data.into()), + &Self::_32BitAddressWithPvm { message_data, .. } => func.write_u32(u16::from(offset + 8), message_data), + &Self::_64BitAddress { message_data, .. } => func.write_u32(u16::from(offset + 12), message_data.into()), + &Self::_64BitAddressWithPvm { message_data, .. } => func.write_u32(u16::from(offset + 12), message_data), } } - pub unsafe fn write_mask_bits(&self, writer: &W, offset: u8) -> Option<()> { + pub unsafe fn write_mask_bits(&self, func: &PciFunc, offset: u8) -> Option<()> { match self { - &Self::_32BitAddressWithPvm { mask_bits, .. } => writer.write_u32(u16::from(offset + 12), mask_bits), - &Self::_64BitAddressWithPvm { mask_bits, .. } => writer.write_u32(u16::from(offset + 16), mask_bits), + &Self::_32BitAddressWithPvm { mask_bits, .. } => func.write_u32(u16::from(offset + 12), mask_bits), + &Self::_64BitAddressWithPvm { mask_bits, .. } => func.write_u32(u16::from(offset + 16), mask_bits), &Self::_32BitAddress { .. } | &Self::_64BitAddress { .. } => return None, } Some(()) } - pub unsafe fn write_all(&self, writer: &W, offset: u8) { - self.write_message_control(writer, offset); - self.write_message_address(writer, offset); - self.write_message_upper_address(writer, offset); - self.write_message_data(writer, offset); - self.write_mask_bits(writer, offset); + pub unsafe fn write_all(&self, func: &PciFunc, offset: u8) { + self.write_message_control(func, offset); + self.write_message_address(func, offset); + self.write_message_upper_address(func, offset); + self.write_message_data(func, offset); + self.write_mask_bits(func, offset); } } @@ -329,8 +329,8 @@ impl MsixCapability { /// Write the first DWORD into configuration space (containing the partially modifiable Message /// Control field). - pub unsafe fn write_a(&self, writer: &W, offset: u8) { - writer.write_u32(u16::from(offset), self.a) + pub unsafe fn write_a(&self, func: &PciFunc, offset: u8) { + func.write_u32(u16::from(offset), self.a) } } From eb2e670cd6f67df447ec009b1ec19870b38af495 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 18:53:19 +0200 Subject: [PATCH 8/8] pcid: Explain why PciBar is used instead of pcid_types::Bar --- pcid/src/pci/bar.rs | 2 ++ pcid/src/pci_header.rs | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pcid/src/pci/bar.rs b/pcid/src/pci/bar.rs index b9e8008f51..b279c44577 100644 --- a/pcid/src/pci/bar.rs +++ b/pcid/src/pci/bar.rs @@ -2,6 +2,8 @@ use std::convert::TryInto; use serde::{Deserialize, Serialize}; +// This type is used instead of [pci_types::Bar] in the driver interface as the +// latter can't be serialized and is missing the convenience functions of [PciBar]. #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] pub enum PciBar { None, diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index 95c960851a..5e450ca26c 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -199,7 +199,6 @@ impl PciEndpointHeader { } /// Return the Headers BARs. - // FIXME use pci_types::Bar instead pub fn bars(&self, access: &impl ConfigRegionAccess) -> [PciBar; 6] { let endpoint_header = self.endpoint_header(access);