This sample driver demonstrates the following basic operations:

* Register a Misc Device
* Create /dev/rust-misc-device
* Open the aforementioned character device
* Operate on the character device via a simple ioctl()
* Close the character device

Signed-off-by: Lee Jones <lee@kernel.org>
---
 samples/rust/Kconfig             | 10 ++++
 samples/rust/Makefile            |  1 +
 samples/rust/rust_misc_device.rs | 80 ++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+)
 create mode 100644 samples/rust/rust_misc_device.rs

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index b0f74a81c8f9..df384e679901 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -20,6 +20,16 @@ config SAMPLE_RUST_MINIMAL
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_MISC_DEVICE
+	tristate "Misc device"
+	help
+	  This option builds the Rust misc device.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_misc_device.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_PRINT
 	tristate "Printing macros"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index c1a5c1655395..ad4b97a98580 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -2,6 +2,7 @@
 ccflags-y += -I$(src)				# needed for trace events
 
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
+obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
 
 rust_print-y := rust_print_main.o rust_print_events.o
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
new file mode 100644
index 000000000000..f50925713f1a
--- /dev/null
+++ b/samples/rust/rust_misc_device.rs
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Rust misc device sample.
+
+use kernel::{
+    c_str,
+    ioctl::_IO,
+    miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
+    prelude::*,
+};
+
+const RUST_MISC_DEV_HELLO: u32 = _IO('R' as u32, 9);
+
+module! {
+    type: RustMiscDeviceModule,
+    name: "rust_misc_device",
+    author: "Lee Jones",
+    description: "Rust misc device sample",
+    license: "GPL",
+}
+
+struct RustMiscDeviceModule {
+    _miscdev: Pin<KBox<MiscDeviceRegistration<RustMiscDevice>>>,
+}
+
+impl kernel::Module for RustMiscDeviceModule {
+    fn init(_module: &'static ThisModule) -> Result<Self> {
+        pr_info!("Initialising Rust Misc Device Sample\n");
+
+        let options = MiscDeviceOptions {
+            name: c_str!("rust-misc-device"),
+        };
+
+        Ok(Self {
+            _miscdev: KBox::pin_init(
+                MiscDeviceRegistration::<RustMiscDevice>::register(options),
+                GFP_KERNEL,
+            )?,
+        })
+    }
+}
+
+struct RustMiscDevice;
+
+#[vtable]
+impl MiscDevice for RustMiscDevice {
+    type Ptr = KBox<Self>;
+
+    fn open() -> Result<KBox<Self>> {
+        pr_info!("Opening Rust Misc Device Sample\n");
+
+        Ok(KBox::new(RustMiscDevice, GFP_KERNEL)?)
+    }
+
+    fn ioctl(
+        _device: <Self::Ptr as kernel::types::ForeignOwnable>::Borrowed<'_>,
+        cmd: u32,
+        _arg: usize,
+    ) -> Result<isize> {
+        pr_info!("IOCTLing Rust Misc Device Sample\n");
+
+        match cmd {
+            RUST_MISC_DEV_HELLO => pr_info!("Hello from the Rust Misc Device\n"),
+            _ => {
+                pr_err!("IOCTL not recognised: {}\n", cmd);
+                return Err(ENOIOCTLCMD);
+            }
+        }
+
+        Ok(0)
+    }
+}
+
+impl Drop for RustMiscDevice {
+    fn drop(&mut self) {
+        pr_info!("Exiting the Rust Misc Device Sample\n");
+    }
+}
-- 
2.47.0.338.g60cca15819-goog

