rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Zhi Wang <zhiw@nvidia.com>
Cc: rust-for-linux@vger.kernel.org, dakr@kernel.org,
	bhelgaas@google.com, kwilczynski@kernel.org, ojeda@kernel.org,
	alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net,
	bjorn3_gh@protonmail.com, lossin@kernel.org,
	a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu,
	linux-kernel@vger.kernel.org, cjia@nvidia.com, smitra@nvidia.com,
	ankita@nvidia.com, aniketa@nvidia.com, kwankhede@nvidia.com,
	targupta@nvidia.com, zhiwang@kernel.org, alwilliamson@nvidia.com,
	acourbot@nvidia.com, joelagnelf@nvidia.com, jhubbard@nvidia.com
Subject: Re: [RFC 1/2] rust: introduce abstractions for fwctl
Date: Thu, 30 Oct 2025 13:22:07 -0300	[thread overview]
Message-ID: <20251030162207.GS1018328@nvidia.com> (raw)
In-Reply-To: <20251030160315.451841-2-zhiw@nvidia.com>

On Thu, Oct 30, 2025 at 04:03:12PM +0000, Zhi Wang wrote:
> +impl<T: FwCtlOps> Registration<T> {
> +    /// Allocate and register a new fwctl device under the given parent device.
> +    pub fn new(parent: &device::Device) -> Result<Self> {
> +        let ops = &FwCtlVTable::<T>::VTABLE as *const _ as *mut _;
> +
> +        // SAFETY: `_fwctl_alloc_device()` allocates a new `fwctl_device`
> +        // and initializes its embedded `struct device`.
> +        let dev = unsafe {
> +            bindings::_fwctl_alloc_device(
> +                parent.as_raw(),
> +                ops,
> +                core::mem::size_of::<bindings::fwctl_device>(),
> +            )
> +        };
> +
> +        let dev = NonNull::new(dev).ok_or(ENOMEM)?;
> +
> +        // SAFETY: `fwctl_register()` expects a valid device from `_fwctl_alloc_device()`.
> +        let ret = unsafe { bindings::fwctl_register(dev.as_ptr()) };

This is a Bound device, not just any device.

> +        if ret != 0 {
> +            // SAFETY: If registration fails, release the allocated fwctl_device().
> +            unsafe {
> +                bindings::put_device(core::ptr::addr_of_mut!((*dev.as_ptr()).dev));

?? Don't open code fwctl_put() - it should be called directly?

> +            }
> +            return Err(Error::from_errno(ret));
> +        }
> +
> +        Ok(Self {
> +            fwctl_dev: dev,
> +            _marker: PhantomData,
> +        })
> +    }
> +
> +    fn as_raw(&self) -> *mut bindings::fwctl_device {
> +        self.fwctl_dev.as_ptr()
> +    }
> +}
> +
> +impl<T: FwCtlOps> Drop for Registration<T> {
> +    fn drop(&mut self) {
> +        // SAFETY: `fwctl_unregister()` expects a valid device from `_fwctl_alloc_device()`.

Incomplete safety statement, the device passed to fwctl_alloc_device must
still be bound prior to calling fwctl_unregister

> +        unsafe {
> +            bindings::fwctl_unregister(self.as_raw());
> +            bindings::put_device(core::ptr::addr_of_mut!((*self.as_raw()).dev));

There for Drop can only do fwctl_put() since otherwise there is no way
to guarantee a Bound device.

unregister has to happen before remove() completes, Danilo had some
approach to this I think he told me?

Jason

  reply	other threads:[~2025-10-30 16:22 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-30 16:03 [RFC 0/2] rust: introduce abstractions for fwctl Zhi Wang
2025-10-30 16:03 ` [RFC 1/2] " Zhi Wang
2025-10-30 16:22   ` Jason Gunthorpe [this message]
2025-10-30 17:19     ` Zhi Wang
2025-10-30 17:24       ` Danilo Krummrich
2025-10-30 17:21     ` Danilo Krummrich
2025-10-30 16:47   ` Danilo Krummrich
2025-11-02 17:26   ` Danilo Krummrich
2025-11-02 22:52     ` Jason Gunthorpe
2025-11-02 18:33   ` Danilo Krummrich
2025-11-02 22:55     ` Jason Gunthorpe
2025-11-03  9:55     ` Zhi Wang
2025-11-03 10:36       ` Danilo Krummrich
2025-10-30 16:03 ` [RFC 2/2] samples: rust: fwctl: add sample code for FwCtl Zhi Wang
2025-10-30 17:29 ` [RFC 0/2] rust: introduce abstractions for fwctl Zhi Wang
2025-10-30 17:52   ` Danilo Krummrich
2025-10-30 17:54     ` Jason Gunthorpe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251030162207.GS1018328@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=alwilliamson@nvidia.com \
    --cc=aniketa@nvidia.com \
    --cc=ankita@nvidia.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=cjia@nvidia.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=kwankhede@nvidia.com \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=smitra@nvidia.com \
    --cc=targupta@nvidia.com \
    --cc=tmgross@umich.edu \
    --cc=zhiw@nvidia.com \
    --cc=zhiwang@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).