[PATCH 00/10] rust: driver: use pointers instead of indices for ID info

[PATCH 00/10] rust: driver: use pointers instead of indices for ID info

[Gary Guo]

Most C drivers use a pointer (and cast to kernel_ulong_t) for driver_data
fields in device_id. Rust code currently does not do this, but rather use
indices. These indices then needs to be translated to `&IdInfo` separately
and this is by a side table.

This leads to open-coded ACPI/OF handling in driver.rs, which is not
desirable. Convert the code to use pointers (or rather, static references)
instead.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
Gary Guo (10):
      rust: driver: remove `IdTable::id`
      rust: driver: simplify `IdArray::new_without_index`
      rust: pci: use `Option<&IdInfo>` for device ID info
      rust: net/phy: remove expansion from doc
      rust: driver: centralize device ID handling
      rust: driver: remove `$module_table_name` from `module_device_table`
      rust: driver: store pointers in `DeviceId`
      rust: driver: remove open-coded matching logic
      rust: driver: remove duplicate ID table
      RFC: rust: driver: support map-like syntax for ID table

 drivers/cpufreq/rcpufreq_dt.rs        |   1 -
 drivers/gpu/drm/nova/driver.rs        |   1 -
 drivers/gpu/drm/tyr/driver.rs         |   1 -
 drivers/gpu/nova-core/driver.rs       |   3 +-
 drivers/pwm/pwm_th1520.rs             |   1 -
 rust/kernel/acpi.rs                   |  14 +--
 rust/kernel/auxiliary.rs              |  18 +--
 rust/kernel/device_id.rs              | 205 +++++++++++++++++++---------------
 rust/kernel/driver.rs                 | 114 ++-----------------
 rust/kernel/i2c.rs                    |  26 ++---
 rust/kernel/net/phy.rs                |  66 +----------
 rust/kernel/of.rs                     |  14 +--
 rust/kernel/pci.rs                    |  24 ++--
 rust/kernel/platform.rs               |   5 +-
 rust/kernel/usb.rs                    |  18 +--
 samples/rust/rust_debugfs.rs          |   1 -
 samples/rust/rust_dma.rs              |   3 +-
 samples/rust/rust_driver_auxiliary.rs |   4 +-
 samples/rust/rust_driver_i2c.rs       |   3 -
 samples/rust/rust_driver_pci.rs       |  11 +-
 samples/rust/rust_driver_platform.rs  |   2 -
 samples/rust/rust_driver_usb.rs       |   1 -
 samples/rust/rust_i2c_client.rs       |   2 -
 samples/rust/rust_soc.rs              |   2 -
 24 files changed, 166 insertions(+), 374 deletions(-)
---
base-commit: 4fa3f5fabb30bf00d7475d5a33459ea83d639bf9
change-id: 20260612-id_info-23eca472ccd8

Best regards,
--  
Gary Guo <gary@garyguo.net>

[PATCH 00/10] rust: driver: use pointers instead of indices for ID info

[Gary Guo]

Most C drivers use a pointer (and cast to kernel_ulong_t) for driver_data
fields in device_id. Rust code currently does not do this, but rather use
indices. These indices then needs to be translated to `&IdInfo` separately
and this is by a side table.

This leads to open-coded ACPI/OF handling in driver.rs, which is not
desirable. Convert the code to use pointers (or rather, static references)
instead.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
Gary Guo (10):
      rust: driver: remove `IdTable::id`
      rust: driver: simplify `IdArray::new_without_index`
      rust: pci: use `Option<&IdInfo>` for device ID info
      rust: net/phy: remove expansion from doc
      rust: driver: centralize device ID handling
      rust: driver: remove `$module_table_name` from `module_device_table`
      rust: driver: store pointers in `DeviceId`
      rust: driver: remove open-coded matching logic
      rust: driver: remove duplicate ID table
      RFC: rust: driver: support map-like syntax for ID table

 drivers/cpufreq/rcpufreq_dt.rs        |   1 -
 drivers/gpu/drm/nova/driver.rs        |   1 -
 drivers/gpu/drm/tyr/driver.rs         |   1 -
 drivers/gpu/nova-core/driver.rs       |   3 +-
 drivers/pwm/pwm_th1520.rs             |   1 -
 rust/kernel/acpi.rs                   |  14 +--
 rust/kernel/auxiliary.rs              |  18 +--
 rust/kernel/device_id.rs              | 205 +++++++++++++++++++---------------
 rust/kernel/driver.rs                 | 114 ++-----------------
 rust/kernel/i2c.rs                    |  26 ++---
 rust/kernel/net/phy.rs                |  66 +----------
 rust/kernel/of.rs                     |  14 +--
 rust/kernel/pci.rs                    |  24 ++--
 rust/kernel/platform.rs               |   5 +-
 rust/kernel/usb.rs                    |  18 +--
 samples/rust/rust_debugfs.rs          |   1 -
 samples/rust/rust_dma.rs              |   3 +-
 samples/rust/rust_driver_auxiliary.rs |   4 +-
 samples/rust/rust_driver_i2c.rs       |   3 -
 samples/rust/rust_driver_pci.rs       |  11 +-
 samples/rust/rust_driver_platform.rs  |   2 -
 samples/rust/rust_driver_usb.rs       |   1 -
 samples/rust/rust_i2c_client.rs       |   2 -
 samples/rust/rust_soc.rs              |   2 -
 24 files changed, 166 insertions(+), 374 deletions(-)
---
base-commit: 4fa3f5fabb30bf00d7475d5a33459ea83d639bf9
change-id: 20260612-id_info-23eca472ccd8

Best regards,
--  
Gary Guo <gary@garyguo.net>


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 01/10] rust: driver: remove `IdTable::id`

[Gary Guo]

This is unused.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/device_id.rs | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 8e9721446014..fbf6d8e6afb9 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -166,9 +166,6 @@ pub trait IdTable<T: RawDeviceId, U> {
     /// Obtain the pointer to the ID table.
     fn as_ptr(&self) -> *const T::RawType;
 
-    /// Obtain the pointer to the bus specific device ID from an index.
-    fn id(&self, index: usize) -> &T::RawType;
-
     /// Obtain the pointer to the driver-specific information from an index.
     fn info(&self, index: usize) -> &U;
 }
@@ -180,10 +177,6 @@ fn as_ptr(&self) -> *const T::RawType {
         core::ptr::from_ref(self).cast()
     }
 
-    fn id(&self, index: usize) -> &T::RawType {
-        &self.raw_ids.ids[index]
-    }
-
     fn info(&self, index: usize) -> &U {
         &self.id_infos[index]
     }

-- 
2.54.0

[PATCH 01/10] rust: driver: remove `IdTable::id`

[Gary Guo]

This is unused.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/device_id.rs | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 8e9721446014..fbf6d8e6afb9 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -166,9 +166,6 @@ pub trait IdTable<T: RawDeviceId, U> {
     /// Obtain the pointer to the ID table.
     fn as_ptr(&self) -> *const T::RawType;
 
-    /// Obtain the pointer to the bus specific device ID from an index.
-    fn id(&self, index: usize) -> &T::RawType;
-
     /// Obtain the pointer to the driver-specific information from an index.
     fn info(&self, index: usize) -> &U;
 }
@@ -180,10 +177,6 @@ fn as_ptr(&self) -> *const T::RawType {
         core::ptr::from_ref(self).cast()
     }
 
-    fn id(&self, index: usize) -> &T::RawType {
-        &self.raw_ids.ids[index]
-    }
-
     fn info(&self, index: usize) -> &U {
         &self.id_infos[index]
     }

-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 02/10] rust: driver: simplify `IdArray::new_without_index`

[Gary Guo]

This method can very easily construct the `IdArray` on its own without
delegating to `Self::build`. Doing so also simplifies the phy device table
macro because it does not need to construct tuples anymore.

This also allows simplification of `new` and `build` which removes the
`unsafe`.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/device_id.rs | 64 ++++++++++++++++++++----------------------------
 rust/kernel/net/phy.rs   |  2 +-
 2 files changed, 28 insertions(+), 38 deletions(-)

diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index fbf6d8e6afb9..eeef3f5e7b63 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -73,19 +73,11 @@ pub struct IdArray<T: RawDeviceId, U, const N: usize> {
     id_infos: [U; N],
 }
 
-impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
+impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
     /// Creates a new instance of the array.
     ///
     /// The contents are derived from the given identifiers and context information.
-    ///
-    /// # Safety
-    ///
-    /// `data_offset` as `None` is always safe.
-    /// If `data_offset` is `Some(data_offset)`, then:
-    /// - `data_offset` must be the correct offset (in bytes) to the context/data field
-    ///   (e.g., the `driver_data` field) within the raw device ID structure.
-    /// - The field at `data_offset` must be correctly sized to hold a `usize`.
-    const unsafe fn build(ids: [(T, U); N], data_offset: Option<usize>) -> Self {
+    pub const fn new(ids: [(T, U); N]) -> Self {
         let mut raw_ids = [const { MaybeUninit::<T::RawType>::uninit() }; N];
         let mut infos = [const { MaybeUninit::uninit() }; N];
 
@@ -94,16 +86,14 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
             // SAFETY: by the safety requirement of `RawDeviceId`, we're guaranteed that `T` is
             // layout-wise compatible with `RawType`.
             raw_ids[i] = unsafe { core::mem::transmute_copy(&ids[i].0) };
-            if let Some(data_offset) = data_offset {
-                // SAFETY: by the safety requirement of this function, this would be effectively
-                // `raw_ids[i].driver_data = i;`.
-                unsafe {
-                    raw_ids[i]
-                        .as_mut_ptr()
-                        .byte_add(data_offset)
-                        .cast::<usize>()
-                        .write(i);
-                }
+            // SAFETY: by the safety requirement of `RawDeviceIdIndex`, this would be effectively
+            // `raw_ids[i].driver_data = i;`.
+            unsafe {
+                raw_ids[i]
+                    .as_mut_ptr()
+                    .byte_add(T::DRIVER_DATA_OFFSET)
+                    .cast::<usize>()
+                    .write(i);
             }
 
             // SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but
@@ -127,32 +117,32 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
             id_infos: unsafe { core::mem::transmute_copy(&infos) },
         }
     }
+}
 
-    /// Creates a new instance of the array without writing index values.
-    ///
-    /// The contents are derived from the given identifiers and context information.
-    /// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead.
-    pub const fn new_without_index(ids: [(T, U); N]) -> Self {
-        // SAFETY: Calling `Self::build` with `offset = None` is always safe,
-        // because no raw memory writes are performed in this case.
-        unsafe { Self::build(ids, None) }
-    }
-
+impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
     /// Reference to the contained [`RawIdArray`].
     pub const fn raw_ids(&self) -> &RawIdArray<T, N> {
         &self.raw_ids
     }
 }
 
-impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
-    /// Creates a new instance of the array.
+impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
+    /// Creates a new instance of the array without writing index values.
     ///
     /// The contents are derived from the given identifiers and context information.
-    pub const fn new(ids: [(T, U); N]) -> Self {
-        // SAFETY: by the safety requirement of `RawDeviceIdIndex`,
-        // `T::DRIVER_DATA_OFFSET` is guaranteed to be the correct offset (in bytes) to
-        // a field within `T::RawType`.
-        unsafe { Self::build(ids, Some(T::DRIVER_DATA_OFFSET)) }
+    /// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead.
+    pub const fn new_without_index(ids: [T; N]) -> Self {
+        // SAFETY: `T` is layout-wise compatible with `T::RawType`, so is the array of them.
+        let raw_ids: [T::RawType; N] = unsafe { core::mem::transmute_copy(&ids) };
+        core::mem::forget(ids);
+
+        Self {
+            raw_ids: RawIdArray {
+                ids: raw_ids,
+                sentinel: MaybeUninit::zeroed(),
+            },
+            id_infos: [(); N],
+        }
     }
 }
 
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 3ca99db5cccf..2868e3a9e02c 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -868,7 +868,7 @@ macro_rules! module_phy_driver {
         const N: usize = $crate::module_phy_driver!(@count_devices $($dev),+);
 
         const TABLE: $crate::device_id::IdArray<$crate::net::phy::DeviceId, (), N> =
-            $crate::device_id::IdArray::new_without_index([ $(($dev,())),+, ]);
+            $crate::device_id::IdArray::new_without_index([ $($dev),+, ]);
 
         $crate::module_device_table!("mdio", phydev, TABLE);
     };

-- 
2.54.0

[PATCH 02/10] rust: driver: simplify `IdArray::new_without_index`

[Gary Guo]

This method can very easily construct the `IdArray` on its own without
delegating to `Self::build`. Doing so also simplifies the phy device table
macro because it does not need to construct tuples anymore.

This also allows simplification of `new` and `build` which removes the
`unsafe`.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/device_id.rs | 64 ++++++++++++++++++++----------------------------
 rust/kernel/net/phy.rs   |  2 +-
 2 files changed, 28 insertions(+), 38 deletions(-)

diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index fbf6d8e6afb9..eeef3f5e7b63 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -73,19 +73,11 @@ pub struct IdArray<T: RawDeviceId, U, const N: usize> {
     id_infos: [U; N],
 }
 
-impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
+impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
     /// Creates a new instance of the array.
     ///
     /// The contents are derived from the given identifiers and context information.
-    ///
-    /// # Safety
-    ///
-    /// `data_offset` as `None` is always safe.
-    /// If `data_offset` is `Some(data_offset)`, then:
-    /// - `data_offset` must be the correct offset (in bytes) to the context/data field
-    ///   (e.g., the `driver_data` field) within the raw device ID structure.
-    /// - The field at `data_offset` must be correctly sized to hold a `usize`.
-    const unsafe fn build(ids: [(T, U); N], data_offset: Option<usize>) -> Self {
+    pub const fn new(ids: [(T, U); N]) -> Self {
         let mut raw_ids = [const { MaybeUninit::<T::RawType>::uninit() }; N];
         let mut infos = [const { MaybeUninit::uninit() }; N];
 
@@ -94,16 +86,14 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
             // SAFETY: by the safety requirement of `RawDeviceId`, we're guaranteed that `T` is
             // layout-wise compatible with `RawType`.
             raw_ids[i] = unsafe { core::mem::transmute_copy(&ids[i].0) };
-            if let Some(data_offset) = data_offset {
-                // SAFETY: by the safety requirement of this function, this would be effectively
-                // `raw_ids[i].driver_data = i;`.
-                unsafe {
-                    raw_ids[i]
-                        .as_mut_ptr()
-                        .byte_add(data_offset)
-                        .cast::<usize>()
-                        .write(i);
-                }
+            // SAFETY: by the safety requirement of `RawDeviceIdIndex`, this would be effectively
+            // `raw_ids[i].driver_data = i;`.
+            unsafe {
+                raw_ids[i]
+                    .as_mut_ptr()
+                    .byte_add(T::DRIVER_DATA_OFFSET)
+                    .cast::<usize>()
+                    .write(i);
             }
 
             // SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but
@@ -127,32 +117,32 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
             id_infos: unsafe { core::mem::transmute_copy(&infos) },
         }
     }
+}
 
-    /// Creates a new instance of the array without writing index values.
-    ///
-    /// The contents are derived from the given identifiers and context information.
-    /// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead.
-    pub const fn new_without_index(ids: [(T, U); N]) -> Self {
-        // SAFETY: Calling `Self::build` with `offset = None` is always safe,
-        // because no raw memory writes are performed in this case.
-        unsafe { Self::build(ids, None) }
-    }
-
+impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
     /// Reference to the contained [`RawIdArray`].
     pub const fn raw_ids(&self) -> &RawIdArray<T, N> {
         &self.raw_ids
     }
 }
 
-impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
-    /// Creates a new instance of the array.
+impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
+    /// Creates a new instance of the array without writing index values.
     ///
     /// The contents are derived from the given identifiers and context information.
-    pub const fn new(ids: [(T, U); N]) -> Self {
-        // SAFETY: by the safety requirement of `RawDeviceIdIndex`,
-        // `T::DRIVER_DATA_OFFSET` is guaranteed to be the correct offset (in bytes) to
-        // a field within `T::RawType`.
-        unsafe { Self::build(ids, Some(T::DRIVER_DATA_OFFSET)) }
+    /// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead.
+    pub const fn new_without_index(ids: [T; N]) -> Self {
+        // SAFETY: `T` is layout-wise compatible with `T::RawType`, so is the array of them.
+        let raw_ids: [T::RawType; N] = unsafe { core::mem::transmute_copy(&ids) };
+        core::mem::forget(ids);
+
+        Self {
+            raw_ids: RawIdArray {
+                ids: raw_ids,
+                sentinel: MaybeUninit::zeroed(),
+            },
+            id_infos: [(); N],
+        }
     }
 }
 
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 3ca99db5cccf..2868e3a9e02c 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -868,7 +868,7 @@ macro_rules! module_phy_driver {
         const N: usize = $crate::module_phy_driver!(@count_devices $($dev),+);
 
         const TABLE: $crate::device_id::IdArray<$crate::net::phy::DeviceId, (), N> =
-            $crate::device_id::IdArray::new_without_index([ $(($dev,())),+, ]);
+            $crate::device_id::IdArray::new_without_index([ $($dev),+, ]);
 
         $crate::module_device_table!("mdio", phydev, TABLE);
     };

-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 03/10] rust: pci: use `Option<&IdInfo>` for device ID info

[Gary Guo]

