| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20260122145234.1839245a.zhiw@nvidia.com>
Date: Thu, 22 Jan 2026 14:52:34 +0200
From: Zhi Wang <zhiw@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
CC: Gary Guo <gary@...yguo.net>, <rust-for-linux@...r.kernel.org>,
<linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<aliceryhl@...gle.com>, <bhelgaas@...gle.com>, <kwilczynski@...nel.org>,
<ojeda@...nel.org>, <alex.gaynor@...il.com>, <boqun.feng@...il.com>,
<bjorn3_gh@...tonmail.com>, <lossin@...nel.org>, <a.hindborg@...nel.org>,
<tmgross@...ch.edu>, <markus.probst@...teo.de>, <helgaas@...nel.org>,
<cjia@...dia.com>, <smitra@...dia.com>, <ankita@...dia.com>,
<aniketa@...dia.com>, <kwankhede@...dia.com>, <targupta@...dia.com>,
<acourbot@...dia.com>, <joelagnelf@...dia.com>, <jhubbard@...dia.com>,
<zhiwang@...nel.org>, <daniel.almeida@...labora.com>
Subject: Re: [PATCH v12 4/5] rust: pci: add config space read/write support
On Thu, 22 Jan 2026 13:40:22 +0100
"Danilo Krummrich" <dakr@...nel.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(),
> }))
>
Powered by blists - more mailing lists