content: usb-monitor-control weekend 3

Binary-Eater · Binary-Eater · commit bb09582fd0bd · 2025-03-02T14:00:30.000-08:00
Describe the HID binding layer work.

Signed-off-by: Rahul Rameshbabu &lt;sergeantsagara@protonmail.com&gt;
diff --git a/content/posts/linux_usb_monitor_control_weekend_3.org b/content/posts/linux_usb_monitor_control_weekend_3.org
@@ -0,0 +1,387 @@
+---
+title: "Supporting USB Monitor Control in the Linux kernel - Weekend 3: Bindings"
+tags: ["kernel", "display", "hid", "usb monitor control"]
+date: 2025-03-02T10:49:38-08:00
+draft: false
+---
+
+The work for binding the Linux kernel HID interface in C is taking shape. I have
+two commits related to the work in my tree. The reason the work is split in two
+was figuring out how to handle the HID device id table as a complex problem on
+its own compared to the rest of the binding work.
+
++ [[https://github.com/Binary-Eater/linux/commit/f6dc91a0eb7eaa6eca08bc246da3372e59ef022d]]
++ [[https://github.com/Binary-Eater/linux/commit/383a97f7a68fe083013e5f8fc11878361b73a022]]
+
+*DISCLAIMER:* The work for this happened on Feb. 22-23, 2025, but I did not have
+time to write the related blog that weekend.
+
+* Initial bindings and macro design
+
+The kernel has done a lot to work to simplify the C ffi in Rust signficantly.
+Specifically, the ~Opaque~ struct under ~rust/kernel/types.rs~ is very helpful.
+
+#+BEGIN_SRC rust
+  #[repr(transparent)]
+  pub struct Device(Opaque<bindings::hid_device>);
+
+  #[repr(transparent)]
+  pub struct DeviceId(Opaque<bindings::hid_device_id>);
+#+END_SRC
+
+What we are doing here is defining two single element [[https://doc.rust-lang.org/book/ch05-01-defining-structs.html#using-tuple-structs-without-named-fields-to-create-different-types]["tuple structs"]] where the
+first element is FFI object that is allowed to make guarantees Rust normally
+would not permit. This will be very useful for passing data in and out of the C
+api layer. We use the ~#[repr(transparent)]~ directive to ensure the Rust types
+we are declaring will have the same memory layout as the C ffi object types
+being used in the single element tuple structs.
+
+By guaranteeing the memory layout is the same, we can implement static methods
+for the type as follows.
+
+#+BEGIN_SRC rust
+  impl Device {
+      unsafe fn from_ptr<'a>(ptr: *mut bindings::hid_device) -> &'a mut Self {
+          let ptr = ptr.cast::<Self>();
+
+          unsafe { &mut *ptr };
+      }
+
+      pub fn vendor(&self) -> u32 {
+          let hdev = self.0.get();
+
+          unsafe { (*hdev).vendor }
+      }
+
+      // Omitted the rest
+  }
+
+  impl DeviceId {
+      unsafe fn from_ptr<'a>(ptr: *mut bindings::hid_device_id) -> &'a mut Self {
+          let ptr = ptr.cast::<Self>();
+
+          unsafe { &mut *ptr };
+      }
+
+      pub fn vendor(&self) -> u32 {
+          let hdev_id = self.0.get();
+
+          unsafe { (*hdev_id).vendor }
+      }
+
+      // Omitted the rest
+  }
+#+END_SRC
+
+We define ~from_ptr~ methods that enable the Rust layer to represent the C types
+using the memory layout equivalent Rust types. We then define some Rust getters
+to "safely" access the underlying C data. I need to go back later and add the
+~SAFETY~ comments in the code.
+
+* Driver adapters and vtables
+
+The Linux kernel module registration interface is well-designed with regards to
+C semantics. Bridging that over to Rust takes quite a bit of massaging. The rust
+binding code had a habit of utilizing a "adapter" and a "vtable". Lets first
+look at the way HID drivers are instantiated in C before delving into the Rust
+code.
+
+#+BEGIN_SRC c
+  /**
+   * struct hid_driver
+   * @name: driver name (e.g. "Footech_bar-wheel")
+   * @id_table: which devices is this driver for (must be non-NULL for probe
+   * 	      to be called)
+   * @dyn_list: list of dynamically added device ids
+   * @dyn_lock: lock protecting @dyn_list
+   * @match: check if the given device is handled by this driver
+   * @probe: new device inserted
+   * @remove: device removed (NULL if not a hot-plug capable driver)
+   * @report_table: on which reports to call raw_event (NULL means all)
+   * @raw_event: if report in report_table, this hook is called (NULL means nop)
+   * @usage_table: on which events to call event (NULL means all)
+   * @event: if usage in usage_table, this hook is called (NULL means nop)
+   * @report: this hook is called after parsing a report (NULL means nop)
+   * @report_fixup: called before report descriptor parsing (NULL means nop)
+   * @input_mapping: invoked on input registering before mapping an usage
+   * @input_mapped: invoked on input registering after mapping an usage
+   * @input_configured: invoked just before the device is registered
+   * @feature_mapping: invoked on feature registering
+   * @suspend: invoked on suspend (NULL means nop)
+   * @resume: invoked on resume if device was not reset (NULL means nop)
+   * @reset_resume: invoked on resume if device was reset (NULL means nop)
+   */
+
+  static struct hid_driver shield_driver = {
+  	.name          = "shield",
+  	.id_table      = shield_devices,
+  	.input_mapping = android_input_mapping,
+  	.probe         = shield_probe,
+  	.remove        = shield_remove,
+  	.raw_event     = shield_raw_event,
+  	.driver = {
+  		.dev_groups = shield_device_groups,
+  	},
+  };
+#+END_SRC
+
+We see that the ~hid_driver~ struct instance used for the ~hid-nvidia-shield~
+driver does not instantiate all members of the struct. This is common practice
+for structs used to bind driver functionality. However, this does not play well
+with Rust, which essentially requires all members of a struct to be initialized.
+We will need some involved logic to work around this.
+
+#+BEGIN_SRC rust
+  struct Adapter<T: Driver> {
+      _p: PhantomData<T>,
+  }
+
+  impl<T: Driver> Adapter<T> {
+      unsafe extern "C" fn probe_callback(
+          hdev: *mut bindings::hid_device,
+          hdev_id: *mut bindings::hid_device_id,
+      ) -> crate::ffi::c_int {
+          from_result(|| {
+              let dev = unsafe { Device::from_ptr(hdev) };
+              let dev_id = unsafe { DeviceId::from_ptr(hdev_id) };
+              T::probe(dev, dev_id)?;
+              Ok(0)
+          })
+      }
+
+      unsafe extern "C" fn remove_callback(hdev: *mut bindings::hid_device,) -> crate::ffi::c_int {
+          from_result(|| {
+              let dev = unsafe { Device::from_ptr(hdev) };
+              T::remove(dev)?;
+              Ok(0)
+          })
+      }
+  }
+#+END_SRC
+
+We define an ~Adapter~ struct capable of providing C linkage functions for
+feedback to C HID API. We make these C linkage functions call the native Rust
+HID ~Driver~ trait methods and provide C style return codes.
+
+We enabled the Rust HID ~Driver~ trait to have a ~vtable~ implementation, which
+means trait implementers do not have to define all the trait methods. This means
+that we need to handle the case when HID methods methods are missing. We create
+a ~DriverVTable~ type that is essentially mapped to ~hid_driver~. We also a
+~Driver~ trair generic ~create_hid_driver~ function to instantiate the wrapped
+~hid_driver~ type.
+
+#+BEGIN_SRC rust
+  #[repr(transparent)]
+  pub struct DriverVTable(Opaque<bindings::hid_driver>);
+
+  // SAFETY: `DriverVTable` doesn't expose any &self method to access internal data, so it's safe to
+  // share `&DriverVTable` across execution context boundaries.
+  unsafe impl Sync for DriverVTable {}
+
+  pub const fn create_hid_driver<T: Driver>(name: &'static CStr) -> DriverVTable {
+      DriverVTable(Opaque::new(bindings::hid_driver {
+          name: name.as_char_ptr(),
+          id_table: /* TODO */,
+          probe: if T::HAS_PROBE {
+              Some(Adapter::<T>::probe_callback)
+          } else {
+              None
+          },
+          remove: if T::HAS_REMOVE {
+              Some(Adapter::<T>::remove_callback)
+          } else {
+              None
+          },
+          // SAFETY: The rest is zeroed out to initialize `struct hid_driver`,
+          // sets `Option<&F>` to be `None`.
+          ..unsafe { core::mem::MaybeUninit::<bindings::hid_driver>::zeroed().assume_init() }
+      }))
+  }
+#+END_SRC
+
+* Registering the driver
+
+Normally for a C HID driver, we would call the ~module_hid_driver~ macro to
+instantiate the HID driver. Calling C-style macros from Rust is not an option.
+We can traverse that the macro eventually calls the ~__hid_register_driver~
+function, which we will want to call from Rust. Similarly, we will want to call
+~hid_unregister_driver~ when unbinding the driver. Both functions can be found
+in ~drivers/hid/hid-core.c~.
+
+We will have to utilize a a new struct and macro in Rust to provide the
+equivalent functionality for a Rust HID driver. Lets first explore the
+~Registration~ struct and implementation.
+
+#+BEGIN_SRC rust
+  pub struct Registration {
+      driver: Pin<&'static mut DriverVTable>,
+  }
+
+  unsafe impl Send for Registration {}
+
+  impl Registration {
+      pub fn register(
+          module: &'static crate::ThisModule,
+          driver: Pin<&'static mut DriverVTable>,
+          name: &'static CStr,
+      ) -> Result<Self> {
+          to_result(unsafe {
+              bindings::__hid_register_driver(driver.0.get(), module.0, name.as_char_ptr())
+          })?;
+
+          Ok(Registration { driver })
+      }
+  }
+
+  impl Drop for Registration {
+      fn drop(&mut self) {
+          unsafe {
+              bindings::hid_unregister_driver(self.driver.0.get())
+          };
+      }
+  }
+#+END_SRC
+
+The ~register~ function is the safe Rust wrapper for the ~__hid_register_driver~
+C call. We essentially use the ~Register~ struct for unregistering the driver
+when the driver is no longer binded. We do this by implementing the ~Drop~ trait
+and calling ~hid_unregister_driver~ in ~drop~.
+
+Working with the ~Register~ struct directly in a Rust HID driver would be
+clunky, so we can instead provide a Rust macro to mask the complexity.
+
+#+BEGIN_SRC rust
+  #[macro_export]
+  macro_rules! module_hid_driver {
+      (driver: $($driver:ident), name: $($name:expr), $(f:tt)*) => {
+          struct Module {
+              _reg: $crate::hid::Registration,
+          }
+
+          $crate::prelude::module! {
+              type: Module,
+              name: $($name),
+              $($f)*
+          }
+
+          const _: () = {
+              static mut DRIVER: $crate::hid::DriverVTable = $($crate::hid::create_hid_driver::<$driver>(NAME, /* TODO pass ID_TABLE */));
+          };
+      }
+  }
+#+END_SRC
+
+The ~macro_rules!~ for ~module_hid_driver~ here is very simple. Also, I need to
+make updates to the signature of ~create_hid_driver~ due to my changes to device
+ID table handling. We call the prelude ~module!~ macro to make the newly define
+~Module~ struct implement what is needed to be a Rust kernel module type.
+
+* Dealing with the HID device ID table
+
+Handling the HID device ID table was one of the most architecturally challenging
+aspects of this project for me. I have lost count of the number of design
+patterns I have scratched while trying to settle on how to handle this. To begin
+with, lets take a look at how this is handled in C.
+
+#+BEGIN_SRC c
+  static const struct hid_device_id shield_devices[] = {
+  	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NVIDIA,
+  			       USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER) },
+  	{ HID_USB_DEVICE(USB_VENDOR_ID_NVIDIA,
+  			 USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER) },
+  	{ }
+  };
+#+END_SRC
+
+We have a C style array where we do not explicitly state the size on the
+left-hand side (lhs) and have a "terminating" element in the right-hand side
+(rhs). This design pattern is completely not compatible with Rust. Rust arrays
+*must* declare their explicit size in the left-hand side and do not trivially
+support the terminator pattern. This made the design tricky. I eventually
+converged to using macros to assist with the arbitrary-sized structure.
+
+The first step was to make the ~HID_USB_DEVICE~ equivalent in Rust.
+
+#+BEGIN_SRC rust
+  #[macro_export]
+  macro_rules! usb_device {
+      (vendor: $($vendor:expr), product: $($product:expr)) => {
+          DeviceId(Opaque::new(bindings::hid_device_id {
+              bus: 0x3, /* BUS_USB */
+              vendor: $($vendor),
+              product: $($product),
+              // SAFETY: The rest is zeroed out to initialize `struct hid_device_id`,
+              // sets `Option<&F>` to be `None`.
+              ..unsafe { core::mem::MaybeUninit::<bindings::hid_device_id>::zeroed().assume_init() }
+          }))
+      }
+  }
+#+END_SRC
+
+The ~usb_device~ macro serves as the Rust equivalent. To deal with terminating
+the table, I made the following macro for internal use only in the ~hid~ Rust
+module.
+
+#+BEGIN_SRC rust
+  macro_rules! term {
+      () => {
+          DeviceId(Opaque::new(bindings::hid_device_id {
+              // SAFETY: The rest is zeroed out to initialize `struct hid_device_id`,
+              // sets `Option<&F>` to be `None`.
+              ..unsafe { $crate::core::mem::MaybeUninit::<bindings::hid_device_id>::zeroed().assume_init() }
+          }))
+      }
+  }
+#+END_SRC
+
+I realize now that this ~term~ macro is useless and will be getting rid of it.
+It's a lot of back and forth on the design. Here is an updated variant of
+~module_hid_driver~ macro for creating a id table structure.
+
+#+BEGIN_SRC rust
+  #[macro_export]
+  macro_rules! module_hid_driver {
+      (@replace_expr $_t:tt $sub:expr) => {$sub};
+
+      (@count_devices $($x:expr),*) => {
+          0usize $(+ $crate::module_hid_driver!(@replace_expr $x 1usize))*
+      };
+
+      (driver: $($driver:ident), id_table: [$($dev_id:expr),+ $(,)?], name: $($name:expr), $(f:tt)*) => {
+          struct Module {
+              _reg: $crate::hid::Registration,
+          }
+
+          $crate::prelude::module! {
+              type: Module,
+              name: $($name),
+              $($f)*
+          }
+
+          const _: () = {
+              const NAME: &'static Cstr = $crate::c_str!($($name));
+
+              static ID_TABLE: [$crate::bindings::hid_device_id;
+                                $crate::module_hid_driver!(@count_devices $($dev_id),+) + 1] = [
+                  $($dev_id.as_ptr()),+,
+                  $crate::bindings::hid_device_id {
+                      // SAFETY: All is zeroed out to initialize `struct hid_device_id`,
+                      // sets `Option<&F>` to be `None`.
+                      ..unsafe { $crate::core::mem::MaybeUninit::<$crate::bindings::hid_device_id>::zeroed().assume_init() }
+                  },
+              ];
+
+              static mut DRIVER: $crate::hid::DriverVTable = $($crate::hid::create_hid_driver::<$driver>(NAME, /* TODO pass ID_TABLE */));
+          };
+      }
+  }
+#+END_SRC
+
+Truthfully, exposing an array of ~bindings::hid_device_id~ is broken. Instead, I
+should build a ~DeviceId~ array and pass the raw pointer of the underlying
+~Opaque~ type to the first element to the C API.
+
+* Next step: probing
+
+With all this work in place, it should be trivial to start probing HID devices
+and exercise the correctness of the bindings.
