From e7a6bb6c8121da6e48e5b798daad1c7c0a547964 Mon Sep 17 00:00:00 2001 From: Jacob Lorentzon <4ldo2@protonmail.com> Date: Tue, 20 Feb 2024 22:27:14 +0000 Subject: [PATCH] Only do one memory:physical map per MCFG entry. --- common/src/dma.rs | 59 +++++++--- common/src/lib.rs | 99 ++++++++++++---- pcid/src/cfg_access/mod.rs | 225 ++++++++++++++++++------------------- pcid/src/main.rs | 3 + 4 files changed, 231 insertions(+), 155 deletions(-) diff --git a/common/src/dma.rs b/common/src/dma.rs index 3a092f1024..ec06623360 100644 --- a/common/src/dma.rs +++ b/common/src/dma.rs @@ -1,4 +1,4 @@ -use std::mem::{self, MaybeUninit, size_of}; +use std::mem::{self, size_of, MaybeUninit}; use std::ops::{Deref, DerefMut}; use std::ptr; @@ -23,13 +23,21 @@ const DMA_MEMTY: MemoryType = { fn alloc_and_map(len: usize) -> Result<(usize, *mut ())> { assert_eq!(len % PAGE_SIZE, 0); unsafe { - let fd = syscall::open(format!("memory:zeroed@{DMA_MEMTY}?phys_contiguous"), syscall::O_CLOEXEC)?; - let virt = syscall::fmap(fd, &syscall::Map { - offset: 0, // ignored - address: 0, // ignored - size: len, - flags: syscall::MapFlags::MAP_PRIVATE | syscall::MapFlags::PROT_READ | syscall::MapFlags::PROT_WRITE, - }); + let fd = syscall::open( + format!("memory:zeroed@{DMA_MEMTY}?phys_contiguous"), + syscall::O_CLOEXEC, + )?; + let virt = syscall::fmap( + fd, + &syscall::Map { + offset: 0, // ignored + address: 0, // ignored + size: len, + flags: syscall::MapFlags::MAP_PRIVATE + | syscall::MapFlags::PROT_READ + | syscall::MapFlags::PROT_WRITE, + }, + ); let _ = syscall::close(fd); let virt = virt?; let phys = syscall::virttophys(virt)?; @@ -57,13 +65,21 @@ impl Dma { pub fn zeroed() -> Result>> { let aligned_len = size_of::().next_multiple_of(PAGE_SIZE); let (phys, virt) = alloc_and_map(aligned_len)?; - Ok(Dma { phys, virt: virt.cast(), aligned_len }) + Ok(Dma { + phys, + virt: virt.cast(), + aligned_len, + }) } } impl Dma> { pub unsafe fn assume_init(self) -> Dma { - let Dma { phys, aligned_len, virt } = self; + let Dma { + phys, + aligned_len, + virt, + } = self; mem::forget(self); Dma { @@ -81,13 +97,24 @@ impl Dma { impl Dma<[T]> { pub fn zeroed_slice(count: usize) -> Result]>> { - let aligned_len = count.checked_mul(size_of::()).unwrap().next_multiple_of(PAGE_SIZE); + let aligned_len = count + .checked_mul(size_of::()) + .unwrap() + .next_multiple_of(PAGE_SIZE); let (phys, virt) = alloc_and_map(aligned_len)?; - Ok(Dma { phys, aligned_len, virt: ptr::slice_from_raw_parts_mut(virt.cast(), count) }) + Ok(Dma { + phys, + aligned_len, + virt: ptr::slice_from_raw_parts_mut(virt.cast(), count), + }) } pub unsafe fn cast_slice(self) -> Dma<[U]> { - let Dma { phys, virt, aligned_len } = self; + let Dma { + phys, + virt, + aligned_len, + } = self; core::mem::forget(self); Dma { @@ -99,7 +126,11 @@ impl Dma<[T]> { } impl Dma<[MaybeUninit]> { pub unsafe fn assume_init(self) -> Dma<[T]> { - let &Dma { phys, aligned_len, virt } = &self; + let &Dma { + phys, + aligned_len, + virt, + } = &self; mem::forget(self); Dma { diff --git a/common/src/lib.rs b/common/src/lib.rs index 05d76bf1dd..2a9240f193 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -1,8 +1,8 @@ #![feature(int_roundings)] -use syscall::PAGE_SIZE; use syscall::error::{Error, Result, EINVAL}; use syscall::flag::{MapFlags, O_CLOEXEC, O_RDONLY, O_RDWR, O_WRONLY}; +use syscall::PAGE_SIZE; pub mod dma; @@ -25,19 +25,38 @@ pub struct Prot { pub write: bool, } impl Prot { - pub const RO: Self = Self { read: true, write: false }; - pub const WO: Self = Self { read: false, write: true }; - pub const RW: Self = Self { read: true, write: true }; + pub const RO: Self = Self { + read: true, + write: false, + }; + pub const WO: Self = Self { + read: false, + write: true, + }; + pub const RW: Self = Self { + read: true, + write: true, + }; } -pub unsafe fn physmap(base_phys: usize, len: usize, Prot { read, write }: Prot, ty: MemoryType) -> Result<*mut ()> { +// TODO: Safe, as the kernel ensures it doesn't conflict with any other memory described in the +// memory map for regular RAM. +pub unsafe fn physmap( + base_phys: usize, + len: usize, + Prot { read, write }: Prot, + ty: MemoryType, +) -> Result<*mut ()> { // TODO: arraystring? - let path = format!("memory:physical@{}", match ty { - MemoryType::Writeback => "wb", - MemoryType::Uncacheable => "uc", - MemoryType::WriteCombining => "wc", - MemoryType::DeviceMemory => "dev", - }); + let path = format!( + "memory:physical@{}", + match ty { + MemoryType::Writeback => "wb", + MemoryType::Uncacheable => "uc", + MemoryType::WriteCombining => "wc", + MemoryType::DeviceMemory => "dev", + } + ); let mode = match (read, write) { (true, true) => O_RDWR, (true, false) => O_RDONLY, @@ -49,12 +68,15 @@ pub unsafe fn physmap(base_phys: usize, len: usize, Prot { read, write }: Prot, prot.set(MapFlags::PROT_WRITE, write); let file = syscall::open(path, O_CLOEXEC | mode)?; - let base = syscall::fmap(file, &syscall::Map { - offset: base_phys, - size: len.next_multiple_of(PAGE_SIZE), - flags: MapFlags::MAP_SHARED | prot, - address: 0, - }); + let base = syscall::fmap( + file, + &syscall::Map { + offset: base_phys, + size: len.next_multiple_of(PAGE_SIZE), + flags: MapFlags::MAP_SHARED | prot, + address: 0, + }, + ); let _ = syscall::close(file); Ok(base? as *mut ()) @@ -62,11 +84,42 @@ pub unsafe fn physmap(base_phys: usize, len: usize, Prot { read, write }: Prot, impl std::fmt::Display for MemoryType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", match self { - Self::Writeback => "wb", - Self::Uncacheable => "uc", - Self::WriteCombining => "wc", - Self::DeviceMemory => "dev", - }) + write!( + f, + "{}", + match self { + Self::Writeback => "wb", + Self::Uncacheable => "uc", + Self::WriteCombining => "wc", + Self::DeviceMemory => "dev", + } + ) + } +} + +pub struct PhysBorrowed { + mem: *mut (), + len: usize, +} +impl PhysBorrowed { + pub fn map(base_phys: usize, len: usize, prot: Prot, ty: MemoryType) -> Result { + let mem = unsafe { physmap(base_phys, len, prot, ty)? }; + Ok(Self { + mem, + len: len.next_multiple_of(PAGE_SIZE), + }) + } + pub fn as_ptr(&self) -> *mut () { + self.mem + } + pub fn mapped_len(&self) -> usize { + self.len + } +} +impl Drop for PhysBorrowed { + fn drop(&mut self) { + unsafe { + let _ = syscall::funmap(self.mem as usize, self.len); + } } } diff --git a/pcid/src/cfg_access/mod.rs b/pcid/src/cfg_access/mod.rs index fd7f6e706a..e3f0d4ed91 100644 --- a/pcid/src/cfg_access/mod.rs +++ b/pcid/src/cfg_access/mod.rs @@ -1,7 +1,7 @@ use std::sync::Mutex; -use std::{fmt, fs, io, mem, ptr, slice}; +use std::{fmt, fs, io, mem, ptr}; -use log::info; +use common::{MemoryType, PhysBorrowed, Prot}; use pci_types::{ConfigRegionAccess, PciAddress}; use fallback::Pci; @@ -11,7 +11,7 @@ mod fallback; pub const MCFG_NAME: [u8; 4] = *b"MCFG"; #[repr(packed)] -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] pub struct Mcfg { // base sdt fields name: [u8; 4], @@ -24,8 +24,6 @@ pub struct Mcfg { creator_id: [u8; 4], creator_revision: u32, _rsvd: [u8; 8], - - base_addrs: [PcieAlloc; 0], } unsafe impl plain::Plain for Mcfg {} @@ -42,10 +40,15 @@ pub struct PcieAlloc { } unsafe impl plain::Plain for PcieAlloc {} +#[derive(Debug)] +struct PcieAllocs<'a>(&'a [PcieAlloc]); + impl Mcfg { - fn with(f: impl FnOnce(&Mcfg) -> io::Result) -> io::Result { + fn with(f: impl FnOnce(&Mcfg, PcieAllocs<'_>) -> io::Result) -> io::Result { let table_dir = fs::read_dir("acpi:tables")?; + // TODO: validate/print MCFG? + for table_direntry in table_dir { let table_path = table_direntry?.path(); @@ -59,7 +62,7 @@ impl Mcfg { if table_filename.get(0..4) == Some(&MCFG_NAME) { let bytes = fs::read(table_path)?.into_boxed_slice(); match Mcfg::parse(&*bytes) { - Some(mcfg) => return f(mcfg), + Some((mcfg, allocs)) => return f(mcfg, allocs), None => { return Err(io::Error::new( io::ErrorKind::InvalidData, @@ -76,129 +79,136 @@ impl Mcfg { )) } - fn parse<'a>(bytes: &'a [u8]) -> Option<&'a Mcfg> { - let mcfg = plain::from_bytes::(bytes).ok()?; - if mcfg.length as usize > bytes.len() { + fn parse<'a>(bytes: &'a [u8]) -> Option<(&'a Mcfg, PcieAllocs<'a>)> { + if bytes.len() < mem::size_of::() { return None; } - Some(mcfg) - } + let (header_bytes, allocs_bytes) = bytes.split_at(mem::size_of::()); - fn at_bus(&self, bus: u8) -> Option<&PcieAlloc> { - Some( - self.base_addr_structs() - .iter() - .find(|addr_struct| (addr_struct.start_bus..=addr_struct.end_bus).contains(&bus))?, - ) - } - - fn base_addr_structs(&self) -> &[PcieAlloc] { - let total_length = self.length as usize; - let len = total_length - mem::size_of::(); - // safe because the length cannot be changed arbitrarily - unsafe { - slice::from_raw_parts( - &self.base_addrs as *const PcieAlloc, - len / mem::size_of::(), - ) + let mcfg = + plain::from_bytes::(header_bytes).expect("packed -> align 1, checked size"); + if mcfg.length as usize != bytes.len() { + log::warn!("MCFG {mcfg:?} length mismatch, expected {}", bytes.len()); + return None; } - } -} + // TODO: Allow invalid bytes not divisible by PcieAlloc? -impl fmt::Debug for Mcfg { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("Mcfg") - .field("name", &self.name) - .field("length", &{ self.length }) - .field("revision", &self.revision) - .field("checksum", &self.checksum) - .field("oem_id", &self.oem_id) - .field("oem_table_id", &{ self.oem_table_id }) - .field("oem_revision", &{ self.oem_revision }) - .field("creator_revision", &{ self.creator_revision }) - .field("creator_id", &self.creator_id) - .field("base_addrs", &self.base_addr_structs()) - .finish() + let allocs_len = + allocs_bytes.len() / mem::size_of::() * mem::size_of::(); + + let allocs = plain::slice_from_bytes::(&allocs_bytes[..allocs_len]) + .expect("packed -> align 1, checked size"); + Some((mcfg, PcieAllocs(allocs))) } } pub struct Pcie { lock: Mutex<()>, - bus_maps: Vec>, + allocs: Vec, fallback: Pci, } +struct Alloc { + seg: u16, + start_bus: u8, + end_bus: u8, + mem: PhysBorrowed, +} + unsafe impl Send for Pcie {} unsafe impl Sync for Pcie {} +const BYTES_PER_BUS: usize = 1 << 20; + impl Pcie { pub fn new() -> Self { - match Mcfg::with(|mcfg| { - let alloc_maps = (0..=255) - .map(|bus| { - if let Some(alloc) = mcfg.at_bus(bus) { - Some(unsafe { Self::physmap_pcie_bus(alloc, bus) }) - } else { - None - } + match Mcfg::with(|mcfg, allocs| { + log::info!("MCFG {mcfg:?} ALLOCS {allocs:?}"); + let mut allocs = allocs + .0 + .iter() + .filter_map(|desc| { + Some(Alloc { + seg: desc.seg_group_num, + start_bus: desc.start_bus, + end_bus: desc.end_bus, + mem: PhysBorrowed::map( + desc.base_addr.try_into().ok()?, + BYTES_PER_BUS + * (usize::from(desc.end_bus) - usize::from(desc.start_bus) + 1), + Prot::RW, + MemoryType::Uncacheable, + ) + .inspect_err(|err| { + log::error!( + "failed to map seg {} bus {}..={}: {}", + { desc.seg_group_num }, + { desc.start_bus }, + { desc.end_bus }, + err + ) + }) + .ok()?, + }) }) .collect::>(); + allocs.sort_by_key(|alloc| (alloc.seg, alloc.start_bus)); + Ok(Self { lock: Mutex::new(()), - bus_maps: alloc_maps, + allocs, fallback: Pci::new(), }) }) { Ok(pcie) => pcie, Err(error) => { - info!("Couldn't retrieve PCIe info, perhaps the kernel is not compiled with acpi? Using the PCI 3.0 configuration space instead. Error: {:?}", error); + log::warn!("Couldn't retrieve PCIe info, perhaps the kernel is not compiled with acpi? Using the PCI 3.0 configuration space instead. Error: {:?}", error); Self { lock: Mutex::new(()), - bus_maps: vec![], + allocs: Vec::new(), fallback: Pci::new(), } } } } - - unsafe fn physmap_pcie_bus(alloc: &PcieAlloc, bus: u8) -> (*mut u32, usize) { - let base_phys = alloc.base_addr as usize + (((bus - alloc.start_bus) as usize) << 20); - let map_size = 1 << 20; - let ptr = common::physmap( - base_phys, - map_size, - common::Prot { - read: true, - write: true, - }, - common::MemoryType::Uncacheable, - ) - .unwrap_or_else(|error| { - panic!( - "failed to physmap pcie configuration space for segment {} bus {} @ {:p}: {:?}", - { alloc.seg_group_num }, - bus, - base_phys as *const u32, - error, - ) - }) as *mut u32; - (ptr, map_size) + fn bus_addr(&self, seg: u16, bus: u8) -> Option<*mut u32> { + let alloc = match self + .allocs + .binary_search_by_key(&(seg, bus), |alloc| (alloc.seg, alloc.start_bus)) + { + Ok(present_idx) => &self.allocs[present_idx], + Err(0) => return None, + Err(above_idx) => { + let below_alloc = &self.allocs[above_idx - 1]; + if bus > below_alloc.end_bus { + return None; + } + below_alloc + } + }; + let bus_off = bus - alloc.start_bus; + Some(unsafe { + alloc + .mem + .as_ptr() + .cast::() + .add(usize::from(bus_off) * BYTES_PER_BUS) + .cast::() + }) } - fn bus_addr_offset_in_bytes(address: PciAddress, offset: u16) -> usize { + fn bus_addr_offset_in_dwords(address: PciAddress, offset: u16) -> usize { assert_eq!(offset & 0xFFFC, offset, "pcie offset not dword-aligned"); assert_eq!(offset & 0x0FFF, offset, "pcie offset larger than 4095"); - ((address.device() as usize) << 15) + (((address.device() as usize) << 15) | ((address.function() as usize) << 12) - | (offset as usize) + | (offset as usize)) + >> 2 } - unsafe fn with_pointer) -> T>( - &self, - address: PciAddress, - offset: u16, - f: F, - ) -> T { + // TODO: A safer interface, using e.g. a VolatileCell or Volatile<'a>. The PhysBorrowed wrapper + // can possibly deref to or provide a Volatile. + fn mmio_addr(&self, address: PciAddress, offset: u16) -> Option<*mut u32> { assert_eq!( address.segment(), 0, @@ -207,17 +217,8 @@ impl Pcie { assert_eq!(offset & 0xFC, offset, "pci offset is not aligned"); - let bus_addr = match self.bus_maps.get(address.bus() as usize) { - Some(Some(bus_addr)) => bus_addr, - Some(None) | None => return f(None), - }; - let virt_pointer = unsafe { - // FIXME use byte_add once stable - (bus_addr.0 as *mut u8).add(Self::bus_addr_offset_in_bytes(address, 0)) as *mut u32 - }; - f(Some(&mut *virt_pointer.offset( - (offset as usize / mem::size_of::()) as isize, - ))) + let bus_addr = self.bus_addr(address.segment(), address.bus())?; + Some(unsafe { bus_addr.add(Self::bus_addr_offset_in_dwords(address, offset)) }) } } @@ -229,30 +230,18 @@ impl ConfigRegionAccess for Pcie { unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 { let _guard = self.lock.lock().unwrap(); - self.with_pointer(address, offset, |pointer| match pointer { - Some(address) => ptr::read_volatile::(address), + match self.mmio_addr(address, offset) { + Some(addr) => addr.read_volatile(), None => self.fallback.read(address, offset), - }) + } } unsafe fn write(&self, address: PciAddress, offset: u16, value: u32) { let _guard = self.lock.lock().unwrap(); - self.with_pointer(address, offset, |pointer| match pointer { - Some(address) => ptr::write_volatile::(address, value), - None => { - self.fallback.write(address, offset, value); - } - }); - } -} - -impl Drop for Pcie { - fn drop(&mut self) { - for &map in &self.bus_maps { - if let Some((ptr, size)) = map { - let _ = unsafe { syscall::funmap(ptr as usize, size) }; - } + match self.mmio_addr(address, offset) { + Some(addr) => addr.write_volatile(value), + None => self.fallback.write(address, offset, value), } } } diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 56f78191bf..e2915272cb 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -1,3 +1,6 @@ +// Already stabilized, TODO: remove when Redox's rustc is updated +#![feature(result_option_inspect)] + use std::fs::{File, metadata, read_dir}; use std::io::prelude::*; use std::os::unix::io::{FromRawFd, RawFd};