* [PATCH v12 0/5] rust: pci: add config space read/write support
@ 2026-01-21 20:22 Zhi Wang
2026-01-21 20:22 ` [PATCH v12 1/5] rust: devres: style for imports Zhi Wang
` (5 more replies)
0 siblings, 6 replies; 25+ messages in thread
From: Zhi Wang @ 2026-01-21 20:22 UTC (permalink / raw)
To: rust-for-linux, linux-pci, linux-kernel
Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor,
boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede,
targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida,
Zhi Wang
In the NVIDIA vGPU RFC [1], the PCI configuration space access is
required in nova-core for preparing gspVFInfo when vGPU support is
enabled. This series is the following up of the discussion with Danilo
for how to introduce support of PCI configuration space access in Rust
PCI abstractions. Alice/Alex/I had a discussion of the next steps of
this patch series in LPC 2025. We agreed that first introducing the
functionality before the BoundedInteger work and other refinement is
settled. [2]
The repo with the patches can be found at [3].
v12:
- Move accessors from IoKnownSize trait to Io trait. (Gary)
- Turn IoKnownSize into a marker trait. (Gary)
- Use build_error!() in Io trait default functions. (Gary)
- Remove extra #[(CONFIG_64BIT)] in Io trait. (Gary/Alice)
- Introduce marker types for configuration space. (Gary)
v11:
I tried to combine IoCapable and IoTryCapable. I found I have to dump
the original forwarding call scheme.
In this re-spin, I moved the accessors into the Io/IoKnownSize traits
and I have to added the default trait functions required by the
compiler. The IoCapable<T> turns into a pure marker trait. Reaching
the accessors from a driver are restricted by the where statements
marked by IoCapable<T>. A backend marks its capablities via implementing
IoCapable<T>. This is quite close to Gary's orginal suggestions.
I also left a assert in the default trait functions (accessors) to
catch the case that a backend implements IoCapable<T> to claim it
support the accessors but forgets to implement the accessors. This
should be helpful when developing a new backend.
- Combine IoCapable and IoTryCapable. (Danilo/Gary)
- Keep IoKnownSize trait. (Alice/Gary)
- Fix the compiling error in pwm_th1520 driver.
v10:
- Merge IoBase trait into Io trait to simplify trait hierarchy.
- Use IoCapable<T> and IoTryCapable<T> functional traits (not marker
traits) to separate infallible and fallible I/O operations. (Danilo/Gary)
I tried implementing the marker traits idea, but I found that the
compiler still requires a default implementation for each function in
the main trait to avoid forcing backends to implement unsupported
methods.
However, Gary's core idea of "capabilities" is extremely valuable. So, I
adopted a dispatching pattern:
- All I/O accessors are defined in the Io trait. They use default
implementations to dispatch calls to the underlying IoCapable and
IoTryCapable traits.
This addresses Alice's concern about the complicated trait hierarchy.
It preserves strict compile-time static checks. The compiler can catch
the case if a driver calls an accessor not implemented by a backend.
- The actual logic resides in IoCapable<T> and IoTryCapable<T>.
Backends can selectively implement IoCapable<T> or IoTryCapable<T> based
on hardware support.
This addresses Markus's concern regarding backends with limited access
sizes (e.g., I2C).
For example, the Mmio backend implements both IoCapable and IoTryCapable
for all sizes, while the PCI config space backend only implements
IoCapable for u8/u16/u32.
v9:
- Rebase the patches to the latest driver-core-testing.
- Move ConfigSpaceSize to pci/io.rs. (Danilo)
- Refine docs. (Danilo)
- Compiling test on Tyr. (Danilo)
v8:
- Rebase to latest driver-core-testing branch.
- Refinement of traits name and hierarchy: (Alice)
* Rename IoInfallible trait to IoKnownSize trait.
* Keep Infallible helpers in Io trait.
v7:
- Rebase to latest driver-core-testing branch.
- Introduce Io64 trait. (Alice)
- Add docs for call_{mmio, config}_{read, write}() macros. (Alex)
- Improve the define_{read, write} macros. (Alex)
- Add SAFETY/CAST in call_config_{read, write}. (Joel)
- Fix typo of method name. (Alex/Joel)
v6:
- Implement config_space() and config_space_extended() in device::Bound
lifecycle. (Danilo)
- Fix the "use" in the comment for generating proper rust docs, verify
the output of rustdoc. (Miguel)
- Improve the comments of PCI configuration space when checking the
output of rustdoc.
v5:
- Remove fallible accessors of PCI configuration space. (Danilo)
- Add #[repr(usize)] for enum ConfigSpace. (Danilo)
- Refine the handling of return value in read accessors. (Danilo)
- Add debug_assert!() in pdev::cfg_size(). (Danilo)
- Add ConfigSpace.as_raw() for extracting the raw value. (Danilo)
- Rebase the patches on top of driver-core-testing branch.
- Convert imports touched by this series to vertical style.
v4:
- Refactor the SIZE constant to be an associated constant. (Alice)
- Remove the default method implementations in the Io trait. (Alice)
- Make cfg_size() private. (Danilo/Bjorn)
- Implement the infallible accessors of ConfigSpace. (Danilo)
- Create a new Io64 trait specifically for 64-bit accessors. (Danilo)
- Provide two separate methods for driver: config_space() and
config_space_extended(). (Danilo)
- Update the sample driver to test the infallible accessors. (Danilo)
v3:
- Turn offset_valid() into a private function of kernel::io:Io. (Alex)
- Separate try and non-try variants. (Danilo)
- Move all the {try_}{read,write}{8,16,32,64} accessors to the I/O trait.
(Danilo)
- Replace the hardcoded MMIO type constraint with a generic trait bound
so that register! macro can be used in other places. (Danilo)
- Fix doctest. (John)
- Add an enum for PCI configuration space size. (Danilo)
- Refine the patch comments. (Bjorn)
v2:
- Factor out common trait as 'Io' and keep the rest routines in original
'Io' as 'Mmio'. (Danilo)
- Rename 'IoRaw' to 'MmioRaw'. Update the bus MMIO implementation to use
'MmioRaw'.
- Introduce pci::Device<Bound>::config_space(). (Danilo)
- Implement both infallible and fallible read/write routines, the device
driver decicdes which version should be used.
This ideas of this series are:
- Factor out common traits for other accessors to share the same
compiling/runtime check like before.
- Introduce IoCapable<T> and IoTryCapable<T> traits to allow backends
to selectively implement only the operations they support.
- Factor the MMIO read/write macros from the define_read! and
define_write! macros. Thus, define_{read, write}! can be used in other
backends.
In detail:
* Introduce `call_mmio_read!` and `call_mmio_write!` helper macros
to encapsulate the unsafe FFI calls.
* Update `define_read!` and `define_write!` macros to delegate to
the call macros.
* Export `define_read` and `define_write` so they can be reused
for other I/O backends (e.g. PCI config space).
- Implement the PCI configuration space access backend in PCI
abstractions.
- Add tests for config space routines in rust PCI sample driver.
[1] https://lore.kernel.org/all/20250903221111.3866249-1-zhiw@nvidia.com/
[2] https://lore.kernel.org/all/DEOMBKIRDXH6.2CF2MR2RB2W2C@nvidia.com/
[3] https://github.com/zhiwang-nvidia/nova-core/tree/rust-for-linux/pci-configuration-space-v12
Zhi Wang (5):
rust: devres: style for imports
rust: io: separate generic I/O helpers from MMIO implementation
rust: io: factor out MMIO read/write macros
rust: pci: add config space read/write support
sample: rust: pci: add tests for config space routines
drivers/gpu/drm/tyr/regs.rs | 1 +
drivers/gpu/nova-core/gsp/sequencer.rs | 5 +-
drivers/gpu/nova-core/regs/macros.rs | 90 +++--
drivers/gpu/nova-core/vbios.rs | 1 +
drivers/pwm/pwm_th1520.rs | 5 +-
rust/kernel/devres.rs | 35 +-
rust/kernel/io.rs | 477 ++++++++++++++++++++-----
rust/kernel/io/mem.rs | 16 +-
rust/kernel/io/poll.rs | 16 +-
rust/kernel/pci.rs | 7 +-
rust/kernel/pci/io.rs | 177 ++++++++-
samples/rust/rust_driver_pci.rs | 29 ++
12 files changed, 713 insertions(+), 146 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v12 1/5] rust: devres: style for imports 2026-01-21 20:22 [PATCH v12 0/5] rust: pci: add config space read/write support Zhi Wang @ 2026-01-21 20:22 ` Zhi Wang 2026-01-22 4:25 ` Alexandre Courbot 2026-01-21 20:22 ` [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation Zhi Wang ` (4 subsequent siblings) 5 siblings, 1 reply; 25+ messages in thread From: Zhi Wang @ 2026-01-21 20:22 UTC (permalink / raw) To: rust-for-linux, linux-pci, linux-kernel Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida, Zhi Wang Convert all imports in the devres to use "kernel vertical" style. Cc: Gary Guo <gary@garyguo.net> Cc: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Gary Guo <gary@garyguo.net> Signed-off-by: Zhi Wang <zhiw@nvidia.com> --- rust/kernel/devres.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index db02f8b1788d..43089511bf76 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -254,8 +254,12 @@ pub fn device(&self) -> &Device { /// # Examples /// /// ```no_run - /// # #![cfg(CONFIG_PCI)] - /// # use kernel::{device::Core, devres::Devres, pci}; + /// #![cfg(CONFIG_PCI)] + /// use kernel::{ + /// device::Core, + /// devres::Devres, + /// pci, // + /// }; /// /// fn from_core(dev: &pci::Device<Core>, devres: Devres<pci::Bar<0x4>>) -> Result { /// let bar = devres.access(dev.as_ref())?; @@ -358,7 +362,13 @@ fn register_foreign<P>(dev: &Device<Bound>, data: P) -> Result /// # Examples /// /// ```no_run -/// use kernel::{device::{Bound, Device}, devres}; +/// use kernel::{ +/// device::{ +/// Bound, +/// Device, // +/// }, +/// devres, // +/// }; /// /// /// Registration of e.g. a class device, IRQ, etc. /// struct Registration; -- 2.51.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v12 1/5] rust: devres: style for imports 2026-01-21 20:22 ` [PATCH v12 1/5] rust: devres: style for imports Zhi Wang @ 2026-01-22 4:25 ` Alexandre Courbot 0 siblings, 0 replies; 25+ messages in thread From: Alexandre Courbot @ 2026-01-22 4:25 UTC (permalink / raw) To: Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, joelagnelf, jhubbard, zhiwang, daniel.almeida On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote: > Convert all imports in the devres to use "kernel vertical" style. > > Cc: Gary Guo <gary@garyguo.net> > Cc: Miguel Ojeda <ojeda@kernel.org> > Reviewed-by: Gary Guo <gary@garyguo.net> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com> ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation 2026-01-21 20:22 [PATCH v12 0/5] rust: pci: add config space read/write support Zhi Wang 2026-01-21 20:22 ` [PATCH v12 1/5] rust: devres: style for imports Zhi Wang @ 2026-01-21 20:22 ` Zhi Wang 2026-01-22 4:28 ` Alexandre Courbot ` (3 more replies) 2026-01-21 20:22 ` [PATCH v12 3/5] rust: io: factor out MMIO read/write macros Zhi Wang ` (3 subsequent siblings) 5 siblings, 4 replies; 25+ messages in thread From: Zhi Wang @ 2026-01-21 20:22 UTC (permalink / raw) To: rust-for-linux, linux-pci, linux-kernel Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida, Zhi Wang The previous Io<SIZE> type combined both the generic I/O access helpers and MMIO implementation details in a single struct. This coupling prevented reusing the I/O helpers for other backends, such as PCI configuration space. Establish a clean separation between the I/O interface and concrete backends by separating generic I/O helpers from MMIO implementation. Introduce a new trait hierarchy to handle different access capabilities: - IoCapable<T>: A marker trait indicating that a backend supports I/O operations of a certain type (u8, u16, u32, or u64). - Io trait: Defines fallible (try_read8, try_write8, etc.) and infallibile (read8, write8, etc.) I/O methods with runtime bounds checking and compile-time bounds checking. - IoKnownSize trait: The marker trait for types support infallible I/O methods. Move the MMIO-specific logic into a dedicated Mmio<SIZE> type that implements the Io traits. Rename IoRaw to MmioRaw and update consumers to use the new types. Cc: Alexandre Courbot <acourbot@nvidia.com> Cc: Alice Ryhl <aliceryhl@google.com> Cc: Bjorn Helgaas <helgaas@kernel.org> Cc: Gary Guo <gary@garyguo.net> Cc: Danilo Krummrich <dakr@kernel.org> Cc: John Hubbard <jhubbard@nvidia.com> Signed-off-by: Zhi Wang <zhiw@nvidia.com> --- drivers/gpu/drm/tyr/regs.rs | 1 + drivers/gpu/nova-core/gsp/sequencer.rs | 5 +- drivers/gpu/nova-core/regs/macros.rs | 90 +++--- drivers/gpu/nova-core/vbios.rs | 1 + drivers/pwm/pwm_th1520.rs | 5 +- rust/kernel/devres.rs | 19 +- rust/kernel/io.rs | 398 ++++++++++++++++++++----- rust/kernel/io/mem.rs | 16 +- rust/kernel/io/poll.rs | 16 +- rust/kernel/pci/io.rs | 12 +- samples/rust/rust_driver_pci.rs | 1 + 11 files changed, 433 insertions(+), 131 deletions(-) diff --git a/drivers/gpu/drm/tyr/regs.rs b/drivers/gpu/drm/tyr/regs.rs index f46933aaa221..d3a541cb37c6 100644 --- a/drivers/gpu/drm/tyr/regs.rs +++ b/drivers/gpu/drm/tyr/regs.rs @@ -11,6 +11,7 @@ use kernel::device::Bound; use kernel::device::Device; use kernel::devres::Devres; +use kernel::io::Io; use kernel::prelude::*; use crate::driver::IoMem; diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs index 2d0369c49092..862cf7f27143 100644 --- a/drivers/gpu/nova-core/gsp/sequencer.rs +++ b/drivers/gpu/nova-core/gsp/sequencer.rs @@ -12,7 +12,10 @@ use kernel::{ device, - io::poll::read_poll_timeout, + io::{ + poll::read_poll_timeout, + Io, // + }, prelude::*, time::{ delay::fsleep, diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs index fd1a815fa57d..ed624be1f39b 100644 --- a/drivers/gpu/nova-core/regs/macros.rs +++ b/drivers/gpu/nova-core/regs/macros.rs @@ -369,16 +369,18 @@ impl $name { /// Read the register from its address in `io`. #[inline(always)] - pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + pub(crate) fn read<T, I>(io: &T) -> Self where + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, { Self(io.read32($offset)) } /// Write the value contained in `self` to the register address in `io`. #[inline(always)] - pub(crate) fn write<const SIZE: usize, T>(self, io: &T) where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + pub(crate) fn write<T, I>(self, io: &T) where + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, { io.write32(self.0, $offset) } @@ -386,11 +388,12 @@ pub(crate) fn write<const SIZE: usize, T>(self, io: &T) where /// Read the register from its address in `io` and run `f` on its value to obtain a new /// value to write back. #[inline(always)] - pub(crate) fn update<const SIZE: usize, T, F>( + pub(crate) fn update<T, I, F>( io: &T, f: F, ) where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, F: ::core::ops::FnOnce(Self) -> Self, { let reg = f(Self::read(io)); @@ -408,12 +411,13 @@ impl $name { /// Read the register from `io`, using the base address provided by `base` and adding /// the register's offset to it. #[inline(always)] - pub(crate) fn read<const SIZE: usize, T, B>( + pub(crate) fn read<T, I, B>( io: &T, #[allow(unused_variables)] base: &B, ) -> Self where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, B: crate::regs::macros::RegisterBase<$base>, { const OFFSET: usize = $name::OFFSET; @@ -428,13 +432,14 @@ pub(crate) fn read<const SIZE: usize, T, B>( /// Write the value contained in `self` to `io`, using the base address provided by /// `base` and adding the register's offset to it. #[inline(always)] - pub(crate) fn write<const SIZE: usize, T, B>( + pub(crate) fn write<T, I, B>( self, io: &T, #[allow(unused_variables)] base: &B, ) where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, B: crate::regs::macros::RegisterBase<$base>, { const OFFSET: usize = $name::OFFSET; @@ -449,12 +454,13 @@ pub(crate) fn write<const SIZE: usize, T, B>( /// the register's offset to it, then run `f` on its value to obtain a new value to /// write back. #[inline(always)] - pub(crate) fn update<const SIZE: usize, T, B, F>( + pub(crate) fn update<T, I, B, F>( io: &T, base: &B, f: F, ) where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, B: crate::regs::macros::RegisterBase<$base>, F: ::core::ops::FnOnce(Self) -> Self, { @@ -474,11 +480,12 @@ impl $name { /// Read the array register at index `idx` from its address in `io`. #[inline(always)] - pub(crate) fn read<const SIZE: usize, T>( + pub(crate) fn read<T, I>( io: &T, idx: usize, ) -> Self where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, { build_assert!(idx < Self::SIZE); @@ -490,12 +497,13 @@ pub(crate) fn read<const SIZE: usize, T>( /// Write the value contained in `self` to the array register with index `idx` in `io`. #[inline(always)] - pub(crate) fn write<const SIZE: usize, T>( + pub(crate) fn write<T, I>( self, io: &T, idx: usize ) where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, { build_assert!(idx < Self::SIZE); @@ -507,12 +515,13 @@ pub(crate) fn write<const SIZE: usize, T>( /// Read the array register at index `idx` in `io` and run `f` on its value to obtain a /// new value to write back. #[inline(always)] - pub(crate) fn update<const SIZE: usize, T, F>( + pub(crate) fn update<T, I, F>( io: &T, idx: usize, f: F, ) where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, F: ::core::ops::FnOnce(Self) -> Self, { let reg = f(Self::read(io, idx)); @@ -524,11 +533,12 @@ pub(crate) fn update<const SIZE: usize, T, F>( /// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the /// access was out-of-bounds. #[inline(always)] - pub(crate) fn try_read<const SIZE: usize, T>( + pub(crate) fn try_read<T, I>( io: &T, idx: usize, ) -> ::kernel::error::Result<Self> where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, { if idx < Self::SIZE { Ok(Self::read(io, idx)) @@ -542,12 +552,13 @@ pub(crate) fn try_read<const SIZE: usize, T>( /// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the /// access was out-of-bounds. #[inline(always)] - pub(crate) fn try_write<const SIZE: usize, T>( + pub(crate) fn try_write<T, I>( self, io: &T, idx: usize, ) -> ::kernel::error::Result where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, { if idx < Self::SIZE { Ok(self.write(io, idx)) @@ -562,12 +573,13 @@ pub(crate) fn try_write<const SIZE: usize, T>( /// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the /// access was out-of-bounds. #[inline(always)] - pub(crate) fn try_update<const SIZE: usize, T, F>( + pub(crate) fn try_update<T, I, F>( io: &T, idx: usize, f: F, ) -> ::kernel::error::Result where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, F: ::core::ops::FnOnce(Self) -> Self, { if idx < Self::SIZE { @@ -593,13 +605,14 @@ impl $name { /// Read the array register at index `idx` from `io`, using the base address provided /// by `base` and adding the register's offset to it. #[inline(always)] - pub(crate) fn read<const SIZE: usize, T, B>( + pub(crate) fn read<T, I, B>( io: &T, #[allow(unused_variables)] base: &B, idx: usize, ) -> Self where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, B: crate::regs::macros::RegisterBase<$base>, { build_assert!(idx < Self::SIZE); @@ -614,14 +627,15 @@ pub(crate) fn read<const SIZE: usize, T, B>( /// Write the value contained in `self` to `io`, using the base address provided by /// `base` and adding the offset of array register `idx` to it. #[inline(always)] - pub(crate) fn write<const SIZE: usize, T, B>( + pub(crate) fn write<T, I, B>( self, io: &T, #[allow(unused_variables)] base: &B, idx: usize ) where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, B: crate::regs::macros::RegisterBase<$base>, { build_assert!(idx < Self::SIZE); @@ -636,13 +650,14 @@ pub(crate) fn write<const SIZE: usize, T, B>( /// by `base` and adding the register's offset to it, then run `f` on its value to /// obtain a new value to write back. #[inline(always)] - pub(crate) fn update<const SIZE: usize, T, B, F>( + pub(crate) fn update<T, I, B, F>( io: &T, base: &B, idx: usize, f: F, ) where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, B: crate::regs::macros::RegisterBase<$base>, F: ::core::ops::FnOnce(Self) -> Self, { @@ -656,12 +671,13 @@ pub(crate) fn update<const SIZE: usize, T, B, F>( /// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the /// access was out-of-bounds. #[inline(always)] - pub(crate) fn try_read<const SIZE: usize, T, B>( + pub(crate) fn try_read<T, I, B>( io: &T, base: &B, idx: usize, ) -> ::kernel::error::Result<Self> where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, B: crate::regs::macros::RegisterBase<$base>, { if idx < Self::SIZE { @@ -677,13 +693,14 @@ pub(crate) fn try_read<const SIZE: usize, T, B>( /// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the /// access was out-of-bounds. #[inline(always)] - pub(crate) fn try_write<const SIZE: usize, T, B>( + pub(crate) fn try_write<T, I, B>( self, io: &T, base: &B, idx: usize, ) -> ::kernel::error::Result where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, B: crate::regs::macros::RegisterBase<$base>, { if idx < Self::SIZE { @@ -700,13 +717,14 @@ pub(crate) fn try_write<const SIZE: usize, T, B>( /// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the /// access was out-of-bounds. #[inline(always)] - pub(crate) fn try_update<const SIZE: usize, T, B, F>( + pub(crate) fn try_update<T, I, B, F>( io: &T, base: &B, idx: usize, f: F, ) -> ::kernel::error::Result where - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, + T: ::core::ops::Deref<Target = I>, + I: ::kernel::io::IoKnownSize + ::kernel::io::IoCapable<u32>, B: crate::regs::macros::RegisterBase<$base>, F: ::core::ops::FnOnce(Self) -> Self, { diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs index abf423560ff4..fe33b519e4d8 100644 --- a/drivers/gpu/nova-core/vbios.rs +++ b/drivers/gpu/nova-core/vbios.rs @@ -6,6 +6,7 @@ use kernel::{ device, + io::Io, prelude::*, ptr::{ Alignable, diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs index e3b7e77356fc..616ca398b2c5 100644 --- a/drivers/pwm/pwm_th1520.rs +++ b/drivers/pwm/pwm_th1520.rs @@ -26,7 +26,10 @@ clk::Clk, device::{Bound, Core, Device}, devres, - io::mem::IoMem, + io::{ + mem::IoMem, + Io, // + }, of, platform, prelude::*, pwm, time, diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 43089511bf76..cdc49677022a 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -74,14 +74,17 @@ struct Inner<T: Send> { /// devres::Devres, /// io::{ /// Io, -/// IoRaw, -/// PhysAddr, +/// IoKnownSize, +/// Mmio, +/// MmioRaw, +/// PhysAddr, // /// }, +/// prelude::*, /// }; /// use core::ops::Deref; /// /// // See also [`pci::Bar`] for a real example. -/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>); +/// struct IoMem<const SIZE: usize>(MmioRaw<SIZE>); /// /// impl<const SIZE: usize> IoMem<SIZE> { /// /// # Safety @@ -96,7 +99,7 @@ struct Inner<T: Send> { /// return Err(ENOMEM); /// } /// -/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?)) +/// Ok(IoMem(MmioRaw::new(addr as usize, SIZE)?)) /// } /// } /// @@ -108,11 +111,11 @@ struct Inner<T: Send> { /// } /// /// impl<const SIZE: usize> Deref for IoMem<SIZE> { -/// type Target = Io<SIZE>; +/// type Target = Mmio<SIZE>; /// /// fn deref(&self) -> &Self::Target { /// // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`. -/// unsafe { Io::from_raw(&self.0) } +/// unsafe { Mmio::from_raw(&self.0) } /// } /// } /// # fn no_run(dev: &Device<Bound>) -> Result<(), Error> { @@ -258,6 +261,10 @@ pub fn device(&self) -> &Device { /// use kernel::{ /// device::Core, /// devres::Devres, + /// io::{ + /// Io, + /// IoKnownSize, // + /// }, /// pci, // /// }; /// diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index a97eb44a9a87..687d32572d6b 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -32,16 +32,16 @@ /// By itself, the existence of an instance of this structure does not provide any guarantees that /// the represented MMIO region does exist or is properly mapped. /// -/// Instead, the bus specific MMIO implementation must convert this raw representation into an `Io` -/// instance providing the actual memory accessors. Only by the conversion into an `Io` structure -/// any guarantees are given. -pub struct IoRaw<const SIZE: usize = 0> { +/// Instead, the bus specific MMIO implementation must convert this raw representation into an +/// `Mmio` instance providing the actual memory accessors. Only by the conversion into an `Mmio` +/// structure any guarantees are given. +pub struct MmioRaw<const SIZE: usize = 0> { addr: usize, maxsize: usize, } -impl<const SIZE: usize> IoRaw<SIZE> { - /// Returns a new `IoRaw` instance on success, an error otherwise. +impl<const SIZE: usize> MmioRaw<SIZE> { + /// Returns a new `MmioRaw` instance on success, an error otherwise. pub fn new(addr: usize, maxsize: usize) -> Result<Self> { if maxsize < SIZE { return Err(EINVAL); @@ -81,14 +81,16 @@ pub fn maxsize(&self) -> usize { /// ffi::c_void, /// io::{ /// Io, -/// IoRaw, +/// IoKnownSize, +/// Mmio, +/// MmioRaw, /// PhysAddr, /// }, /// }; /// use core::ops::Deref; /// /// // See also `pci::Bar` for a real example. -/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>); +/// struct IoMem<const SIZE: usize>(MmioRaw<SIZE>); /// /// impl<const SIZE: usize> IoMem<SIZE> { /// /// # Safety @@ -103,7 +105,7 @@ pub fn maxsize(&self) -> usize { /// return Err(ENOMEM); /// } /// -/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?)) +/// Ok(IoMem(MmioRaw::new(addr as usize, SIZE)?)) /// } /// } /// @@ -115,11 +117,11 @@ pub fn maxsize(&self) -> usize { /// } /// /// impl<const SIZE: usize> Deref for IoMem<SIZE> { -/// type Target = Io<SIZE>; +/// type Target = Mmio<SIZE>; /// /// fn deref(&self) -> &Self::Target { /// // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`. -/// unsafe { Io::from_raw(&self.0) } +/// unsafe { Mmio::from_raw(&self.0) } /// } /// } /// @@ -133,29 +135,31 @@ pub fn maxsize(&self) -> usize { /// # } /// ``` #[repr(transparent)] -pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>); +pub struct Mmio<const SIZE: usize = 0>(MmioRaw<SIZE>); macro_rules! define_read { - ($(#[$attr:meta])* $name:ident, $try_name:ident, $c_fn:ident -> $type_name:ty) => { + (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $c_fn:ident -> $type_name:ty) => { /// Read IO data from a given offset known at compile time. /// /// Bound checks are performed on compile time, hence if the offset is not known at compile /// time, the build will fail. $(#[$attr])* #[inline] - pub fn $name(&self, offset: usize) -> $type_name { + $vis fn $name(&self, offset: usize) -> $type_name { let addr = self.io_addr_assert::<$type_name>(offset); // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. unsafe { bindings::$c_fn(addr as *const c_void) } } + }; + (fallible, $(#[$attr:meta])* $vis:vis $try_name:ident, $c_fn:ident -> $type_name:ty) => { /// Read IO data from a given offset. /// /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is /// out of bounds. $(#[$attr])* - pub fn $try_name(&self, offset: usize) -> Result<$type_name> { + $vis fn $try_name(&self, offset: usize) -> Result<$type_name> { let addr = self.io_addr::<$type_name>(offset)?; // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. @@ -163,74 +167,95 @@ pub fn $try_name(&self, offset: usize) -> Result<$type_name> { } }; } +pub(crate) use define_read; macro_rules! define_write { - ($(#[$attr:meta])* $name:ident, $try_name:ident, $c_fn:ident <- $type_name:ty) => { + (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $c_fn:ident <- $type_name:ty) => { /// Write IO data from a given offset known at compile time. /// /// Bound checks are performed on compile time, hence if the offset is not known at compile /// time, the build will fail. $(#[$attr])* #[inline] - pub fn $name(&self, value: $type_name, offset: usize) { + $vis fn $name(&self, value: $type_name, offset: usize) { let addr = self.io_addr_assert::<$type_name>(offset); // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. unsafe { bindings::$c_fn(value, addr as *mut c_void) } } + }; + (fallible, $(#[$attr:meta])* $vis:vis $try_name:ident, $c_fn:ident <- $type_name:ty) => { /// Write IO data from a given offset. /// /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is /// out of bounds. $(#[$attr])* - pub fn $try_name(&self, value: $type_name, offset: usize) -> Result { + $vis fn $try_name(&self, value: $type_name, offset: usize) -> Result { let addr = self.io_addr::<$type_name>(offset)?; // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - unsafe { bindings::$c_fn(value, addr as *mut c_void) } + unsafe { bindings::$c_fn(value, addr as *mut c_void) }; Ok(()) } }; } - -impl<const SIZE: usize> Io<SIZE> { - /// Converts an `IoRaw` into an `Io` instance, providing the accessors to the MMIO mapping. - /// - /// # Safety - /// - /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size - /// `maxsize`. - pub unsafe fn from_raw(raw: &IoRaw<SIZE>) -> &Self { - // SAFETY: `Io` is a transparent wrapper around `IoRaw`. - unsafe { &*core::ptr::from_ref(raw).cast() } +pub(crate) use define_write; + +/// Checks whether an access of type `U` at the given `offset` +/// is valid within this region. +#[inline] +const fn offset_valid<U>(offset: usize, size: usize) -> bool { + let type_size = core::mem::size_of::<U>(); + if let Some(end) = offset.checked_add(type_size) { + end <= size && offset % type_size == 0 + } else { + false } +} + +/// Marker trait indicating that an I/O backend supports operations of a certain type. +/// +/// Different I/O backends can implement this trait to expose only the operations they support. +/// +/// For example, a PCI configuration space may implement `IoCapable<u8>`, `IoCapable<u16>`, +/// and `IoCapable<u32>`, but not `IoCapable<u64>`, while an MMIO region on a 64-bit +/// system might implement all four. +pub trait IoCapable<T> {} + +/// Types implementing this trait (e.g. MMIO BARs or PCI config regions) +/// can perform I/O operations on regions of memory. +/// +/// This is an abstract representation to be implemented by arbitrary I/O +/// backends (e.g. MMIO, PCI config space, etc.). +/// +/// The [`Io`] trait provides: +/// - Base address and size information +/// - Helper methods for offset validation and address calculation +/// - Fallible (runtime checked) accessors for different data widths +/// +/// Which I/O methods are available depends on which [`IoCapable<T>`] traits +/// are implemented for the type. +/// +/// # Examples +/// +/// For MMIO regions, all widths (u8, u16, u32, and u64 on 64-bit systems) are typically +/// supported. For PCI configuration space, u8, u16, and u32 are supported but u64 is not. +pub trait Io { + /// Minimum usable size of this region. + const MIN_SIZE: usize; /// Returns the base address of this mapping. - #[inline] - pub fn addr(&self) -> usize { - self.0.addr() - } + fn addr(&self) -> usize; /// Returns the maximum size of this mapping. - #[inline] - pub fn maxsize(&self) -> usize { - self.0.maxsize() - } - - #[inline] - const fn offset_valid<U>(offset: usize, size: usize) -> bool { - let type_size = core::mem::size_of::<U>(); - if let Some(end) = offset.checked_add(type_size) { - end <= size && offset % type_size == 0 - } else { - false - } - } + fn maxsize(&self) -> usize; + /// Returns the absolute I/O address for a given `offset`, + /// performing runtime bound checks. #[inline] fn io_addr<U>(&self, offset: usize) -> Result<usize> { - if !Self::offset_valid::<U>(offset, self.maxsize()) { + if !offset_valid::<U>(offset, self.maxsize()) { return Err(EINVAL); } @@ -239,50 +264,285 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> { self.addr().checked_add(offset).ok_or(EINVAL) } + /// Returns the absolute I/O address for a given `offset`, + /// performing compile-time bound checks. #[inline] fn io_addr_assert<U>(&self, offset: usize) -> usize { - build_assert!(Self::offset_valid::<U>(offset, SIZE)); + build_assert!(offset_valid::<U>(offset, Self::MIN_SIZE)); self.addr() + offset } - define_read!(read8, try_read8, readb -> u8); - define_read!(read16, try_read16, readw -> u16); - define_read!(read32, try_read32, readl -> u32); + /// Fallible 8-bit read with runtime bounds check. + #[inline(always)] + fn try_read8(&self, _offset: usize) -> Result<u8> + where + Self: IoCapable<u8>, + { + build_error!("Backend does not support fallible 8-bit read") + } + + /// Fallible 16-bit read with runtime bounds check. + #[inline(always)] + fn try_read16(&self, _offset: usize) -> Result<u16> + where + Self: IoCapable<u16>, + { + build_error!("Backend does not support fallible 16-bit read") + } + + /// Fallible 32-bit read with runtime bounds check. + #[inline(always)] + fn try_read32(&self, _offset: usize) -> Result<u32> + where + Self: IoCapable<u32>, + { + build_error!("Backend does not support fallible 32-bit read") + } + + /// Fallible 64-bit read with runtime bounds check. + #[inline(always)] + fn try_read64(&self, _offset: usize) -> Result<u64> + where + Self: IoCapable<u64>, + { + build_error!("Backend does not support fallible 64-bit read") + } + + /// Fallible 8-bit write with runtime bounds check. + #[inline(always)] + fn try_write8(&self, _value: u8, _offset: usize) -> Result + where + Self: IoCapable<u8>, + { + build_error!("Backend does not support fallible 8-bit write") + } + + /// Fallible 16-bit write with runtime bounds check. + #[inline(always)] + fn try_write16(&self, _value: u16, _offset: usize) -> Result + where + Self: IoCapable<u16>, + { + build_error!("Backend does not support fallible 16-bit write") + } + + /// Fallible 32-bit write with runtime bounds check. + #[inline(always)] + fn try_write32(&self, _value: u32, _offset: usize) -> Result + where + Self: IoCapable<u32>, + { + build_error!("Backend does not support fallible 32-bit write") + } + + /// Fallible 64-bit write with runtime bounds check. + #[inline(always)] + fn try_write64(&self, _value: u64, _offset: usize) -> Result + where + Self: IoCapable<u64>, + { + build_error!("Backend does not support fallible 64-bit write") + } + + /// Infallible 8-bit read with compile-time bounds check. + #[inline(always)] + fn read8(&self, _offset: usize) -> u8 + where + Self: IoKnownSize + IoCapable<u8>, + { + build_error!("Backend does not support infallible 8-bit read") + } + + /// Infallible 16-bit read with compile-time bounds check. + #[inline(always)] + fn read16(&self, _offset: usize) -> u16 + where + Self: IoKnownSize + IoCapable<u16>, + { + build_error!("Backend does not support infallible 16-bit read") + } + + /// Infallible 32-bit read with compile-time bounds check. + #[inline(always)] + fn read32(&self, _offset: usize) -> u32 + where + Self: IoKnownSize + IoCapable<u32>, + { + build_error!("Backend does not support infallible 32-bit read") + } + + /// Infallible 64-bit read with compile-time bounds check. + #[inline(always)] + fn read64(&self, _offset: usize) -> u64 + where + Self: IoKnownSize + IoCapable<u64>, + { + build_error!("Backend does not support infallible 64-bit read") + } + + /// Infallible 8-bit write with compile-time bounds check. + #[inline(always)] + fn write8(&self, _value: u8, _offset: usize) + where + Self: IoKnownSize + IoCapable<u8>, + { + build_error!("Backend does not support infallible 8-bit write") + } + + /// Infallible 16-bit write with compile-time bounds check. + #[inline(always)] + fn write16(&self, _value: u16, _offset: usize) + where + Self: IoKnownSize + IoCapable<u16>, + { + build_error!("Backend does not support infallible 16-bit write") + } + + /// Infallible 32-bit write with compile-time bounds check. + #[inline(always)] + fn write32(&self, _value: u32, _offset: usize) + where + Self: IoKnownSize + IoCapable<u32>, + { + build_error!("Backend does not support infallible 32-bit write") + } + + /// Infallible 64-bit write with compile-time bounds check. + #[inline(always)] + fn write64(&self, _value: u64, _offset: usize) + where + Self: IoKnownSize + IoCapable<u64>, + { + build_error!("Backend does not support infallible 64-bit write") + } +} + +/// Marker trait for types with a known size at compile time. +/// +/// This trait is implemented by I/O backends that have a compile-time known size, +/// enabling the use of infallible I/O accessors with compile-time bounds checking. +/// +/// Types implementing this trait can use the infallible methods in [`Io`] trait +/// (e.g., `read8`, `write32`), which require `Self: IoKnownSize` bound. +pub trait IoKnownSize: Io {} + +// MMIO regions support 8, 16, and 32-bit accesses. +impl<const SIZE: usize> IoCapable<u8> for Mmio<SIZE> {} +impl<const SIZE: usize> IoCapable<u16> for Mmio<SIZE> {} +impl<const SIZE: usize> IoCapable<u32> for Mmio<SIZE> {} + +// MMIO regions on 64-bit systems also support 64-bit accesses. +#[cfg(CONFIG_64BIT)] +impl<const SIZE: usize> IoCapable<u64> for Mmio<SIZE> {} + +impl<const SIZE: usize> Io for Mmio<SIZE> { + const MIN_SIZE: usize = SIZE; + + /// Returns the base address of this mapping. + #[inline] + fn addr(&self) -> usize { + self.0.addr() + } + + /// Returns the maximum size of this mapping. + #[inline] + fn maxsize(&self) -> usize { + self.0.maxsize() + } + + define_read!(fallible, try_read8, readb -> u8); + define_read!(fallible, try_read16, readw -> u16); + define_read!(fallible, try_read32, readl -> u32); define_read!( + fallible, #[cfg(CONFIG_64BIT)] - read64, try_read64, readq -> u64 ); - define_read!(read8_relaxed, try_read8_relaxed, readb_relaxed -> u8); - define_read!(read16_relaxed, try_read16_relaxed, readw_relaxed -> u16); - define_read!(read32_relaxed, try_read32_relaxed, readl_relaxed -> u32); + define_write!(fallible, try_write8, writeb <- u8); + define_write!(fallible, try_write16, writew <- u16); + define_write!(fallible, try_write32, writel <- u32); + define_write!( + fallible, + #[cfg(CONFIG_64BIT)] + try_write64, + writeq <- u64 + ); + + define_read!(infallible, read8, readb -> u8); + define_read!(infallible, read16, readw -> u16); + define_read!(infallible, read32, readl -> u32); define_read!( + infallible, #[cfg(CONFIG_64BIT)] - read64_relaxed, - try_read64_relaxed, - readq_relaxed -> u64 + read64, + readq -> u64 ); - define_write!(write8, try_write8, writeb <- u8); - define_write!(write16, try_write16, writew <- u16); - define_write!(write32, try_write32, writel <- u32); + define_write!(infallible, write8, writeb <- u8); + define_write!(infallible, write16, writew <- u16); + define_write!(infallible, write32, writel <- u32); define_write!( + infallible, #[cfg(CONFIG_64BIT)] write64, - try_write64, writeq <- u64 ); +} + +impl<const SIZE: usize> IoKnownSize for Mmio<SIZE> {} + +impl<const SIZE: usize> Mmio<SIZE> { + /// Converts an `MmioRaw` into an `Mmio` instance, providing the accessors to the MMIO mapping. + /// + /// # Safety + /// + /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size + /// `maxsize`. + pub unsafe fn from_raw(raw: &MmioRaw<SIZE>) -> &Self { + // SAFETY: `Mmio` is a transparent wrapper around `MmioRaw`. + unsafe { &*core::ptr::from_ref(raw).cast() } + } + + define_read!(infallible, pub read8_relaxed, readb_relaxed -> u8); + define_read!(infallible, pub read16_relaxed, readw_relaxed -> u16); + define_read!(infallible, pub read32_relaxed, readl_relaxed -> u32); + define_read!( + infallible, + #[cfg(CONFIG_64BIT)] + pub read64_relaxed, + readq_relaxed -> u64 + ); + + define_read!(fallible, pub try_read8_relaxed, readb_relaxed -> u8); + define_read!(fallible, pub try_read16_relaxed, readw_relaxed -> u16); + define_read!(fallible, pub try_read32_relaxed, readl_relaxed -> u32); + define_read!( + fallible, + #[cfg(CONFIG_64BIT)] + pub try_read64_relaxed, + readq_relaxed -> u64 + ); + + define_write!(infallible, pub write8_relaxed, writeb_relaxed <- u8); + define_write!(infallible, pub write16_relaxed, writew_relaxed <- u16); + define_write!(infallible, pub write32_relaxed, writel_relaxed <- u32); + define_write!( + infallible, + #[cfg(CONFIG_64BIT)] + pub write64_relaxed, + writeq_relaxed <- u64 + ); - define_write!(write8_relaxed, try_write8_relaxed, writeb_relaxed <- u8); - define_write!(write16_relaxed, try_write16_relaxed, writew_relaxed <- u16); - define_write!(write32_relaxed, try_write32_relaxed, writel_relaxed <- u32); + define_write!(fallible, pub try_write8_relaxed, writeb_relaxed <- u8); + define_write!(fallible, pub try_write16_relaxed, writew_relaxed <- u16); + define_write!(fallible, pub try_write32_relaxed, writel_relaxed <- u32); define_write!( + fallible, #[cfg(CONFIG_64BIT)] - write64_relaxed, - try_write64_relaxed, + pub try_write64_relaxed, writeq_relaxed <- u64 ); } diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs index e4878c131c6d..620022cff401 100644 --- a/rust/kernel/io/mem.rs +++ b/rust/kernel/io/mem.rs @@ -16,8 +16,8 @@ Region, Resource, // }, - Io, - IoRaw, // + Mmio, + MmioRaw, // }, prelude::*, }; @@ -212,7 +212,7 @@ pub fn new<'a>(io_request: IoRequest<'a>) -> impl PinInit<Devres<Self>, Error> + } impl<const SIZE: usize> Deref for ExclusiveIoMem<SIZE> { - type Target = Io<SIZE>; + type Target = Mmio<SIZE>; fn deref(&self) -> &Self::Target { &self.iomem @@ -226,10 +226,10 @@ fn deref(&self) -> &Self::Target { /// /// # Invariants /// -/// [`IoMem`] always holds an [`IoRaw`] instance that holds a valid pointer to the +/// [`IoMem`] always holds an [`MmioRaw`] instance that holds a valid pointer to the /// start of the I/O memory mapped region. pub struct IoMem<const SIZE: usize = 0> { - io: IoRaw<SIZE>, + io: MmioRaw<SIZE>, } impl<const SIZE: usize> IoMem<SIZE> { @@ -264,7 +264,7 @@ fn ioremap(resource: &Resource) -> Result<Self> { return Err(ENOMEM); } - let io = IoRaw::new(addr as usize, size)?; + let io = MmioRaw::new(addr as usize, size)?; let io = IoMem { io }; Ok(io) @@ -287,10 +287,10 @@ fn drop(&mut self) { } impl<const SIZE: usize> Deref for IoMem<SIZE> { - type Target = Io<SIZE>; + type Target = Mmio<SIZE>; fn deref(&self) -> &Self::Target { // SAFETY: Safe as by the invariant of `IoMem`. - unsafe { Io::from_raw(&self.io) } + unsafe { Mmio::from_raw(&self.io) } } } diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs index b1a2570364f4..75d1b3e8596c 100644 --- a/rust/kernel/io/poll.rs +++ b/rust/kernel/io/poll.rs @@ -45,12 +45,16 @@ /// # Examples /// /// ```no_run -/// use kernel::io::{Io, poll::read_poll_timeout}; +/// use kernel::io::{ +/// Io, +/// Mmio, +/// poll::read_poll_timeout, // +/// }; /// use kernel::time::Delta; /// /// const HW_READY: u16 = 0x01; /// -/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result { +/// fn wait_for_hardware<const SIZE: usize>(io: &Mmio<SIZE>) -> Result { /// read_poll_timeout( /// // The `op` closure reads the value of a specific status register. /// || io.try_read16(0x1000), @@ -128,12 +132,16 @@ pub fn read_poll_timeout<Op, Cond, T>( /// # Examples /// /// ```no_run -/// use kernel::io::{poll::read_poll_timeout_atomic, Io}; +/// use kernel::io::{ +/// Io, +/// Mmio, +/// poll::read_poll_timeout_atomic, // +/// }; /// use kernel::time::Delta; /// /// const HW_READY: u16 = 0x01; /// -/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result { +/// fn wait_for_hardware<const SIZE: usize>(io: &Mmio<SIZE>) -> Result { /// read_poll_timeout_atomic( /// // The `op` closure reads the value of a specific status register. /// || io.try_read16(0x1000), diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs index 70e3854e7d8d..e3377397666e 100644 --- a/rust/kernel/pci/io.rs +++ b/rust/kernel/pci/io.rs @@ -8,8 +8,8 @@ device, devres::Devres, io::{ - Io, - IoRaw, // + Mmio, + MmioRaw, // }, prelude::*, sync::aref::ARef, // @@ -27,7 +27,7 @@ /// memory mapped PCI BAR and its size. pub struct Bar<const SIZE: usize = 0> { pdev: ARef<Device>, - io: IoRaw<SIZE>, + io: MmioRaw<SIZE>, num: i32, } @@ -63,7 +63,7 @@ pub(super) fn new(pdev: &Device, num: u32, name: &CStr) -> Result<Self> { return Err(ENOMEM); } - let io = match IoRaw::new(ioptr, len as usize) { + let io = match MmioRaw::new(ioptr, len as usize) { Ok(io) => io, Err(err) => { // SAFETY: @@ -117,11 +117,11 @@ fn drop(&mut self) { } impl<const SIZE: usize> Deref for Bar<SIZE> { - type Target = Io<SIZE>; + type Target = Mmio<SIZE>; fn deref(&self) -> &Self::Target { // SAFETY: By the type invariant of `Self`, the MMIO range in `self.io` is properly mapped. - unsafe { Io::from_raw(&self.io) } + unsafe { Mmio::from_raw(&self.io) } } } diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs index ef04c6401e6a..38c949efce38 100644 --- a/samples/rust/rust_driver_pci.rs +++ b/samples/rust/rust_driver_pci.rs @@ -7,6 +7,7 @@ use kernel::{ device::Core, devres::Devres, + io::Io, pci, prelude::*, sync::aref::ARef, // -- 2.51.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation 2026-01-21 20:22 ` [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation Zhi Wang @ 2026-01-22 4:28 ` Alexandre Courbot 2026-01-22 11:52 ` Gary Guo ` (2 subsequent siblings) 3 siblings, 0 replies; 25+ messages in thread From: Alexandre Courbot @ 2026-01-22 4:28 UTC (permalink / raw) To: Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, joelagnelf, jhubbard, zhiwang, daniel.almeida On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote: > The previous Io<SIZE> type combined both the generic I/O access helpers > and MMIO implementation details in a single struct. This coupling prevented > reusing the I/O helpers for other backends, such as PCI configuration > space. > > Establish a clean separation between the I/O interface and concrete > backends by separating generic I/O helpers from MMIO implementation. > > Introduce a new trait hierarchy to handle different access capabilities: > > - IoCapable<T>: A marker trait indicating that a backend supports I/O > operations of a certain type (u8, u16, u32, or u64). > > - Io trait: Defines fallible (try_read8, try_write8, etc.) and infallibile > (read8, write8, etc.) I/O methods with runtime bounds checking and > compile-time bounds checking. > > - IoKnownSize trait: The marker trait for types support infallible I/O > methods. > > Move the MMIO-specific logic into a dedicated Mmio<SIZE> type that > implements the Io traits. Rename IoRaw to MmioRaw and update consumers to > use the new types. > > Cc: Alexandre Courbot <acourbot@nvidia.com> > Cc: Alice Ryhl <aliceryhl@google.com> > Cc: Bjorn Helgaas <helgaas@kernel.org> > Cc: Gary Guo <gary@garyguo.net> > Cc: Danilo Krummrich <dakr@kernel.org> > Cc: John Hubbard <jhubbard@nvidia.com> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> I still hope we can make this evolve into something with less macros and more accurate error handling (maybe following my earlier proposal if its ideas are still relevant), but this is a great first step - thanks for doing it! [1] https://lore.kernel.org/all/DEMV14GBQWMC.28TXT8E5YO5NW@nvidia.com/ Reviewed-by: Alexandre Courbot <acourbot@nvidia.com> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation 2026-01-21 20:22 ` [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation Zhi Wang 2026-01-22 4:28 ` Alexandre Courbot @ 2026-01-22 11:52 ` Gary Guo 2026-01-22 12:52 ` Danilo Krummrich 2026-01-23 9:13 ` Alice Ryhl 3 siblings, 0 replies; 25+ messages in thread From: Gary Guo @ 2026-01-22 11:52 UTC (permalink / raw) To: Zhi Wang, rust-for-linux, linux-pci, linux-kernel Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida On Wed Jan 21, 2026 at 8:22 PM GMT, Zhi Wang wrote: > The previous Io<SIZE> type combined both the generic I/O access helpers > and MMIO implementation details in a single struct. This coupling prevented > reusing the I/O helpers for other backends, such as PCI configuration > space. > > Establish a clean separation between the I/O interface and concrete > backends by separating generic I/O helpers from MMIO implementation. > > Introduce a new trait hierarchy to handle different access capabilities: > > - IoCapable<T>: A marker trait indicating that a backend supports I/O > operations of a certain type (u8, u16, u32, or u64). > > - Io trait: Defines fallible (try_read8, try_write8, etc.) and infallibile > (read8, write8, etc.) I/O methods with runtime bounds checking and > compile-time bounds checking. > > - IoKnownSize trait: The marker trait for types support infallible I/O > methods. > > Move the MMIO-specific logic into a dedicated Mmio<SIZE> type that > implements the Io traits. Rename IoRaw to MmioRaw and update consumers to > use the new types. > > Cc: Alexandre Courbot <acourbot@nvidia.com> > Cc: Alice Ryhl <aliceryhl@google.com> > Cc: Bjorn Helgaas <helgaas@kernel.org> > Cc: Gary Guo <gary@garyguo.net> > Cc: Danilo Krummrich <dakr@kernel.org> > Cc: John Hubbard <jhubbard@nvidia.com> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> Reviewed-by: Gary Guo <gary@garyguo.net> > --- > drivers/gpu/drm/tyr/regs.rs | 1 + > drivers/gpu/nova-core/gsp/sequencer.rs | 5 +- > drivers/gpu/nova-core/regs/macros.rs | 90 +++--- > drivers/gpu/nova-core/vbios.rs | 1 + > drivers/pwm/pwm_th1520.rs | 5 +- > rust/kernel/devres.rs | 19 +- > rust/kernel/io.rs | 398 ++++++++++++++++++++----- > rust/kernel/io/mem.rs | 16 +- > rust/kernel/io/poll.rs | 16 +- > rust/kernel/pci/io.rs | 12 +- > samples/rust/rust_driver_pci.rs | 1 + > 11 files changed, 433 insertions(+), 131 deletions(-) ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation 2026-01-21 20:22 ` [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation Zhi Wang 2026-01-22 4:28 ` Alexandre Courbot 2026-01-22 11:52 ` Gary Guo @ 2026-01-22 12:52 ` Danilo Krummrich 2026-01-23 9:13 ` Alice Ryhl 3 siblings, 0 replies; 25+ messages in thread From: Danilo Krummrich @ 2026-01-22 12:52 UTC (permalink / raw) To: Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida, ukleinek, m.wilczynski (Cc: Uwe, Michal) On Wed Jan 21, 2026 at 9:22 PM CET, Zhi Wang wrote: > The previous Io<SIZE> type combined both the generic I/O access helpers > and MMIO implementation details in a single struct. This coupling prevented > reusing the I/O helpers for other backends, such as PCI configuration > space. > > Establish a clean separation between the I/O interface and concrete > backends by separating generic I/O helpers from MMIO implementation. > > Introduce a new trait hierarchy to handle different access capabilities: > > - IoCapable<T>: A marker trait indicating that a backend supports I/O > operations of a certain type (u8, u16, u32, or u64). > > - Io trait: Defines fallible (try_read8, try_write8, etc.) and infallibile > (read8, write8, etc.) I/O methods with runtime bounds checking and > compile-time bounds checking. > > - IoKnownSize trait: The marker trait for types support infallible I/O > methods. > > Move the MMIO-specific logic into a dedicated Mmio<SIZE> type that > implements the Io traits. Rename IoRaw to MmioRaw and update consumers to > use the new types. > > Cc: Alexandre Courbot <acourbot@nvidia.com> > Cc: Alice Ryhl <aliceryhl@google.com> > Cc: Bjorn Helgaas <helgaas@kernel.org> > Cc: Gary Guo <gary@garyguo.net> > Cc: Danilo Krummrich <dakr@kernel.org> > Cc: John Hubbard <jhubbard@nvidia.com> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> > --- > drivers/gpu/drm/tyr/regs.rs | 1 + > drivers/gpu/nova-core/gsp/sequencer.rs | 5 +- > drivers/gpu/nova-core/regs/macros.rs | 90 +++--- > drivers/gpu/nova-core/vbios.rs | 1 + > drivers/pwm/pwm_th1520.rs | 5 +- > rust/kernel/devres.rs | 19 +- > rust/kernel/io.rs | 398 ++++++++++++++++++++----- > rust/kernel/io/mem.rs | 16 +- > rust/kernel/io/poll.rs | 16 +- > rust/kernel/pci/io.rs | 12 +- > samples/rust/rust_driver_pci.rs | 1 + > 11 files changed, 433 insertions(+), 131 deletions(-) <snip> > diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs > index e3b7e77356fc..616ca398b2c5 100644 > --- a/drivers/pwm/pwm_th1520.rs > +++ b/drivers/pwm/pwm_th1520.rs > @@ -26,7 +26,10 @@ > clk::Clk, > device::{Bound, Core, Device}, > devres, > - io::mem::IoMem, > + io::{ > + mem::IoMem, > + Io, // > + }, > of, platform, > prelude::*, > pwm, time, ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation 2026-01-21 20:22 ` [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation Zhi Wang ` (2 preceding siblings ...) 2026-01-22 12:52 ` Danilo Krummrich @ 2026-01-23 9:13 ` Alice Ryhl 3 siblings, 0 replies; 25+ messages in thread From: Alice Ryhl @ 2026-01-23 9:13 UTC (permalink / raw) To: Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, dakr, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida On Wed, Jan 21, 2026 at 10:22:08PM +0200, Zhi Wang wrote: > The previous Io<SIZE> type combined both the generic I/O access helpers > and MMIO implementation details in a single struct. This coupling prevented > reusing the I/O helpers for other backends, such as PCI configuration > space. > > Establish a clean separation between the I/O interface and concrete > backends by separating generic I/O helpers from MMIO implementation. > > Introduce a new trait hierarchy to handle different access capabilities: > > - IoCapable<T>: A marker trait indicating that a backend supports I/O > operations of a certain type (u8, u16, u32, or u64). > > - Io trait: Defines fallible (try_read8, try_write8, etc.) and infallibile > (read8, write8, etc.) I/O methods with runtime bounds checking and > compile-time bounds checking. > > - IoKnownSize trait: The marker trait for types support infallible I/O > methods. > > Move the MMIO-specific logic into a dedicated Mmio<SIZE> type that > implements the Io traits. Rename IoRaw to MmioRaw and update consumers to > use the new types. > > Cc: Alexandre Courbot <acourbot@nvidia.com> > Cc: Alice Ryhl <aliceryhl@google.com> > Cc: Bjorn Helgaas <helgaas@kernel.org> > Cc: Gary Guo <gary@garyguo.net> > Cc: Danilo Krummrich <dakr@kernel.org> > Cc: John Hubbard <jhubbard@nvidia.com> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> Reviewed-by: Alice Ryhl <aliceryhl@google.com> ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v12 3/5] rust: io: factor out MMIO read/write macros 2026-01-21 20:22 [PATCH v12 0/5] rust: pci: add config space read/write support Zhi Wang 2026-01-21 20:22 ` [PATCH v12 1/5] rust: devres: style for imports Zhi Wang 2026-01-21 20:22 ` [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation Zhi Wang @ 2026-01-21 20:22 ` Zhi Wang 2026-01-22 4:30 ` Alexandre Courbot 2026-01-21 20:22 ` [PATCH v12 4/5] rust: pci: add config space read/write support Zhi Wang ` (2 subsequent siblings) 5 siblings, 1 reply; 25+ messages in thread From: Zhi Wang @ 2026-01-21 20:22 UTC (permalink / raw) To: rust-for-linux, linux-pci, linux-kernel Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida, Zhi Wang Refactor the existing MMIO accessors to use common call macros instead of inlining the bindings calls in each `define_{read,write}!` expansion. This factoring separates the common offset/bounds checks from the low-level call pattern, making it easier to add additional I/O accessor families. No functional change intended. Cc: Alexandre Courbot <acourbot@nvidia.com> Signed-off-by: Zhi Wang <zhiw@nvidia.com> --- rust/kernel/io.rs | 147 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 102 insertions(+), 45 deletions(-) diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index 687d32572d6b..fcd1b156a14a 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -137,8 +137,65 @@ pub fn maxsize(&self) -> usize { #[repr(transparent)] pub struct Mmio<const SIZE: usize = 0>(MmioRaw<SIZE>); +/// Internal helper macros used to invoke C MMIO read functions. +/// +/// This macro is intended to be used by higher-level MMIO access macros (define_read) and provides +/// a unified expansion for infallible vs. fallible read semantics. It emits a direct call into the +/// corresponding C helper and performs the required cast to the Rust return type. +/// +/// # Parameters +/// +/// * `$c_fn` – The C function performing the MMIO read. +/// * `$self` – The I/O backend object. +/// * `$ty` – The type of the value to be read. +/// * `$addr` – The MMIO address to read. +/// +/// This macro does not perform any validation; all invariants must be upheld by the higher-level +/// abstraction invoking it. +macro_rules! call_mmio_read { + (infallible, $c_fn:ident, $self:ident, $type:ty, $addr:expr) => { + // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. + unsafe { bindings::$c_fn($addr as *const c_void) as $type } + }; + + (fallible, $c_fn:ident, $self:ident, $type:ty, $addr:expr) => {{ + // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. + Ok(unsafe { bindings::$c_fn($addr as *const c_void) as $type }) + }}; +} + +/// Internal helper macros used to invoke C MMIO write functions. +/// +/// This macro is intended to be used by higher-level MMIO access macros (define_write) and provides +/// a unified expansion for infallible vs. fallible write semantics. It emits a direct call into the +/// corresponding C helper and performs the required cast to the Rust return type. +/// +/// # Parameters +/// +/// * `$c_fn` – The C function performing the MMIO write. +/// * `$self` – The I/O backend object. +/// * `$ty` – The type of the written value. +/// * `$addr` – The MMIO address to write. +/// * `$value` – The value to write. +/// +/// This macro does not perform any validation; all invariants must be upheld by the higher-level +/// abstraction invoking it. +macro_rules! call_mmio_write { + (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => { + // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. + unsafe { bindings::$c_fn($value, $addr as *mut c_void) } + }; + + (fallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {{ + // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. + unsafe { bindings::$c_fn($value, $addr as *mut c_void) }; + Ok(()) + }}; +} + macro_rules! define_read { - (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $c_fn:ident -> $type_name:ty) => { + (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $call_macro:ident($c_fn:ident) -> + $type_name:ty) => { /// Read IO data from a given offset known at compile time. /// /// Bound checks are performed on compile time, hence if the offset is not known at compile @@ -148,12 +205,13 @@ macro_rules! define_read { $vis fn $name(&self, offset: usize) -> $type_name { let addr = self.io_addr_assert::<$type_name>(offset); - // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - unsafe { bindings::$c_fn(addr as *const c_void) } + // SAFETY: By the type invariant `addr` is a valid address for IO operations. + $call_macro!(infallible, $c_fn, self, $type_name, addr) } }; - (fallible, $(#[$attr:meta])* $vis:vis $try_name:ident, $c_fn:ident -> $type_name:ty) => { + (fallible, $(#[$attr:meta])* $vis:vis $try_name:ident, $call_macro:ident($c_fn:ident) -> + $type_name:ty) => { /// Read IO data from a given offset. /// /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is @@ -162,15 +220,16 @@ macro_rules! define_read { $vis fn $try_name(&self, offset: usize) -> Result<$type_name> { let addr = self.io_addr::<$type_name>(offset)?; - // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - Ok(unsafe { bindings::$c_fn(addr as *const c_void) }) + // SAFETY: By the type invariant `addr` is a valid address for IO operations. + $call_macro!(fallible, $c_fn, self, $type_name, addr) } }; } pub(crate) use define_read; macro_rules! define_write { - (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $c_fn:ident <- $type_name:ty) => { + (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $call_macro:ident($c_fn:ident) <- + $type_name:ty) => { /// Write IO data from a given offset known at compile time. /// /// Bound checks are performed on compile time, hence if the offset is not known at compile @@ -180,12 +239,12 @@ macro_rules! define_write { $vis fn $name(&self, value: $type_name, offset: usize) { let addr = self.io_addr_assert::<$type_name>(offset); - // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - unsafe { bindings::$c_fn(value, addr as *mut c_void) } + $call_macro!(infallible, $c_fn, self, $type_name, addr, value); } }; - (fallible, $(#[$attr:meta])* $vis:vis $try_name:ident, $c_fn:ident <- $type_name:ty) => { + (fallible, $(#[$attr:meta])* $vis:vis $try_name:ident, $call_macro:ident($c_fn:ident) <- + $type_name:ty) => { /// Write IO data from a given offset. /// /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is @@ -194,9 +253,7 @@ macro_rules! define_write { $vis fn $try_name(&self, value: $type_name, offset: usize) -> Result { let addr = self.io_addr::<$type_name>(offset)?; - // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - unsafe { bindings::$c_fn(value, addr as *mut c_void) }; - Ok(()) + $call_macro!(fallible, $c_fn, self, $type_name, addr, value) } }; } @@ -451,44 +508,44 @@ fn maxsize(&self) -> usize { self.0.maxsize() } - define_read!(fallible, try_read8, readb -> u8); - define_read!(fallible, try_read16, readw -> u16); - define_read!(fallible, try_read32, readl -> u32); + define_read!(fallible, try_read8, call_mmio_read(readb) -> u8); + define_read!(fallible, try_read16, call_mmio_read(readw) -> u16); + define_read!(fallible, try_read32, call_mmio_read(readl) -> u32); define_read!( fallible, #[cfg(CONFIG_64BIT)] try_read64, - readq -> u64 + call_mmio_read(readq) -> u64 ); - define_write!(fallible, try_write8, writeb <- u8); - define_write!(fallible, try_write16, writew <- u16); - define_write!(fallible, try_write32, writel <- u32); + define_write!(fallible, try_write8, call_mmio_write(writeb) <- u8); + define_write!(fallible, try_write16, call_mmio_write(writew) <- u16); + define_write!(fallible, try_write32, call_mmio_write(writel) <- u32); define_write!( fallible, #[cfg(CONFIG_64BIT)] try_write64, - writeq <- u64 + call_mmio_write(writeq) <- u64 ); - define_read!(infallible, read8, readb -> u8); - define_read!(infallible, read16, readw -> u16); - define_read!(infallible, read32, readl -> u32); + define_read!(infallible, read8, call_mmio_read(readb) -> u8); + define_read!(infallible, read16, call_mmio_read(readw) -> u16); + define_read!(infallible, read32, call_mmio_read(readl) -> u32); define_read!( infallible, #[cfg(CONFIG_64BIT)] read64, - readq -> u64 + call_mmio_read(readq) -> u64 ); - define_write!(infallible, write8, writeb <- u8); - define_write!(infallible, write16, writew <- u16); - define_write!(infallible, write32, writel <- u32); + define_write!(infallible, write8, call_mmio_write(writeb) <- u8); + define_write!(infallible, write16, call_mmio_write(writew) <- u16); + define_write!(infallible, write32, call_mmio_write(writel) <- u32); define_write!( infallible, #[cfg(CONFIG_64BIT)] write64, - writeq <- u64 + call_mmio_write(writeq) <- u64 ); } @@ -506,43 +563,43 @@ pub unsafe fn from_raw(raw: &MmioRaw<SIZE>) -> &Self { unsafe { &*core::ptr::from_ref(raw).cast() } } - define_read!(infallible, pub read8_relaxed, readb_relaxed -> u8); - define_read!(infallible, pub read16_relaxed, readw_relaxed -> u16); - define_read!(infallible, pub read32_relaxed, readl_relaxed -> u32); + define_read!(infallible, pub read8_relaxed, call_mmio_read(readb_relaxed) -> u8); + define_read!(infallible, pub read16_relaxed, call_mmio_read(readw_relaxed) -> u16); + define_read!(infallible, pub read32_relaxed, call_mmio_read(readl_relaxed) -> u32); define_read!( infallible, #[cfg(CONFIG_64BIT)] pub read64_relaxed, - readq_relaxed -> u64 + call_mmio_read(readq_relaxed) -> u64 ); - define_read!(fallible, pub try_read8_relaxed, readb_relaxed -> u8); - define_read!(fallible, pub try_read16_relaxed, readw_relaxed -> u16); - define_read!(fallible, pub try_read32_relaxed, readl_relaxed -> u32); + define_read!(fallible, pub try_read8_relaxed, call_mmio_read(readb_relaxed) -> u8); + define_read!(fallible, pub try_read16_relaxed, call_mmio_read(readw_relaxed) -> u16); + define_read!(fallible, pub try_read32_relaxed, call_mmio_read(readl_relaxed) -> u32); define_read!( fallible, #[cfg(CONFIG_64BIT)] pub try_read64_relaxed, - readq_relaxed -> u64 + call_mmio_read(readq_relaxed) -> u64 ); - define_write!(infallible, pub write8_relaxed, writeb_relaxed <- u8); - define_write!(infallible, pub write16_relaxed, writew_relaxed <- u16); - define_write!(infallible, pub write32_relaxed, writel_relaxed <- u32); + define_write!(infallible, pub write8_relaxed, call_mmio_write(writeb_relaxed) <- u8); + define_write!(infallible, pub write16_relaxed, call_mmio_write(writew_relaxed) <- u16); + define_write!(infallible, pub write32_relaxed, call_mmio_write(writel_relaxed) <- u32); define_write!( infallible, #[cfg(CONFIG_64BIT)] pub write64_relaxed, - writeq_relaxed <- u64 + call_mmio_write(writeq_relaxed) <- u64 ); - define_write!(fallible, pub try_write8_relaxed, writeb_relaxed <- u8); - define_write!(fallible, pub try_write16_relaxed, writew_relaxed <- u16); - define_write!(fallible, pub try_write32_relaxed, writel_relaxed <- u32); + define_write!(fallible, pub try_write8_relaxed, call_mmio_write(writeb_relaxed) <- u8); + define_write!(fallible, pub try_write16_relaxed, call_mmio_write(writew_relaxed) <- u16); + define_write!(fallible, pub try_write32_relaxed, call_mmio_write(writel_relaxed) <- u32); define_write!( fallible, #[cfg(CONFIG_64BIT)] pub try_write64_relaxed, - writeq_relaxed <- u64 + call_mmio_write(writeq_relaxed) <- u64 ); } -- 2.51.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v12 3/5] rust: io: factor out MMIO read/write macros 2026-01-21 20:22 ` [PATCH v12 3/5] rust: io: factor out MMIO read/write macros Zhi Wang @ 2026-01-22 4:30 ` Alexandre Courbot 2026-01-22 11:54 ` Gary Guo 0 siblings, 1 reply; 25+ messages in thread From: Alexandre Courbot @ 2026-01-22 4:30 UTC (permalink / raw) To: Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, joelagnelf, jhubbard, zhiwang, daniel.almeida On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote: > Refactor the existing MMIO accessors to use common call macros > instead of inlining the bindings calls in each `define_{read,write}!` > expansion. > > This factoring separates the common offset/bounds checks from the > low-level call pattern, making it easier to add additional I/O accessor > families. > > No functional change intended. > > Cc: Alexandre Courbot <acourbot@nvidia.com> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> I still want to eventually switch to a trait hierarchy that removes the need for these intricate macros, but that's a few more step ahead. For now this integrates well with the current design. Reviewed-by: Alexandre Courbot <acourbot@nvidia.com> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 3/5] rust: io: factor out MMIO read/write macros 2026-01-22 4:30 ` Alexandre Courbot @ 2026-01-22 11:54 ` Gary Guo 0 siblings, 0 replies; 25+ messages in thread From: Gary Guo @ 2026-01-22 11:54 UTC (permalink / raw) To: Alexandre Courbot, Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, joelagnelf, jhubbard, zhiwang, daniel.almeida On Thu Jan 22, 2026 at 4:30 AM GMT, Alexandre Courbot wrote: > On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote: >> Refactor the existing MMIO accessors to use common call macros >> instead of inlining the bindings calls in each `define_{read,write}!` >> expansion. >> >> This factoring separates the common offset/bounds checks from the >> low-level call pattern, making it easier to add additional I/O accessor >> families. >> >> No functional change intended. >> >> Cc: Alexandre Courbot <acourbot@nvidia.com> >> Signed-off-by: Zhi Wang <zhiw@nvidia.com> > > I still want to eventually switch to a trait hierarchy that removes the > need for these intricate macros, but that's a few more step ahead. For > now this integrates well with the current design. I am not sure a actual hierarchy is needed, but the suggestion you have to add `unchecked` method and have the bound checking be part of `Io` should remove the need of an fallible and infallible variants. I think we still need macros to actually dispatch the read/write functions anyway as they're different for each access size. Best, Gary ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v12 4/5] rust: pci: add config space read/write support 2026-01-21 20:22 [PATCH v12 0/5] rust: pci: add config space read/write support Zhi Wang ` (2 preceding siblings ...) 2026-01-21 20:22 ` [PATCH v12 3/5] rust: io: factor out MMIO read/write macros Zhi Wang @ 2026-01-21 20:22 ` Zhi Wang 2026-01-22 4:25 ` Alexandre Courbot 2026-01-22 11:59 ` Gary Guo 2026-01-21 20:22 ` [PATCH v12 5/5] sample: rust: pci: add tests for config space routines Zhi Wang 2026-01-24 0:12 ` [PATCH v12 0/5] rust: pci: add config space read/write support Danilo Krummrich 5 siblings, 2 replies; 25+ messages in thread From: Zhi Wang @ 2026-01-21 20:22 UTC (permalink / raw) To: rust-for-linux, linux-pci, linux-kernel Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida, Zhi Wang Drivers might need to access PCI config space for querying capability structures and access the registers inside the structures. For Rust drivers need to access PCI config space, the Rust PCI abstraction needs to support it in a way that upholds Rust's safety principles. Introduce a `ConfigSpace` wrapper in Rust PCI abstraction to provide safe accessors for PCI config space. The new type implements the `Io` trait and `IoCapable<T>` for u8, u16, and u32 to share offset validation and bound-checking logic with other I/O backends. The `ConfigSpace` type uses marker types (`Normal` and `Extended`) to represent configuration space sizes at the type level. Cc: Alexandre Courbot <acourbot@nvidia.com> Cc: Danilo Krummrich <dakr@kernel.org> Cc: Gary Guo <gary@garyguo.net> Cc: Joel Fernandes <joelagnelf@nvidia.com> Signed-off-by: Zhi Wang <zhiw@nvidia.com> --- rust/kernel/pci.rs | 7 +- rust/kernel/pci/io.rs | 167 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 172 insertions(+), 2 deletions(-) diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 82e128431f08..9020959ce0c7 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -40,7 +40,12 @@ ClassMask, Vendor, // }; -pub use self::io::Bar; +pub use self::io::{ + Bar, + ConfigSpaceSize, + Extended, + Normal, // +}; pub use self::irq::{ IrqType, IrqTypes, diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs index e3377397666e..39df41d0eaab 100644 --- a/rust/kernel/pci/io.rs +++ b/rust/kernel/pci/io.rs @@ -8,13 +8,149 @@ device, devres::Devres, io::{ + define_read, + define_write, + Io, + IoCapable, + IoKnownSize, Mmio, MmioRaw, // }, prelude::*, sync::aref::ARef, // }; -use core::ops::Deref; +use core::{ + marker::PhantomData, + ops::Deref, // +}; + +/// Marker type for normal (256-byte) PCI configuration space. +pub struct Normal; + +/// Marker type for extended (4096-byte) PCIe configuration space. +pub struct Extended; + +/// Trait for PCI configuration space size markers. +/// +/// This trait is implemented by [`Normal`] and [`Extended`] to provide +/// compile-time knowledge of the configuration space size. +pub trait ConfigSpaceSize { + /// The size of this configuration space in bytes. + const SIZE: usize; +} + +impl ConfigSpaceSize for Normal { + const SIZE: usize = 256; +} + +impl ConfigSpaceSize for Extended { + const SIZE: usize = 4096; +} + +/// The PCI configuration space of a device. +/// +/// Provides typed read and write accessors for configuration registers +/// using the standard `pci_read_config_*` and `pci_write_config_*` helpers. +/// +/// The generic parameter `S` indicates the maximum size of the configuration space. +/// Use [`Normal`] for 256-byte legacy configuration space or [`Extended`] for +/// 4096-byte PCIe extended configuration space (default). +pub struct ConfigSpace<'a, S: ConfigSpaceSize = Extended> { + pub(crate) pdev: &'a Device<device::Bound>, + _marker: PhantomData<S>, +} + +/// Internal helper macros used to invoke C PCI configuration space read functions. +/// +/// This macro is intended to be used by higher-level PCI configuration space access macros +/// (define_read) and provides a unified expansion for infallible vs. fallible read semantics. It +/// emits a direct call into the corresponding C helper and performs the required cast to the Rust +/// return type. +/// +/// # Parameters +/// +/// * `$c_fn` – The C function performing the PCI configuration space write. +/// * `$self` – The I/O backend object. +/// * `$ty` – The type of the value to read. +/// * `$addr` – The PCI configuration space offset to read. +/// +/// This macro does not perform any validation; all invariants must be upheld by the higher-level +/// abstraction invoking it. +macro_rules! call_config_read { + (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr) => {{ + let mut val: $ty = 0; + // SAFETY: By the type invariant `$self.pdev` is a valid address. + // CAST: The offset is cast to `i32` because the C functions expect a 32-bit signed offset + // parameter. PCI configuration space size is at most 4096 bytes, so the value always fits + // within `i32` without truncation or sign change. + // Return value from C function is ignored in infallible accessors. + let _ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, &mut val) }; + val + }}; +} + +/// Internal helper macros used to invoke C PCI configuration space write functions. +/// +/// This macro is intended to be used by higher-level PCI configuration space access macros +/// (define_write) and provides a unified expansion for infallible vs. fallible read semantics. It +/// emits a direct call into the corresponding C helper and performs the required cast to the Rust +/// return type. +/// +/// # Parameters +/// +/// * `$c_fn` – The C function performing the PCI configuration space write. +/// * `$self` – The I/O backend object. +/// * `$ty` – The type of the written value. +/// * `$addr` – The configuration space offset to write. +/// * `$value` – The value to write. +/// +/// This macro does not perform any validation; all invariants must be upheld by the higher-level +/// abstraction invoking it. +macro_rules! call_config_write { + (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => { + // SAFETY: By the type invariant `$self.pdev` is a valid address. + // CAST: The offset is cast to `i32` because the C functions expect a 32-bit signed offset + // parameter. PCI configuration space size is at most 4096 bytes, so the value always fits + // within `i32` without truncation or sign change. + // Return value from C function is ignored in infallible accessors. + let _ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, $value) }; + }; +} + +// PCI configuration space supports 8, 16, and 32-bit accesses. +impl<'a, S: ConfigSpaceSize> IoCapable<u8> for ConfigSpace<'a, S> {} +impl<'a, S: ConfigSpaceSize> IoCapable<u16> for ConfigSpace<'a, S> {} +impl<'a, S: ConfigSpaceSize> IoCapable<u32> for ConfigSpace<'a, S> {} + +impl<'a, S: ConfigSpaceSize> Io for ConfigSpace<'a, S> { + const MIN_SIZE: usize = S::SIZE; + + /// Returns the base address of the I/O region. It is always 0 for configuration space. + #[inline] + fn addr(&self) -> usize { + 0 + } + + /// Returns the maximum size of the configuration space. + #[inline] + fn maxsize(&self) -> usize { + self.pdev.cfg_size().map_or(0, |v| v) + } + + // PCI configuration space does not support fallible operations. + // The default implementations from the Io trait are not used. + + define_read!(infallible, read8, call_config_read(pci_read_config_byte) -> u8); + define_read!(infallible, read16, call_config_read(pci_read_config_word) -> u16); + define_read!(infallible, read32, call_config_read(pci_read_config_dword) -> u32); + + define_write!(infallible, write8, call_config_write(pci_write_config_byte) <- u8); + define_write!(infallible, write16, call_config_write(pci_write_config_word) <- u16); + define_write!(infallible, write32, call_config_write(pci_write_config_dword) <- u32); +} + +/// Marker trait indicating ConfigSpace has a known size at compile time. +impl<'a, S: ConfigSpaceSize> IoKnownSize for ConfigSpace<'a, S> {} /// A PCI BAR to perform I/O-Operations on. /// @@ -144,4 +280,33 @@ pub fn iomap_region<'a>( ) -> impl PinInit<Devres<Bar>, Error> + 'a { self.iomap_region_sized::<0>(bar, name) } + + /// Returns the size of configuration space in bytes. + fn cfg_size(&self) -> Result<usize> { + // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`. + let size = unsafe { (*self.as_raw()).cfg_size }; + match size { + 256 | 4096 => Ok(size as usize), + _ => { + debug_assert!(false); + Err(EINVAL) + } + } + } + + /// Return an initialized normal (256-byte) config space object. + pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, Normal>> { + Ok(ConfigSpace { + pdev: self, + _marker: PhantomData, + }) + } + + /// Return an initialized extended (4096-byte) config space object. + pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a, Extended>> { + Ok(ConfigSpace { + pdev: self, + _marker: PhantomData, + }) + } } -- 2.51.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v12 4/5] rust: pci: add config space read/write support 2026-01-21 20:22 ` [PATCH v12 4/5] rust: pci: add config space read/write support Zhi Wang @ 2026-01-22 4:25 ` Alexandre Courbot 2026-01-22 11:59 ` Gary Guo 1 sibling, 0 replies; 25+ messages in thread From: Alexandre Courbot @ 2026-01-22 4:25 UTC (permalink / raw) To: Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, joelagnelf, jhubbard, zhiwang, daniel.almeida On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote: <snip> > + /// Return an initialized normal (256-byte) config space object. > + pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, Normal>> { > + Ok(ConfigSpace { > + pdev: self, > + _marker: PhantomData, > + }) > + } > + > + /// Return an initialized extended (4096-byte) config space object. > + pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a, Extended>> { > + Ok(ConfigSpace { > + pdev: self, > + _marker: PhantomData, > + }) > + } Why are we returning a `Result` given that none of these methods can ever fail? I mentioned that in earlier revisions too [1], but it looks like we should invoke `cfg_size` to validate the size first, lest we return a `ConfigSpace` that is larger than what is actually supported. [1] https://lore.kernel.org/all/DEHUBNKNSNXH.14A4OGQKY5KZR@nvidia.com/#t ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 4/5] rust: pci: add config space read/write support 2026-01-21 20:22 ` [PATCH v12 4/5] rust: pci: add config space read/write support Zhi Wang 2026-01-22 4:25 ` Alexandre Courbot @ 2026-01-22 11:59 ` Gary Guo 2026-01-22 12:40 ` Danilo Krummrich 1 sibling, 1 reply; 25+ messages in thread From: Gary Guo @ 2026-01-22 11:59 UTC (permalink / raw) To: Zhi Wang, rust-for-linux, linux-pci, linux-kernel Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida On Wed Jan 21, 2026 at 8:22 PM GMT, Zhi Wang wrote: > Drivers might need to access PCI config space for querying capability > structures and access the registers inside the structures. > > For Rust drivers need to access PCI config space, the Rust PCI abstraction > needs to support it in a way that upholds Rust's safety principles. > > Introduce a `ConfigSpace` wrapper in Rust PCI abstraction to provide safe > accessors for PCI config space. The new type implements the `Io` trait and > `IoCapable<T>` for u8, u16, and u32 to share offset validation and > bound-checking logic with other I/O backends. > > The `ConfigSpace` type uses marker types (`Normal` and `Extended`) to > represent configuration space sizes at the type level. > > Cc: Alexandre Courbot <acourbot@nvidia.com> > Cc: Danilo Krummrich <dakr@kernel.org> > Cc: Gary Guo <gary@garyguo.net> > Cc: Joel Fernandes <joelagnelf@nvidia.com> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> > --- > rust/kernel/pci.rs | 7 +- > rust/kernel/pci/io.rs | 167 +++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 172 insertions(+), 2 deletions(-) > > diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs > index 82e128431f08..9020959ce0c7 100644 > --- a/rust/kernel/pci.rs > +++ b/rust/kernel/pci.rs > @@ -40,7 +40,12 @@ > ClassMask, > Vendor, // > }; > -pub use self::io::Bar; > +pub use self::io::{ > + Bar, > + ConfigSpaceSize, > + Extended, > + Normal, // > +}; > pub use self::irq::{ > IrqType, > IrqTypes, > diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs > index e3377397666e..39df41d0eaab 100644 > --- a/rust/kernel/pci/io.rs > +++ b/rust/kernel/pci/io.rs > @@ -8,13 +8,149 @@ > device, > devres::Devres, > io::{ > + define_read, > + define_write, > + Io, > + IoCapable, > + IoKnownSize, > Mmio, > MmioRaw, // > }, > prelude::*, > sync::aref::ARef, // > }; > -use core::ops::Deref; > +use core::{ > + marker::PhantomData, > + ops::Deref, // > +}; > + > +/// Marker type for normal (256-byte) PCI configuration space. > +pub struct Normal; > + > +/// Marker type for extended (4096-byte) PCIe configuration space. > +pub struct Extended; > + > +/// Trait for PCI configuration space size markers. > +/// > +/// This trait is implemented by [`Normal`] and [`Extended`] to provide > +/// compile-time knowledge of the configuration space size. > +pub trait ConfigSpaceSize { > + /// The size of this configuration space in bytes. > + const SIZE: usize; > +} > + > +impl ConfigSpaceSize for Normal { > + const SIZE: usize = 256; > +} > + > +impl ConfigSpaceSize for Extended { > + const SIZE: usize = 4096; > +} > + > +/// The PCI configuration space of a device. > +/// > +/// Provides typed read and write accessors for configuration registers > +/// using the standard `pci_read_config_*` and `pci_write_config_*` helpers. > +/// > +/// The generic parameter `S` indicates the maximum size of the configuration space. > +/// Use [`Normal`] for 256-byte legacy configuration space or [`Extended`] for > +/// 4096-byte PCIe extended configuration space (default). > +pub struct ConfigSpace<'a, S: ConfigSpaceSize = Extended> { > + pub(crate) pdev: &'a Device<device::Bound>, > + _marker: PhantomData<S>, > +} > + > +/// Internal helper macros used to invoke C PCI configuration space read functions. > +/// > +/// This macro is intended to be used by higher-level PCI configuration space access macros > +/// (define_read) and provides a unified expansion for infallible vs. fallible read semantics. It > +/// emits a direct call into the corresponding C helper and performs the required cast to the Rust > +/// return type. > +/// > +/// # Parameters > +/// > +/// * `$c_fn` – The C function performing the PCI configuration space write. > +/// * `$self` – The I/O backend object. > +/// * `$ty` – The type of the value to read. > +/// * `$addr` – The PCI configuration space offset to read. > +/// > +/// This macro does not perform any validation; all invariants must be upheld by the higher-level > +/// abstraction invoking it. > +macro_rules! call_config_read { > + (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr) => {{ > + let mut val: $ty = 0; > + // SAFETY: By the type invariant `$self.pdev` is a valid address. > + // CAST: The offset is cast to `i32` because the C functions expect a 32-bit signed offset > + // parameter. PCI configuration space size is at most 4096 bytes, so the value always fits > + // within `i32` without truncation or sign change. > + // Return value from C function is ignored in infallible accessors. > + let _ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, &mut val) }; > + val > + }}; > +} > + > +/// Internal helper macros used to invoke C PCI configuration space write functions. > +/// > +/// This macro is intended to be used by higher-level PCI configuration space access macros > +/// (define_write) and provides a unified expansion for infallible vs. fallible read semantics. It > +/// emits a direct call into the corresponding C helper and performs the required cast to the Rust > +/// return type. > +/// > +/// # Parameters > +/// > +/// * `$c_fn` – The C function performing the PCI configuration space write. > +/// * `$self` – The I/O backend object. > +/// * `$ty` – The type of the written value. > +/// * `$addr` – The configuration space offset to write. > +/// * `$value` – The value to write. > +/// > +/// This macro does not perform any validation; all invariants must be upheld by the higher-level > +/// abstraction invoking it. > +macro_rules! call_config_write { > + (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => { > + // SAFETY: By the type invariant `$self.pdev` is a valid address. > + // CAST: The offset is cast to `i32` because the C functions expect a 32-bit signed offset > + // parameter. PCI configuration space size is at most 4096 bytes, so the value always fits > + // within `i32` without truncation or sign change. > + // Return value from C function is ignored in infallible accessors. > + let _ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, $value) }; > + }; > +} > + > +// PCI configuration space supports 8, 16, and 32-bit accesses. > +impl<'a, S: ConfigSpaceSize> IoCapable<u8> for ConfigSpace<'a, S> {} > +impl<'a, S: ConfigSpaceSize> IoCapable<u16> for ConfigSpace<'a, S> {} > +impl<'a, S: ConfigSpaceSize> IoCapable<u32> for ConfigSpace<'a, S> {} > + > +impl<'a, S: ConfigSpaceSize> Io for ConfigSpace<'a, S> { > + const MIN_SIZE: usize = S::SIZE; > + > + /// Returns the base address of the I/O region. It is always 0 for configuration space. > + #[inline] > + fn addr(&self) -> usize { > + 0 > + } > + > + /// Returns the maximum size of the configuration space. > + #[inline] > + fn maxsize(&self) -> usize { > + self.pdev.cfg_size().map_or(0, |v| v) > + } > + > + // PCI configuration space does not support fallible operations. > + // The default implementations from the Io trait are not used. > + > + define_read!(infallible, read8, call_config_read(pci_read_config_byte) -> u8); > + define_read!(infallible, read16, call_config_read(pci_read_config_word) -> u16); > + define_read!(infallible, read32, call_config_read(pci_read_config_dword) -> u32); > + > + define_write!(infallible, write8, call_config_write(pci_write_config_byte) <- u8); > + define_write!(infallible, write16, call_config_write(pci_write_config_word) <- u16); > + define_write!(infallible, write32, call_config_write(pci_write_config_dword) <- u32); > +} > + > +/// Marker trait indicating ConfigSpace has a known size at compile time. > +impl<'a, S: ConfigSpaceSize> IoKnownSize for ConfigSpace<'a, S> {} > > /// A PCI BAR to perform I/O-Operations on. > /// > @@ -144,4 +280,33 @@ pub fn iomap_region<'a>( > ) -> impl PinInit<Devres<Bar>, Error> + 'a { > self.iomap_region_sized::<0>(bar, name) > } > + > + /// Returns the size of configuration space in bytes. > + fn cfg_size(&self) -> Result<usize> { > + // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`. > + let size = unsafe { (*self.as_raw()).cfg_size }; > + match size { > + 256 | 4096 => Ok(size as usize), > + _ => { > + debug_assert!(false); > + Err(EINVAL) > + } > + } > + } This method is only invoked from maxsize, which turns error into `0`. Do apart from the debug assertion, the error code is pointless. I think this function should just return `usize` as it's specified in the device (we should trust the C side that the value is sensible). The check, as Alex mentioned, need to be done when ConfigSpace is created in the first place and is too late when you already hand out `Ok(ConfigSpace)`. Best, Gary > + > + /// Return an initialized normal (256-byte) config space object. > + pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, Normal>> { > + Ok(ConfigSpace { > + pdev: self, > + _marker: PhantomData, > + }) > + } > + > + /// Return an initialized extended (4096-byte) config space object. > + pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a, Extended>> { > + Ok(ConfigSpace { > + pdev: self, > + _marker: PhantomData, > + }) > + } > } ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 4/5] rust: pci: add config space read/write support 2026-01-22 11:59 ` Gary Guo @ 2026-01-22 12:40 ` Danilo Krummrich 2026-01-22 12:52 ` Zhi Wang 2026-01-22 14:26 ` Gary Guo 0 siblings, 2 replies; 25+ messages in thread From: Danilo Krummrich @ 2026-01-22 12:40 UTC (permalink / raw) To: Gary Guo Cc: Zhi Wang, rust-for-linux, linux-pci, linux-kernel, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida On Thu Jan 22, 2026 at 12:59 PM CET, Gary Guo wrote: > On Wed Jan 21, 2026 at 8:22 PM GMT, Zhi Wang wrote: >> + /// Returns the size of configuration space in bytes. >> + fn cfg_size(&self) -> Result<usize> { >> + // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`. >> + let size = unsafe { (*self.as_raw()).cfg_size }; >> + match size { >> + 256 | 4096 => Ok(size as usize), >> + _ => { >> + debug_assert!(false); >> + Err(EINVAL) >> + } >> + } >> + } > > This method is only invoked from maxsize, which turns error into `0`. Do apart > from the debug assertion, the error code is pointless. I think this function > should just return `usize` as it's specified in the device (we should trust the > C side that the value is sensible). That seems reasonable, but I also think we should keep the enum ConfigSpaceSize we had before and call the new trait ConfigSpaceKind instead, such that this method becomes: fn cfg_size(&self) -> ConfigSpaceSize; > The check, as Alex mentioned, need to be done when ConfigSpace is created in > the first place and is too late when you already hand out `Ok(ConfigSpace)`. We need the check for config_space_extended(), but not for config_space(), as it represents the minimum size, i.e. it's always valid. Here's a diff of what I think this should look like on top of this series. (@Zhi: If we all agree on the diff and nothing else comes up you don't need to resend. :) diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 9020959ce0c7..1d1a253e5d5d 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -42,6 +42,7 @@ }; pub use self::io::{ Bar, + ConfigSpaceKind, ConfigSpaceSize, Extended, Normal, // diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs index 39df41d0eaab..5dbdfe516418 100644 --- a/rust/kernel/pci/io.rs +++ b/rust/kernel/pci/io.rs @@ -24,6 +24,31 @@ ops::Deref, // }; +/// Represents the size of a PCI configuration space. +/// +/// PCI devices can have either a *normal* (legacy) configuration space of 256 bytes, +/// or an *extended* configuration space of 4096 bytes as defined in the PCI Express +/// specification. +#[repr(usize)] +#[derive(PartialEq)] +pub enum ConfigSpaceSize { + /// 256-byte legacy PCI configuration space. + Normal = 256, + + /// 4096-byte PCIe extended configuration space. + Extended = 4096, +} + +impl ConfigSpaceSize { + /// Get the raw value of this enum. + #[inline(always)] + pub const fn into_raw(self) -> usize { + // CAST: PCI configuration space size is at most 4096 bytes, so the value always fits + // within `usize` without truncation or sign change. + self as usize + } +} + /// Marker type for normal (256-byte) PCI configuration space. pub struct Normal; @@ -34,16 +59,16 @@ /// /// This trait is implemented by [`Normal`] and [`Extended`] to provide /// compile-time knowledge of the configuration space size. -pub trait ConfigSpaceSize { +pub trait ConfigSpaceKind { /// The size of this configuration space in bytes. const SIZE: usize; } -impl ConfigSpaceSize for Normal { +impl ConfigSpaceKind for Normal { const SIZE: usize = 256; } -impl ConfigSpaceSize for Extended { +impl ConfigSpaceKind for Extended { const SIZE: usize = 4096; } @@ -55,7 +80,7 @@ impl ConfigSpaceSize for Extended { /// The generic parameter `S` indicates the maximum size of the configuration space. /// Use [`Normal`] for 256-byte legacy configuration space or [`Extended`] for /// 4096-byte PCIe extended configuration space (default). -pub struct ConfigSpace<'a, S: ConfigSpaceSize = Extended> { +pub struct ConfigSpace<'a, S: ConfigSpaceKind = Extended> { pub(crate) pdev: &'a Device<device::Bound>, _marker: PhantomData<S>, } @@ -118,11 +143,11 @@ macro_rules! call_config_write { } // PCI configuration space supports 8, 16, and 32-bit accesses. -impl<'a, S: ConfigSpaceSize> IoCapable<u8> for ConfigSpace<'a, S> {} -impl<'a, S: ConfigSpaceSize> IoCapable<u16> for ConfigSpace<'a, S> {} -impl<'a, S: ConfigSpaceSize> IoCapable<u32> for ConfigSpace<'a, S> {} +impl<'a, S: ConfigSpaceKind> IoCapable<u8> for ConfigSpace<'a, S> {} +impl<'a, S: ConfigSpaceKind> IoCapable<u16> for ConfigSpace<'a, S> {} +impl<'a, S: ConfigSpaceKind> IoCapable<u32> for ConfigSpace<'a, S> {} -impl<'a, S: ConfigSpaceSize> Io for ConfigSpace<'a, S> { +impl<'a, S: ConfigSpaceKind> Io for ConfigSpace<'a, S> { const MIN_SIZE: usize = S::SIZE; /// Returns the base address of the I/O region. It is always 0 for configuration space. @@ -134,7 +159,7 @@ fn addr(&self) -> usize { /// Returns the maximum size of the configuration space. #[inline] fn maxsize(&self) -> usize { - self.pdev.cfg_size().map_or(0, |v| v) + self.pdev.cfg_size().into_raw() } // PCI configuration space does not support fallible operations. @@ -150,7 +175,7 @@ fn maxsize(&self) -> usize { } /// Marker trait indicating ConfigSpace has a known size at compile time. -impl<'a, S: ConfigSpaceSize> IoKnownSize for ConfigSpace<'a, S> {} +impl<'a, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'a, S> {} /// A PCI BAR to perform I/O-Operations on. /// @@ -281,29 +306,35 @@ pub fn iomap_region<'a>( self.iomap_region_sized::<0>(bar, name) } - /// Returns the size of configuration space in bytes. - fn cfg_size(&self) -> Result<usize> { + /// Returns the size of configuration space. + fn cfg_size(&self) -> ConfigSpaceSize { // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`. let size = unsafe { (*self.as_raw()).cfg_size }; match size { - 256 | 4096 => Ok(size as usize), + 256 => ConfigSpaceSize::Normal, + 4096 => ConfigSpaceSize::Extended, _ => { - debug_assert!(false); - Err(EINVAL) + // PANIC: The PCI subsystem only ever reports the configuration space size as either + // `ConfigSpaceSize::Normal` or `ConfigSpaceSize::Extended`. + unreachable!(); } } } /// Return an initialized normal (256-byte) config space object. - pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, Normal>> { - Ok(ConfigSpace { + pub fn config_space<'a>(&'a self) -> ConfigSpace<'a, Normal> { + ConfigSpace { pdev: self, _marker: PhantomData, - }) + } } /// Return an initialized extended (4096-byte) config space object. pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a, Extended>> { + if self.cfg_size() != ConfigSpaceSize::Extended { + return Err(EINVAL); + } + Ok(ConfigSpace { pdev: self, _marker: PhantomData, diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs index 1bc5bd1a8df5..8eea79e858a2 100644 --- a/samples/rust/rust_driver_pci.rs +++ b/samples/rust/rust_driver_pci.rs @@ -67,8 +67,8 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> Result<u32> { Ok(bar.read32(Regs::COUNT)) } - fn config_space(pdev: &pci::Device<Bound>) -> Result { - let config = pdev.config_space()?; + fn config_space(pdev: &pci::Device<Bound>) { + let config = pdev.config_space(); // TODO: use the register!() macro for defining PCI configuration space registers once it // has been move out of nova-core. @@ -89,8 +89,6 @@ fn config_space(pdev: &pci::Device<Bound>) -> Result { "pci-testdev config space read32 BAR 0: {:x}\n", config.read32(0x10) ); - - Ok(()) } } @@ -123,7 +121,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er "pci-testdev data-match count: {}\n", Self::testdev(info, bar)? ); - Self::config_space(pdev)?; + Self::config_space(pdev); }, pdev: pdev.into(), })) ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v12 4/5] rust: pci: add config space read/write support 2026-01-22 12:40 ` Danilo Krummrich @ 2026-01-22 12:52 ` Zhi Wang 2026-01-22 14:26 ` Gary Guo 1 sibling, 0 replies; 25+ messages in thread From: Zhi Wang @ 2026-01-22 12:52 UTC (permalink / raw) To: Danilo Krummrich Cc: Gary Guo, rust-for-linux, linux-pci, linux-kernel, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida On Thu, 22 Jan 2026 13:40:22 +0100 "Danilo Krummrich" <dakr@kernel.org> wrote: > On Thu Jan 22, 2026 at 12:59 PM CET, Gary Guo wrote: > > On Wed Jan 21, 2026 at 8:22 PM GMT, Zhi Wang wrote: > (@Zhi: If we all agree on the diff and nothing else comes up you don't > need to resend. :) > Hi Danilo: Thanks so much for this. I went through the code. It looks good to me. Z. > diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs > index 9020959ce0c7..1d1a253e5d5d 100644 > --- a/rust/kernel/pci.rs > +++ b/rust/kernel/pci.rs > @@ -42,6 +42,7 @@ > }; > pub use self::io::{ > Bar, > + ConfigSpaceKind, > ConfigSpaceSize, > Extended, > Normal, // > diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs > index 39df41d0eaab..5dbdfe516418 100644 > --- a/rust/kernel/pci/io.rs > +++ b/rust/kernel/pci/io.rs > @@ -24,6 +24,31 @@ > ops::Deref, // > }; > > +/// Represents the size of a PCI configuration space. > +/// > +/// PCI devices can have either a *normal* (legacy) configuration space > of 256 bytes, +/// or an *extended* configuration space of 4096 bytes as > defined in the PCI Express +/// specification. > +#[repr(usize)] > +#[derive(PartialEq)] > +pub enum ConfigSpaceSize { > + /// 256-byte legacy PCI configuration space. > + Normal = 256, > + > + /// 4096-byte PCIe extended configuration space. > + Extended = 4096, > +} > + > +impl ConfigSpaceSize { > + /// Get the raw value of this enum. > + #[inline(always)] > + pub const fn into_raw(self) -> usize { > + // CAST: PCI configuration space size is at most 4096 bytes, so > the value always fits > + // within `usize` without truncation or sign change. > + self as usize > + } > +} > + > /// Marker type for normal (256-byte) PCI configuration space. > pub struct Normal; > > @@ -34,16 +59,16 @@ > /// > /// This trait is implemented by [`Normal`] and [`Extended`] to provide > /// compile-time knowledge of the configuration space size. > -pub trait ConfigSpaceSize { > +pub trait ConfigSpaceKind { > /// The size of this configuration space in bytes. > const SIZE: usize; > } > > -impl ConfigSpaceSize for Normal { > +impl ConfigSpaceKind for Normal { > const SIZE: usize = 256; > } > > -impl ConfigSpaceSize for Extended { > +impl ConfigSpaceKind for Extended { > const SIZE: usize = 4096; > } > > @@ -55,7 +80,7 @@ impl ConfigSpaceSize for Extended { > /// The generic parameter `S` indicates the maximum size of the > configuration space. /// Use [`Normal`] for 256-byte legacy > configuration space or [`Extended`] for /// 4096-byte PCIe extended > configuration space (default). -pub struct ConfigSpace<'a, S: > ConfigSpaceSize = Extended> { +pub struct ConfigSpace<'a, S: > ConfigSpaceKind = Extended> { pub(crate) pdev: &'a Device<device::Bound>, > _marker: PhantomData<S>, > } > @@ -118,11 +143,11 @@ macro_rules! call_config_write { > } > > // PCI configuration space supports 8, 16, and 32-bit accesses. > -impl<'a, S: ConfigSpaceSize> IoCapable<u8> for ConfigSpace<'a, S> {} > -impl<'a, S: ConfigSpaceSize> IoCapable<u16> for ConfigSpace<'a, S> {} > -impl<'a, S: ConfigSpaceSize> IoCapable<u32> for ConfigSpace<'a, S> {} > +impl<'a, S: ConfigSpaceKind> IoCapable<u8> for ConfigSpace<'a, S> {} > +impl<'a, S: ConfigSpaceKind> IoCapable<u16> for ConfigSpace<'a, S> {} > +impl<'a, S: ConfigSpaceKind> IoCapable<u32> for ConfigSpace<'a, S> {} > > -impl<'a, S: ConfigSpaceSize> Io for ConfigSpace<'a, S> { > +impl<'a, S: ConfigSpaceKind> Io for ConfigSpace<'a, S> { > const MIN_SIZE: usize = S::SIZE; > > /// Returns the base address of the I/O region. It is always 0 for > configuration space. @@ -134,7 +159,7 @@ fn addr(&self) -> usize { > /// Returns the maximum size of the configuration space. > #[inline] > fn maxsize(&self) -> usize { > - self.pdev.cfg_size().map_or(0, |v| v) > + self.pdev.cfg_size().into_raw() > } > > // PCI configuration space does not support fallible operations. > @@ -150,7 +175,7 @@ fn maxsize(&self) -> usize { > } > > /// Marker trait indicating ConfigSpace has a known size at compile > time. -impl<'a, S: ConfigSpaceSize> IoKnownSize for ConfigSpace<'a, S> {} > +impl<'a, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'a, S> {} > > /// A PCI BAR to perform I/O-Operations on. > /// > @@ -281,29 +306,35 @@ pub fn iomap_region<'a>( > self.iomap_region_sized::<0>(bar, name) > } > > - /// Returns the size of configuration space in bytes. > - fn cfg_size(&self) -> Result<usize> { > + /// Returns the size of configuration space. > + fn cfg_size(&self) -> ConfigSpaceSize { > // SAFETY: `self.as_raw` is a valid pointer to a `struct > pci_dev`. let size = unsafe { (*self.as_raw()).cfg_size }; > match size { > - 256 | 4096 => Ok(size as usize), > + 256 => ConfigSpaceSize::Normal, > + 4096 => ConfigSpaceSize::Extended, > _ => { > - debug_assert!(false); > - Err(EINVAL) > + // PANIC: The PCI subsystem only ever reports the > configuration space size as either > + // `ConfigSpaceSize::Normal` or > `ConfigSpaceSize::Extended`. > + unreachable!(); > } > } > } > > /// Return an initialized normal (256-byte) config space object. > - pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, > Normal>> { > - Ok(ConfigSpace { > + pub fn config_space<'a>(&'a self) -> ConfigSpace<'a, Normal> { > + ConfigSpace { > pdev: self, > _marker: PhantomData, > - }) > + } > } > > /// Return an initialized extended (4096-byte) config space object. > pub fn config_space_extended<'a>(&'a self) -> > Result<ConfigSpace<'a, Extended>> { > + if self.cfg_size() != ConfigSpaceSize::Extended { > + return Err(EINVAL); > + } > + > Ok(ConfigSpace { > pdev: self, > _marker: PhantomData, > diff --git a/samples/rust/rust_driver_pci.rs > b/samples/rust/rust_driver_pci.rs index 1bc5bd1a8df5..8eea79e858a2 100644 > --- a/samples/rust/rust_driver_pci.rs > +++ b/samples/rust/rust_driver_pci.rs > @@ -67,8 +67,8 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> > Result<u32> { Ok(bar.read32(Regs::COUNT)) > } > > - fn config_space(pdev: &pci::Device<Bound>) -> Result { > - let config = pdev.config_space()?; > + fn config_space(pdev: &pci::Device<Bound>) { > + let config = pdev.config_space(); > > // TODO: use the register!() macro for defining PCI > configuration space registers once it // has been move out of nova-core. > @@ -89,8 +89,6 @@ fn config_space(pdev: &pci::Device<Bound>) -> Result { > "pci-testdev config space read32 BAR 0: {:x}\n", > config.read32(0x10) > ); > - > - Ok(()) > } > } > > @@ -123,7 +121,7 @@ fn probe(pdev: &pci::Device<Core>, info: > &Self::IdInfo) -> impl PinInit<Self, Er "pci-testdev data-match count: > {}\n", Self::testdev(info, bar)? > ); > - Self::config_space(pdev)?; > + Self::config_space(pdev); > }, > pdev: pdev.into(), > })) > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 4/5] rust: pci: add config space read/write support 2026-01-22 12:40 ` Danilo Krummrich 2026-01-22 12:52 ` Zhi Wang @ 2026-01-22 14:26 ` Gary Guo 1 sibling, 0 replies; 25+ messages in thread From: Gary Guo @ 2026-01-22 14:26 UTC (permalink / raw) To: Danilo Krummrich, Gary Guo Cc: Zhi Wang, rust-for-linux, linux-pci, linux-kernel, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida On Thu Jan 22, 2026 at 12:40 PM GMT, Danilo Krummrich wrote: > On Thu Jan 22, 2026 at 12:59 PM CET, Gary Guo wrote: >> On Wed Jan 21, 2026 at 8:22 PM GMT, Zhi Wang wrote: >>> + /// Returns the size of configuration space in bytes. >>> + fn cfg_size(&self) -> Result<usize> { >>> + // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`. >>> + let size = unsafe { (*self.as_raw()).cfg_size }; >>> + match size { >>> + 256 | 4096 => Ok(size as usize), >>> + _ => { >>> + debug_assert!(false); >>> + Err(EINVAL) >>> + } >>> + } >>> + } >> >> This method is only invoked from maxsize, which turns error into `0`. Do apart >> from the debug assertion, the error code is pointless. I think this function >> should just return `usize` as it's specified in the device (we should trust the >> C side that the value is sensible). > > That seems reasonable, but I also think we should keep the enum ConfigSpaceSize > we had before and call the new trait ConfigSpaceKind instead, such that this > method becomes: > > fn cfg_size(&self) -> ConfigSpaceSize; > >> The check, as Alex mentioned, need to be done when ConfigSpace is created in >> the first place and is too late when you already hand out `Ok(ConfigSpace)`. > > We need the check for config_space_extended(), but not for config_space(), as it > represents the minimum size, i.e. it's always valid. > > Here's a diff of what I think this should look like on top of this series. The proposal looks good to me. Some comments below. Reviewed-by: Gary Guo <gary@garyguo.net> > > (@Zhi: If we all agree on the diff and nothing else comes up you don't need to > resend. :) > > diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs > index 9020959ce0c7..1d1a253e5d5d 100644 > --- a/rust/kernel/pci.rs > +++ b/rust/kernel/pci.rs > @@ -42,6 +42,7 @@ > }; > pub use self::io::{ > Bar, > + ConfigSpaceKind, > ConfigSpaceSize, > Extended, > Normal, // > diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs > index 39df41d0eaab..5dbdfe516418 100644 > --- a/rust/kernel/pci/io.rs > +++ b/rust/kernel/pci/io.rs > @@ -24,6 +24,31 @@ > ops::Deref, // > }; > > +/// Represents the size of a PCI configuration space. > +/// > +/// PCI devices can have either a *normal* (legacy) configuration space of 256 bytes, > +/// or an *extended* configuration space of 4096 bytes as defined in the PCI Express > +/// specification. > +#[repr(usize)] > +#[derive(PartialEq)] When `PartialEq` is derived, I would also derive `Eq` unless there's no reflexivity in comparison. > +pub enum ConfigSpaceSize { > + /// 256-byte legacy PCI configuration space. > + Normal = 256, > + > + /// 4096-byte PCIe extended configuration space. > + Extended = 4096, > +} > + > +impl ConfigSpaceSize { > + /// Get the raw value of this enum. > + #[inline(always)] > + pub const fn into_raw(self) -> usize { > + // CAST: PCI configuration space size is at most 4096 bytes, so the value always fits > + // within `usize` without truncation or sign change. > + self as usize > + } > +} > + > /// Marker type for normal (256-byte) PCI configuration space. > pub struct Normal; > > @@ -34,16 +59,16 @@ > /// > /// This trait is implemented by [`Normal`] and [`Extended`] to provide > /// compile-time knowledge of the configuration space size. > -pub trait ConfigSpaceSize { > +pub trait ConfigSpaceKind { > /// The size of this configuration space in bytes. > const SIZE: usize; > } > > -impl ConfigSpaceSize for Normal { > +impl ConfigSpaceKind for Normal { > const SIZE: usize = 256; > } > > -impl ConfigSpaceSize for Extended { > +impl ConfigSpaceKind for Extended { > const SIZE: usize = 4096; > } > > @@ -55,7 +80,7 @@ impl ConfigSpaceSize for Extended { > /// The generic parameter `S` indicates the maximum size of the configuration space. > /// Use [`Normal`] for 256-byte legacy configuration space or [`Extended`] for > /// 4096-byte PCIe extended configuration space (default). > -pub struct ConfigSpace<'a, S: ConfigSpaceSize = Extended> { > +pub struct ConfigSpace<'a, S: ConfigSpaceKind = Extended> { > pub(crate) pdev: &'a Device<device::Bound>, > _marker: PhantomData<S>, > } > @@ -118,11 +143,11 @@ macro_rules! call_config_write { > } > > // PCI configuration space supports 8, 16, and 32-bit accesses. > -impl<'a, S: ConfigSpaceSize> IoCapable<u8> for ConfigSpace<'a, S> {} > -impl<'a, S: ConfigSpaceSize> IoCapable<u16> for ConfigSpace<'a, S> {} > -impl<'a, S: ConfigSpaceSize> IoCapable<u32> for ConfigSpace<'a, S> {} > +impl<'a, S: ConfigSpaceKind> IoCapable<u8> for ConfigSpace<'a, S> {} > +impl<'a, S: ConfigSpaceKind> IoCapable<u16> for ConfigSpace<'a, S> {} > +impl<'a, S: ConfigSpaceKind> IoCapable<u32> for ConfigSpace<'a, S> {} > > -impl<'a, S: ConfigSpaceSize> Io for ConfigSpace<'a, S> { > +impl<'a, S: ConfigSpaceKind> Io for ConfigSpace<'a, S> { > const MIN_SIZE: usize = S::SIZE; > > /// Returns the base address of the I/O region. It is always 0 for configuration space. > @@ -134,7 +159,7 @@ fn addr(&self) -> usize { > /// Returns the maximum size of the configuration space. > #[inline] > fn maxsize(&self) -> usize { > - self.pdev.cfg_size().map_or(0, |v| v) > + self.pdev.cfg_size().into_raw() > } > > // PCI configuration space does not support fallible operations. > @@ -150,7 +175,7 @@ fn maxsize(&self) -> usize { > } > > /// Marker trait indicating ConfigSpace has a known size at compile time. > -impl<'a, S: ConfigSpaceSize> IoKnownSize for ConfigSpace<'a, S> {} > +impl<'a, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'a, S> {} > > /// A PCI BAR to perform I/O-Operations on. > /// > @@ -281,29 +306,35 @@ pub fn iomap_region<'a>( > self.iomap_region_sized::<0>(bar, name) > } > > - /// Returns the size of configuration space in bytes. > - fn cfg_size(&self) -> Result<usize> { > + /// Returns the size of configuration space. > + fn cfg_size(&self) -> ConfigSpaceSize { If you keep this `fn` instead of `pub fn`, then the `ConfigSpaceSize` type being `pub` is not very useful as it cannot be invoked by the user. > // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`. > let size = unsafe { (*self.as_raw()).cfg_size }; > match size { > - 256 | 4096 => Ok(size as usize), > + 256 => ConfigSpaceSize::Normal, > + 4096 => ConfigSpaceSize::Extended, > _ => { > - debug_assert!(false); > - Err(EINVAL) > + // PANIC: The PCI subsystem only ever reports the configuration space size as either > + // `ConfigSpaceSize::Normal` or `ConfigSpaceSize::Extended`. > + unreachable!(); > } > } > } > > /// Return an initialized normal (256-byte) config space object. > - pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, Normal>> { > - Ok(ConfigSpace { > + pub fn config_space<'a>(&'a self) -> ConfigSpace<'a, Normal> { Nice. Less failing path = good. > + ConfigSpace { > pdev: self, > _marker: PhantomData, > - }) > + } > } > > /// Return an initialized extended (4096-byte) config space object. > pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a, Extended>> { > + if self.cfg_size() != ConfigSpaceSize::Extended { > + return Err(EINVAL); > + } > + > Ok(ConfigSpace { > pdev: self, > _marker: PhantomData, > diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs > index 1bc5bd1a8df5..8eea79e858a2 100644 > --- a/samples/rust/rust_driver_pci.rs > +++ b/samples/rust/rust_driver_pci.rs > @@ -67,8 +67,8 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> Result<u32> { > Ok(bar.read32(Regs::COUNT)) > } > > - fn config_space(pdev: &pci::Device<Bound>) -> Result { > - let config = pdev.config_space()?; > + fn config_space(pdev: &pci::Device<Bound>) { > + let config = pdev.config_space(); > > // TODO: use the register!() macro for defining PCI configuration space registers once it > // has been move out of nova-core. > @@ -89,8 +89,6 @@ fn config_space(pdev: &pci::Device<Bound>) -> Result { > "pci-testdev config space read32 BAR 0: {:x}\n", > config.read32(0x10) > ); > - > - Ok(()) > } > } > > @@ -123,7 +121,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er > "pci-testdev data-match count: {}\n", > Self::testdev(info, bar)? > ); > - Self::config_space(pdev)?; > + Self::config_space(pdev); > }, > pdev: pdev.into(), > })) ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v12 5/5] sample: rust: pci: add tests for config space routines 2026-01-21 20:22 [PATCH v12 0/5] rust: pci: add config space read/write support Zhi Wang ` (3 preceding siblings ...) 2026-01-21 20:22 ` [PATCH v12 4/5] rust: pci: add config space read/write support Zhi Wang @ 2026-01-21 20:22 ` Zhi Wang 2026-01-22 4:31 ` Alexandre Courbot 2026-01-22 12:00 ` Gary Guo 2026-01-24 0:12 ` [PATCH v12 0/5] rust: pci: add config space read/write support Danilo Krummrich 5 siblings, 2 replies; 25+ messages in thread From: Zhi Wang @ 2026-01-21 20:22 UTC (permalink / raw) To: rust-for-linux, linux-pci, linux-kernel Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida, Zhi Wang Add tests exercising the PCI configuration space helpers. Suggested-by: Danilo Krummrich <dakr@kernel.org> Signed-off-by: Zhi Wang <zhiw@nvidia.com> --- samples/rust/rust_driver_pci.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs index 38c949efce38..1bc5bd1a8df5 100644 --- a/samples/rust/rust_driver_pci.rs +++ b/samples/rust/rust_driver_pci.rs @@ -5,6 +5,7 @@ //! To make this driver probe, QEMU must be run with `-device pci-testdev`. use kernel::{ + device::Bound, device::Core, devres::Devres, io::Io, @@ -65,6 +66,32 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> Result<u32> { Ok(bar.read32(Regs::COUNT)) } + + fn config_space(pdev: &pci::Device<Bound>) -> Result { + let config = pdev.config_space()?; + + // TODO: use the register!() macro for defining PCI configuration space registers once it + // has been move out of nova-core. + dev_info!( + pdev.as_ref(), + "pci-testdev config space read8 rev ID: {:x}\n", + config.read8(0x8) + ); + + dev_info!( + pdev.as_ref(), + "pci-testdev config space read16 vendor ID: {:x}\n", + config.read16(0) + ); + + dev_info!( + pdev.as_ref(), + "pci-testdev config space read32 BAR 0: {:x}\n", + config.read32(0x10) + ); + + Ok(()) + } } impl pci::Driver for SampleDriver { @@ -96,6 +123,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er "pci-testdev data-match count: {}\n", Self::testdev(info, bar)? ); + Self::config_space(pdev)?; }, pdev: pdev.into(), })) -- 2.51.0 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v12 5/5] sample: rust: pci: add tests for config space routines 2026-01-21 20:22 ` [PATCH v12 5/5] sample: rust: pci: add tests for config space routines Zhi Wang @ 2026-01-22 4:31 ` Alexandre Courbot 2026-01-26 5:08 ` Alexandre Courbot 2026-01-22 12:00 ` Gary Guo 1 sibling, 1 reply; 25+ messages in thread From: Alexandre Courbot @ 2026-01-22 4:31 UTC (permalink / raw) To: Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, joelagnelf, jhubbard, zhiwang, daniel.almeida On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote: > Add tests exercising the PCI configuration space helpers. > > Suggested-by: Danilo Krummrich <dakr@kernel.org> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> > --- > samples/rust/rust_driver_pci.rs | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs > index 38c949efce38..1bc5bd1a8df5 100644 > --- a/samples/rust/rust_driver_pci.rs > +++ b/samples/rust/rust_driver_pci.rs > @@ -5,6 +5,7 @@ > //! To make this driver probe, QEMU must be run with `-device pci-testdev`. > > use kernel::{ > + device::Bound, > device::Core, > devres::Devres, > io::Io, > @@ -65,6 +66,32 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> Result<u32> { > > Ok(bar.read32(Regs::COUNT)) > } > + > + fn config_space(pdev: &pci::Device<Bound>) -> Result { > + let config = pdev.config_space()?; > + > + // TODO: use the register!() macro for defining PCI configuration space registers once it I'll rebase the `register!` series on top of this and try to address this item. For now, Reviewed-by: Alexandre Courbot <acourbot@nvidia.com> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 5/5] sample: rust: pci: add tests for config space routines 2026-01-22 4:31 ` Alexandre Courbot @ 2026-01-26 5:08 ` Alexandre Courbot 2026-01-26 9:05 ` Zhi Wang 0 siblings, 1 reply; 25+ messages in thread From: Alexandre Courbot @ 2026-01-26 5:08 UTC (permalink / raw) To: Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, joelagnelf, jhubbard, zhiwang, daniel.almeida On Thu Jan 22, 2026 at 1:31 PM JST, Alexandre Courbot wrote: > On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote: >> Add tests exercising the PCI configuration space helpers. >> >> Suggested-by: Danilo Krummrich <dakr@kernel.org> >> Signed-off-by: Zhi Wang <zhiw@nvidia.com> >> --- >> samples/rust/rust_driver_pci.rs | 28 ++++++++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> >> diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs >> index 38c949efce38..1bc5bd1a8df5 100644 >> --- a/samples/rust/rust_driver_pci.rs >> +++ b/samples/rust/rust_driver_pci.rs >> @@ -5,6 +5,7 @@ >> //! To make this driver probe, QEMU must be run with `-device pci-testdev`. >> >> use kernel::{ >> + device::Bound, >> device::Core, >> devres::Devres, >> io::Io, >> @@ -65,6 +66,32 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> Result<u32> { >> >> Ok(bar.read32(Regs::COUNT)) >> } >> + >> + fn config_space(pdev: &pci::Device<Bound>) -> Result { >> + let config = pdev.config_space()?; >> + >> + // TODO: use the register!() macro for defining PCI configuration space registers once it > > I'll rebase the `register!` series on top of this and try to address > this item. Actually... is this expected to work? > + dev_info!( > + pdev.as_ref(), > + "pci-testdev config space read8 rev ID: {:x}\n", > + config.read8(0x8) > + ); We are performing I/O on `config`, but since this is a sample device with no hardware backing, what is providing the data for the registers? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 5/5] sample: rust: pci: add tests for config space routines 2026-01-26 5:08 ` Alexandre Courbot @ 2026-01-26 9:05 ` Zhi Wang 2026-01-26 10:21 ` Zhi Wang 2026-01-26 10:35 ` Alexandre Courbot 0 siblings, 2 replies; 25+ messages in thread From: Zhi Wang @ 2026-01-26 9:05 UTC (permalink / raw) To: Alexandre Courbot Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, joelagnelf, jhubbard, zhiwang, daniel.almeida On Mon, 26 Jan 2026 14:08:30 +0900 "Alexandre Courbot" <acourbot@nvidia.com> wrote: > On Thu Jan 22, 2026 at 1:31 PM JST, Alexandre Courbot wrote: > > On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote: > >> Add tests exercising the PCI configuration space helpers. > >> > >> Suggested-by: Danilo Krummrich <dakr@kernel.org> > >> Signed-off-by: Zhi Wang <zhiw@nvidia.com> > >> --- > >> samples/rust/rust_driver_pci.rs | 28 ++++++++++++++++++++++++++++ > >> 1 file changed, 28 insertions(+) > >> > >> diff --git a/samples/rust/rust_driver_pci.rs > >> b/samples/rust/rust_driver_pci.rs index 38c949efce38..1bc5bd1a8df5 > >> 100644 --- a/samples/rust/rust_driver_pci.rs > >> +++ b/samples/rust/rust_driver_pci.rs > >> @@ -5,6 +5,7 @@ > >> //! To make this driver probe, QEMU must be run with `-device > >> pci-testdev`. > >> use kernel::{ > >> + device::Bound, > >> device::Core, > >> devres::Devres, > >> io::Io, > >> @@ -65,6 +66,32 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> > >> Result<u32> { > >> Ok(bar.read32(Regs::COUNT)) > >> } > >> + > >> + fn config_space(pdev: &pci::Device<Bound>) -> Result { > >> + let config = pdev.config_space()?; > >> + > >> + // TODO: use the register!() macro for defining PCI > >> configuration space registers once it > > > > I'll rebase the `register!` series on top of this and try to address > > this item. > > Actually... is this expected to work? > > > + dev_info!( > > + pdev.as_ref(), > > + "pci-testdev config space read8 rev ID: {:x}\n", > > + config.read8(0x8) > > + ); > > We are performing I/O on `config`, but since this is a sample device > with no hardware backing, what is providing the data for the > registers? > We are running the sample driver in the virtual machine. QEMU provides a emulated PCI test device (-device pci-testdev) for this purpose. E.g. with the following command line, I am able to run and test this in the virtual machine. qemu-system-x86_64 \ -enable-kvm \ -cpu host \ -m 8192 \ -smp 24 \ -drive file=/home/inno/vm/xubuntu.img,if=virtio \ -netdev user,id=net0 \ -device virtio-net-pci,netdev=net0 \ -device usb-tablet \ -device pci-testdev \ -usb ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 5/5] sample: rust: pci: add tests for config space routines 2026-01-26 9:05 ` Zhi Wang @ 2026-01-26 10:21 ` Zhi Wang 2026-01-26 10:35 ` Alexandre Courbot 1 sibling, 0 replies; 25+ messages in thread From: Zhi Wang @ 2026-01-26 10:21 UTC (permalink / raw) To: Alexandre Courbot Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, joelagnelf, jhubbard, zhiwang, daniel.almeida On Mon, 26 Jan 2026 09:05:31 +0000 Zhi Wang <zhiw@nvidia.com> wrote: > On Mon, 26 Jan 2026 14:08:30 +0900 > "Alexandre Courbot" <acourbot@nvidia.com> wrote: > > > On Thu Jan 22, 2026 at 1:31 PM JST, Alexandre Courbot wrote: > > > On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote: > > >> Add tests exercising the PCI configuration space helpers. > > >> > > >> Suggested-by: Danilo Krummrich <dakr@kernel.org> > > >> Signed-off-by: Zhi Wang <zhiw@nvidia.com> > > >> --- > > >> samples/rust/rust_driver_pci.rs | 28 ++++++++++++++++++++++++++++ > > >> 1 file changed, 28 insertions(+) > > >> > > >> diff --git a/samples/rust/rust_driver_pci.rs > > >> b/samples/rust/rust_driver_pci.rs index 38c949efce38..1bc5bd1a8df5 > > >> 100644 --- a/samples/rust/rust_driver_pci.rs > > >> +++ b/samples/rust/rust_driver_pci.rs > > >> @@ -5,6 +5,7 @@ > > >> //! To make this driver probe, QEMU must be run with `-device > > >> pci-testdev`. > > >> use kernel::{ > > >> + device::Bound, > > >> device::Core, > > >> devres::Devres, > > >> io::Io, > > >> @@ -65,6 +66,32 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> > > >> Result<u32> { > > >> Ok(bar.read32(Regs::COUNT)) > > >> } > > >> + > > >> + fn config_space(pdev: &pci::Device<Bound>) -> Result { > > >> + let config = pdev.config_space()?; > > >> + > > >> + // TODO: use the register!() macro for defining PCI > > >> configuration space registers once it > > > > > > I'll rebase the `register!` series on top of this and try to address > > > this item. > > > > Actually... is this expected to work? > > > > > + dev_info!( > > > + pdev.as_ref(), > > > + "pci-testdev config space read8 rev ID: {:x}\n", > > > + config.read8(0x8) > > > + ); > > > > We are performing I/O on `config`, but since this is a sample device > > with no hardware backing, what is providing the data for the > > registers? > > > > We are running the sample driver in the virtual machine. QEMU provides > a emulated PCI test device (-device pci-testdev) for this purpose. E.g. > with the following command line, I am able to run and test this in the > virtual machine. > > qemu-system-x86_64 \ > -enable-kvm \ > -cpu host \ > -m 8192 \ > -smp 24 \ > -drive file=/home/inno/vm/xubuntu.img,if=virtio \ > -netdev user,id=net0 \ > -device virtio-net-pci,netdev=net0 \ > -device usb-tablet \ > -device pci-testdev \ > -usb The dmesg in the VM: [ 2.762517] rust_driver_pci 0000:00:04.0: pci-testdev data-match count:1 [ 2.764368] rust_driver_pci 0000:00:04.0: pci-testdev config space read8 rev ID: 0 [ 2.766478] rust_driver_pci 0000:00:04.0: pci-testdev config space read16 vendor ID: 1b36 [ 2.770523] rust_driver_pci 0000:00:04.0: pci-testdev config space read32 BAR 0: febd2000 Z. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 5/5] sample: rust: pci: add tests for config space routines 2026-01-26 9:05 ` Zhi Wang 2026-01-26 10:21 ` Zhi Wang @ 2026-01-26 10:35 ` Alexandre Courbot 1 sibling, 0 replies; 25+ messages in thread From: Alexandre Courbot @ 2026-01-26 10:35 UTC (permalink / raw) To: Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, joelagnelf, jhubbard, zhiwang, daniel.almeida On Mon Jan 26, 2026 at 6:05 PM JST, Zhi Wang wrote: > On Mon, 26 Jan 2026 14:08:30 +0900 > "Alexandre Courbot" <acourbot@nvidia.com> wrote: > >> On Thu Jan 22, 2026 at 1:31 PM JST, Alexandre Courbot wrote: >> > On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote: >> >> Add tests exercising the PCI configuration space helpers. >> >> >> >> Suggested-by: Danilo Krummrich <dakr@kernel.org> >> >> Signed-off-by: Zhi Wang <zhiw@nvidia.com> >> >> --- >> >> samples/rust/rust_driver_pci.rs | 28 ++++++++++++++++++++++++++++ >> >> 1 file changed, 28 insertions(+) >> >> >> >> diff --git a/samples/rust/rust_driver_pci.rs >> >> b/samples/rust/rust_driver_pci.rs index 38c949efce38..1bc5bd1a8df5 >> >> 100644 --- a/samples/rust/rust_driver_pci.rs >> >> +++ b/samples/rust/rust_driver_pci.rs >> >> @@ -5,6 +5,7 @@ >> >> //! To make this driver probe, QEMU must be run with `-device >> >> pci-testdev`. >> >> use kernel::{ >> >> + device::Bound, >> >> device::Core, >> >> devres::Devres, >> >> io::Io, >> >> @@ -65,6 +66,32 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> >> >> Result<u32> { >> >> Ok(bar.read32(Regs::COUNT)) >> >> } >> >> + >> >> + fn config_space(pdev: &pci::Device<Bound>) -> Result { >> >> + let config = pdev.config_space()?; >> >> + >> >> + // TODO: use the register!() macro for defining PCI >> >> configuration space registers once it >> > >> > I'll rebase the `register!` series on top of this and try to address >> > this item. >> >> Actually... is this expected to work? >> >> > + dev_info!( >> > + pdev.as_ref(), >> > + "pci-testdev config space read8 rev ID: {:x}\n", >> > + config.read8(0x8) >> > + ); >> >> We are performing I/O on `config`, but since this is a sample device >> with no hardware backing, what is providing the data for the >> registers? >> > > We are running the sample driver in the virtual machine. QEMU provides > a emulated PCI test device (-device pci-testdev) for this purpose. E.g. > with the following command line, I am able to run and test this in the > virtual machine. > > qemu-system-x86_64 \ > -enable-kvm \ > -cpu host \ > -m 8192 \ > -smp 24 \ > -drive file=/home/inno/vm/xubuntu.img,if=virtio \ > -netdev user,id=net0 \ > -device virtio-net-pci,netdev=net0 \ > -device usb-tablet \ > -device pci-testdev \ > -usb Ah, that makes sense! Thanks for the explanation. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 5/5] sample: rust: pci: add tests for config space routines 2026-01-21 20:22 ` [PATCH v12 5/5] sample: rust: pci: add tests for config space routines Zhi Wang 2026-01-22 4:31 ` Alexandre Courbot @ 2026-01-22 12:00 ` Gary Guo 1 sibling, 0 replies; 25+ messages in thread From: Gary Guo @ 2026-01-22 12:00 UTC (permalink / raw) To: Zhi Wang, rust-for-linux, linux-pci, linux-kernel Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida On Wed Jan 21, 2026 at 8:22 PM GMT, Zhi Wang wrote: > Add tests exercising the PCI configuration space helpers. > > Suggested-by: Danilo Krummrich <dakr@kernel.org> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> Reviewed-by: Gary Guo <gary@garyguo.net> > --- > samples/rust/rust_driver_pci.rs | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 0/5] rust: pci: add config space read/write support 2026-01-21 20:22 [PATCH v12 0/5] rust: pci: add config space read/write support Zhi Wang ` (4 preceding siblings ...) 2026-01-21 20:22 ` [PATCH v12 5/5] sample: rust: pci: add tests for config space routines Zhi Wang @ 2026-01-24 0:12 ` Danilo Krummrich 5 siblings, 0 replies; 25+ messages in thread From: Danilo Krummrich @ 2026-01-24 0:12 UTC (permalink / raw) To: Zhi Wang Cc: rust-for-linux, linux-pci, linux-kernel, aliceryhl, bhelgaas, kwilczynski, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, helgaas, cjia, smitra, ankita, aniketa, kwankhede, targupta, acourbot, joelagnelf, jhubbard, zhiwang, daniel.almeida On Wed Jan 21, 2026 at 9:22 PM CET, Zhi Wang wrote: > Zhi Wang (5): > rust: devres: style for imports > rust: io: separate generic I/O helpers from MMIO implementation [ Add #[expect(unused)] to define_{read,write}!(). - Danilo ] > rust: io: factor out MMIO read/write macros > rust: pci: add config space read/write support [ Applied the diff from [1], considering subsequent comment; remove #[expect(unused)] from define_{read,write}!(). - Danilo ] > sample: rust: pci: add tests for config space routines Applied to driver-core-testing, thanks! ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2026-01-26 10:35 UTC | newest] Thread overview: 25+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-01-21 20:22 [PATCH v12 0/5] rust: pci: add config space read/write support Zhi Wang 2026-01-21 20:22 ` [PATCH v12 1/5] rust: devres: style for imports Zhi Wang 2026-01-22 4:25 ` Alexandre Courbot 2026-01-21 20:22 ` [PATCH v12 2/5] rust: io: separate generic I/O helpers from MMIO implementation Zhi Wang 2026-01-22 4:28 ` Alexandre Courbot 2026-01-22 11:52 ` Gary Guo 2026-01-22 12:52 ` Danilo Krummrich 2026-01-23 9:13 ` Alice Ryhl 2026-01-21 20:22 ` [PATCH v12 3/5] rust: io: factor out MMIO read/write macros Zhi Wang 2026-01-22 4:30 ` Alexandre Courbot 2026-01-22 11:54 ` Gary Guo 2026-01-21 20:22 ` [PATCH v12 4/5] rust: pci: add config space read/write support Zhi Wang 2026-01-22 4:25 ` Alexandre Courbot 2026-01-22 11:59 ` Gary Guo 2026-01-22 12:40 ` Danilo Krummrich 2026-01-22 12:52 ` Zhi Wang 2026-01-22 14:26 ` Gary Guo 2026-01-21 20:22 ` [PATCH v12 5/5] sample: rust: pci: add tests for config space routines Zhi Wang 2026-01-22 4:31 ` Alexandre Courbot 2026-01-26 5:08 ` Alexandre Courbot 2026-01-26 9:05 ` Zhi Wang 2026-01-26 10:21 ` Zhi Wang 2026-01-26 10:35 ` Alexandre Courbot 2026-01-22 12:00 ` Gary Guo 2026-01-24 0:12 ` [PATCH v12 0/5] rust: pci: add config space read/write support Danilo Krummrich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox