linux-kernel - Re: [PATCH RFC 3/3] rust: hid: demo the core abstractions for probe and remove

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <m2cm4c4m7teaca3tog774tl25ynngicg4eav4i7xiqyrxsqwju@leh5og5d6uba>
Date: Thu, 13 Mar 2025 18:05:36 +0100
From: Benjamin Tissoires <bentiss@...nel.org>
To: Rahul Rameshbabu <sergeantsagara@...tonmail.com>
Cc: linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org, 
	linux-input@...r.kernel.org, dri-devel@...ts.freedesktop.org, Jiri Kosina <jikos@...nel.org>, 
	Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, Benno Lossin <benno.lossin@...ton.me>, 
	Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, 
	Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>
Subject: Re: [PATCH RFC 3/3] rust: hid: demo the core abstractions for probe
 and remove

On Mar 13 2025, Rahul Rameshbabu wrote:
> This is a very basic "hello, world!" implementation to illustrate that the
> probe and remove callbacks are working as expected. I chose an arbitrary
> device I had on hand for populating in the HID device id table.

Nice to see this live :)

Though as I mentioned in the previous patch, I'd prefer having one full
driver in a single patch and one separate patch with the skeleton in the
docs.

Do you have any meaningful processing that needs to be done in the
report fixup or in the .raw_event or .event callbacks?

It would be much more interesting to show how this works together on a
minimalistic driver.

FWIW, the driver in itself, though incomplete, looks familiar, which is
a good thing: we've got a probe and a remove. This is common with all
the other HID drivers, so it's not a brand new thing.

I wonder however how and if we could enforce the remove() to be
automated by the binding or rust, to not have to deal with resource
leaking. Because the removal part, especially on failure, is the hardest
part.

Cheers,
Benjamin

> 
>   [  +0.012968] monitor_control: Probing HID device vendor: 2389 product: 29204 using Rust!
>   [  +0.000108] monitor_control: Removing HID device vendor: 2389 product: 29204 using Rust!
> 
> Signed-off-by: Rahul Rameshbabu <sergeantsagara@...tonmail.com>
> ---
>  drivers/hid/hid_monitor_control.rs | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid_monitor_control.rs b/drivers/hid/hid_monitor_control.rs
> index 18afd69a56d5..aeb6e4058a6b 100644
> --- a/drivers/hid/hid_monitor_control.rs
> +++ b/drivers/hid/hid_monitor_control.rs
> @@ -8,17 +8,22 @@
>      Driver,
>  };
>  
> +const USB_VENDOR_ID_NVIDIA: u32 = 0x0955;
> +const USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER: u32 = 0x7214;
> +
>  struct HidMonitorControl;
>  
>  #[vtable]
>  impl Driver for HidMonitorControl {
>      fn probe(dev: &mut hid::Device, id: &hid::DeviceId) -> Result<()> {
>          /* TODO implement */
> +        pr_info!("Probing HID device vendor: {} product: {} using Rust!\n", id.vendor(), id.product());
>          Ok(())
>      }
>  
>      fn remove(dev: &mut hid::Device) {
>          /* TODO implement */
> +        pr_info!("Removing HID device vendor: {} product: {} using Rust!\n", dev.vendor(), dev.product());
>      }
>  }
>  
> @@ -26,8 +31,8 @@ fn remove(dev: &mut hid::Device) {
>      driver: HidMonitorControl,
>      id_table: [
>          kernel::usb_device! {
> -            vendor: /* TODO fill in */,
> -            product: /* TODO fill in */,
> +            vendor: USB_VENDOR_ID_NVIDIA,
> +            product: USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER,
>          },
>      ],
>      name: "monitor_control",
> -- 
> 2.47.2
> 
>