It is possible that `pci_device_id_any` will be passed to the driver, e.g.
`driver_override` is used on the device. Therefore, the driver must be able
to handle the case where `driver_data` is 0. Thus, update the `probe`
functions to get `Option`.

The current code cannot tell if the info does not exist or is the first
entry; however this will be achievable once the code is updated to use a
`&'static IdInfo` pointer instead of indices.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/gpu/nova-core/driver.rs       | 2 +-
 rust/kernel/pci.rs                    | 6 +++---
 samples/rust/rust_dma.rs              | 2 +-
 samples/rust/rust_driver_auxiliary.rs | 2 +-
 samples/rust/rust_driver_pci.rs       | 3 ++-
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 5738d4ac521b..5a5f0b63e0f3 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -70,7 +70,7 @@ impl pci::Driver for NovaCoreDriver {
 
     fn probe<'bound>(
         pdev: &'bound pci::Device<Core<'_>>,
-        _info: &'bound Self::IdInfo,
+        _info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         pin_init::pin_init_scope(move || {
             dev_dbg!(pdev, "Probe Nova Core GPU driver.\n");
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 5071cae6543f..0e055e4df99e 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -113,7 +113,7 @@ extern "C" fn probe_callback(
         let info = T::ID_TABLE.info(id.index());
 
         from_result(|| {
-            let data = T::probe(pdev, info);
+            let data = T::probe(pdev, Some(info));
 
             pdev.as_ref().set_drvdata(data)?;
             Ok(0)
@@ -284,7 +284,7 @@ macro_rules! pci_device_table {
 ///
 ///     fn probe<'bound>(
 ///         _pdev: &'bound pci::Device<Core<'_>>,
-///         _id_info: &'bound Self::IdInfo,
+///         _id_info: Option<&'bound Self::IdInfo>,
 ///     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
 ///         Err(ENODEV)
 ///     }
@@ -313,7 +313,7 @@ pub trait Driver {
     /// attempt to initialize the device here.
     fn probe<'bound>(
         dev: &'bound Device<device::Core<'_>>,
-        id_info: &'bound Self::IdInfo,
+        id_info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
 
     /// PCI driver unbind.
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 5046b4628d0e..9beb37275e0d 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -63,7 +63,7 @@ impl pci::Driver for DmaSampleDriver {
 
     fn probe<'bound>(
         pdev: &'bound pci::Device<Core<'_>>,
-        _info: &'bound Self::IdInfo,
+        _info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self, Error> + 'bound {
         pin_init::pin_init_scope(move || {
             dev_info!(pdev, "Probe DMA test driver.\n");
diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
index 2c1351040e45..73c63afc046a 100644
--- a/samples/rust/rust_driver_auxiliary.rs
+++ b/samples/rust/rust_driver_auxiliary.rs
@@ -79,7 +79,7 @@ impl pci::Driver for ParentDriver {
 
     fn probe<'bound>(
         pdev: &'bound pci::Device<Core<'_>>,
-        _info: &'bound Self::IdInfo,
+        _info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         Ok(ParentData {
             // SAFETY: `ParentData` is the driver's private data, which is dropped when the
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 1aa8197d8698..5547dd704a1b 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -144,7 +144,7 @@ impl pci::Driver for SampleDriver {
 
     fn probe<'bound>(
         pdev: &'bound pci::Device<Core<'_>>,
-        info: &'bound Self::IdInfo,
+        info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         let vendor = pdev.vendor_id();
         dev_dbg!(
@@ -153,6 +153,7 @@ fn probe<'bound>(
             vendor,
             pdev.device_id()
         );
+        let info = info.ok_or(ENODEV)?;
 
         pdev.enable_device_mem()?;
         pdev.set_master();

-- 
2.54.0

[PATCH 03/10] rust: pci: use `Option<&IdInfo>` for device ID info

[Gary Guo]

It is possible that `pci_device_id_any` will be passed to the driver, e.g.
`driver_override` is used on the device. Therefore, the driver must be able
to handle the case where `driver_data` is 0. Thus, update the `probe`
functions to get `Option`.

The current code cannot tell if the info does not exist or is the first
entry; however this will be achievable once the code is updated to use a
`&'static IdInfo` pointer instead of indices.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/gpu/nova-core/driver.rs       | 2 +-
 rust/kernel/pci.rs                    | 6 +++---
 samples/rust/rust_dma.rs              | 2 +-
 samples/rust/rust_driver_auxiliary.rs | 2 +-
 samples/rust/rust_driver_pci.rs       | 3 ++-
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 5738d4ac521b..5a5f0b63e0f3 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -70,7 +70,7 @@ impl pci::Driver for NovaCoreDriver {
 
     fn probe<'bound>(
         pdev: &'bound pci::Device<Core<'_>>,
-        _info: &'bound Self::IdInfo,
+        _info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         pin_init::pin_init_scope(move || {
             dev_dbg!(pdev, "Probe Nova Core GPU driver.\n");
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 5071cae6543f..0e055e4df99e 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -113,7 +113,7 @@ extern "C" fn probe_callback(
         let info = T::ID_TABLE.info(id.index());
 
         from_result(|| {
-            let data = T::probe(pdev, info);
+            let data = T::probe(pdev, Some(info));
 
             pdev.as_ref().set_drvdata(data)?;
             Ok(0)
@@ -284,7 +284,7 @@ macro_rules! pci_device_table {
 ///
 ///     fn probe<'bound>(
 ///         _pdev: &'bound pci::Device<Core<'_>>,
-///         _id_info: &'bound Self::IdInfo,
+///         _id_info: Option<&'bound Self::IdInfo>,
 ///     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
 ///         Err(ENODEV)
 ///     }
@@ -313,7 +313,7 @@ pub trait Driver {
     /// attempt to initialize the device here.
     fn probe<'bound>(
         dev: &'bound Device<device::Core<'_>>,
-        id_info: &'bound Self::IdInfo,
+        id_info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
 
     /// PCI driver unbind.
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 5046b4628d0e..9beb37275e0d 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -63,7 +63,7 @@ impl pci::Driver for DmaSampleDriver {
 
     fn probe<'bound>(
         pdev: &'bound pci::Device<Core<'_>>,
-        _info: &'bound Self::IdInfo,
+        _info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self, Error> + 'bound {
         pin_init::pin_init_scope(move || {
             dev_info!(pdev, "Probe DMA test driver.\n");
diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
index 2c1351040e45..73c63afc046a 100644
--- a/samples/rust/rust_driver_auxiliary.rs
+++ b/samples/rust/rust_driver_auxiliary.rs
@@ -79,7 +79,7 @@ impl pci::Driver for ParentDriver {
 
     fn probe<'bound>(
         pdev: &'bound pci::Device<Core<'_>>,
-        _info: &'bound Self::IdInfo,
+        _info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         Ok(ParentData {
             // SAFETY: `ParentData` is the driver's private data, which is dropped when the
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 1aa8197d8698..5547dd704a1b 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -144,7 +144,7 @@ impl pci::Driver for SampleDriver {
 
     fn probe<'bound>(
         pdev: &'bound pci::Device<Core<'_>>,
-        info: &'bound Self::IdInfo,
+        info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         let vendor = pdev.vendor_id();
         dev_dbg!(
@@ -153,6 +153,7 @@ fn probe<'bound>(
             vendor,
             pdev.device_id()
         );
+        let info = info.ok_or(ENODEV)?;
 
         pdev.enable_device_mem()?;
         pdev.set_master();

-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 04/10] rust: net/phy: remove expansion from doc

[Gary Guo]

The expansion serves little purpose and it can easily diverge.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/net/phy.rs | 56 --------------------------------------------------
 1 file changed, 56 deletions(-)

diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 2868e3a9e02c..965ecca7d55f 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -800,62 +800,6 @@ const fn as_int(&self) -> u32 {
 /// }
 /// # }
 /// ```
-///
-/// This expands to the following code:
-///
-/// ```ignore
-/// use kernel::net::phy::{self, DeviceId};
-/// use kernel::prelude::*;
-///
-/// struct Module {
-///     _reg: ::kernel::net::phy::Registration,
-/// }
-///
-/// module! {
-///     type: Module,
-///     name: "rust_sample_phy",
-///     authors: ["Rust for Linux Contributors"],
-///     description: "Rust sample PHYs driver",
-///     license: "GPL",
-/// }
-///
-/// struct PhySample;
-///
-/// #[vtable]
-/// impl phy::Driver for PhySample {
-///     const NAME: &'static CStr = c"PhySample";
-///     const PHY_DEVICE_ID: phy::DeviceId = phy::DeviceId::new_with_exact_mask(0x00000001);
-/// }
-///
-/// const _: () = {
-///     static mut DRIVERS: [::kernel::net::phy::DriverVTable; 1] =
-///         [::kernel::net::phy::create_phy_driver::<PhySample>()];
-///
-///     impl ::kernel::Module for Module {
-///         fn init(module: &'static ::kernel::ThisModule) -> Result<Self> {
-///             let drivers = unsafe { &mut DRIVERS };
-///             let mut reg = ::kernel::net::phy::Registration::register(
-///                 module,
-///                 ::core::pin::Pin::static_mut(drivers),
-///             )?;
-///             Ok(Module { _reg: reg })
-///         }
-///     }
-/// };
-///
-/// const N: usize = 1;
-///
-/// const TABLE: ::kernel::device_id::IdArray<::kernel::net::phy::DeviceId, (), N> =
-///     ::kernel::device_id::IdArray::new_without_index([
-///         ::kernel::net::phy::DeviceId(
-///             ::kernel::bindings::mdio_device_id {
-///                 phy_id: 0x00000001,
-///                 phy_id_mask: 0xffffffff,
-///             }),
-///     ]);
-///
-/// ::kernel::module_device_table!("mdio", phydev, TABLE);
-/// ```
 #[macro_export]
 macro_rules! module_phy_driver {
     (@replace_expr $_t:tt $sub:expr) => {$sub};

-- 
2.54.0

[PATCH 04/10] rust: net/phy: remove expansion from doc

[Gary Guo]

The expansion serves little purpose and it can easily diverge.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/net/phy.rs | 56 --------------------------------------------------
 1 file changed, 56 deletions(-)

diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 2868e3a9e02c..965ecca7d55f 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -800,62 +800,6 @@ const fn as_int(&self) -> u32 {
 /// }
 /// # }
 /// ```
-///
-/// This expands to the following code:
-///
-/// ```ignore
-/// use kernel::net::phy::{self, DeviceId};
-/// use kernel::prelude::*;
-///
-/// struct Module {
-///     _reg: ::kernel::net::phy::Registration,
-/// }
-///
-/// module! {
-///     type: Module,
-///     name: "rust_sample_phy",
-///     authors: ["Rust for Linux Contributors"],
-///     description: "Rust sample PHYs driver",
-///     license: "GPL",
-/// }
-///
-/// struct PhySample;
-///
-/// #[vtable]
-/// impl phy::Driver for PhySample {
-///     const NAME: &'static CStr = c"PhySample";
-///     const PHY_DEVICE_ID: phy::DeviceId = phy::DeviceId::new_with_exact_mask(0x00000001);
-/// }
-///
-/// const _: () = {
-///     static mut DRIVERS: [::kernel::net::phy::DriverVTable; 1] =
-///         [::kernel::net::phy::create_phy_driver::<PhySample>()];
-///
-///     impl ::kernel::Module for Module {
-///         fn init(module: &'static ::kernel::ThisModule) -> Result<Self> {
-///             let drivers = unsafe { &mut DRIVERS };
-///             let mut reg = ::kernel::net::phy::Registration::register(
-///                 module,
-///                 ::core::pin::Pin::static_mut(drivers),
-///             )?;
-///             Ok(Module { _reg: reg })
-///         }
-///     }
-/// };
-///
-/// const N: usize = 1;
-///
-/// const TABLE: ::kernel::device_id::IdArray<::kernel::net::phy::DeviceId, (), N> =
-///     ::kernel::device_id::IdArray::new_without_index([
-///         ::kernel::net::phy::DeviceId(
-///             ::kernel::bindings::mdio_device_id {
-///                 phy_id: 0x00000001,
-///                 phy_id_mask: 0xffffffff,
-///             }),
-///     ]);
-///
-/// ::kernel::module_device_table!("mdio", phydev, TABLE);
-/// ```
 #[macro_export]
 macro_rules! module_phy_driver {
     (@replace_expr $_t:tt $sub:expr) => {$sub};

-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 05/10] rust: driver: centralize device ID handling

[Gary Guo]

Move the `IdArray` creation from individual buses to be handled by shared
code in `device_id.rs`.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/acpi.rs      | 10 ++--------
 rust/kernel/auxiliary.rs | 10 ++--------
 rust/kernel/device_id.rs | 31 ++++++++++++++++++++++++++++++-
 rust/kernel/i2c.rs       | 10 ++--------
 rust/kernel/net/phy.rs   | 10 ++++------
 rust/kernel/of.rs        | 10 ++--------
 rust/kernel/pci.rs       | 10 ++--------
 rust/kernel/usb.rs       | 10 ++--------
 8 files changed, 46 insertions(+), 55 deletions(-)

diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
index 9b8efa623130..315f2f2af446 100644
--- a/rust/kernel/acpi.rs
+++ b/rust/kernel/acpi.rs
@@ -53,13 +53,7 @@ pub const fn new(id: &'static CStr) -> Self {
 /// Create an ACPI `IdTable` with an "alias" for modpost.
 #[macro_export]
 macro_rules! acpi_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::acpi::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("acpi", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("acpi", $crate::acpi::DeviceId, $($tt)*);
     };
 }
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index c42928d5a239..59787c9bff26 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -181,14 +181,8 @@ fn index(&self) -> usize {
 /// Create a auxiliary `IdTable` with its alias for modpost.
 #[macro_export]
 macro_rules! auxiliary_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::auxiliary::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("auxiliary", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("auxiliary", $crate::auxiliary::DeviceId, $($tt)*);
     };
 }
 
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index eeef3f5e7b63..0239f89d5f69 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -175,7 +175,36 @@ fn info(&self, index: usize) -> &U {
 /// Create device table alias for modpost.
 #[macro_export]
 macro_rules! module_device_table {
-    ($table_type: literal, $module_table_name:ident, $table_name:ident) => {
+    (
+        $table_type: literal, $device_id_ty: ty,
+        $table_name: ident, $module_table_name: ident, $id_info_type: ty,
+        [$(($id: expr, $info:expr $(,)?)),* $(,)?]
+    ) => {
+        const $table_name: $crate::device_id::IdArray<
+            $device_id_ty,
+            $id_info_type,
+            { <[$device_id_ty]>::len(&[$($id,)*]) },
+        > = $crate::device_id::IdArray::new([$(($id, $info),)*]);
+
+        $crate::module_device_table!($table_type, $module_table_name, $table_name);
+    };
+
+    // Case for no ID info.
+    (
+        $table_type: literal, $device_id_ty: ty,
+        $table_name: ident, $module_table_name: ident, @none,
+        [$($id: expr),* $(,)?]
+    ) => {
+        const $table_name: $crate::device_id::IdArray<
+            $device_id_ty,
+            (),
+            { <[$device_id_ty]>::len(&[$($id,)*]) },
+        > = $crate::device_id::IdArray::new_without_index([$($id),*]);
+
+        $crate::module_device_table!($table_type, $module_table_name, $table_name);
+    };
+
+    ($table_type: literal, $module_table_name: ident, $table_name:ident) => {
         #[rustfmt::skip]
         #[export_name =
             concat!("__mod_device_table__", line!(),
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 7655d61daac8..a7d9b88ae616 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -77,14 +77,8 @@ fn index(&self) -> usize {
 /// Create a I2C `IdTable` with its alias for modpost.
 #[macro_export]
 macro_rules! i2c_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::i2c::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("i2c", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("i2c", $crate::i2c::DeviceId, $($tt)*);
     };
 }
 
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 965ecca7d55f..166572861e61 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -809,12 +809,10 @@ macro_rules! module_phy_driver {
     };
 
     (@device_table [$($dev:expr),+]) => {
-        const N: usize = $crate::module_phy_driver!(@count_devices $($dev),+);
-
-        const TABLE: $crate::device_id::IdArray<$crate::net::phy::DeviceId, (), N> =
-            $crate::device_id::IdArray::new_without_index([ $($dev),+, ]);
-
-        $crate::module_device_table!("mdio", phydev, TABLE);
+        $crate::module_device_table!(
+            "mdio", $crate::net::phy::DeviceId,
+            phydev, TABLE, @none, [$($dev),+]
+        );
     };
 
     (drivers: [$($driver:ident),+ $(,)?], device_table: [$($dev:expr),+ $(,)?], $($f:tt)*) => {
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 58b20c367f99..35aa6d36d309 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -53,13 +53,7 @@ pub const fn new(compatible: &'static CStr) -> Self {
 /// Create an OF `IdTable` with an "alias" for modpost.
 #[macro_export]
 macro_rules! of_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::of::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("of", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("of", $crate::of::DeviceId, $($tt)*);
     };
 }
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 0e055e4df99e..34e07a53244d 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -245,14 +245,8 @@ fn index(&self) -> usize {
 /// Create a PCI `IdTable` with its alias for modpost.
 #[macro_export]
 macro_rules! pci_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::pci::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("pci", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("pci", $crate::pci::DeviceId, $($tt)*);
     };
 }
 
diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 7aff0c82d0af..154919ee1e19 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -254,14 +254,8 @@ fn index(&self) -> usize {
 /// Create a USB `IdTable` with its alias for modpost.
 #[macro_export]
 macro_rules! usb_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::usb::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("usb", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("usb", $crate::usb::DeviceId, $($tt)*);
     };
 }
 

-- 
2.54.0

[PATCH 05/10] rust: driver: centralize device ID handling

[Gary Guo]

Move the `IdArray` creation from individual buses to be handled by shared
code in `device_id.rs`.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/acpi.rs      | 10 ++--------
 rust/kernel/auxiliary.rs | 10 ++--------
 rust/kernel/device_id.rs | 31 ++++++++++++++++++++++++++++++-
 rust/kernel/i2c.rs       | 10 ++--------
 rust/kernel/net/phy.rs   | 10 ++++------
 rust/kernel/of.rs        | 10 ++--------
 rust/kernel/pci.rs       | 10 ++--------
 rust/kernel/usb.rs       | 10 ++--------
 8 files changed, 46 insertions(+), 55 deletions(-)

diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
index 9b8efa623130..315f2f2af446 100644
--- a/rust/kernel/acpi.rs
+++ b/rust/kernel/acpi.rs
@@ -53,13 +53,7 @@ pub const fn new(id: &'static CStr) -> Self {
 /// Create an ACPI `IdTable` with an "alias" for modpost.
 #[macro_export]
 macro_rules! acpi_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::acpi::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("acpi", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("acpi", $crate::acpi::DeviceId, $($tt)*);
     };
 }
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index c42928d5a239..59787c9bff26 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -181,14 +181,8 @@ fn index(&self) -> usize {
 /// Create a auxiliary `IdTable` with its alias for modpost.
 #[macro_export]
 macro_rules! auxiliary_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::auxiliary::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("auxiliary", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("auxiliary", $crate::auxiliary::DeviceId, $($tt)*);
     };
 }
 
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index eeef3f5e7b63..0239f89d5f69 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -175,7 +175,36 @@ fn info(&self, index: usize) -> &U {
 /// Create device table alias for modpost.
 #[macro_export]
 macro_rules! module_device_table {
-    ($table_type: literal, $module_table_name:ident, $table_name:ident) => {
+    (
+        $table_type: literal, $device_id_ty: ty,
+        $table_name: ident, $module_table_name: ident, $id_info_type: ty,
+        [$(($id: expr, $info:expr $(,)?)),* $(,)?]
+    ) => {
+        const $table_name: $crate::device_id::IdArray<
+            $device_id_ty,
+            $id_info_type,
+            { <[$device_id_ty]>::len(&[$($id,)*]) },
+        > = $crate::device_id::IdArray::new([$(($id, $info),)*]);
+
+        $crate::module_device_table!($table_type, $module_table_name, $table_name);
+    };
+
+    // Case for no ID info.
+    (
+        $table_type: literal, $device_id_ty: ty,
+        $table_name: ident, $module_table_name: ident, @none,
+        [$($id: expr),* $(,)?]
+    ) => {
+        const $table_name: $crate::device_id::IdArray<
+            $device_id_ty,
+            (),
+            { <[$device_id_ty]>::len(&[$($id,)*]) },
+        > = $crate::device_id::IdArray::new_without_index([$($id),*]);
+
+        $crate::module_device_table!($table_type, $module_table_name, $table_name);
+    };
+
+    ($table_type: literal, $module_table_name: ident, $table_name:ident) => {
         #[rustfmt::skip]
         #[export_name =
             concat!("__mod_device_table__", line!(),
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 7655d61daac8..a7d9b88ae616 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -77,14 +77,8 @@ fn index(&self) -> usize {
 /// Create a I2C `IdTable` with its alias for modpost.
 #[macro_export]
 macro_rules! i2c_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::i2c::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("i2c", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("i2c", $crate::i2c::DeviceId, $($tt)*);
     };
 }
 
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 965ecca7d55f..166572861e61 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -809,12 +809,10 @@ macro_rules! module_phy_driver {
     };
 
     (@device_table [$($dev:expr),+]) => {
-        const N: usize = $crate::module_phy_driver!(@count_devices $($dev),+);
-
-        const TABLE: $crate::device_id::IdArray<$crate::net::phy::DeviceId, (), N> =
-            $crate::device_id::IdArray::new_without_index([ $($dev),+, ]);
-
-        $crate::module_device_table!("mdio", phydev, TABLE);
+        $crate::module_device_table!(
+            "mdio", $crate::net::phy::DeviceId,
+            phydev, TABLE, @none, [$($dev),+]
+        );
     };
 
     (drivers: [$($driver:ident),+ $(,)?], device_table: [$($dev:expr),+ $(,)?], $($f:tt)*) => {
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 58b20c367f99..35aa6d36d309 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -53,13 +53,7 @@ pub const fn new(compatible: &'static CStr) -> Self {
 /// Create an OF `IdTable` with an "alias" for modpost.
 #[macro_export]
 macro_rules! of_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::of::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("of", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("of", $crate::of::DeviceId, $($tt)*);
     };
 }
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 0e055e4df99e..34e07a53244d 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -245,14 +245,8 @@ fn index(&self) -> usize {
 /// Create a PCI `IdTable` with its alias for modpost.
 #[macro_export]
 macro_rules! pci_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::pci::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("pci", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("pci", $crate::pci::DeviceId, $($tt)*);
     };
 }
 
diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 7aff0c82d0af..154919ee1e19 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -254,14 +254,8 @@ fn index(&self) -> usize {
 /// Create a USB `IdTable` with its alias for modpost.
 #[macro_export]
 macro_rules! usb_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
-            $crate::usb::DeviceId,
-            $id_info_type,
-            { $table_data.len() },
-        > = $crate::device_id::IdArray::new($table_data);
-
-        $crate::module_device_table!("usb", $module_table_name, $table_name);
+    ($($tt:tt)*) => {
+        $crate::module_device_table!("usb", $crate::usb::DeviceId, $($tt)*);
     };
 }
 

-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 06/10] rust: driver: remove `$module_table_name` from `module_device_table`

[Gary Guo]

Wrap the generated code in a `const _: ()` block to avoid symbol conflict.
This removes the need of creating a new identifier.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/cpufreq/rcpufreq_dt.rs        |  1 -
 drivers/gpu/drm/nova/driver.rs        |  1 -
 drivers/gpu/drm/tyr/driver.rs         |  1 -
 drivers/gpu/nova-core/driver.rs       |  1 -
 drivers/pwm/pwm_th1520.rs             |  1 -
 rust/kernel/device_id.rs              | 30 ++++++++++++++++--------------
 rust/kernel/i2c.rs                    |  3 ---
 rust/kernel/net/phy.rs                |  2 +-
 rust/kernel/pci.rs                    |  1 -
 rust/kernel/platform.rs               |  2 --
 rust/kernel/usb.rs                    |  1 -
 samples/rust/rust_debugfs.rs          |  1 -
 samples/rust/rust_dma.rs              |  1 -
 samples/rust/rust_driver_auxiliary.rs |  2 --
 samples/rust/rust_driver_i2c.rs       |  3 ---
 samples/rust/rust_driver_pci.rs       |  1 -
 samples/rust/rust_driver_platform.rs  |  2 --
 samples/rust/rust_driver_usb.rs       |  1 -
 samples/rust/rust_i2c_client.rs       |  2 --
 samples/rust/rust_soc.rs              |  2 --
 20 files changed, 17 insertions(+), 42 deletions(-)

diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
index 10106fa13095..145daa12072f 100644
--- a/drivers/cpufreq/rcpufreq_dt.rs
+++ b/drivers/cpufreq/rcpufreq_dt.rs
@@ -194,7 +194,6 @@ fn register_em(policy: &mut cpufreq::Policy) {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <CPUFreqDTDriver as platform::Driver>::IdInfo,
     [(of::DeviceId::new(c"operating-points-v2"), ())]
 );
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 48933d86ddda..43f15cdfeb09 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -43,7 +43,6 @@ pub(crate) struct NovaData {
 
 kernel::auxiliary_device_table!(
     AUX_TABLE,
-    MODULE_AUX_TABLE,
     <NovaDriver as auxiliary::Driver>::IdInfo,
     [(
         auxiliary::DeviceId::new(NOVA_CORE_MODULE_NAME, AUXILIARY_NAME),
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index d063bc664cc1..218e9af899c7 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -87,7 +87,6 @@ fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <TyrPlatformDriver as platform::Driver>::IdInfo,
     [
         (of::DeviceId::new(c"rockchip,rk3588-mali"), ()),
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 5a5f0b63e0f3..0c53b7239ac9 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -40,7 +40,6 @@ pub(crate) struct NovaCore<'bound> {
 
 kernel::pci_device_table!(
     PCI_TABLE,
-    MODULE_PCI_TABLE,
     <NovaCoreDriver as pci::Driver>::IdInfo,
     [
         // Modern NVIDIA GPUs will show up as either VGA or 3D controllers.
diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
index 3e3fa51ccef9..1df752330e8f 100644
--- a/drivers/pwm/pwm_th1520.rs
+++ b/drivers/pwm/pwm_th1520.rs
@@ -303,7 +303,6 @@ fn drop(self: Pin<&mut Self>) {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <Th1520PwmPlatformDriver as platform::Driver>::IdInfo,
     [(of::DeviceId::new(c"thead,th1520-pwm"), ())]
 );
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 0239f89d5f69..84852a2d9ad7 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -177,7 +177,7 @@ fn info(&self, index: usize) -> &U {
 macro_rules! module_device_table {
     (
         $table_type: literal, $device_id_ty: ty,
-        $table_name: ident, $module_table_name: ident, $id_info_type: ty,
+        $table_name: ident, $id_info_type: ty,
         [$(($id: expr, $info:expr $(,)?)),* $(,)?]
     ) => {
         const $table_name: $crate::device_id::IdArray<
@@ -186,13 +186,13 @@ macro_rules! module_device_table {
             { <[$device_id_ty]>::len(&[$($id,)*]) },
         > = $crate::device_id::IdArray::new([$(($id, $info),)*]);
 
-        $crate::module_device_table!($table_type, $module_table_name, $table_name);
+        $crate::module_device_table!($table_type, $table_name);
     };
 
     // Case for no ID info.
     (
         $table_type: literal, $device_id_ty: ty,
-        $table_name: ident, $module_table_name: ident, @none,
+        $table_name: ident, @none,
         [$($id: expr),* $(,)?]
     ) => {
         const $table_name: $crate::device_id::IdArray<
@@ -201,18 +201,20 @@ macro_rules! module_device_table {
             { <[$device_id_ty]>::len(&[$($id,)*]) },
         > = $crate::device_id::IdArray::new_without_index([$($id),*]);
 
-        $crate::module_device_table!($table_type, $module_table_name, $table_name);
+        $crate::module_device_table!($table_type, $table_name);
     };
 
-    ($table_type: literal, $module_table_name: ident, $table_name:ident) => {
-        #[rustfmt::skip]
-        #[export_name =
-            concat!("__mod_device_table__", line!(),
-                    "__kmod_", module_path!(),
-                    "__", $table_type,
-                    "__", stringify!($table_name))
-        ]
-        static $module_table_name: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
-            unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
+    ($table_type: literal, $table_name:ident) => {
+        const _: () = {
+            #[rustfmt::skip]
+            #[export_name =
+                concat!("__mod_device_table__", line!(),
+                        "__kmod_", module_path!(),
+                        "__", $table_type,
+                        "__", stringify!($table_name))
+            ]
+            static TABLE: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
+                unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
+        };
     };
 }
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index a7d9b88ae616..55c89ba3a82a 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -261,7 +261,6 @@ macro_rules! module_i2c_driver {
 ///
 /// kernel::acpi_device_table!(
 ///     ACPI_TABLE,
-///     MODULE_ACPI_TABLE,
 ///     <MyDriver as i2c::Driver>::IdInfo,
 ///     [
 ///         (acpi::DeviceId::new(c"LNUXBEEF"), ())
@@ -270,7 +269,6 @@ macro_rules! module_i2c_driver {
 ///
 /// kernel::i2c_device_table!(
 ///     I2C_TABLE,
-///     MODULE_I2C_TABLE,
 ///     <MyDriver as i2c::Driver>::IdInfo,
 ///     [
 ///          (i2c::DeviceId::new(c"rust_driver_i2c"), ())
@@ -279,7 +277,6 @@ macro_rules! module_i2c_driver {
 ///
 /// kernel::of_device_table!(
 ///     OF_TABLE,
-///     MODULE_OF_TABLE,
 ///     <MyDriver as i2c::Driver>::IdInfo,
 ///     [
 ///         (of::DeviceId::new(c"test,device"), ())
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 166572861e61..1e86b901c391 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -811,7 +811,7 @@ macro_rules! module_phy_driver {
     (@device_table [$($dev:expr),+]) => {
         $crate::module_device_table!(
             "mdio", $crate::net::phy::DeviceId,
-            phydev, TABLE, @none, [$($dev),+]
+            TABLE, @none, [$($dev),+]
         );
     };
 
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 34e07a53244d..a3dd48f76353 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -261,7 +261,6 @@ macro_rules! pci_device_table {
 ///
 /// kernel::pci_device_table!(
 ///     PCI_TABLE,
-///     MODULE_PCI_TABLE,
 ///     <MyDriver as pci::Driver>::IdInfo,
 ///     [
 ///         (
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 9b362e0495d3..210a815925ce 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -176,7 +176,6 @@ macro_rules! module_platform_driver {
 ///
 /// kernel::of_device_table!(
 ///     OF_TABLE,
-///     MODULE_OF_TABLE,
 ///     <MyDriver as platform::Driver>::IdInfo,
 ///     [
 ///         (of::DeviceId::new(c"test,device"), ())
@@ -185,7 +184,6 @@ macro_rules! module_platform_driver {
 ///
 /// kernel::acpi_device_table!(
 ///     ACPI_TABLE,
-///     MODULE_ACPI_TABLE,
 ///     <MyDriver as platform::Driver>::IdInfo,
 ///     [
 ///         (acpi::DeviceId::new(c"LNUXBEEF"), ())
diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 154919ee1e19..500b5e0ba4ea 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -271,7 +271,6 @@ macro_rules! usb_device_table {
 ///
 /// kernel::usb_device_table!(
 ///     USB_TABLE,
-///     MODULE_USB_TABLE,
 ///     <MyDriver as usb::Driver>::IdInfo,
 ///     [
 ///         (usb::DeviceId::from_id(0x1234, 0x5678), ()),
diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
index 1f59e08aaa4b..181fd98ae5b6 100644
--- a/samples/rust/rust_debugfs.rs
+++ b/samples/rust/rust_debugfs.rs
@@ -110,7 +110,6 @@ fn from_str(s: &str) -> Result<Self> {
 
 kernel::acpi_device_table!(
     ACPI_TABLE,
-    MODULE_ACPI_TABLE,
     <RustDebugFs as platform::Driver>::IdInfo,
     [(acpi::DeviceId::new(c"LNUXBEEF"), ())]
 );
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 9beb37275e0d..80c309ce07e9 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -51,7 +51,6 @@ unsafe impl kernel::transmute::FromBytes for MyStruct {}
 
 kernel::pci_device_table!(
     PCI_TABLE,
-    MODULE_PCI_TABLE,
     <DmaSampleDriver as pci::Driver>::IdInfo,
     [(pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5), ())]
 );
diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
index 73c63afc046a..704567a072ba 100644
--- a/samples/rust/rust_driver_auxiliary.rs
+++ b/samples/rust/rust_driver_auxiliary.rs
@@ -24,7 +24,6 @@
 
 kernel::auxiliary_device_table!(
     AUX_TABLE,
-    MODULE_AUX_TABLE,
     <AuxiliaryDriver as auxiliary::Driver>::IdInfo,
     [(auxiliary::DeviceId::new(MODULE_NAME, AUXILIARY_NAME), ())]
 );
@@ -66,7 +65,6 @@ struct ParentData<'bound> {
 
 kernel::pci_device_table!(
     PCI_TABLE,
-    MODULE_PCI_TABLE,
     <ParentDriver as pci::Driver>::IdInfo,
     [(pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5), ())]
 );
diff --git a/samples/rust/rust_driver_i2c.rs b/samples/rust/rust_driver_i2c.rs
index ead8263a7d48..a0df0c6097c4 100644
--- a/samples/rust/rust_driver_i2c.rs
+++ b/samples/rust/rust_driver_i2c.rs
@@ -14,21 +14,18 @@
 
 kernel::acpi_device_table! {
     ACPI_TABLE,
-    MODULE_ACPI_TABLE,
     <SampleDriver as i2c::Driver>::IdInfo,
     [(acpi::DeviceId::new(c"LNUXBEEF"), 0)]
 }
 
 kernel::i2c_device_table! {
     I2C_TABLE,
-    MODULE_I2C_TABLE,
     <SampleDriver as i2c::Driver>::IdInfo,
     [(i2c::DeviceId::new(c"rust_driver_i2c"), 0)]
 }
 
 kernel::of_device_table! {
     OF_TABLE,
-    MODULE_OF_TABLE,
     <SampleDriver as i2c::Driver>::IdInfo,
     [(of::DeviceId::new(c"test,rust_driver_i2c"), 0)]
 }
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 5547dd704a1b..2282191e6292 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -74,7 +74,6 @@ struct SampleDriverData<'bound> {
 
 kernel::pci_device_table!(
     PCI_TABLE,
-    MODULE_PCI_TABLE,
     <SampleDriver as pci::Driver>::IdInfo,
     [(
         pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5),
diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index ec0d6cac4f57..710145b3605a 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -87,14 +87,12 @@ struct SampleDriver {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <SampleDriver as platform::Driver>::IdInfo,
     [(of::DeviceId::new(c"test,rust-device"), Info(42))]
 );
 
 kernel::acpi_device_table!(
     ACPI_TABLE,
-    MODULE_ACPI_TABLE,
     <SampleDriver as platform::Driver>::IdInfo,
     [(acpi::DeviceId::new(c"LNUXBEEF"), Info(0))]
 );
diff --git a/samples/rust/rust_driver_usb.rs b/samples/rust/rust_driver_usb.rs
index 02bd5085f9bc..284042c5969b 100644
--- a/samples/rust/rust_driver_usb.rs
+++ b/samples/rust/rust_driver_usb.rs
@@ -19,7 +19,6 @@ struct SampleDriver {
 
 kernel::usb_device_table!(
     USB_TABLE,
-    MODULE_USB_TABLE,
     <SampleDriver as usb::Driver>::IdInfo,
     [(usb::DeviceId::from_id(0x1234, 0x5678), ()),]
 );
diff --git a/samples/rust/rust_i2c_client.rs b/samples/rust/rust_i2c_client.rs
index 2d876f4e3ee0..c8a23875ef5b 100644
--- a/samples/rust/rust_i2c_client.rs
+++ b/samples/rust/rust_i2c_client.rs
@@ -87,14 +87,12 @@ struct SampleDriver {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <SampleDriver as platform::Driver>::IdInfo,
     [(of::DeviceId::new(c"test,rust-device"), ())]
 );
 
 kernel::acpi_device_table!(
     ACPI_TABLE,
-    MODULE_ACPI_TABLE,
     <SampleDriver as platform::Driver>::IdInfo,
     [(acpi::DeviceId::new(c"LNUXBEEF"), ())]
 );
diff --git a/samples/rust/rust_soc.rs b/samples/rust/rust_soc.rs
index 808d58200eb6..f5e5f2f9adf7 100644
--- a/samples/rust/rust_soc.rs
+++ b/samples/rust/rust_soc.rs
@@ -23,14 +23,12 @@ struct SampleSocDriver {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <SampleSocDriver as platform::Driver>::IdInfo,
     [(of::DeviceId::new(c"test,rust-device"), ())]
 );
 
 kernel::acpi_device_table!(
     ACPI_TABLE,
-    MODULE_ACPI_TABLE,
     <SampleSocDriver as platform::Driver>::IdInfo,
     [(acpi::DeviceId::new(c"LNUXBEEF"), ())]
 );

-- 
2.54.0

[PATCH 06/10] rust: driver: remove `$module_table_name` from `module_device_table`

[Gary Guo]

Wrap the generated code in a `const _: ()` block to avoid symbol conflict.
This removes the need of creating a new identifier.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/cpufreq/rcpufreq_dt.rs        |  1 -
 drivers/gpu/drm/nova/driver.rs        |  1 -
 drivers/gpu/drm/tyr/driver.rs         |  1 -
 drivers/gpu/nova-core/driver.rs       |  1 -
 drivers/pwm/pwm_th1520.rs             |  1 -
 rust/kernel/device_id.rs              | 30 ++++++++++++++++--------------
 rust/kernel/i2c.rs                    |  3 ---
 rust/kernel/net/phy.rs                |  2 +-
 rust/kernel/pci.rs                    |  1 -
 rust/kernel/platform.rs               |  2 --
 rust/kernel/usb.rs                    |  1 -
 samples/rust/rust_debugfs.rs          |  1 -
 samples/rust/rust_dma.rs              |  1 -
 samples/rust/rust_driver_auxiliary.rs |  2 --
 samples/rust/rust_driver_i2c.rs       |  3 ---
 samples/rust/rust_driver_pci.rs       |  1 -
 samples/rust/rust_driver_platform.rs  |  2 --
 samples/rust/rust_driver_usb.rs       |  1 -
 samples/rust/rust_i2c_client.rs       |  2 --
 samples/rust/rust_soc.rs              |  2 --
 20 files changed, 17 insertions(+), 42 deletions(-)

diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
index 10106fa13095..145daa12072f 100644
--- a/drivers/cpufreq/rcpufreq_dt.rs
+++ b/drivers/cpufreq/rcpufreq_dt.rs
@@ -194,7 +194,6 @@ fn register_em(policy: &mut cpufreq::Policy) {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <CPUFreqDTDriver as platform::Driver>::IdInfo,
     [(of::DeviceId::new(c"operating-points-v2"), ())]
 );
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 48933d86ddda..43f15cdfeb09 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -43,7 +43,6 @@ pub(crate) struct NovaData {
 
 kernel::auxiliary_device_table!(
     AUX_TABLE,
-    MODULE_AUX_TABLE,
     <NovaDriver as auxiliary::Driver>::IdInfo,
     [(
         auxiliary::DeviceId::new(NOVA_CORE_MODULE_NAME, AUXILIARY_NAME),
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index d063bc664cc1..218e9af899c7 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -87,7 +87,6 @@ fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <TyrPlatformDriver as platform::Driver>::IdInfo,
     [
         (of::DeviceId::new(c"rockchip,rk3588-mali"), ()),
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 5a5f0b63e0f3..0c53b7239ac9 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -40,7 +40,6 @@ pub(crate) struct NovaCore<'bound> {
 
 kernel::pci_device_table!(
     PCI_TABLE,
-    MODULE_PCI_TABLE,
     <NovaCoreDriver as pci::Driver>::IdInfo,
     [
         // Modern NVIDIA GPUs will show up as either VGA or 3D controllers.
diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
index 3e3fa51ccef9..1df752330e8f 100644
--- a/drivers/pwm/pwm_th1520.rs
+++ b/drivers/pwm/pwm_th1520.rs
@@ -303,7 +303,6 @@ fn drop(self: Pin<&mut Self>) {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <Th1520PwmPlatformDriver as platform::Driver>::IdInfo,
     [(of::DeviceId::new(c"thead,th1520-pwm"), ())]
 );
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 0239f89d5f69..84852a2d9ad7 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -177,7 +177,7 @@ fn info(&self, index: usize) -> &U {
 macro_rules! module_device_table {
     (
         $table_type: literal, $device_id_ty: ty,
-        $table_name: ident, $module_table_name: ident, $id_info_type: ty,
+        $table_name: ident, $id_info_type: ty,
         [$(($id: expr, $info:expr $(,)?)),* $(,)?]
     ) => {
         const $table_name: $crate::device_id::IdArray<
@@ -186,13 +186,13 @@ macro_rules! module_device_table {
             { <[$device_id_ty]>::len(&[$($id,)*]) },
         > = $crate::device_id::IdArray::new([$(($id, $info),)*]);
 
-        $crate::module_device_table!($table_type, $module_table_name, $table_name);
+        $crate::module_device_table!($table_type, $table_name);
     };
 
     // Case for no ID info.
     (
         $table_type: literal, $device_id_ty: ty,
-        $table_name: ident, $module_table_name: ident, @none,
+        $table_name: ident, @none,
         [$($id: expr),* $(,)?]
     ) => {
         const $table_name: $crate::device_id::IdArray<
@@ -201,18 +201,20 @@ macro_rules! module_device_table {
             { <[$device_id_ty]>::len(&[$($id,)*]) },
         > = $crate::device_id::IdArray::new_without_index([$($id),*]);
 
-        $crate::module_device_table!($table_type, $module_table_name, $table_name);
+        $crate::module_device_table!($table_type, $table_name);
     };
 
-    ($table_type: literal, $module_table_name: ident, $table_name:ident) => {
-        #[rustfmt::skip]
-        #[export_name =
-            concat!("__mod_device_table__", line!(),
-                    "__kmod_", module_path!(),
-                    "__", $table_type,
-                    "__", stringify!($table_name))
-        ]
-        static $module_table_name: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
-            unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
+    ($table_type: literal, $table_name:ident) => {
+        const _: () = {
+            #[rustfmt::skip]
+            #[export_name =
+                concat!("__mod_device_table__", line!(),
+                        "__kmod_", module_path!(),
+                        "__", $table_type,
+                        "__", stringify!($table_name))
+            ]
+            static TABLE: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
+                unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
+        };
     };
 }
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index a7d9b88ae616..55c89ba3a82a 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -261,7 +261,6 @@ macro_rules! module_i2c_driver {
 ///
 /// kernel::acpi_device_table!(
 ///     ACPI_TABLE,
-///     MODULE_ACPI_TABLE,
 ///     <MyDriver as i2c::Driver>::IdInfo,
 ///     [
 ///         (acpi::DeviceId::new(c"LNUXBEEF"), ())
@@ -270,7 +269,6 @@ macro_rules! module_i2c_driver {
 ///
 /// kernel::i2c_device_table!(
 ///     I2C_TABLE,
-///     MODULE_I2C_TABLE,
 ///     <MyDriver as i2c::Driver>::IdInfo,
 ///     [
 ///          (i2c::DeviceId::new(c"rust_driver_i2c"), ())
@@ -279,7 +277,6 @@ macro_rules! module_i2c_driver {
 ///
 /// kernel::of_device_table!(
 ///     OF_TABLE,
-///     MODULE_OF_TABLE,
 ///     <MyDriver as i2c::Driver>::IdInfo,
 ///     [
 ///         (of::DeviceId::new(c"test,device"), ())
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 166572861e61..1e86b901c391 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -811,7 +811,7 @@ macro_rules! module_phy_driver {
     (@device_table [$($dev:expr),+]) => {
         $crate::module_device_table!(
             "mdio", $crate::net::phy::DeviceId,
-            phydev, TABLE, @none, [$($dev),+]
+            TABLE, @none, [$($dev),+]
         );
     };
 
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 34e07a53244d..a3dd48f76353 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -261,7 +261,6 @@ macro_rules! pci_device_table {
 ///
 /// kernel::pci_device_table!(
 ///     PCI_TABLE,
-///     MODULE_PCI_TABLE,
 ///     <MyDriver as pci::Driver>::IdInfo,
 ///     [
 ///         (
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 9b362e0495d3..210a815925ce 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -176,7 +176,6 @@ macro_rules! module_platform_driver {
 ///
 /// kernel::of_device_table!(
 ///     OF_TABLE,
-///     MODULE_OF_TABLE,
 ///     <MyDriver as platform::Driver>::IdInfo,
 ///     [
 ///         (of::DeviceId::new(c"test,device"), ())
@@ -185,7 +184,6 @@ macro_rules! module_platform_driver {
 ///
 /// kernel::acpi_device_table!(
 ///     ACPI_TABLE,
-///     MODULE_ACPI_TABLE,
 ///     <MyDriver as platform::Driver>::IdInfo,
 ///     [
 ///         (acpi::DeviceId::new(c"LNUXBEEF"), ())
diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 154919ee1e19..500b5e0ba4ea 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -271,7 +271,6 @@ macro_rules! usb_device_table {
 ///
 /// kernel::usb_device_table!(
 ///     USB_TABLE,
-///     MODULE_USB_TABLE,
 ///     <MyDriver as usb::Driver>::IdInfo,
 ///     [
 ///         (usb::DeviceId::from_id(0x1234, 0x5678), ()),
diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
index 1f59e08aaa4b..181fd98ae5b6 100644
--- a/samples/rust/rust_debugfs.rs
+++ b/samples/rust/rust_debugfs.rs
@@ -110,7 +110,6 @@ fn from_str(s: &str) -> Result<Self> {
 
 kernel::acpi_device_table!(
     ACPI_TABLE,
-    MODULE_ACPI_TABLE,
     <RustDebugFs as platform::Driver>::IdInfo,
     [(acpi::DeviceId::new(c"LNUXBEEF"), ())]
 );
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 9beb37275e0d..80c309ce07e9 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -51,7 +51,6 @@ unsafe impl kernel::transmute::FromBytes for MyStruct {}
 
 kernel::pci_device_table!(
     PCI_TABLE,
-    MODULE_PCI_TABLE,
     <DmaSampleDriver as pci::Driver>::IdInfo,
     [(pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5), ())]
 );
diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
index 73c63afc046a..704567a072ba 100644
--- a/samples/rust/rust_driver_auxiliary.rs
+++ b/samples/rust/rust_driver_auxiliary.rs
@@ -24,7 +24,6 @@
 
 kernel::auxiliary_device_table!(
     AUX_TABLE,
-    MODULE_AUX_TABLE,
     <AuxiliaryDriver as auxiliary::Driver>::IdInfo,
     [(auxiliary::DeviceId::new(MODULE_NAME, AUXILIARY_NAME), ())]
 );
@@ -66,7 +65,6 @@ struct ParentData<'bound> {
 
 kernel::pci_device_table!(
     PCI_TABLE,
-    MODULE_PCI_TABLE,
     <ParentDriver as pci::Driver>::IdInfo,
     [(pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5), ())]
 );
diff --git a/samples/rust/rust_driver_i2c.rs b/samples/rust/rust_driver_i2c.rs
index ead8263a7d48..a0df0c6097c4 100644
--- a/samples/rust/rust_driver_i2c.rs
+++ b/samples/rust/rust_driver_i2c.rs
@@ -14,21 +14,18 @@
 
 kernel::acpi_device_table! {
     ACPI_TABLE,
-    MODULE_ACPI_TABLE,
     <SampleDriver as i2c::Driver>::IdInfo,
     [(acpi::DeviceId::new(c"LNUXBEEF"), 0)]
 }
 
 kernel::i2c_device_table! {
     I2C_TABLE,
-    MODULE_I2C_TABLE,
     <SampleDriver as i2c::Driver>::IdInfo,
     [(i2c::DeviceId::new(c"rust_driver_i2c"), 0)]
 }
 
 kernel::of_device_table! {
     OF_TABLE,
-    MODULE_OF_TABLE,
     <SampleDriver as i2c::Driver>::IdInfo,
     [(of::DeviceId::new(c"test,rust_driver_i2c"), 0)]
 }
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 5547dd704a1b..2282191e6292 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -74,7 +74,6 @@ struct SampleDriverData<'bound> {
 
 kernel::pci_device_table!(
     PCI_TABLE,
-    MODULE_PCI_TABLE,
     <SampleDriver as pci::Driver>::IdInfo,
     [(
         pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5),
diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
index ec0d6cac4f57..710145b3605a 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -87,14 +87,12 @@ struct SampleDriver {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <SampleDriver as platform::Driver>::IdInfo,
     [(of::DeviceId::new(c"test,rust-device"), Info(42))]
 );
 
 kernel::acpi_device_table!(
     ACPI_TABLE,
-    MODULE_ACPI_TABLE,
     <SampleDriver as platform::Driver>::IdInfo,
     [(acpi::DeviceId::new(c"LNUXBEEF"), Info(0))]
 );
diff --git a/samples/rust/rust_driver_usb.rs b/samples/rust/rust_driver_usb.rs
index 02bd5085f9bc..284042c5969b 100644
--- a/samples/rust/rust_driver_usb.rs
+++ b/samples/rust/rust_driver_usb.rs
@@ -19,7 +19,6 @@ struct SampleDriver {
 
 kernel::usb_device_table!(
     USB_TABLE,
-    MODULE_USB_TABLE,
     <SampleDriver as usb::Driver>::IdInfo,
     [(usb::DeviceId::from_id(0x1234, 0x5678), ()),]
 );
diff --git a/samples/rust/rust_i2c_client.rs b/samples/rust/rust_i2c_client.rs
index 2d876f4e3ee0..c8a23875ef5b 100644
--- a/samples/rust/rust_i2c_client.rs
+++ b/samples/rust/rust_i2c_client.rs
@@ -87,14 +87,12 @@ struct SampleDriver {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <SampleDriver as platform::Driver>::IdInfo,
     [(of::DeviceId::new(c"test,rust-device"), ())]
 );
 
 kernel::acpi_device_table!(
     ACPI_TABLE,
-    MODULE_ACPI_TABLE,
     <SampleDriver as platform::Driver>::IdInfo,
     [(acpi::DeviceId::new(c"LNUXBEEF"), ())]
 );
diff --git a/samples/rust/rust_soc.rs b/samples/rust/rust_soc.rs
index 808d58200eb6..f5e5f2f9adf7 100644
--- a/samples/rust/rust_soc.rs
+++ b/samples/rust/rust_soc.rs
@@ -23,14 +23,12 @@ struct SampleSocDriver {
 
 kernel::of_device_table!(
     OF_TABLE,
-    MODULE_OF_TABLE,
     <SampleSocDriver as platform::Driver>::IdInfo,
     [(of::DeviceId::new(c"test,rust-device"), ())]
 );
 
 kernel::acpi_device_table!(
     ACPI_TABLE,
-    MODULE_ACPI_TABLE,
     <SampleSocDriver as platform::Driver>::IdInfo,
     [(acpi::DeviceId::new(c"LNUXBEEF"), ())]
 );

-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 07/10] rust: driver: store pointers in `DeviceId`

[Gary Guo]

The common practice in C drivers is to store pointers into `driver_data`
field of device IDs. The Rust code is however currently storing indices
into the fields and then carry a side table that maps the index to
pointers.

It is much simpler to just have `DeviceId` carry the pointer like C code
does. However, just doing so naively would cause a "pointers cannot be cast
to integers during const eval" error, as kernel_ulong_t does not have
provenance while pointers do, and Rust forbids `expose_provenance` during
consteval.

Work around this limitation by wrapping raw IDs in `MaybeUninit`.
`MaybeUninit` is allowed to host arbitrary bytes with or without
provenance, so we can just then use `unsafe` to store a pointer with
provenance there. This has the same effect as changing the C-side
definition to use `void*` instead of `kernel_ulong_t`, but without actually
changing the C side.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/acpi.rs      |  4 ---
 rust/kernel/auxiliary.rs |  8 ++---
 rust/kernel/device_id.rs | 88 +++++++++++++++++++++++++++++-------------------
 rust/kernel/driver.rs    | 14 ++++----
 rust/kernel/i2c.rs       |  7 ++--
 rust/kernel/of.rs        |  4 ---
 rust/kernel/pci.rs       | 11 +++---
 rust/kernel/usb.rs       |  7 ++--
 8 files changed, 73 insertions(+), 70 deletions(-)

diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
index 315f2f2af446..ea2ce61ee393 100644
--- a/rust/kernel/acpi.rs
+++ b/rust/kernel/acpi.rs
@@ -25,10 +25,6 @@ unsafe impl RawDeviceId for DeviceId {
 // SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::acpi_device_id, driver_data);
-
-    fn index(&self) -> usize {
-        self.0.driver_data
-    }
 }
 
 impl DeviceId {
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index 59787c9bff26..aa13d8866a19 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -93,7 +93,9 @@ extern "C" fn probe_callback(
         // SAFETY: `DeviceId` is a `#[repr(transparent)`] wrapper of `struct auxiliary_device_id`
         // and does not add additional invariants, so it's safe to transmute.
         let id = unsafe { &*id.cast::<DeviceId>() };
-        let info = T::ID_TABLE.info(id.index());
+
+        // SAFETY: `id` comes from `T::ID_TABLE` which is of type `IdArray<_, T::IdInfo>`.
+        let info = unsafe { id.info_unchecked::<T::IdInfo>() };
 
         from_result(|| {
             let data = T::probe(adev, info);
@@ -169,10 +171,6 @@ unsafe impl RawDeviceId for DeviceId {
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize =
         core::mem::offset_of!(bindings::auxiliary_device_id, driver_data);
-
-    fn index(&self) -> usize {
-        self.0.driver_data
-    }
 }
 
 /// IdTable type for auxiliary drivers.
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 84852a2d9ad7..59453588df0e 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -5,7 +5,10 @@
 //! Each bus / subsystem that matches device and driver through a bus / subsystem specific ID is
 //! expected to implement [`RawDeviceId`].
 
-use core::mem::MaybeUninit;
+use core::{
+    marker::PhantomData,
+    mem::MaybeUninit, //
+};
 
 /// Marker trait to indicate a Rust device ID type represents a corresponding C device ID type.
 ///
@@ -47,15 +50,48 @@ pub unsafe trait RawDeviceIdIndex: RawDeviceId {
     /// The offset (in bytes) to the context/data field in the raw device ID.
     const DRIVER_DATA_OFFSET: usize;
 
-    /// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceIdIndex`]
-    /// trait.
-    fn index(&self) -> usize;
+    /// Obtain the data pointer stored inside the device ID.
+    ///
+    /// # Safety
+    ///
+    /// `&Self` must be stored inside a `IdArray<Self, U>`.
+    unsafe fn info_unchecked<U>(&self) -> &'static U {
+        // SAFETY: By safety requirement of the trait, this is `self.driver_data as *const U` and by
+        // the safety requirement of the function, this is stored in `IdArray<Self, U>` so is
+        // convertible to `&'static U`.
+        unsafe {
+            core::ptr::from_ref(self)
+                .byte_add(Self::DRIVER_DATA_OFFSET)
+                .cast::<&U>()
+                .read()
+        }
+    }
+
+    /// Obtain the data pointer stored inside the device ID.
+    ///
+    /// # Safety
+    ///
+    /// `&Self` must be stored inside a `IdArray<Self, U>`, or has NULL (or 0) as driver data.
+    unsafe fn info_unchecked_opt<U>(&self) -> Option<&'static U> {
+        // SAFETY: By safety requirement of the trait, this is `self.driver_data as *const U` and by
+        // the safety requirement of the function, if this is stored in `IdArray<Self, U>`, this is
+        // convertible to `Option<&'static U>`. Otherwise it is NULL which is `None` as
+        // `Option<&U>`.
+        unsafe {
+            core::ptr::from_ref(self)
+                .byte_add(Self::DRIVER_DATA_OFFSET)
+                .cast::<Option<&U>>()
+                .read()
+        }
+    }
 }
 
 /// A zero-terminated device id array.
 #[repr(C)]
 pub struct RawIdArray<T: RawDeviceId, const N: usize> {
-    ids: [T::RawType; N],
+    // This is `MaybeUninit<T::RawType>` so any bytes inside it can carry provenance in CTFE.
+    // If this were `T::RawType`, integer fields would not be able to contain pointers.
+    ids: [MaybeUninit<T::RawType>; N],
     sentinel: MaybeUninit<T::RawType>,
 }
 
@@ -68,18 +104,17 @@ pub const fn size(&self) -> usize {
 
 /// A zero-terminated device id array, followed by context data.
 #[repr(C)]
-pub struct IdArray<T: RawDeviceId, U, const N: usize> {
+pub struct IdArray<T: RawDeviceId, U: 'static, const N: usize> {
     raw_ids: RawIdArray<T, N>,
-    id_infos: [U; N],
+    phantom: PhantomData<&'static U>,
 }
 
-impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
+impl<T: RawDeviceId + RawDeviceIdIndex, U: 'static, const N: usize> IdArray<T, U, N> {
     /// Creates a new instance of the array.
     ///
     /// The contents are derived from the given identifiers and context information.
-    pub const fn new(ids: [(T, U); N]) -> Self {
+    pub const fn new(ids: [(T, &'static U); N]) -> Self {
         let mut raw_ids = [const { MaybeUninit::<T::RawType>::uninit() }; N];
-        let mut infos = [const { MaybeUninit::uninit() }; N];
 
         let mut i = 0usize;
         while i < N {
@@ -87,18 +122,15 @@ impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
             // layout-wise compatible with `RawType`.
             raw_ids[i] = unsafe { core::mem::transmute_copy(&ids[i].0) };
             // SAFETY: by the safety requirement of `RawDeviceIdIndex`, this would be effectively
-            // `raw_ids[i].driver_data = i;`.
+            // `raw_ids[i].driver_data = ids[i].1;`.
             unsafe {
                 raw_ids[i]
                     .as_mut_ptr()
                     .byte_add(T::DRIVER_DATA_OFFSET)
-                    .cast::<usize>()
-                    .write(i);
+                    .cast::<&U>()
+                    .write(ids[i].1);
             }
 
-            // SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but
-            // later forget `ids`.
-            infos[i] = MaybeUninit::new(unsafe { core::ptr::read(&ids[i].1) });
             i += 1;
         }
 
@@ -106,20 +138,15 @@ impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
 
         Self {
             raw_ids: RawIdArray {
-                // SAFETY: this is effectively `array_assume_init`, which is unstable, so we use
-                // `transmute_copy` instead. We have initialized all elements of `raw_ids` so this
-                // `array_assume_init` is safe.
-                ids: unsafe { core::mem::transmute_copy(&raw_ids) },
+                ids: raw_ids,
                 sentinel: MaybeUninit::zeroed(),
             },
-            // SAFETY: We have initialized all elements of `infos` so this `array_assume_init` is
-            // safe.
-            id_infos: unsafe { core::mem::transmute_copy(&infos) },
+            phantom: PhantomData,
         }
     }
 }
 
-impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
+impl<T: RawDeviceId, U: 'static, const N: usize> IdArray<T, U, N> {
     /// Reference to the contained [`RawIdArray`].
     pub const fn raw_ids(&self) -> &RawIdArray<T, N> {
         &self.raw_ids
@@ -133,7 +160,7 @@ impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
     /// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead.
     pub const fn new_without_index(ids: [T; N]) -> Self {
         // SAFETY: `T` is layout-wise compatible with `T::RawType`, so is the array of them.
-        let raw_ids: [T::RawType; N] = unsafe { core::mem::transmute_copy(&ids) };
+        let raw_ids: [MaybeUninit<T::RawType>; N] = unsafe { core::mem::transmute_copy(&ids) };
         core::mem::forget(ids);
 
         Self {
@@ -141,7 +168,7 @@ impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
                 ids: raw_ids,
                 sentinel: MaybeUninit::zeroed(),
             },
-            id_infos: [(); N],
+            phantom: PhantomData,
         }
     }
 }
@@ -155,9 +182,6 @@ impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
 pub trait IdTable<T: RawDeviceId, U> {
     /// Obtain the pointer to the ID table.
     fn as_ptr(&self) -> *const T::RawType;
-
-    /// Obtain the pointer to the driver-specific information from an index.
-    fn info(&self, index: usize) -> &U;
 }
 
 impl<T: RawDeviceId, U, const N: usize> IdTable<T, U> for IdArray<T, U, N> {
@@ -166,10 +190,6 @@ fn as_ptr(&self) -> *const T::RawType {
         // to access the sentinel.
         core::ptr::from_ref(self).cast()
     }
-
-    fn info(&self, index: usize) -> &U {
-        &self.id_infos[index]
-    }
 }
 
 /// Create device table alias for modpost.
@@ -184,7 +204,7 @@ macro_rules! module_device_table {
             $device_id_ty,
             $id_info_type,
             { <[$device_id_ty]>::len(&[$($id,)*]) },
-        > = $crate::device_id::IdArray::new([$(($id, $info),)*]);
+        > = $crate::device_id::IdArray::new([$(($id, &$info),)*]);
 
         $crate::module_device_table!($table_type, $table_name);
     };
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index bf5ba0d27553..824899d76fed 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -107,6 +107,7 @@
 use crate::{
     acpi,
     device,
+    device_id::RawDeviceIdIndex,
     of,
     prelude::*,
     types::Opaque,
@@ -350,7 +351,8 @@ fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
                 // and does not add additional invariants, so it's safe to transmute.
                 let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
 
-                Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceIdIndex>::index(id)))
+                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
+                Some(unsafe { id.info_unchecked::<Self::IdInfo>() })
             }
         }
     }
@@ -381,9 +383,8 @@ fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
                 // and does not add additional invariants, so it's safe to transmute.
                 let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
 
-                return Some(table.info(
-                    <of::DeviceId as crate::device_id::RawDeviceIdIndex>::index(id),
-                ));
+                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
+                return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
             }
         }
 
@@ -412,9 +413,8 @@ fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
                 //   and does not add additional invariants, so it's safe to transmute.
                 let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
 
-                return Some(table.info(
-                    <of::DeviceId as crate::device_id::RawDeviceIdIndex>::index(id),
-                ));
+                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
+                return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
             }
         }
 
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 55c89ba3a82a..9e551c7e8e41 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -65,10 +65,6 @@ unsafe impl RawDeviceId for DeviceId {
 // SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::i2c_device_id, driver_data);
-
-    fn index(&self) -> usize {
-        self.0.driver_data
-    }
 }
 
 /// IdTable type for I2C
@@ -212,7 +208,8 @@ fn i2c_id_info(dev: &I2cClient) -> Option<&'static <Self as driver::Adapter>::Id
         // does not add additional invariants, so it's safe to transmute.
         let id = unsafe { &*raw_id.cast::<DeviceId>() };
 
-        Some(table.info(<DeviceId as RawDeviceIdIndex>::index(id)))
+        // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
+        Some(unsafe { id.info_unchecked::<T::IdInfo>() })
     }
 }
 
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 35aa6d36d309..d0318f62afd7 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -25,10 +25,6 @@ unsafe impl RawDeviceId for DeviceId {
 // SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `data` field.
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
-
-    fn index(&self) -> usize {
-        self.0.data as usize
-    }
 }
 
 impl DeviceId {
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index a3dd48f76353..a630c7fc6a85 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -110,10 +110,13 @@ extern "C" fn probe_callback(
         // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct pci_device_id` and
         // does not add additional invariants, so it's safe to transmute.
         let id = unsafe { &*id.cast::<DeviceId>() };
-        let info = T::ID_TABLE.info(id.index());
+
+        // SAFETY: `id` comes from `T::ID_TABLE` which is of type `IdArray<_, T::IdInfo>` or
+        // `pci_device_id_any` which has 0 as driver_data.
+        let info = unsafe { id.info_unchecked_opt::<T::IdInfo>() };
 
         from_result(|| {
-            let data = T::probe(pdev, Some(info));
+            let data = T::probe(pdev, info);
 
             pdev.as_ref().set_drvdata(data)?;
             Ok(0)
@@ -233,10 +236,6 @@ unsafe impl RawDeviceId for DeviceId {
 // SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data);
-
-    fn index(&self) -> usize {
-        self.0.driver_data
-    }
 }
 
 /// `IdTable` type for PCI.
diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 500b5e0ba4ea..8aeff5011755 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -89,7 +89,8 @@ extern "C" fn probe_callback(
             // does not add additional invariants, so it's safe to transmute.
             let id = unsafe { &*id.cast::<DeviceId>() };
 
-            let info = T::ID_TABLE.info(id.index());
+            // SAFETY: `id` comes from `T::ID_TABLE` which is of type `IdArray<_, T::IdInfo>`.
+            let info = unsafe { id.info_unchecked::<T::IdInfo>() };
             let data = T::probe(intf, id, info);
 
             let dev: &device::Device<device::CoreInternal<'_>> = intf.as_ref();
@@ -242,10 +243,6 @@ unsafe impl RawDeviceId for DeviceId {
 // SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_info` field.
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::usb_device_id, driver_info);
-
-    fn index(&self) -> usize {
-        self.0.driver_info
-    }
 }
 
 /// [`IdTable`](kernel::device_id::IdTable) type for USB.

-- 
2.54.0

[PATCH 07/10] rust: driver: store pointers in `DeviceId`

[Gary Guo]

The common practice in C drivers is to store pointers into `driver_data`
field of device IDs. The Rust code is however currently storing indices
into the fields and then carry a side table that maps the index to
pointers.

It is much simpler to just have `DeviceId` carry the pointer like C code
does. However, just doing so naively would cause a "pointers cannot be cast
to integers during const eval" error, as kernel_ulong_t does not have
provenance while pointers do, and Rust forbids `expose_provenance` during
consteval.

Work around this limitation by wrapping raw IDs in `MaybeUninit`.
`MaybeUninit` is allowed to host arbitrary bytes with or without
provenance, so we can just then use `unsafe` to store a pointer with
provenance there. This has the same effect as changing the C-side
definition to use `void*` instead of `kernel_ulong_t`, but without actually
changing the C side.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/acpi.rs      |  4 ---
 rust/kernel/auxiliary.rs |  8 ++---
 rust/kernel/device_id.rs | 88 +++++++++++++++++++++++++++++-------------------
 rust/kernel/driver.rs    | 14 ++++----
 rust/kernel/i2c.rs       |  7 ++--
 rust/kernel/of.rs        |  4 ---
 rust/kernel/pci.rs       | 11 +++---
 rust/kernel/usb.rs       |  7 ++--
 8 files changed, 73 insertions(+), 70 deletions(-)

diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
index 315f2f2af446..ea2ce61ee393 100644
--- a/rust/kernel/acpi.rs
+++ b/rust/kernel/acpi.rs
@@ -25,10 +25,6 @@ unsafe impl RawDeviceId for DeviceId {
 // SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::acpi_device_id, driver_data);
-
-    fn index(&self) -> usize {
-        self.0.driver_data
-    }
 }
 
 impl DeviceId {
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index 59787c9bff26..aa13d8866a19 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -93,7 +93,9 @@ extern "C" fn probe_callback(
         // SAFETY: `DeviceId` is a `#[repr(transparent)`] wrapper of `struct auxiliary_device_id`
         // and does not add additional invariants, so it's safe to transmute.
         let id = unsafe { &*id.cast::<DeviceId>() };
-        let info = T::ID_TABLE.info(id.index());
+
+        // SAFETY: `id` comes from `T::ID_TABLE` which is of type `IdArray<_, T::IdInfo>`.
+        let info = unsafe { id.info_unchecked::<T::IdInfo>() };
 
         from_result(|| {
             let data = T::probe(adev, info);
@@ -169,10 +171,6 @@ unsafe impl RawDeviceId for DeviceId {
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize =
         core::mem::offset_of!(bindings::auxiliary_device_id, driver_data);
-
-    fn index(&self) -> usize {
-        self.0.driver_data
-    }
 }
 
 /// IdTable type for auxiliary drivers.
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 84852a2d9ad7..59453588df0e 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -5,7 +5,10 @@
 //! Each bus / subsystem that matches device and driver through a bus / subsystem specific ID is
 //! expected to implement [`RawDeviceId`].
 
-use core::mem::MaybeUninit;
+use core::{
+    marker::PhantomData,
+    mem::MaybeUninit, //
+};
 
 /// Marker trait to indicate a Rust device ID type represents a corresponding C device ID type.
 ///
@@ -47,15 +50,48 @@ pub unsafe trait RawDeviceIdIndex: RawDeviceId {
     /// The offset (in bytes) to the context/data field in the raw device ID.
     const DRIVER_DATA_OFFSET: usize;
 
-    /// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceIdIndex`]
-    /// trait.
-    fn index(&self) -> usize;
+    /// Obtain the data pointer stored inside the device ID.
+    ///
+    /// # Safety
+    ///
+    /// `&Self` must be stored inside a `IdArray<Self, U>`.
+    unsafe fn info_unchecked<U>(&self) -> &'static U {
+        // SAFETY: By safety requirement of the trait, this is `self.driver_data as *const U` and by
+        // the safety requirement of the function, this is stored in `IdArray<Self, U>` so is
+        // convertible to `&'static U`.
+        unsafe {
+            core::ptr::from_ref(self)
+                .byte_add(Self::DRIVER_DATA_OFFSET)
+                .cast::<&U>()
+                .read()
+        }
+    }
+
+    /// Obtain the data pointer stored inside the device ID.
+    ///
+    /// # Safety
+    ///
+    /// `&Self` must be stored inside a `IdArray<Self, U>`, or has NULL (or 0) as driver data.
+    unsafe fn info_unchecked_opt<U>(&self) -> Option<&'static U> {
+        // SAFETY: By safety requirement of the trait, this is `self.driver_data as *const U` and by
+        // the safety requirement of the function, if this is stored in `IdArray<Self, U>`, this is
+        // convertible to `Option<&'static U>`. Otherwise it is NULL which is `None` as
+        // `Option<&U>`.
+        unsafe {
+            core::ptr::from_ref(self)
+                .byte_add(Self::DRIVER_DATA_OFFSET)
+                .cast::<Option<&U>>()
+                .read()
+        }
+    }
 }
 
 /// A zero-terminated device id array.
 #[repr(C)]
 pub struct RawIdArray<T: RawDeviceId, const N: usize> {
-    ids: [T::RawType; N],
+    // This is `MaybeUninit<T::RawType>` so any bytes inside it can carry provenance in CTFE.
+    // If this were `T::RawType`, integer fields would not be able to contain pointers.
+    ids: [MaybeUninit<T::RawType>; N],
     sentinel: MaybeUninit<T::RawType>,
 }
 
@@ -68,18 +104,17 @@ pub const fn size(&self) -> usize {
 
 /// A zero-terminated device id array, followed by context data.
 #[repr(C)]
-pub struct IdArray<T: RawDeviceId, U, const N: usize> {
+pub struct IdArray<T: RawDeviceId, U: 'static, const N: usize> {
     raw_ids: RawIdArray<T, N>,
-    id_infos: [U; N],
+    phantom: PhantomData<&'static U>,
 }
 
-impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
+impl<T: RawDeviceId + RawDeviceIdIndex, U: 'static, const N: usize> IdArray<T, U, N> {
     /// Creates a new instance of the array.
     ///
     /// The contents are derived from the given identifiers and context information.
-    pub const fn new(ids: [(T, U); N]) -> Self {
+    pub const fn new(ids: [(T, &'static U); N]) -> Self {
         let mut raw_ids = [const { MaybeUninit::<T::RawType>::uninit() }; N];
-        let mut infos = [const { MaybeUninit::uninit() }; N];
 
         let mut i = 0usize;
         while i < N {
@@ -87,18 +122,15 @@ impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
             // layout-wise compatible with `RawType`.
             raw_ids[i] = unsafe { core::mem::transmute_copy(&ids[i].0) };
             // SAFETY: by the safety requirement of `RawDeviceIdIndex`, this would be effectively
-            // `raw_ids[i].driver_data = i;`.
+            // `raw_ids[i].driver_data = ids[i].1;`.
             unsafe {
                 raw_ids[i]
                     .as_mut_ptr()
                     .byte_add(T::DRIVER_DATA_OFFSET)
-                    .cast::<usize>()
-                    .write(i);
+                    .cast::<&U>()
+                    .write(ids[i].1);
             }
 
-            // SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but
-            // later forget `ids`.
-            infos[i] = MaybeUninit::new(unsafe { core::ptr::read(&ids[i].1) });
             i += 1;
         }
 
@@ -106,20 +138,15 @@ impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
 
         Self {
             raw_ids: RawIdArray {
-                // SAFETY: this is effectively `array_assume_init`, which is unstable, so we use
-                // `transmute_copy` instead. We have initialized all elements of `raw_ids` so this
-                // `array_assume_init` is safe.
-                ids: unsafe { core::mem::transmute_copy(&raw_ids) },
+                ids: raw_ids,
                 sentinel: MaybeUninit::zeroed(),
             },
-            // SAFETY: We have initialized all elements of `infos` so this `array_assume_init` is
-            // safe.
-            id_infos: unsafe { core::mem::transmute_copy(&infos) },
+            phantom: PhantomData,
         }
     }
 }
 
-impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
+impl<T: RawDeviceId, U: 'static, const N: usize> IdArray<T, U, N> {
     /// Reference to the contained [`RawIdArray`].
     pub const fn raw_ids(&self) -> &RawIdArray<T, N> {
         &self.raw_ids
@@ -133,7 +160,7 @@ impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
     /// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead.
     pub const fn new_without_index(ids: [T; N]) -> Self {
         // SAFETY: `T` is layout-wise compatible with `T::RawType`, so is the array of them.
-        let raw_ids: [T::RawType; N] = unsafe { core::mem::transmute_copy(&ids) };
+        let raw_ids: [MaybeUninit<T::RawType>; N] = unsafe { core::mem::transmute_copy(&ids) };
         core::mem::forget(ids);
 
         Self {
@@ -141,7 +168,7 @@ impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
                 ids: raw_ids,
                 sentinel: MaybeUninit::zeroed(),
             },
-            id_infos: [(); N],
+            phantom: PhantomData,
         }
     }
 }
@@ -155,9 +182,6 @@ impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
 pub trait IdTable<T: RawDeviceId, U> {
     /// Obtain the pointer to the ID table.
     fn as_ptr(&self) -> *const T::RawType;
-
-    /// Obtain the pointer to the driver-specific information from an index.
-    fn info(&self, index: usize) -> &U;
 }
 
 impl<T: RawDeviceId, U, const N: usize> IdTable<T, U> for IdArray<T, U, N> {
@@ -166,10 +190,6 @@ fn as_ptr(&self) -> *const T::RawType {
         // to access the sentinel.
         core::ptr::from_ref(self).cast()
     }
-
-    fn info(&self, index: usize) -> &U {
-        &self.id_infos[index]
-    }
 }
 
 /// Create device table alias for modpost.
@@ -184,7 +204,7 @@ macro_rules! module_device_table {
             $device_id_ty,
             $id_info_type,
             { <[$device_id_ty]>::len(&[$($id,)*]) },
-        > = $crate::device_id::IdArray::new([$(($id, $info),)*]);
+        > = $crate::device_id::IdArray::new([$(($id, &$info),)*]);
 
         $crate::module_device_table!($table_type, $table_name);
     };
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index bf5ba0d27553..824899d76fed 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -107,6 +107,7 @@
 use crate::{
     acpi,
     device,
+    device_id::RawDeviceIdIndex,
     of,
     prelude::*,
     types::Opaque,
@@ -350,7 +351,8 @@ fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
                 // and does not add additional invariants, so it's safe to transmute.
                 let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
 
-                Some(table.info(<acpi::DeviceId as crate::device_id::RawDeviceIdIndex>::index(id)))
+                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
+                Some(unsafe { id.info_unchecked::<Self::IdInfo>() })
             }
         }
     }
@@ -381,9 +383,8 @@ fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
                 // and does not add additional invariants, so it's safe to transmute.
                 let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
 
-                return Some(table.info(
-                    <of::DeviceId as crate::device_id::RawDeviceIdIndex>::index(id),
-                ));
+                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
+                return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
             }
         }
 
@@ -412,9 +413,8 @@ fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
                 //   and does not add additional invariants, so it's safe to transmute.
                 let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
 
-                return Some(table.info(
-                    <of::DeviceId as crate::device_id::RawDeviceIdIndex>::index(id),
-                ));
+                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
+                return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
             }
         }
 
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 55c89ba3a82a..9e551c7e8e41 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -65,10 +65,6 @@ unsafe impl RawDeviceId for DeviceId {
 // SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::i2c_device_id, driver_data);
-
-    fn index(&self) -> usize {
-        self.0.driver_data
-    }
 }
 
 /// IdTable type for I2C
@@ -212,7 +208,8 @@ fn i2c_id_info(dev: &I2cClient) -> Option<&'static <Self as driver::Adapter>::Id
         // does not add additional invariants, so it's safe to transmute.
         let id = unsafe { &*raw_id.cast::<DeviceId>() };
 
-        Some(table.info(<DeviceId as RawDeviceIdIndex>::index(id)))
+        // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
+        Some(unsafe { id.info_unchecked::<T::IdInfo>() })
     }
 }
 
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 35aa6d36d309..d0318f62afd7 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -25,10 +25,6 @@ unsafe impl RawDeviceId for DeviceId {
 // SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `data` field.
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
-
-    fn index(&self) -> usize {
-        self.0.data as usize
-    }
 }
 
 impl DeviceId {
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index a3dd48f76353..a630c7fc6a85 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -110,10 +110,13 @@ extern "C" fn probe_callback(
         // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct pci_device_id` and
         // does not add additional invariants, so it's safe to transmute.
         let id = unsafe { &*id.cast::<DeviceId>() };
-        let info = T::ID_TABLE.info(id.index());
+
+        // SAFETY: `id` comes from `T::ID_TABLE` which is of type `IdArray<_, T::IdInfo>` or
+        // `pci_device_id_any` which has 0 as driver_data.
+        let info = unsafe { id.info_unchecked_opt::<T::IdInfo>() };
 
         from_result(|| {
-            let data = T::probe(pdev, Some(info));
+            let data = T::probe(pdev, info);
 
             pdev.as_ref().set_drvdata(data)?;
             Ok(0)
@@ -233,10 +236,6 @@ unsafe impl RawDeviceId for DeviceId {
 // SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data);
-
-    fn index(&self) -> usize {
-        self.0.driver_data
-    }
 }
 
 /// `IdTable` type for PCI.
diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 500b5e0ba4ea..8aeff5011755 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -89,7 +89,8 @@ extern "C" fn probe_callback(
             // does not add additional invariants, so it's safe to transmute.
             let id = unsafe { &*id.cast::<DeviceId>() };
 
-            let info = T::ID_TABLE.info(id.index());
+            // SAFETY: `id` comes from `T::ID_TABLE` which is of type `IdArray<_, T::IdInfo>`.
+            let info = unsafe { id.info_unchecked::<T::IdInfo>() };
             let data = T::probe(intf, id, info);
 
             let dev: &device::Device<device::CoreInternal<'_>> = intf.as_ref();
@@ -242,10 +243,6 @@ unsafe impl RawDeviceId for DeviceId {
 // SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_info` field.
 unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::usb_device_id, driver_info);
-
-    fn index(&self) -> usize {
-        self.0.driver_info
-    }
 }
 
 /// [`IdTable`](kernel::device_id::IdTable) type for USB.

-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 08/10] rust: driver: remove open-coded matching logic

[Gary Guo]

With device ID info now including pointers instead of indices, the
open-coded ACPI/OF matching is no longer needed and can be replaced with
`device_get_match_data`.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/driver.rs   | 114 ++++--------------------------------------------
 rust/kernel/i2c.rs      |   6 ++-
 rust/kernel/platform.rs |   3 +-
 3 files changed, 15 insertions(+), 108 deletions(-)

diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index 824899d76fed..a881f5ef99ec 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -107,7 +107,6 @@
 use crate::{
     acpi,
     device,
-    device_id::RawDeviceIdIndex,
     of,
     prelude::*,
     types::Opaque,
@@ -325,117 +324,22 @@ pub trait Adapter {
     /// The [`acpi::IdTable`] of the corresponding driver
     fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>>;
 
-    /// Returns the driver's private data from the matching entry in the [`acpi::IdTable`], if any.
-    ///
-    /// If this returns `None`, it means there is no match with an entry in the [`acpi::IdTable`].
-    fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        #[cfg(not(CONFIG_ACPI))]
-        {
-            let _ = dev;
-            None
-        }
-
-        #[cfg(CONFIG_ACPI)]
-        {
-            let table = Self::acpi_id_table()?;
-
-            // SAFETY:
-            // - `table` has static lifetime, hence it's valid for read,
-            // - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
-            let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
-
-            if raw_id.is_null() {
-                None
-            } else {
-                // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct acpi_device_id`
-                // and does not add additional invariants, so it's safe to transmute.
-                let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
-
-                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
-                Some(unsafe { id.info_unchecked::<Self::IdInfo>() })
-            }
-        }
-    }
-
     /// The [`of::IdTable`] of the corresponding driver.
     fn of_id_table() -> Option<of::IdTable<Self::IdInfo>>;
 
-    /// Returns the driver's private data from the matching entry in the [`of::IdTable`], if any.
-    ///
-    /// If this returns `None`, it means there is no match with an entry in the [`of::IdTable`].
-    fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        let table = Self::of_id_table()?;
-
-        #[cfg(not(any(CONFIG_OF, CONFIG_ACPI)))]
-        {
-            let _ = (dev, table);
-        }
-
-        #[cfg(CONFIG_OF)]
-        {
-            // SAFETY:
-            // - `table` has static lifetime, hence it's valid for read,
-            // - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
-            let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), dev.as_raw()) };
-
-            if !raw_id.is_null() {
-                // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id`
-                // and does not add additional invariants, so it's safe to transmute.
-                let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
-
-                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
-                return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
-            }
-        }
-
-        #[cfg(CONFIG_ACPI)]
-        {
-            use core::ptr;
-            use device::property::FwNode;
-
-            let mut raw_id = ptr::null();
-
-            let fwnode = dev.fwnode().map_or(ptr::null_mut(), FwNode::as_raw);
-
-            // SAFETY: `fwnode` is a pointer to a valid `fwnode_handle`. A null pointer will be
-            // passed through the function.
-            let adev = unsafe { bindings::to_acpi_device_node(fwnode) };
-
-            // SAFETY:
-            // - `adev` is a valid pointer to `acpi_device` or is null. It is guaranteed to be
-            //   valid as long as `dev` is alive.
-            // - `table` has static lifetime, hence it's valid for read.
-            if unsafe { acpi_of_match_device(adev, table.as_ptr(), &raw mut raw_id) } {
-                // SAFETY:
-                // - the function returns true, therefore `raw_id` has been set to a pointer to a
-                //   valid `of_device_id`.
-                // - `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id`
-                //   and does not add additional invariants, so it's safe to transmute.
-                let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
-
-                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
-                return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
-            }
-        }
-
-        None
-    }
-
     /// Returns the driver's private data from the matching entry of any of the ID tables, if any.
     ///
     /// If this returns `None`, it means that there is no match in any of the ID tables directly
     /// associated with a [`device::Device`].
-    fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        let id = Self::acpi_id_info(dev);
-        if id.is_some() {
-            return id;
-        }
-
-        let id = Self::of_id_info(dev);
-        if id.is_some() {
-            return id;
-        }
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that the `dev` matched data is of type `Self::IdInfo`.
+    unsafe fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
+        // SAFETY: `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
+        let data = unsafe { bindings::device_get_match_data(dev.as_raw()) };
 
-        None
+        // SAFETY: Per safety requirement, `data` is of type `Self::IdInfo`.
+        unsafe { data.cast::<Self::IdInfo>().as_ref() }
     }
 }
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 9e551c7e8e41..07680fd2f3fc 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -149,8 +149,10 @@ extern "C" fn probe_callback(idev: *mut bindings::i2c_client) -> kernel::ffi::c_
         // INVARIANT: `idev` is valid for the duration of `probe_callback()`.
         let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal<'_>>>() };
 
-        let info =
-            Self::i2c_id_info(idev).or_else(|| <Self as driver::Adapter>::id_info(idev.as_ref()));
+        let info = Self::i2c_id_info(idev).or_else(|| {
+            // SAFETY: `idev` matched data is of type `Self::IdInfo`.
+            unsafe { <Self as driver::Adapter>::id_info(idev.as_ref()) }
+        });
 
         from_result(|| {
             let data = T::probe(idev, info);
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 210a815925ce..e12e88113ca5 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -100,7 +100,8 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff
         //
         // INVARIANT: `pdev` is valid for the duration of `probe_callback()`.
         let pdev = unsafe { &*pdev.cast::<Device<device::CoreInternal<'_>>>() };
-        let info = <Self as driver::Adapter>::id_info(pdev.as_ref());
+        // SAFETY: `pdev` matched data is of type `Self::IdInfo`.
+        let info = unsafe { <Self as driver::Adapter>::id_info(pdev.as_ref()) };
 
         from_result(|| {
             let data = T::probe(pdev, info);

-- 
2.54.0

[PATCH 08/10] rust: driver: remove open-coded matching logic

[Gary Guo]

With device ID info now including pointers instead of indices, the
open-coded ACPI/OF matching is no longer needed and can be replaced with
`device_get_match_data`.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/driver.rs   | 114 ++++--------------------------------------------
 rust/kernel/i2c.rs      |   6 ++-
 rust/kernel/platform.rs |   3 +-
 3 files changed, 15 insertions(+), 108 deletions(-)

diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index 824899d76fed..a881f5ef99ec 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -107,7 +107,6 @@
 use crate::{
     acpi,
     device,
-    device_id::RawDeviceIdIndex,
     of,
     prelude::*,
     types::Opaque,
@@ -325,117 +324,22 @@ pub trait Adapter {
     /// The [`acpi::IdTable`] of the corresponding driver
     fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>>;
 
-    /// Returns the driver's private data from the matching entry in the [`acpi::IdTable`], if any.
-    ///
-    /// If this returns `None`, it means there is no match with an entry in the [`acpi::IdTable`].
-    fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        #[cfg(not(CONFIG_ACPI))]
-        {
-            let _ = dev;
-            None
-        }
-
-        #[cfg(CONFIG_ACPI)]
-        {
-            let table = Self::acpi_id_table()?;
-
-            // SAFETY:
-            // - `table` has static lifetime, hence it's valid for read,
-            // - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
-            let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
-
-            if raw_id.is_null() {
-                None
-            } else {
-                // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct acpi_device_id`
-                // and does not add additional invariants, so it's safe to transmute.
-                let id = unsafe { &*raw_id.cast::<acpi::DeviceId>() };
-
-                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
-                Some(unsafe { id.info_unchecked::<Self::IdInfo>() })
-            }
-        }
-    }
-
     /// The [`of::IdTable`] of the corresponding driver.
     fn of_id_table() -> Option<of::IdTable<Self::IdInfo>>;
 
-    /// Returns the driver's private data from the matching entry in the [`of::IdTable`], if any.
-    ///
-    /// If this returns `None`, it means there is no match with an entry in the [`of::IdTable`].
-    fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        let table = Self::of_id_table()?;
-
-        #[cfg(not(any(CONFIG_OF, CONFIG_ACPI)))]
-        {
-            let _ = (dev, table);
-        }
-
-        #[cfg(CONFIG_OF)]
-        {
-            // SAFETY:
-            // - `table` has static lifetime, hence it's valid for read,
-            // - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
-            let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), dev.as_raw()) };
-
-            if !raw_id.is_null() {
-                // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id`
-                // and does not add additional invariants, so it's safe to transmute.
-                let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
-
-                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
-                return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
-            }
-        }
-
-        #[cfg(CONFIG_ACPI)]
-        {
-            use core::ptr;
-            use device::property::FwNode;
-
-            let mut raw_id = ptr::null();
-
-            let fwnode = dev.fwnode().map_or(ptr::null_mut(), FwNode::as_raw);
-
-            // SAFETY: `fwnode` is a pointer to a valid `fwnode_handle`. A null pointer will be
-            // passed through the function.
-            let adev = unsafe { bindings::to_acpi_device_node(fwnode) };
-
-            // SAFETY:
-            // - `adev` is a valid pointer to `acpi_device` or is null. It is guaranteed to be
-            //   valid as long as `dev` is alive.
-            // - `table` has static lifetime, hence it's valid for read.
-            if unsafe { acpi_of_match_device(adev, table.as_ptr(), &raw mut raw_id) } {
-                // SAFETY:
-                // - the function returns true, therefore `raw_id` has been set to a pointer to a
-                //   valid `of_device_id`.
-                // - `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id`
-                //   and does not add additional invariants, so it's safe to transmute.
-                let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
-
-                // SAFETY: `id` comes from `table` which is of type `IdArray<_, Self::IdInfo>`.
-                return Some(unsafe { id.info_unchecked::<Self::IdInfo>() });
-            }
-        }
-
-        None
-    }
-
     /// Returns the driver's private data from the matching entry of any of the ID tables, if any.
     ///
     /// If this returns `None`, it means that there is no match in any of the ID tables directly
     /// associated with a [`device::Device`].
-    fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
-        let id = Self::acpi_id_info(dev);
-        if id.is_some() {
-            return id;
-        }
-
-        let id = Self::of_id_info(dev);
-        if id.is_some() {
-            return id;
-        }
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that the `dev` matched data is of type `Self::IdInfo`.
+    unsafe fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
+        // SAFETY: `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
+        let data = unsafe { bindings::device_get_match_data(dev.as_raw()) };
 
-        None
+        // SAFETY: Per safety requirement, `data` is of type `Self::IdInfo`.
+        unsafe { data.cast::<Self::IdInfo>().as_ref() }
     }
 }
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 9e551c7e8e41..07680fd2f3fc 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -149,8 +149,10 @@ extern "C" fn probe_callback(idev: *mut bindings::i2c_client) -> kernel::ffi::c_
         // INVARIANT: `idev` is valid for the duration of `probe_callback()`.
         let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal<'_>>>() };
 
-        let info =
-            Self::i2c_id_info(idev).or_else(|| <Self as driver::Adapter>::id_info(idev.as_ref()));
+        let info = Self::i2c_id_info(idev).or_else(|| {
+            // SAFETY: `idev` matched data is of type `Self::IdInfo`.
+            unsafe { <Self as driver::Adapter>::id_info(idev.as_ref()) }
+        });
 
         from_result(|| {
             let data = T::probe(idev, info);
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 210a815925ce..e12e88113ca5 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -100,7 +100,8 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff
         //
         // INVARIANT: `pdev` is valid for the duration of `probe_callback()`.
         let pdev = unsafe { &*pdev.cast::<Device<device::CoreInternal<'_>>>() };
-        let info = <Self as driver::Adapter>::id_info(pdev.as_ref());
+        // SAFETY: `pdev` matched data is of type `Self::IdInfo`.
+        let info = unsafe { <Self as driver::Adapter>::id_info(pdev.as_ref()) };
 
         from_result(|| {
             let data = T::probe(pdev, info);

-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 09/10] rust: driver: remove duplicate ID table

[Gary Guo]

Previously, `IdArray` contains both device ID table and info table so we
keep a separate copy for MODULE_DEVICE_TABLE for hotplug (which needs to be
just the device ID table). With the info being changed to be carried via
pointers, `IdArray` is now layout compatible with raw ID table and hence
there is no longer a need to keep the distinction.

Deduplicate the code, and remove the redundant copy for hotplug purpose by
just giving the `IdArray` instance a proper symbol name.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/device_id.rs | 76 +++++++++++++++++-------------------------------
 1 file changed, 27 insertions(+), 49 deletions(-)

diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 59453588df0e..26618bcda276 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -86,28 +86,23 @@ unsafe fn info_unchecked_opt<U>(&self) -> Option<&'static U> {
     }
 }
 
-/// A zero-terminated device id array.
+/// A zero-terminated device id array, followed by context data.
 #[repr(C)]
-pub struct RawIdArray<T: RawDeviceId, const N: usize> {
+pub struct IdArray<T: RawDeviceId, U: 'static, const N: usize> {
     // This is `MaybeUninit<T::RawType>` so any bytes inside it can carry provenance in CTFE.
     // If this were `T::RawType`, integer fields would not be able to contain pointers.
     ids: [MaybeUninit<T::RawType>; N],
     sentinel: MaybeUninit<T::RawType>,
+    phantom: PhantomData<&'static U>,
 }
 
-impl<T: RawDeviceId, const N: usize> RawIdArray<T, N> {
-    #[doc(hidden)]
-    pub const fn size(&self) -> usize {
-        core::mem::size_of::<Self>()
-    }
-}
+// SAFETY: device ID is plain data plus a `&'static U` and can thus be sent between threads safely
+// if `&U` can.
+unsafe impl<T: RawDeviceId, U: Sync + 'static, const N: usize> Send for IdArray<T, U, N> {}
 
-/// A zero-terminated device id array, followed by context data.
-#[repr(C)]
-pub struct IdArray<T: RawDeviceId, U: 'static, const N: usize> {
-    raw_ids: RawIdArray<T, N>,
-    phantom: PhantomData<&'static U>,
-}
+// SAFETY: device ID is plain data plus a `&'static U` and can thus be shared between threads safely
+// if `&U` can.
+unsafe impl<T: RawDeviceId, U: Sync + 'static, const N: usize> Sync for IdArray<T, U, N> {}
 
 impl<T: RawDeviceId + RawDeviceIdIndex, U: 'static, const N: usize> IdArray<T, U, N> {
     /// Creates a new instance of the array.
@@ -137,22 +132,13 @@ impl<T: RawDeviceId + RawDeviceIdIndex, U: 'static, const N: usize> IdArray<T, U
         core::mem::forget(ids);
 
         Self {
-            raw_ids: RawIdArray {
-                ids: raw_ids,
-                sentinel: MaybeUninit::zeroed(),
-            },
+            ids: raw_ids,
+            sentinel: MaybeUninit::zeroed(),
             phantom: PhantomData,
         }
     }
 }
 
-impl<T: RawDeviceId, U: 'static, const N: usize> IdArray<T, U, N> {
-    /// Reference to the contained [`RawIdArray`].
-    pub const fn raw_ids(&self) -> &RawIdArray<T, N> {
-        &self.raw_ids
-    }
-}
-
 impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
     /// Creates a new instance of the array without writing index values.
     ///
@@ -164,10 +150,8 @@ impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
         core::mem::forget(ids);
 
         Self {
-            raw_ids: RawIdArray {
-                ids: raw_ids,
-                sentinel: MaybeUninit::zeroed(),
-            },
+            ids: raw_ids,
+            sentinel: MaybeUninit::zeroed(),
             phantom: PhantomData,
         }
     }
@@ -200,13 +184,17 @@ macro_rules! module_device_table {
         $table_name: ident, $id_info_type: ty,
         [$(($id: expr, $info:expr $(,)?)),* $(,)?]
     ) => {
-        const $table_name: $crate::device_id::IdArray<
+        #[export_name =
+            concat!("__mod_device_table__", line!(),
+                    "__kmod_", module_path!(),
+                    "__", $table_type,
+                    "__", stringify!($table_name))
+        ]
+        static $table_name: $crate::device_id::IdArray<
             $device_id_ty,
             $id_info_type,
             { <[$device_id_ty]>::len(&[$($id,)*]) },
         > = $crate::device_id::IdArray::new([$(($id, &$info),)*]);
-
-        $crate::module_device_table!($table_type, $table_name);
     };
 
     // Case for no ID info.
@@ -215,26 +203,16 @@ macro_rules! module_device_table {
         $table_name: ident, @none,
         [$($id: expr),* $(,)?]
     ) => {
-        const $table_name: $crate::device_id::IdArray<
+        #[export_name =
+            concat!("__mod_device_table__", line!(),
+                    "__kmod_", module_path!(),
+                    "__", $table_type,
+                    "__", stringify!($table_name))
+        ]
+        static $table_name: $crate::device_id::IdArray<
             $device_id_ty,
             (),
             { <[$device_id_ty]>::len(&[$($id,)*]) },
         > = $crate::device_id::IdArray::new_without_index([$($id),*]);
-
-        $crate::module_device_table!($table_type, $table_name);
-    };
-
-    ($table_type: literal, $table_name:ident) => {
-        const _: () = {
-            #[rustfmt::skip]
-            #[export_name =
-                concat!("__mod_device_table__", line!(),
-                        "__kmod_", module_path!(),
-                        "__", $table_type,
-                        "__", stringify!($table_name))
-            ]
-            static TABLE: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
-                unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
-        };
     };
 }

-- 
2.54.0

[PATCH 09/10] rust: driver: remove duplicate ID table

[Gary Guo]

Previously, `IdArray` contains both device ID table and info table so we
keep a separate copy for MODULE_DEVICE_TABLE for hotplug (which needs to be
just the device ID table). With the info being changed to be carried via
pointers, `IdArray` is now layout compatible with raw ID table and hence
there is no longer a need to keep the distinction.

Deduplicate the code, and remove the redundant copy for hotplug purpose by
just giving the `IdArray` instance a proper symbol name.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/device_id.rs | 76 +++++++++++++++++-------------------------------
 1 file changed, 27 insertions(+), 49 deletions(-)

diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 59453588df0e..26618bcda276 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -86,28 +86,23 @@ unsafe fn info_unchecked_opt<U>(&self) -> Option<&'static U> {
     }
 }
 
-/// A zero-terminated device id array.
+/// A zero-terminated device id array, followed by context data.
 #[repr(C)]
-pub struct RawIdArray<T: RawDeviceId, const N: usize> {
+pub struct IdArray<T: RawDeviceId, U: 'static, const N: usize> {
     // This is `MaybeUninit<T::RawType>` so any bytes inside it can carry provenance in CTFE.
     // If this were `T::RawType`, integer fields would not be able to contain pointers.
     ids: [MaybeUninit<T::RawType>; N],
     sentinel: MaybeUninit<T::RawType>,
+    phantom: PhantomData<&'static U>,
 }
 
-impl<T: RawDeviceId, const N: usize> RawIdArray<T, N> {
-    #[doc(hidden)]
-    pub const fn size(&self) -> usize {
-        core::mem::size_of::<Self>()
-    }
-}
+// SAFETY: device ID is plain data plus a `&'static U` and can thus be sent between threads safely
+// if `&U` can.
+unsafe impl<T: RawDeviceId, U: Sync + 'static, const N: usize> Send for IdArray<T, U, N> {}
 
-/// A zero-terminated device id array, followed by context data.
-#[repr(C)]
-pub struct IdArray<T: RawDeviceId, U: 'static, const N: usize> {
-    raw_ids: RawIdArray<T, N>,
-    phantom: PhantomData<&'static U>,
-}
+// SAFETY: device ID is plain data plus a `&'static U` and can thus be shared between threads safely
+// if `&U` can.
+unsafe impl<T: RawDeviceId, U: Sync + 'static, const N: usize> Sync for IdArray<T, U, N> {}
 
 impl<T: RawDeviceId + RawDeviceIdIndex, U: 'static, const N: usize> IdArray<T, U, N> {
     /// Creates a new instance of the array.
@@ -137,22 +132,13 @@ impl<T: RawDeviceId + RawDeviceIdIndex, U: 'static, const N: usize> IdArray<T, U
         core::mem::forget(ids);
 
         Self {
-            raw_ids: RawIdArray {
-                ids: raw_ids,
-                sentinel: MaybeUninit::zeroed(),
-            },
+            ids: raw_ids,
+            sentinel: MaybeUninit::zeroed(),
             phantom: PhantomData,
         }
     }
 }
 
-impl<T: RawDeviceId, U: 'static, const N: usize> IdArray<T, U, N> {
-    /// Reference to the contained [`RawIdArray`].
-    pub const fn raw_ids(&self) -> &RawIdArray<T, N> {
-        &self.raw_ids
-    }
-}
-
 impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
     /// Creates a new instance of the array without writing index values.
     ///
@@ -164,10 +150,8 @@ impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
         core::mem::forget(ids);
 
         Self {
-            raw_ids: RawIdArray {
-                ids: raw_ids,
-                sentinel: MaybeUninit::zeroed(),
-            },
+            ids: raw_ids,
+            sentinel: MaybeUninit::zeroed(),
             phantom: PhantomData,
         }
     }
@@ -200,13 +184,17 @@ macro_rules! module_device_table {
         $table_name: ident, $id_info_type: ty,
         [$(($id: expr, $info:expr $(,)?)),* $(,)?]
     ) => {
-        const $table_name: $crate::device_id::IdArray<
+        #[export_name =
+            concat!("__mod_device_table__", line!(),
+                    "__kmod_", module_path!(),
+                    "__", $table_type,
+                    "__", stringify!($table_name))
+        ]
+        static $table_name: $crate::device_id::IdArray<
             $device_id_ty,
             $id_info_type,
             { <[$device_id_ty]>::len(&[$($id,)*]) },
         > = $crate::device_id::IdArray::new([$(($id, &$info),)*]);
-
-        $crate::module_device_table!($table_type, $table_name);
     };
 
     // Case for no ID info.
@@ -215,26 +203,16 @@ macro_rules! module_device_table {
         $table_name: ident, @none,
         [$($id: expr),* $(,)?]
     ) => {
-        const $table_name: $crate::device_id::IdArray<
+        #[export_name =
+            concat!("__mod_device_table__", line!(),
+                    "__kmod_", module_path!(),
+                    "__", $table_type,
+                    "__", stringify!($table_name))
+        ]
+        static $table_name: $crate::device_id::IdArray<
             $device_id_ty,
             (),
             { <[$device_id_ty]>::len(&[$($id,)*]) },
         > = $crate::device_id::IdArray::new_without_index([$($id),*]);
-
-        $crate::module_device_table!($table_type, $table_name);
-    };
-
-    ($table_type: literal, $table_name:ident) => {
-        const _: () = {
-            #[rustfmt::skip]
-            #[export_name =
-                concat!("__mod_device_table__", line!(),
-                        "__kmod_", module_path!(),
-                        "__", $table_type,
-                        "__", stringify!($table_name))
-            ]
-            static TABLE: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
-                unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
-        };
     };
 }

-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 10/10] RFC: rust: driver: support map-like syntax for ID table

[Gary Guo]

The device ID table and its associated info is really just a map. Add a
syntax to `module_device_table` macro that reflects that.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/device_id.rs        | 11 +++++++++++
 samples/rust/rust_driver_pci.rs |  7 +++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 26618bcda276..7c61cdcc9427 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -183,6 +183,17 @@ macro_rules! module_device_table {
         $table_type: literal, $device_id_ty: ty,
         $table_name: ident, $id_info_type: ty,
         [$(($id: expr, $info:expr $(,)?)),* $(,)?]
+    ) => {
+        $crate::module_device_table!(
+            $table_type, $device_id_ty, $table_name, $id_info_type,
+            {$($id=>$info,)*}
+        );
+    };
+
+    (
+        $table_type: literal, $device_id_ty: ty,
+        $table_name: ident, $id_info_type: ty,
+        {$($id: expr => $info:expr),* $(,)?}
     ) => {
         #[export_name =
             concat!("__mod_device_table__", line!(),
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 2282191e6292..652819dff082 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -75,10 +75,9 @@ struct SampleDriverData<'bound> {
 kernel::pci_device_table!(
     PCI_TABLE,
     <SampleDriver as pci::Driver>::IdInfo,
-    [(
-        pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5),
-        TestIndex::NO_EVENTFD
-    )]
+    {
+        pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5) => TestIndex::NO_EVENTFD,
+    }
 );
 
 impl SampleDriverData<'_> {

-- 
2.54.0

[PATCH 10/10] RFC: rust: driver: support map-like syntax for ID table

[Gary Guo]

The device ID table and its associated info is really just a map. Add a
syntax to `module_device_table` macro that reflects that.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/device_id.rs        | 11 +++++++++++
 samples/rust/rust_driver_pci.rs |  7 +++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 26618bcda276..7c61cdcc9427 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -183,6 +183,17 @@ macro_rules! module_device_table {
         $table_type: literal, $device_id_ty: ty,
         $table_name: ident, $id_info_type: ty,
         [$(($id: expr, $info:expr $(,)?)),* $(,)?]
+    ) => {
+        $crate::module_device_table!(
+            $table_type, $device_id_ty, $table_name, $id_info_type,
+            {$($id=>$info,)*}
+        );
+    };
+
+    (
+        $table_type: literal, $device_id_ty: ty,
+        $table_name: ident, $id_info_type: ty,
+        {$($id: expr => $info:expr),* $(,)?}
     ) => {
         #[export_name =
             concat!("__mod_device_table__", line!(),
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 2282191e6292..652819dff082 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -75,10 +75,9 @@ struct SampleDriverData<'bound> {
 kernel::pci_device_table!(
     PCI_TABLE,
     <SampleDriver as pci::Driver>::IdInfo,
-    [(
-        pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5),
-        TestIndex::NO_EVENTFD
-    )]
+    {
+        pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5) => TestIndex::NO_EVENTFD,
+    }
 );
 
 impl SampleDriverData<'_> {

-- 
2.54.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 02/10] rust: driver: simplify `IdArray::new_without_index`

[sashiko-bot]

> This method can very easily construct the `IdArray` on its own without
> delegating to `Self::build`. Doing so also simplifies the phy device table
> macro because it does not need to construct tuples anymore.
> 
> This also allows simplification of `new` and `build` which removes the
> `unsafe`.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=2

Re: [PATCH 10/10] RFC: rust: driver: support map-like syntax for ID table

[sashiko-bot]

> The device ID table and its associated info is really just a map. Add a
> syntax to `module_device_table` macro that reflects that.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=10

Re: [PATCH 03/10] rust: pci: use `Option<&IdInfo>` for device ID info

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The dynamic ID matching in the PCI core returns a pointer to a dynamically allocated ID without holding a lock or reference, leading to a Use-After-Free if the ID is concurrently removed via sysfs.
--

commit d1889a1b7c5b08bca9b93c95bb95cf7a983ddfea
Author: Gary Guo <gary@garyguo.net>

rust: pci: use Option<&IdInfo> for device ID info

This commit updates the Rust PCI probe functions to accept an Option<&IdInfo>
instead of a direct reference. This allows the driver to handle situations
where the driver_data is 0, such as when driver_override is used.

[ ... ]

> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 5071cae6543fd..0e055e4df99ee 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -113,7 +113,7 @@ extern "C" fn probe_callback(
>          let info = T::ID_TABLE.info(id.index());
>  
>          from_result(|| {
> -            let data = T::probe(pdev, info);
> +            let data = T::probe(pdev, Some(info));

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing
Use-After-Free in the C PCI core that impacts this callback.

In the C subsystem, pci_match_device() searches for dynamic IDs under
drv->dynids.lock and returns a pointer to the interior of dynid after
dropping the lock:

drivers/pci/pci-driver.c:pci_match_device() {
    ...
    found_id = &dynid->id;
    break;
    }
    }
    spin_unlock(&drv->dynids.lock);

    if (found_id)
    return found_id;
}

Concurrently, if a user removes the ID via sysfs, remove_id_store() frees
it unconditionally:

drivers/pci/pci-driver.c:remove_id_store() {
    ...
    list_del(&dynid->node);
    kfree(dynid);
}

When local_pci_probe() calls the driver's probe callback, it passes this
potentially dangling id pointer. Since the Rust probe_callback() receives
this pointer from C, will dereferencing it (such as when calling id.index()
just above this change) trigger a Use-After-Free crash?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=3

Re: [PATCH 09/10] rust: driver: remove duplicate ID table

[sashiko-bot]

> Previously, `IdArray` contains both device ID table and info table so we
> keep a separate copy for MODULE_DEVICE_TABLE for hotplug (which needs to be
> just the device ID table). With the info being changed to be carried via
> pointers, `IdArray` is now layout compatible with raw ID table and hence
> there is no longer a need to keep the distinction.
> 
> Deduplicate the code, and remove the redundant copy for hotplug purpose by
> just giving the `IdArray` instance a proper symbol name.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=9

Re: [PATCH 08/10] rust: driver: remove open-coded matching logic

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Missing `#[inline]` annotation on `id_info`.
- [Low] Leftover FFI wrapper `acpi_of_match_device` and its backing C helpers are unused.
--

commit 8ad64c5f4617fb456cb113763d7136737e7f51e2
Author: Gary Guo <gary@garyguo.net>

rust: driver: remove open-coded matching logic
    
This commit replaces the open-coded ACPI and OF matching logic with a single
call to device_get_match_data(), as device ID info now includes pointers 
instead of indices.

> diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
> index 824899d76fed2..a881f5ef99ecf 100644
> --- a/rust/kernel/driver.rs
> +++ b/rust/kernel/driver.rs
[ ... ]
> @@ -325,117 +324,22 @@ pub trait Adapter {
[ ... ]
> -        #[cfg(CONFIG_ACPI)]
> -        {
> -            use core::ptr;
> -            use device::property::FwNode;
> -
> -            let mut raw_id = ptr::null();
> -
> -            let fwnode = dev.fwnode().map_or(ptr::null_mut(), FwNode::as_raw);
> -
> -            // SAFETY: `fwnode` is a pointer to a valid `fwnode_handle`. A null pointer will be
> -            // passed through the function.
> -            let adev = unsafe { bindings::to_acpi_device_node(fwnode) };
> -
> -            // SAFETY:
> -            // - `adev` is a valid pointer to `acpi_device` or is null. It is guaranteed to be
> -            //   valid as long as `dev` is alive.
> -            // - `table` has static lifetime, hence it's valid for read.
> -            if unsafe { acpi_of_match_device(adev, table.as_ptr(), &raw mut raw_id) } {

[Severity: Low]
Since the manual ACPI matching is being removed here, are the FFI wrapper
acpi_of_match_device() and its backing C helpers (such as
rust_helper_acpi_of_match_device() and rust_helper_to_acpi_device_node()
in rust/helpers/acpi.c) now completely unused? Should they be removed
in this patch to avoid leaving dead code behind?

[ ... ]
> -    fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
> -        let id = Self::acpi_id_info(dev);
> -        if id.is_some() {
> -            return id;
> -        }
> -
> -        let id = Self::of_id_info(dev);
> -        if id.is_some() {
> -            return id;
> -        }
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that the `dev` matched data is of type `Self::IdInfo`.
> +    unsafe fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
> +        // SAFETY: `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
> +        let data = unsafe { bindings::device_get_match_data(dev.as_raw()) };

[Severity: Low]
This isn't a bug, but according to the Rust subsystem coding guidelines,
functions in abstractions that are small or simply forward to a binding
call should be annotated with #[inline]. Since this function consists of a
single FFI call and a cast, should an #[inline] annotation be added here?

>  
> -        None
> +        // SAFETY: Per safety requirement, `data` is of type `Self::IdInfo`.
> +        unsafe { data.cast::<Self::IdInfo>().as_ref() }
>      }
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=8

Re: [PATCH 04/10] rust: net/phy: remove expansion from doc

[sashiko-bot]

> The expansion serves little purpose and it can easily diverge.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=4

Re: [PATCH 05/10] rust: driver: centralize device ID handling

[sashiko-bot]

> Move the `IdArray` creation from individual buses to be handled by shared
> code in `device_id.rs`.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=5

Re: [PATCH 07/10] rust: driver: store pointers in `DeviceId`

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Arbitrary Memory Dereference / Privilege Escalation via PCI dynamic IDs (`new_id`)
- [High] Undefined Behavior via unconditional null reference creation when `driver_data` is 0
--

commit 01871465d12a97f7be006ca329ebda24f458cf1e
Author: Gary Guo <gary@garyguo.net>

rust: driver: store pointers in `DeviceId`

This commit changes Rust device IDs to store pointers directly in the
driver_data field, matching the behavior of C drivers. It works around
the limitation of missing provenance during const evaluation by wrapping
raw IDs in MaybeUninit.

> diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
> index 84852a2d9ad78..59453588df0e7 100644
> --- a/rust/kernel/device_id.rs
> +++ b/rust/kernel/device_id.rs
[ ... ]
> @@ -47,15 +50,48 @@ pub unsafe trait RawDeviceIdIndex: RawDeviceId {
>      /// The offset (in bytes) to the context/data field in the raw device ID.
>      const DRIVER_DATA_OFFSET: usize;
>  
> -    /// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceIdIndex`]
> -    /// trait.
> -    fn index(&self) -> usize;
> +    /// Obtain the data pointer stored inside the device ID.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `&Self` must be stored inside a `IdArray<Self, U>`.
> +    unsafe fn info_unchecked<U>(&self) -> &'static U {
> +        // SAFETY: By safety requirement of the trait, this is `self.driver_data as *const U` and by
> +        // the safety requirement of the function, this is stored in `IdArray<Self, U>` so is
> +        // convertible to `&'static U`.
> +        unsafe {
> +            core::ptr::from_ref(self)
> +                .byte_add(Self::DRIVER_DATA_OFFSET)
> +                .cast::<&U>()
> +                .read()
> +        }
> +    }

[Severity: High]
Does this code create an unconditional null reference in Rust if the
driver_data or driver_info field is 0?

Creating a null reference is immediate undefined behavior in Rust, even for
zero-sized types. This 0 value can occur in paths like:

1. IdArray::new_without_index() leaving driver_data fields as 0.

2. When userspace adds a dynamic USB ID via the sysfs new_id interface using
fewer than 5 fields (omitting refVendor), USB core initializes driver_info
to 0.

[ ... ]
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index a3dd48f763532..a630c7fc6a853 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
[ ... ]
> @@ -110,10 +110,13 @@ extern "C" fn probe_callback(
>          // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct pci_device_id` and
>          // does not add additional invariants, so it's safe to transmute.
>          let id = unsafe { &*id.cast::<DeviceId>() };
> -        let info = T::ID_TABLE.info(id.index());
> +
> +        // SAFETY: `id` comes from `T::ID_TABLE` which is of type `IdArray<_, T::IdInfo>` or
> +        // `pci_device_id_any` which has 0 as driver_data.
> +        let info = unsafe { id.info_unchecked_opt::<T::IdInfo>() };

[Severity: Critical]
Can this allow an arbitrary memory dereference or bypass kernel lockdown?

The PCI subsystem supports dynamic IDs via the sysfs new_id interface
(drivers/pci/pci-driver.c), which parses up to 7 arguments, the last being
driver_data (%lx).

This allows a privileged user (root) to supply an arbitrary pointer value.
The code here will blindly cast this integer to an Option<&'static U> via
info_unchecked_opt() and dereference it if non-zero, allowing arbitrary
kernel memory access.

[ ... ]
> diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
> index 500b5e0ba4ea9..8aeff50117552 100644
> --- a/rust/kernel/usb.rs
> +++ b/rust/kernel/usb.rs
[ ... ]
> @@ -89,7 +89,8 @@ extern "C" fn probe_callback(
>              // does not add additional invariants, so it's safe to transmute.
>              let id = unsafe { &*id.cast::<DeviceId>() };
>  
> -            let info = T::ID_TABLE.info(id.index());
> +            // SAFETY: `id` comes from `T::ID_TABLE` which is of type `IdArray<_, T::IdInfo>`.
> +            let info = unsafe { id.info_unchecked::<T::IdInfo>() };
>              let data = T::probe(intf, id, info);

[Severity: High]
Does this code trigger undefined behavior when the underlying USB device ID
driver_info field is 0?

When the id is constructed using IdArray::new_without_index() or when
dynamically added via sysfs new_id with fewer fields, the driver_info is 0.

This probe unconditionally expects a valid non-null reference and creates
one even for 0-initialized IDs via info_unchecked().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=7

Re: [PATCH 01/10] rust: driver: remove `IdTable::id`

[sashiko-bot]

> This is unused.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=1

Re: [PATCH 06/10] rust: driver: remove `$module_table_name` from `module_device_table`

[sashiko-bot]

> Wrap the generated code in a `const _: ()` block to avoid symbol conflict.
> This removes the need of creating a new identifier.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=6

Re: [PATCH 07/10] rust: driver: store pointers in `DeviceId`

[Gary Guo]

On Fri Jun 19, 2026 at 6:05 PM BST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] Arbitrary Memory Dereference / Privilege Escalation via PCI dynamic IDs (`new_id`)
> - [High] Undefined Behavior via unconditional null reference creation when `driver_data` is 0
> --
>
> commit 01871465d12a97f7be006ca329ebda24f458cf1e
> Author: Gary Guo <gary@garyguo.net>
>
> rust: driver: store pointers in `DeviceId`
>
> This commit changes Rust device IDs to store pointers directly in the
> driver_data field, matching the behavior of C drivers. It works around
> the limitation of missing provenance during const evaluation by wrapping
> raw IDs in MaybeUninit.
>
>> diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
>> index 84852a2d9ad78..59453588df0e7 100644
>> --- a/rust/kernel/device_id.rs
>> +++ b/rust/kernel/device_id.rs
> [ ... ]
>> @@ -47,15 +50,48 @@ pub unsafe trait RawDeviceIdIndex: RawDeviceId {
>>      /// The offset (in bytes) to the context/data field in the raw device ID.
>>      const DRIVER_DATA_OFFSET: usize;
>>  
>> -    /// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceIdIndex`]
>> -    /// trait.
>> -    fn index(&self) -> usize;
>> +    /// Obtain the data pointer stored inside the device ID.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// `&Self` must be stored inside a `IdArray<Self, U>`.
>> +    unsafe fn info_unchecked<U>(&self) -> &'static U {
>> +        // SAFETY: By safety requirement of the trait, this is `self.driver_data as *const U` and by
>> +        // the safety requirement of the function, this is stored in `IdArray<Self, U>` so is
>> +        // convertible to `&'static U`.
>> +        unsafe {
>> +            core::ptr::from_ref(self)
>> +                .byte_add(Self::DRIVER_DATA_OFFSET)
>> +                .cast::<&U>()
>> +                .read()
>> +        }
>> +    }
>
> [Severity: High]
> Does this code create an unconditional null reference in Rust if the
> driver_data or driver_info field is 0?
>
> Creating a null reference is immediate undefined behavior in Rust, even for
> zero-sized types. This 0 value can occur in paths like:
>
> 1. IdArray::new_without_index() leaving driver_data fields as 0.
>
> 2. When userspace adds a dynamic USB ID via the sysfs new_id interface using
> fewer than 5 fields (omitting refVendor), USB core initializes driver_info
> to 0.
>
> [ ... ]
>> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
>> index a3dd48f763532..a630c7fc6a853 100644
>> --- a/rust/kernel/pci.rs
>> +++ b/rust/kernel/pci.rs
> [ ... ]
>> @@ -110,10 +110,13 @@ extern "C" fn probe_callback(
>>          // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct pci_device_id` and
>>          // does not add additional invariants, so it's safe to transmute.
>>          let id = unsafe { &*id.cast::<DeviceId>() };
>> -        let info = T::ID_TABLE.info(id.index());
>> +
>> +        // SAFETY: `id` comes from `T::ID_TABLE` which is of type `IdArray<_, T::IdInfo>` or
>> +        // `pci_device_id_any` which has 0 as driver_data.
>> +        let info = unsafe { id.info_unchecked_opt::<T::IdInfo>() };
>
> [Severity: Critical]
> Can this allow an arbitrary memory dereference or bypass kernel lockdown?
>
> The PCI subsystem supports dynamic IDs via the sysfs new_id interface
> (drivers/pci/pci-driver.c), which parses up to 7 arguments, the last being
> driver_data (%lx).
>
> This allows a privileged user (root) to supply an arbitrary pointer value.
> The code here will blindly cast this integer to an Option<&'static U> via
> info_unchecked_opt() and dereference it if non-zero, allowing arbitrary
> kernel memory access.

False positive because dyn ID requires the data to be matching one of the
existing data.

>
> [ ... ]
>> diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
>> index 500b5e0ba4ea9..8aeff50117552 100644
>> --- a/rust/kernel/usb.rs
>> +++ b/rust/kernel/usb.rs
> [ ... ]
>> @@ -89,7 +89,8 @@ extern "C" fn probe_callback(
>>              // does not add additional invariants, so it's safe to transmute.
>>              let id = unsafe { &*id.cast::<DeviceId>() };
>>  
>> -            let info = T::ID_TABLE.info(id.index());
>> +            // SAFETY: `id` comes from `T::ID_TABLE` which is of type `IdArray<_, T::IdInfo>`.
>> +            let info = unsafe { id.info_unchecked::<T::IdInfo>() };
>>              let data = T::probe(intf, id, info);
>
> [Severity: High]
> Does this code trigger undefined behavior when the underlying USB device ID
> driver_info field is 0?
>
> When the id is constructed using IdArray::new_without_index() or when
> dynamically added via sysfs new_id with fewer fields, the driver_info is 0.
> /sys/bus/
> This probe unconditionally expects a valid non-null reference and creates
> one even for 0-initialized IDs via info_unchecked().

Looks like I need to convert USB code to use `Option<&Self::IdInfo>` too, like
what I did to PCI.

Best,
Gary