From a04ed2821a22a0338980d632b7f26099f2edf569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Tue, 4 Jun 2024 18:45:56 +0200 Subject: [PATCH 1/3] feat: add forwarding `impl ConfigRegionAccess for &T` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- src/lib.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 9bd0574..b170a19 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -92,6 +92,23 @@ pub trait ConfigRegionAccess { unsafe fn write(&self, address: PciAddress, offset: u16, value: u32); } +impl ConfigRegionAccess for &T { + #[inline] + fn function_exists(&self, address: PciAddress) -> bool { + (**self).function_exists(address) + } + + #[inline] + unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 { + (**self).read(address, offset) + } + + #[inline] + unsafe fn write(&self, address: PciAddress, offset: u16, value: u32) { + (**self).write(address, offset, value) + } +} + #[non_exhaustive] #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum HeaderType { From d243ca39a10e60f53f203c205b2b3d63adbe9209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Tue, 4 Jun 2024 18:47:55 +0200 Subject: [PATCH 2/3] feat: don't take explicit references to `ConfigRegionAccess` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- src/capability/mod.rs | 2 +- src/capability/msi.rs | 18 +++++++-------- src/capability/msix.rs | 4 ++-- src/lib.rs | 50 +++++++++++++++++++++--------------------- 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/capability/mod.rs b/src/capability/mod.rs index c01079a..e341185 100644 --- a/src/capability/mod.rs +++ b/src/capability/mod.rs @@ -64,7 +64,7 @@ impl PciCapability { id: u8, address: PciCapabilityAddress, extension: u16, - access: &impl ConfigRegionAccess, + access: impl ConfigRegionAccess, ) -> Option { match id { 0x00 => None, // null capability diff --git a/src/capability/msi.rs b/src/capability/msi.rs index 30a036a..4428fe5 100644 --- a/src/capability/msi.rs +++ b/src/capability/msi.rs @@ -82,18 +82,18 @@ impl MsiCapability { self.multiple_message_capable } - pub fn ctrl(&self, access: &impl ConfigRegionAccess) -> u32 { + pub fn ctrl(&self, access: impl ConfigRegionAccess) -> u32 { unsafe { access.read(self.address.address, self.address.offset) } } /// Is MSI capability enabled? - pub fn is_enabled(&self, access: &impl ConfigRegionAccess) -> bool { + pub fn is_enabled(&self, access: impl ConfigRegionAccess) -> bool { let reg = unsafe { access.read(self.address.address, self.address.offset) }; reg.get_bit(16) } /// Enable or disable MSI capability - pub fn set_enabled(&self, enabled: bool, access: &impl ConfigRegionAccess) { + pub fn set_enabled(&self, enabled: bool, access: impl ConfigRegionAccess) { let mut reg = unsafe { access.read(self.address.address, self.address.offset) }; reg.set_bit(16, enabled); unsafe { access.write(self.address.address, self.address.offset, reg) }; @@ -101,14 +101,14 @@ impl MsiCapability { /// Set how many interrupts the device will use. If requested count is bigger than supported count, /// the second will be used. - pub fn set_multiple_message_enable(&self, data: MultipleMessageSupport, access: &impl ConfigRegionAccess) { + pub fn set_multiple_message_enable(&self, data: MultipleMessageSupport, access: impl ConfigRegionAccess) { let mut reg = unsafe { access.read(self.address.address, self.address.offset) }; reg.set_bits(4..7, (data.min(self.multiple_message_capable)) as u32); unsafe { access.write(self.address.address, self.address.offset, reg) }; } /// Return how many interrupts the device is using - pub fn get_multiple_message_enable(&self, access: &impl ConfigRegionAccess) -> MultipleMessageSupport { + pub fn get_multiple_message_enable(&self, access: impl ConfigRegionAccess) -> MultipleMessageSupport { let reg = unsafe { access.read(self.address.address, self.address.offset) }; MultipleMessageSupport::try_from(reg.get_bits(4..7) as u8).unwrap_or(MultipleMessageSupport::Int1) } @@ -125,7 +125,7 @@ impl MsiCapability { address: u32, vector: u8, trigger_mode: TriggerMode, - access: &impl ConfigRegionAccess, + access: impl ConfigRegionAccess, ) { unsafe { access.write(self.address.address, self.address.offset + 0x4, address) } let data_offset = if self.is_64bit { 0xC } else { 0x8 }; @@ -140,7 +140,7 @@ impl MsiCapability { /// # Note /// Only supported on when device supports 64-bit addressing and per-vector masking. Otherwise /// returns `0` - pub fn get_message_mask(&self, access: &impl ConfigRegionAccess) -> u32 { + pub fn get_message_mask(&self, access: impl ConfigRegionAccess) -> u32 { if self.is_64bit && self.per_vector_masking { unsafe { access.read(self.address.address, self.address.offset + 0x10) } } else { @@ -153,7 +153,7 @@ impl MsiCapability { /// # Note /// Only supported on when device supports 64-bit addressing and per-vector masking. Otherwise /// will do nothing - pub fn set_message_mask(&self, access: &impl ConfigRegionAccess, mask: u32) { + pub fn set_message_mask(&self, access: impl ConfigRegionAccess, mask: u32) { if self.is_64bit && self.per_vector_masking { unsafe { access.write(self.address.address, self.address.offset + 0x10, mask) } } @@ -163,7 +163,7 @@ impl MsiCapability { /// /// # Note /// Only supported on when device supports 64-bit addressing. Otherwise will return `0` - pub fn get_pending(&self, access: &impl ConfigRegionAccess) -> u32 { + pub fn get_pending(&self, access: impl ConfigRegionAccess) -> u32 { if self.is_64bit { unsafe { access.read(self.address.address, self.address.offset + 0x14) } } else { diff --git a/src/capability/msix.rs b/src/capability/msix.rs index 84522e2..bd90111 100644 --- a/src/capability/msix.rs +++ b/src/capability/msix.rs @@ -16,7 +16,7 @@ impl MsixCapability { pub(crate) fn new( address: PciCapabilityAddress, control: u16, - access: &impl ConfigRegionAccess, + access: impl ConfigRegionAccess, ) -> MsixCapability { let table_size = control.get_bits(0..11) + 1; let table = unsafe { access.read(address.address, address.offset + 0x04) }; @@ -31,7 +31,7 @@ impl MsixCapability { /// `[MsixCapability::table_bar]` and `[MsixCapability::table_offset]`. The caller is therefore /// responsible for configuring this separately, as this crate does not have access to /// arbitrary physical memory. - pub fn set_enabled(&mut self, enabled: bool, access: &impl ConfigRegionAccess) { + pub fn set_enabled(&mut self, enabled: bool, access: impl ConfigRegionAccess) { let mut control = unsafe { access.read(self.address.address, self.address.offset) }; control.set_bit(31, enabled); unsafe { diff --git a/src/lib.rs b/src/lib.rs index b170a19..8f93690 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -146,12 +146,12 @@ impl PciHeader { PciHeader(address) } - pub fn id(&self, access: &impl ConfigRegionAccess) -> (VendorId, DeviceId) { + pub fn id(&self, access: impl ConfigRegionAccess) -> (VendorId, DeviceId) { let id = unsafe { access.read(self.0, 0x00) }; (id.get_bits(0..16) as VendorId, id.get_bits(16..32) as DeviceId) } - pub fn header_type(&self, access: &impl ConfigRegionAccess) -> HeaderType { + pub fn header_type(&self, access: impl ConfigRegionAccess) -> HeaderType { /* * Read bits 0..=6 of the Header Type. Bit 7 dictates whether the device has multiple functions and so * isn't returned here. @@ -164,7 +164,7 @@ impl PciHeader { } } - pub fn has_multiple_functions(&self, access: &impl ConfigRegionAccess) -> bool { + pub fn has_multiple_functions(&self, access: impl ConfigRegionAccess) -> bool { /* * Reads bit 7 of the Header Type, which is 1 if the device has multiple functions. */ @@ -173,7 +173,7 @@ impl PciHeader { pub fn revision_and_class( &self, - access: &impl ConfigRegionAccess, + access: impl ConfigRegionAccess, ) -> (DeviceRevision, BaseClass, SubClass, Interface) { let field = unsafe { access.read(self.0, 0x08) }; ( @@ -184,17 +184,17 @@ impl PciHeader { ) } - pub fn status(&self, access: &impl ConfigRegionAccess) -> StatusRegister { + pub fn status(&self, access: impl ConfigRegionAccess) -> StatusRegister { let data = unsafe { access.read(self.0, 0x4).get_bits(16..32) }; StatusRegister::new(data as u16) } - pub fn command(&self, access: &impl ConfigRegionAccess) -> CommandRegister { + pub fn command(&self, access: impl ConfigRegionAccess) -> CommandRegister { let data = unsafe { access.read(self.0, 0x4).get_bits(0..16) }; CommandRegister::from_bits_retain(data as u16) } - pub fn update_command(&self, access: &impl ConfigRegionAccess, f: F) + pub fn update_command(&self, access: impl ConfigRegionAccess, f: F) where F: FnOnce(CommandRegister) -> CommandRegister, { @@ -256,7 +256,7 @@ impl PciHeader { pub struct EndpointHeader(PciAddress); impl EndpointHeader { - pub fn from_header(header: PciHeader, access: &impl ConfigRegionAccess) -> Option { + pub fn from_header(header: PciHeader, access: impl ConfigRegionAccess) -> Option { match header.header_type(access) { HeaderType::Endpoint => Some(EndpointHeader(header.0)), _ => None, @@ -267,23 +267,23 @@ impl EndpointHeader { PciHeader(self.0) } - pub fn status(&self, access: &impl ConfigRegionAccess) -> StatusRegister { + pub fn status(&self, access: impl ConfigRegionAccess) -> StatusRegister { self.header().status(access) } - pub fn command(&self, access: &impl ConfigRegionAccess) -> CommandRegister { + pub fn command(&self, access: impl ConfigRegionAccess) -> CommandRegister { self.header().command(access) } - pub fn update_command(&self, access: &impl ConfigRegionAccess, f: F) + pub fn update_command(&self, access: impl ConfigRegionAccess, f: F) where F: FnOnce(CommandRegister) -> CommandRegister, { self.header().update_command(access, f); } - pub fn capability_pointer(&self, access: &impl ConfigRegionAccess) -> u16 { - let status = self.status(access); + pub fn capability_pointer(&self, access: impl ConfigRegionAccess) -> u16 { + let status = self.status(&access); if status.has_capability_list() { unsafe { access.read(self.0, 0x34).get_bits(0..8) as u16 } } else { @@ -296,7 +296,7 @@ impl EndpointHeader { CapabilityIterator::new(self.0, pointer, access) } - pub fn subsystem(&self, access: &impl ConfigRegionAccess) -> (SubsystemId, SubsystemVendorId) { + pub fn subsystem(&self, access: impl ConfigRegionAccess) -> (SubsystemId, SubsystemVendorId) { let data = unsafe { access.read(self.0, 0x2c) }; (data.get_bits(16..32) as u16, data.get_bits(0..16) as u16) } @@ -306,7 +306,7 @@ impl EndpointHeader { /// ### Note /// 64-bit memory BARs use two slots, so if one is decoded in e.g. slot #0, this method should not be called /// for slot #1 - pub fn bar(&self, slot: u8, access: &impl ConfigRegionAccess) -> Option { + pub fn bar(&self, slot: u8, access: impl ConfigRegionAccess) -> Option { if slot >= 6 { return None; } @@ -397,10 +397,10 @@ impl EndpointHeader { pub unsafe fn write_bar( &mut self, slot: u8, - access: &impl ConfigRegionAccess, + access: impl ConfigRegionAccess, value: usize, ) -> Result<(), BarWriteError> { - match self.bar(slot, access) { + match self.bar(slot, &access) { Some(Bar::Memory64 { .. }) => { let offset = 0x10 + (slot as u16) * 4; unsafe { @@ -424,7 +424,7 @@ impl EndpointHeader { } } - pub fn interrupt(&self, access: &impl ConfigRegionAccess) -> (InterruptPin, InterruptLine) { + pub fn interrupt(&self, access: impl ConfigRegionAccess) -> (InterruptPin, InterruptLine) { // According to the PCI Express Specification 4.0, Min_Gnt/Max_Lat registers // must be read-only and hardwired to 00h. let data = unsafe { access.read(self.0, 0x3c) }; @@ -494,7 +494,7 @@ impl EndpointHeader { pub struct PciPciBridgeHeader(PciAddress); impl PciPciBridgeHeader { - pub fn from_header(header: PciHeader, access: &impl ConfigRegionAccess) -> Option { + pub fn from_header(header: PciHeader, access: impl ConfigRegionAccess) -> Option { match header.header_type(access) { HeaderType::PciPciBridge => Some(PciPciBridgeHeader(header.0)), _ => None, @@ -505,32 +505,32 @@ impl PciPciBridgeHeader { PciHeader(self.0) } - pub fn status(&self, access: &impl ConfigRegionAccess) -> StatusRegister { + pub fn status(&self, access: impl ConfigRegionAccess) -> StatusRegister { self.header().status(access) } - pub fn command(&self, access: &impl ConfigRegionAccess) -> CommandRegister { + pub fn command(&self, access: impl ConfigRegionAccess) -> CommandRegister { self.header().command(access) } - pub fn update_command(&self, access: &impl ConfigRegionAccess, f: F) + pub fn update_command(&self, access: impl ConfigRegionAccess, f: F) where F: FnOnce(CommandRegister) -> CommandRegister, { self.header().update_command(access, f); } - pub fn primary_bus_number(&self, access: &impl ConfigRegionAccess) -> u8 { + pub fn primary_bus_number(&self, access: impl ConfigRegionAccess) -> u8 { let data = unsafe { access.read(self.0, 0x18).get_bits(0..8) }; data as u8 } - pub fn secondary_bus_number(&self, access: &impl ConfigRegionAccess) -> u8 { + pub fn secondary_bus_number(&self, access: impl ConfigRegionAccess) -> u8 { let data = unsafe { access.read(self.0, 0x18).get_bits(8..16) }; data as u8 } - pub fn subordinate_bus_number(&self, access: &impl ConfigRegionAccess) -> u8 { + pub fn subordinate_bus_number(&self, access: impl ConfigRegionAccess) -> u8 { let data = unsafe { access.read(self.0, 0x18).get_bits(16..24) }; data as u8 } From 7087796d42104cf64f9a788ed5fbab5cf92b3374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Tue, 4 Jun 2024 18:51:28 +0200 Subject: [PATCH 3/3] feat(breaking): remove lifetime parameter from `CapabilityIterator` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The potential lifetime parameter is now part of `T`. Signed-off-by: Martin Kröning --- src/capability/mod.rs | 12 ++++++------ src/lib.rs | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/capability/mod.rs b/src/capability/mod.rs index e341185..546de4b 100644 --- a/src/capability/mod.rs +++ b/src/capability/mod.rs @@ -89,19 +89,19 @@ impl PciCapability { } } -pub struct CapabilityIterator<'a, T: ConfigRegionAccess> { +pub struct CapabilityIterator { address: PciAddress, offset: u16, - access: &'a T, + access: T, } -impl<'a, T: ConfigRegionAccess> CapabilityIterator<'a, T> { - pub(crate) fn new(address: PciAddress, offset: u16, access: &'a T) -> CapabilityIterator<'a, T> { +impl CapabilityIterator { + pub(crate) fn new(address: PciAddress, offset: u16, access: T) -> CapabilityIterator { CapabilityIterator { address, offset, access } } } -impl<'a, T: ConfigRegionAccess> Iterator for CapabilityIterator<'a, T> { +impl Iterator for CapabilityIterator { type Item = PciCapability; fn next(&mut self) -> Option { @@ -117,7 +117,7 @@ impl<'a, T: ConfigRegionAccess> Iterator for CapabilityIterator<'a, T> { id as u8, PciCapabilityAddress { address: self.address, offset: self.offset }, extension, - self.access, + &self.access, ); self.offset = next_ptr as u16; if let Some(cap) = cap { diff --git a/src/lib.rs b/src/lib.rs index 8f93690..21d5e7f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -291,8 +291,8 @@ impl EndpointHeader { } } - pub fn capabilities<'a, T: ConfigRegionAccess>(&self, access: &'a T) -> CapabilityIterator<'a, T> { - let pointer = self.capability_pointer(access); + pub fn capabilities(&self, access: T) -> CapabilityIterator { + let pointer = self.capability_pointer(&access); CapabilityIterator::new(self.0, pointer, access) }