On Thu, Dec 05, 2024 at 04:25:20PM +0000, Lee Jones wrote:
> This sample driver demonstrates the following basic operations:
> 
> * Register a Misc Device
> * Create /dev/rust-misc-device
> * Open the aforementioned character device
> * Operate on the character device via a simple ioctl()
> * Close the character device
> 
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>  samples/rust/Kconfig             | 10 ++++
>  samples/rust/Makefile            |  1 +
>  samples/rust/rust_misc_device.rs | 80 ++++++++++++++++++++++++++++++++
>  3 files changed, 91 insertions(+)
>  create mode 100644 samples/rust/rust_misc_device.rs
> 
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index b0f74a81c8f9..df384e679901 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -20,6 +20,16 @@ config SAMPLE_RUST_MINIMAL
>  
>  	  If unsure, say N.
>  
> +config SAMPLE_RUST_MISC_DEVICE
> +	tristate "Misc device"
> +	help
> +	  This option builds the Rust misc device.
> +
> +	  To compile this as a module, choose M here:
> +	  the module will be called rust_misc_device.
> +
> +	  If unsure, say N.
> +
>  config SAMPLE_RUST_PRINT
>  	tristate "Printing macros"
>  	help
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index c1a5c1655395..ad4b97a98580 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -2,6 +2,7 @@
>  ccflags-y += -I$(src)				# needed for trace events
>  
>  obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
> +obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
>  obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
>  
>  rust_print-y := rust_print_main.o rust_print_events.o
> diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> new file mode 100644
> index 000000000000..f50925713f1a
> --- /dev/null
> +++ b/samples/rust/rust_misc_device.rs
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 Google LLC.
> +
> +//! Rust misc device sample.
> +
> +use kernel::{
> +    c_str,
> +    ioctl::_IO,
> +    miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
> +    prelude::*,
> +};
> +
> +const RUST_MISC_DEV_HELLO: u32 = _IO('R' as u32, 9);
> +
> +module! {
> +    type: RustMiscDeviceModule,
> +    name: "rust_misc_device",
> +    author: "Lee Jones",
> +    description: "Rust misc device sample",
> +    license: "GPL",
> +}
> +
> +struct RustMiscDeviceModule {
> +    _miscdev: Pin<KBox<MiscDeviceRegistration<RustMiscDevice>>>,

Why do we add examples where we ask people to allocate those structures with
kmalloc()?

`MiscDevice` should switch to the generic `Registration` type in [1] and use
`InPlaceModule`, such that those structures land in the .data section of the
binary.

[1] https://lore.kernel.org/rust-for-linux/20241205141533.111830-3-dakr@kernel.org/

> +}
> +
> +impl kernel::Module for RustMiscDeviceModule {
> +    fn init(_module: &'static ThisModule) -> Result<Self> {
> +        pr_info!("Initialising Rust Misc Device Sample\n");
> +
> +        let options = MiscDeviceOptions {
> +            name: c_str!("rust-misc-device"),
> +        };
> +
> +        Ok(Self {
> +            _miscdev: KBox::pin_init(
> +                MiscDeviceRegistration::<RustMiscDevice>::register(options),
> +                GFP_KERNEL,
> +            )?,
> +        })
> +    }
> +}
> +
> +struct RustMiscDevice;
> +
> +#[vtable]
> +impl MiscDevice for RustMiscDevice {
> +    type Ptr = KBox<Self>;
> +
> +    fn open() -> Result<KBox<Self>> {
> +        pr_info!("Opening Rust Misc Device Sample\n");
> +
> +        Ok(KBox::new(RustMiscDevice, GFP_KERNEL)?)
> +    }
> +
> +    fn ioctl(
> +        _device: <Self::Ptr as kernel::types::ForeignOwnable>::Borrowed<'_>,
> +        cmd: u32,
> +        _arg: usize,
> +    ) -> Result<isize> {
> +        pr_info!("IOCTLing Rust Misc Device Sample\n");
> +
> +        match cmd {
> +            RUST_MISC_DEV_HELLO => pr_info!("Hello from the Rust Misc Device\n"),
> +            _ => {
> +                pr_err!("IOCTL not recognised: {}\n", cmd);
> +                return Err(ENOIOCTLCMD);
> +            }
> +        }
> +
> +        Ok(0)
> +    }
> +}
> +
> +impl Drop for RustMiscDevice {
> +    fn drop(&mut self) {
> +        pr_info!("Exiting the Rust Misc Device Sample\n");
> +    }
> +}
> -- 
> 2.47.0.338.g60cca15819-goog
> 
>

