rust: pci: Use explicit types in FFI callbacks

[PATCH v2] rust: pci: Use explicit types in FFI callbacks

Posted by herculoxz 2 months, 1 week ago

From: Abhinav Ananthu <abhinav.ogl@gmail.com>

Update PCI FFI callback signatures to use `c_int` from the prelude,
instead of accessing it via `kernel::ffi::c_int`.

This follows Rust-for-Linux coding guidelines and improves ABI
correctness when interfacing with C code.

Signed-off-by: Abhinav Ananthu <abhinav.ogl@gmail.com>
---
 rust/kernel/pci.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 5ce07999168e..fbeeaec4e044 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -61,7 +61,7 @@ impl<T: Driver + 'static> Adapter<T> {
     extern "C" fn probe_callback(
         pdev: *mut bindings::pci_dev,
         id: *const bindings::pci_device_id,
-    ) -> kernel::ffi::c_int {
+    ) -> c_int {
         // SAFETY: The PCI bus only ever calls the probe callback with a valid pointer to a
         // `struct pci_dev`.
         //
@@ -333,7 +333,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
         // `ioptr` is valid by the safety requirements.
         // `num` is valid by the safety requirements.
         unsafe {
-            bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
+            bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut c_void);
             bindings::pci_release_region(pdev.as_raw(), num);
         }
     }
-- 
2.34.1

Re: [PATCH v2] rust: pci: Use explicit types in FFI callbacks

Posted by Benno Lossin 2 months, 1 week ago

On Tue Jul 29, 2025 at 12:29 PM CEST, herculoxz wrote:
> From: Abhinav Ananthu <abhinav.ogl@gmail.com>
>
> Update PCI FFI callback signatures to use `c_int` from the prelude,
> instead of accessing it via `kernel::ffi::c_int`.
>
> This follows Rust-for-Linux coding guidelines and improves ABI
> correctness when interfacing with C code.
>
> Signed-off-by: Abhinav Ananthu <abhinav.ogl@gmail.com>

Reviewed-by: Benno Lossin <lossin@kernel.org>

---
Cheers,
Benno

> ---
>  rust/kernel/pci.rs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Re: [PATCH v2] rust: pci: Use explicit types in FFI callbacks

Posted by Miguel Ojeda 2 months, 1 week ago

On Tue, Jul 29, 2025 at 12:30 PM herculoxz <abhinav.ogl@gmail.com> wrote:
>
> and improves ABI
> correctness when interfacing with C code.

I think this still sounds like it is fixing an ABI issue -- I would
probably just remove that second sentence.

(But no need for a v3 -- I think it can be fixed on apply unless
Danilo wants it).

Thanks!

Cheers,
Miguel

Re: [PATCH v2] rust: pci: Use explicit types in FFI callbacks

Posted by Danilo Krummrich 2 months, 1 week ago

On Tue Jul 29, 2025 at 12:52 PM CEST, Miguel Ojeda wrote:
> On Tue, Jul 29, 2025 at 12:30 PM herculoxz <abhinav.ogl@gmail.com> wrote:
>>
>> and improves ABI
>> correctness when interfacing with C code.
>
> I think this still sounds like it is fixing an ABI issue -- I would
> probably just remove that second sentence.

I agree, the types exported via prelude are the ones from kernel::ffi.

> (But no need for a v3 -- I think it can be fixed on apply unless
> Danilo wants it).

Yeah, I can fix it up when applying the patch.

I also think the subject from v1, i.e. "use c_* types via kernel prelude", was
better. This one is a bit misleading, the types in the FFI callbacks are already
explicit.

Unless I hear otherwise, I will also revert the subject to the one of v1 when I
apply the patch.

Thanks,
Danilo

Re: [PATCH v2] rust: pci: Use explicit types in FFI callbacks

Posted by Alice Ryhl 2 months, 1 week ago

On Tue, Jul 29, 2025 at 04:45:50PM +0200, Danilo Krummrich wrote:
> On Tue Jul 29, 2025 at 12:52 PM CEST, Miguel Ojeda wrote:
> > On Tue, Jul 29, 2025 at 12:30 PM herculoxz <abhinav.ogl@gmail.com> wrote:
> >>
> >> and improves ABI
> >> correctness when interfacing with C code.
> >
> > I think this still sounds like it is fixing an ABI issue -- I would
> > probably just remove that second sentence.
> 
> I agree, the types exported via prelude are the ones from kernel::ffi.
> 
> > (But no need for a v3 -- I think it can be fixed on apply unless
> > Danilo wants it).
> 
> Yeah, I can fix it up when applying the patch.
> 
> I also think the subject from v1, i.e. "use c_* types via kernel prelude", was
> better. This one is a bit misleading, the types in the FFI callbacks are already
> explicit.
> 
> Unless I hear otherwise, I will also revert the subject to the one of v1 when I
> apply the patch.

With the above changes:

Reviewed-by: Alice Ryhl <aliceryhl@google.com>