Date: Thu, 18 Dec 2025 08:05:36 +0000
In-Reply-To: <20251218013910.459045-2-ttabi@nvidia.com>
Precedence: bulk
Mime-Version: 1.0
References: <20251218013910.459045-1-ttabi@nvidia.com> <20251218013910.459045-2-ttabi@nvidia.com>
Message-ID: <aUO10KftOHuOwDeP@google.com>
Subject: Re: [PATCH v3 1/9] rust: pci: add PCI device name method
From: Alice Ryhl <aliceryhl@google.com>
To: Timur Tabi <ttabi@nvidia.com>
Cc: Gary Guo <gary@garyguo.net>, Danilo Krummrich <dakr@kernel.org>, rust-for-linux@vger.kernel.org, 
	John Hubbard <jhubbard@nvidia.com>, Joel Fernandes <joelagnelf@nvidia.com>, 
	Alexandre Courbot <acourbot@nvidia.com>, Lyude Paul <lyude@redhat.com>, nouveau@lists.freedesktop.org
Content-Type: text/plain; charset="utf-8"

On Wed, Dec 17, 2025 at 07:39:02PM -0600, Timur Tabi wrote:
> Add a name() method to the PCI `Device` type, which returns a CStr
> that contains the device name, typically the BDF address.
> 
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/pci.rs | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 82e128431f08..125fb39f4316 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -427,6 +427,43 @@ pub fn pci_class(&self) -> Class {
>          // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
>          Class::from_raw(unsafe { (*self.as_raw()).class })
>      }
> +
> +    /// Returns the PCI device name.
> +    ///
> +    /// This returns the device name in the format "DDDD:BB:DD.F" where:
> +    /// - DDDD is the PCI domain (4 hex digits)
> +    /// - BB is the bus number (2 hex digits)
> +    /// - DD is the device number (2 hex digits)
> +    /// - F is the function number (1 hex digit)
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::{c_str, debugfs::Dir, device::Core, pci, prelude::*};
> +    /// fn create_debugfs(pdev: &pci::Device<Core>) -> Result {
> +    ///     let dir = Dir::new(pdev.name());
> +    ///     Ok(())
> +    /// }
> +    /// ```
> +    #[inline]
> +    pub fn name(&self) -> &CStr {
> +        // SAFETY: By its type invariant `self.as_raw` is always a valid pointer to a
> +        // `struct pci_dev`, which contains a `struct device dev` member.
> +        unsafe {

What's the actual unsafe operation in this block? We generally try to
limit the scope of unsafe as much as possible.

Alice

> +            let pci_dev = self.as_raw();
> +            let dev = &raw const (*pci_dev).dev;
> +
> +            // If init_name is set, use it; otherwise use the kobject name
> +            let init_name = (*dev).init_name;
> +            let name_ptr = if !init_name.is_null() {
> +                init_name
> +            } else {
> +                (*dev).kobj.name
> +            };
> +
> +            CStr::from_char_ptr(name_ptr)
> +        }
> +    }
>  }
>  
>  impl Device<device::Core> {
> -- 
> 2.52.0
>