On Thu, Dec 05, 2024 at 04:25:20PM +0000, Lee Jones wrote:
> This sample driver demonstrates the following basic operations:
> 
> * Register a Misc Device
> * Create /dev/rust-misc-device
> * Open the aforementioned character device
> * Operate on the character device via a simple ioctl()
> * Close the character device
> 
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>  samples/rust/Kconfig             | 10 ++++
>  samples/rust/Makefile            |  1 +
>  samples/rust/rust_misc_device.rs | 80 ++++++++++++++++++++++++++++++++
>  3 files changed, 91 insertions(+)
>  create mode 100644 samples/rust/rust_misc_device.rs
> 
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index b0f74a81c8f9..df384e679901 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -20,6 +20,16 @@ config SAMPLE_RUST_MINIMAL
>  
>  	  If unsure, say N.
>  
> +config SAMPLE_RUST_MISC_DEVICE
> +	tristate "Misc device"
> +	help
> +	  This option builds the Rust misc device.
> +
> +	  To compile this as a module, choose M here:
> +	  the module will be called rust_misc_device.
> +
> +	  If unsure, say N.
> +
>  config SAMPLE_RUST_PRINT
>  	tristate "Printing macros"
>  	help
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index c1a5c1655395..ad4b97a98580 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -2,6 +2,7 @@
>  ccflags-y += -I$(src)				# needed for trace events
>  
>  obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
> +obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
>  obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
>  
>  rust_print-y := rust_print_main.o rust_print_events.o
> diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> new file mode 100644
> index 000000000000..f50925713f1a
> --- /dev/null
> +++ b/samples/rust/rust_misc_device.rs
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 Google LLC.
> +
> +//! Rust misc device sample.
> +
> +use kernel::{
> +    c_str,
> +    ioctl::_IO,
> +    miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
> +    prelude::*,
> +};
> +
> +const RUST_MISC_DEV_HELLO: u32 = _IO('R' as u32, 9);
> +
> +module! {
> +    type: RustMiscDeviceModule,
> +    name: "rust_misc_device",
> +    author: "Lee Jones",
> +    description: "Rust misc device sample",
> +    license: "GPL",
> +}
> +
> +struct RustMiscDeviceModule {
> +    _miscdev: Pin<KBox<MiscDeviceRegistration<RustMiscDevice>>>,
> +}
> +
> +impl kernel::Module for RustMiscDeviceModule {
> +    fn init(_module: &'static ThisModule) -> Result<Self> {
> +        pr_info!("Initialising Rust Misc Device Sample\n");
> +
> +        let options = MiscDeviceOptions {
> +            name: c_str!("rust-misc-device"),
> +        };
> +
> +        Ok(Self {
> +            _miscdev: KBox::pin_init(
> +                MiscDeviceRegistration::<RustMiscDevice>::register(options),
> +                GFP_KERNEL,
> +            )?,
> +        })
> +    }
> +}
> +
> +struct RustMiscDevice;
> +
> +#[vtable]
> +impl MiscDevice for RustMiscDevice {
> +    type Ptr = KBox<Self>;
> +
> +    fn open() -> Result<KBox<Self>> {
> +        pr_info!("Opening Rust Misc Device Sample\n");
> +
> +        Ok(KBox::new(RustMiscDevice, GFP_KERNEL)?)
> +    }
> +
> +    fn ioctl(
> +        _device: <Self::Ptr as kernel::types::ForeignOwnable>::Borrowed<'_>,
> +        cmd: u32,
> +        _arg: usize,
> +    ) -> Result<isize> {
> +        pr_info!("IOCTLing Rust Misc Device Sample\n");
> +
> +        match cmd {
> +            RUST_MISC_DEV_HELLO => pr_info!("Hello from the Rust Misc Device\n"),
> +            _ => {
> +                pr_err!("IOCTL not recognised: {}\n", cmd);
> +                return Err(ENOIOCTLCMD);

To quote errno.h:
	These should never be seen by user programs

The correct value here is ENOTTY.

Yeah, fun stuff.  Not intuitive at all, sorry.

thanks,

greg k-h

On Fri, 06 Dec 2024, Greg KH wrote:

> On Thu, Dec 05, 2024 at 04:25:20PM +0000, Lee Jones wrote:
> > This sample driver demonstrates the following basic operations:
> > 
> > * Register a Misc Device
> > * Create /dev/rust-misc-device
> > * Open the aforementioned character device
> > * Operate on the character device via a simple ioctl()
> > * Close the character device
> > 
> > Signed-off-by: Lee Jones <lee@kernel.org>
> > ---
> >  samples/rust/Kconfig             | 10 ++++
> >  samples/rust/Makefile            |  1 +
> >  samples/rust/rust_misc_device.rs | 80 ++++++++++++++++++++++++++++++++
> >  3 files changed, 91 insertions(+)
> >  create mode 100644 samples/rust/rust_misc_device.rs
> > 
> > diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> > index b0f74a81c8f9..df384e679901 100644
> > --- a/samples/rust/Kconfig
> > +++ b/samples/rust/Kconfig
> > @@ -20,6 +20,16 @@ config SAMPLE_RUST_MINIMAL
> >  
> >  	  If unsure, say N.
> >  
> > +config SAMPLE_RUST_MISC_DEVICE
> > +	tristate "Misc device"
> > +	help
> > +	  This option builds the Rust misc device.
> > +
> > +	  To compile this as a module, choose M here:
> > +	  the module will be called rust_misc_device.
> > +
> > +	  If unsure, say N.
> > +
> >  config SAMPLE_RUST_PRINT
> >  	tristate "Printing macros"
> >  	help
> > diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> > index c1a5c1655395..ad4b97a98580 100644
> > --- a/samples/rust/Makefile
> > +++ b/samples/rust/Makefile
> > @@ -2,6 +2,7 @@
> >  ccflags-y += -I$(src)				# needed for trace events
> >  
> >  obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
> > +obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
> >  obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
> >  
> >  rust_print-y := rust_print_main.o rust_print_events.o
> > diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> > new file mode 100644
> > index 000000000000..f50925713f1a
> > --- /dev/null
> > +++ b/samples/rust/rust_misc_device.rs
> > @@ -0,0 +1,80 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +// Copyright (C) 2024 Google LLC.
> > +
> > +//! Rust misc device sample.
> > +
> > +use kernel::{
> > +    c_str,
> > +    ioctl::_IO,
> > +    miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
> > +    prelude::*,
> > +};
> > +
> > +const RUST_MISC_DEV_HELLO: u32 = _IO('R' as u32, 9);
> > +
> > +module! {
> > +    type: RustMiscDeviceModule,
> > +    name: "rust_misc_device",
> > +    author: "Lee Jones",
> > +    description: "Rust misc device sample",
> > +    license: "GPL",
> > +}
> > +
> > +struct RustMiscDeviceModule {
> > +    _miscdev: Pin<KBox<MiscDeviceRegistration<RustMiscDevice>>>,
> > +}
> > +
> > +impl kernel::Module for RustMiscDeviceModule {
> > +    fn init(_module: &'static ThisModule) -> Result<Self> {
> > +        pr_info!("Initialising Rust Misc Device Sample\n");
> > +
> > +        let options = MiscDeviceOptions {
> > +            name: c_str!("rust-misc-device"),
> > +        };
> > +
> > +        Ok(Self {
> > +            _miscdev: KBox::pin_init(
> > +                MiscDeviceRegistration::<RustMiscDevice>::register(options),
> > +                GFP_KERNEL,
> > +            )?,
> > +        })
> > +    }
> > +}
> > +
> > +struct RustMiscDevice;
> > +
> > +#[vtable]
> > +impl MiscDevice for RustMiscDevice {
> > +    type Ptr = KBox<Self>;
> > +
> > +    fn open() -> Result<KBox<Self>> {
> > +        pr_info!("Opening Rust Misc Device Sample\n");
> > +
> > +        Ok(KBox::new(RustMiscDevice, GFP_KERNEL)?)
> > +    }
> > +
> > +    fn ioctl(
> > +        _device: <Self::Ptr as kernel::types::ForeignOwnable>::Borrowed<'_>,
> > +        cmd: u32,
> > +        _arg: usize,
> > +    ) -> Result<isize> {
> > +        pr_info!("IOCTLing Rust Misc Device Sample\n");
> > +
> > +        match cmd {
> > +            RUST_MISC_DEV_HELLO => pr_info!("Hello from the Rust Misc Device\n"),
> > +            _ => {
> > +                pr_err!("IOCTL not recognised: {}\n", cmd);
> > +                return Err(ENOIOCTLCMD);
> 
> To quote errno.h:
> 	These should never be seen by user programs
> 
> The correct value here is ENOTTY.
> 
> Yeah, fun stuff.  Not intuitive at all, sorry.

I read the instructions in: Documentation/driver-api/ioctl.rst

"When the ioctl callback is called with an unknown command number, the
 handler returns either -ENOTTY or -ENOIOCTLCMD, which also results in
 -ENOTTY being returned from the system call. Some subsystems return
 -ENOSYS or -EINVAL here for historic reasons, but this is wrong."

This makes it sound like the system call itself converts one to the
other.  Either way, I'll make the change.  Thanks.

-- 
Lee Jones [李琼斯]

On Fri, Dec 6, 2024, at 07:49, Greg KH wrote:
> On Thu, Dec 05, 2024 at 04:25:20PM +0000, Lee Jones wrote:
>> +
>> +#[vtable]
>> +impl MiscDevice for RustMiscDevice {
>> +    type Ptr = KBox<Self>;
>> +
>> +    fn open() -> Result<KBox<Self>> {
>> +        pr_info!("Opening Rust Misc Device Sample\n");
>> +
>> +        Ok(KBox::new(RustMiscDevice, GFP_KERNEL)?)
>> +    }
>> +
>> +    fn ioctl(
>> +        _device: <Self::Ptr as kernel::types::ForeignOwnable>::Borrowed<'_>,
>> +        cmd: u32,
>> +        _arg: usize,
>> +    ) -> Result<isize> {
>> +        pr_info!("IOCTLing Rust Misc Device Sample\n");
>> +
>> +        match cmd {
>> +            RUST_MISC_DEV_HELLO => pr_info!("Hello from the Rust Misc Device\n"),
>> +            _ => {
>> +                pr_err!("IOCTL not recognised: {}\n", cmd);
>> +                return Err(ENOIOCTLCMD);
>
> To quote errno.h:
> 	These should never be seen by user programs
>
> The correct value here is ENOTTY.
>
> Yeah, fun stuff.  Not intuitive at all, sorry.

That should get handled by vfs_ioctl() converting the
ENOIOCTLCMD to ENOTTY.

       Arnd

On Fri, Dec 06, 2024 at 07:52:46AM +0100, Arnd Bergmann wrote:
> On Fri, Dec 6, 2024, at 07:49, Greg KH wrote:
> > On Thu, Dec 05, 2024 at 04:25:20PM +0000, Lee Jones wrote:
> >> +
> >> +#[vtable]
> >> +impl MiscDevice for RustMiscDevice {
> >> +    type Ptr = KBox<Self>;
> >> +
> >> +    fn open() -> Result<KBox<Self>> {
> >> +        pr_info!("Opening Rust Misc Device Sample\n");
> >> +
> >> +        Ok(KBox::new(RustMiscDevice, GFP_KERNEL)?)
> >> +    }
> >> +
> >> +    fn ioctl(
> >> +        _device: <Self::Ptr as kernel::types::ForeignOwnable>::Borrowed<'_>,
> >> +        cmd: u32,
> >> +        _arg: usize,
> >> +    ) -> Result<isize> {
> >> +        pr_info!("IOCTLing Rust Misc Device Sample\n");
> >> +
> >> +        match cmd {
> >> +            RUST_MISC_DEV_HELLO => pr_info!("Hello from the Rust Misc Device\n"),
> >> +            _ => {
> >> +                pr_err!("IOCTL not recognised: {}\n", cmd);
> >> +                return Err(ENOIOCTLCMD);
> >
> > To quote errno.h:
> > 	These should never be seen by user programs
> >
> > The correct value here is ENOTTY.
> >
> > Yeah, fun stuff.  Not intuitive at all, sorry.
> 
> That should get handled by vfs_ioctl() converting the
> ENOIOCTLCMD to ENOTTY.

Ah, I always miss that, for some reason I thought that didn't happen
there, but happened in subsystems that did ENOIOCTLCMD for their
sub-ioctl handlers.  And yet it's always been that way, nevermind...

Anyway, I always recommend just using ENOTTY to be "safe", the usual way
people get this wrong is using EINVAL.

thanks,

greg k-h

On Fri, 06 Dec 2024, Greg Kroah-Hartman wrote:

> On Fri, Dec 06, 2024 at 07:52:46AM +0100, Arnd Bergmann wrote:
> > On Fri, Dec 6, 2024, at 07:49, Greg KH wrote:
> > > On Thu, Dec 05, 2024 at 04:25:20PM +0000, Lee Jones wrote:
> > >> +
> > >> +#[vtable]
> > >> +impl MiscDevice for RustMiscDevice {
> > >> +    type Ptr = KBox<Self>;
> > >> +
> > >> +    fn open() -> Result<KBox<Self>> {
> > >> +        pr_info!("Opening Rust Misc Device Sample\n");
> > >> +
> > >> +        Ok(KBox::new(RustMiscDevice, GFP_KERNEL)?)
> > >> +    }
> > >> +
> > >> +    fn ioctl(
> > >> +        _device: <Self::Ptr as kernel::types::ForeignOwnable>::Borrowed<'_>,
> > >> +        cmd: u32,
> > >> +        _arg: usize,
> > >> +    ) -> Result<isize> {
> > >> +        pr_info!("IOCTLing Rust Misc Device Sample\n");
> > >> +
> > >> +        match cmd {
> > >> +            RUST_MISC_DEV_HELLO => pr_info!("Hello from the Rust Misc Device\n"),
> > >> +            _ => {
> > >> +                pr_err!("IOCTL not recognised: {}\n", cmd);
> > >> +                return Err(ENOIOCTLCMD);
> > >
> > > To quote errno.h:
> > > 	These should never be seen by user programs
> > >
> > > The correct value here is ENOTTY.
> > >
> > > Yeah, fun stuff.  Not intuitive at all, sorry.
> > 
> > That should get handled by vfs_ioctl() converting the
> > ENOIOCTLCMD to ENOTTY.
> 
> Ah, I always miss that, for some reason I thought that didn't happen
> there, but happened in subsystems that did ENOIOCTLCMD for their
> sub-ioctl handlers.  And yet it's always been that way, nevermind...
> 
> Anyway, I always recommend just using ENOTTY to be "safe", the usual way
> people get this wrong is using EINVAL.

No matter.  I can change it.

-- 
Lee Jones [李琼斯]

On Thu, Dec 05, 2024 at 04:25:20PM +0000, Lee Jones wrote:
> This sample driver demonstrates the following basic operations:
> 
> * Register a Misc Device
> * Create /dev/rust-misc-device
> * Open the aforementioned character device
> * Operate on the character device via a simple ioctl()
> * Close the character device

Nit, the driver doesn't demonstrate open/close, it "provides" open/close
functionality if someone wants to use it :)

> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>  samples/rust/Kconfig             | 10 ++++
>  samples/rust/Makefile            |  1 +
>  samples/rust/rust_misc_device.rs | 80 ++++++++++++++++++++++++++++++++
>  3 files changed, 91 insertions(+)
>  create mode 100644 samples/rust/rust_misc_device.rs
> 
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index b0f74a81c8f9..df384e679901 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -20,6 +20,16 @@ config SAMPLE_RUST_MINIMAL
>  
>  	  If unsure, say N.
>  
> +config SAMPLE_RUST_MISC_DEVICE
> +	tristate "Misc device"
> +	help
> +	  This option builds the Rust misc device.
> +
> +	  To compile this as a module, choose M here:
> +	  the module will be called rust_misc_device.
> +
> +	  If unsure, say N.
> +
>  config SAMPLE_RUST_PRINT
>  	tristate "Printing macros"
>  	help
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index c1a5c1655395..ad4b97a98580 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -2,6 +2,7 @@
>  ccflags-y += -I$(src)				# needed for trace events
>  
>  obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
> +obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
>  obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
>  
>  rust_print-y := rust_print_main.o rust_print_events.o
> diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> new file mode 100644
> index 000000000000..f50925713f1a
> --- /dev/null
> +++ b/samples/rust/rust_misc_device.rs
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 Google LLC.
> +
> +//! Rust misc device sample.
> +
> +use kernel::{
> +    c_str,
> +    ioctl::_IO,
> +    miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
> +    prelude::*,
> +};
> +
> +const RUST_MISC_DEV_HELLO: u32 = _IO('R' as u32, 9);
> +
> +module! {
> +    type: RustMiscDeviceModule,
> +    name: "rust_misc_device",
> +    author: "Lee Jones",
> +    description: "Rust misc device sample",
> +    license: "GPL",
> +}
> +
> +struct RustMiscDeviceModule {
> +    _miscdev: Pin<KBox<MiscDeviceRegistration<RustMiscDevice>>>,
> +}
> +
> +impl kernel::Module for RustMiscDeviceModule {
> +    fn init(_module: &'static ThisModule) -> Result<Self> {
> +        pr_info!("Initialising Rust Misc Device Sample\n");
> +
> +        let options = MiscDeviceOptions {
> +            name: c_str!("rust-misc-device"),
> +        };
> +
> +        Ok(Self {
> +            _miscdev: KBox::pin_init(
> +                MiscDeviceRegistration::<RustMiscDevice>::register(options),
> +                GFP_KERNEL,
> +            )?,
> +        })
> +    }
> +}
> +
> +struct RustMiscDevice;
> +
> +#[vtable]
> +impl MiscDevice for RustMiscDevice {
> +    type Ptr = KBox<Self>;
> +
> +    fn open() -> Result<KBox<Self>> {
> +        pr_info!("Opening Rust Misc Device Sample\n");

I'd prefer this to be dev_info() to start with please.

thanks,

greg k-h

On Fri, 06 Dec 2024, Greg KH wrote:

> On Thu, Dec 05, 2024 at 04:25:20PM +0000, Lee Jones wrote:
> > This sample driver demonstrates the following basic operations:
> > 
> > * Register a Misc Device
> > * Create /dev/rust-misc-device
> > * Open the aforementioned character device
> > * Operate on the character device via a simple ioctl()
> > * Close the character device
> 
> Nit, the driver doesn't demonstrate open/close, it "provides" open/close
> functionality if someone wants to use it :)

Okay!  :)

> > Signed-off-by: Lee Jones <lee@kernel.org>
> > ---
> >  samples/rust/Kconfig             | 10 ++++
> >  samples/rust/Makefile            |  1 +
> >  samples/rust/rust_misc_device.rs | 80 ++++++++++++++++++++++++++++++++
> >  3 files changed, 91 insertions(+)
> >  create mode 100644 samples/rust/rust_misc_device.rs
> > 
> > diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> > index b0f74a81c8f9..df384e679901 100644
> > --- a/samples/rust/Kconfig
> > +++ b/samples/rust/Kconfig
> > @@ -20,6 +20,16 @@ config SAMPLE_RUST_MINIMAL
> >  
> >  	  If unsure, say N.
> >  
> > +config SAMPLE_RUST_MISC_DEVICE
> > +	tristate "Misc device"
> > +	help
> > +	  This option builds the Rust misc device.
> > +
> > +	  To compile this as a module, choose M here:
> > +	  the module will be called rust_misc_device.
> > +
> > +	  If unsure, say N.
> > +
> >  config SAMPLE_RUST_PRINT
> >  	tristate "Printing macros"
> >  	help
> > diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> > index c1a5c1655395..ad4b97a98580 100644
> > --- a/samples/rust/Makefile
> > +++ b/samples/rust/Makefile
> > @@ -2,6 +2,7 @@
> >  ccflags-y += -I$(src)				# needed for trace events
> >  
> >  obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
> > +obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
> >  obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
> >  
> >  rust_print-y := rust_print_main.o rust_print_events.o
> > diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> > new file mode 100644
> > index 000000000000..f50925713f1a
> > --- /dev/null
> > +++ b/samples/rust/rust_misc_device.rs
> > @@ -0,0 +1,80 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +// Copyright (C) 2024 Google LLC.
> > +
> > +//! Rust misc device sample.
> > +
> > +use kernel::{
> > +    c_str,
> > +    ioctl::_IO,
> > +    miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
> > +    prelude::*,
> > +};
> > +
> > +const RUST_MISC_DEV_HELLO: u32 = _IO('R' as u32, 9);
> > +
> > +module! {
> > +    type: RustMiscDeviceModule,
> > +    name: "rust_misc_device",
> > +    author: "Lee Jones",
> > +    description: "Rust misc device sample",
> > +    license: "GPL",
> > +}
> > +
> > +struct RustMiscDeviceModule {
> > +    _miscdev: Pin<KBox<MiscDeviceRegistration<RustMiscDevice>>>,
> > +}
> > +
> > +impl kernel::Module for RustMiscDeviceModule {
> > +    fn init(_module: &'static ThisModule) -> Result<Self> {
> > +        pr_info!("Initialising Rust Misc Device Sample\n");
> > +
> > +        let options = MiscDeviceOptions {
> > +            name: c_str!("rust-misc-device"),
> > +        };
> > +
> > +        Ok(Self {
> > +            _miscdev: KBox::pin_init(
> > +                MiscDeviceRegistration::<RustMiscDevice>::register(options),
> > +                GFP_KERNEL,
> > +            )?,
> > +        })
> > +    }
> > +}
> > +
> > +struct RustMiscDevice;
> > +
> > +#[vtable]
> > +impl MiscDevice for RustMiscDevice {
> > +    type Ptr = KBox<Self>;
> > +
> > +    fn open() -> Result<KBox<Self>> {
> > +        pr_info!("Opening Rust Misc Device Sample\n");
> 
> I'd prefer this to be dev_info() to start with please.

This is not possible at the moment.  Please see the cover-letter.

-- 
Lee Jones [李琼斯]

On Fri, Dec 06, 2024 at 07:14:30AM +0000, Lee Jones wrote:
> On Fri, 06 Dec 2024, Greg KH wrote:
> > > +    fn open() -> Result<KBox<Self>> {
> > > +        pr_info!("Opening Rust Misc Device Sample\n");
> > 
> > I'd prefer this to be dev_info() to start with please.
> 
> This is not possible at the moment.  Please see the cover-letter.

Then why make the change to miscdevice.rs if that pointer provided there
doesn't work for you here?

confused,

greg k-h

On Fri, 06 Dec 2024, Greg KH wrote:

> On Fri, Dec 06, 2024 at 07:14:30AM +0000, Lee Jones wrote:
> > On Fri, 06 Dec 2024, Greg KH wrote:
> > > > +    fn open() -> Result<KBox<Self>> {
> > > > +        pr_info!("Opening Rust Misc Device Sample\n");
> > > 
> > > I'd prefer this to be dev_info() to start with please.
> > 
> > This is not possible at the moment.  Please see the cover-letter.
> 
> Then why make the change to miscdevice.rs if that pointer provided there
> doesn't work for you here?

It's half the puzzle to get it working.  We're waiting on the other
(more complex) part from Alice before we can make use of it.  Would you
like me to remove it from the set until we have all of the pieces?

-- 
Lee Jones [李琼斯]

On Fri, Dec 06, 2024 at 07:35:58AM +0000, Lee Jones wrote:
> On Fri, 06 Dec 2024, Greg KH wrote:
> 
> > On Fri, Dec 06, 2024 at 07:14:30AM +0000, Lee Jones wrote:
> > > On Fri, 06 Dec 2024, Greg KH wrote:
> > > > > +    fn open() -> Result<KBox<Self>> {
> > > > > +        pr_info!("Opening Rust Misc Device Sample\n");
> > > > 
> > > > I'd prefer this to be dev_info() to start with please.
> > > 
> > > This is not possible at the moment.  Please see the cover-letter.
> > 
> > Then why make the change to miscdevice.rs if that pointer provided there
> > doesn't work for you here?
> 
> It's half the puzzle to get it working.  We're waiting on the other
> (more complex) part from Alice before we can make use of it.  Would you
> like me to remove it from the set until we have all of the pieces?

I'm confused, if you have the reference here, what is preventing it from
being used?

And yes, if it doesn't work, it shouldn't be part of this series :)

thanks,

greg k-h

On Fri, 06 Dec 2024, Greg KH wrote:

> On Fri, Dec 06, 2024 at 07:35:58AM +0000, Lee Jones wrote:
> > On Fri, 06 Dec 2024, Greg KH wrote:
> > 
> > > On Fri, Dec 06, 2024 at 07:14:30AM +0000, Lee Jones wrote:
> > > > On Fri, 06 Dec 2024, Greg KH wrote:
> > > > > > +    fn open() -> Result<KBox<Self>> {
> > > > > > +        pr_info!("Opening Rust Misc Device Sample\n");
> > > > > 
> > > > > I'd prefer this to be dev_info() to start with please.
> > > > 
> > > > This is not possible at the moment.  Please see the cover-letter.
> > > 
> > > Then why make the change to miscdevice.rs if that pointer provided there
> > > doesn't work for you here?
> > 
> > It's half the puzzle to get it working.  We're waiting on the other
> > (more complex) part from Alice before we can make use of it.  Would you
> > like me to remove it from the set until we have all of the pieces?
> 
> I'm confused, if you have the reference here, what is preventing it from
> being used?
> 
> And yes, if it doesn't work, it shouldn't be part of this series :)

It works - we can use dev_*() in .init, but not after (i.e. in open(),
ioctl(), etc).

I'll pull it from the series and let Alice re-post it when we have all
of the parts.

-- 
Lee Jones [李琼斯]