`acpi::DeviceId` is an abstraction around `struct acpi_device_id`.
This is used by subsequent patches, in particular the i2c driver
abstractions, to create ACPI device ID tables.
Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
MAINTAINERS | 1 +
rust/kernel/acpi.rs | 60 +++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
3 files changed, 62 insertions(+)
create mode 100644 rust/kernel/acpi.rs
diff --git a/MAINTAINERS b/MAINTAINERS
index 1e918319cff4..3e59a177ac0c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -302,6 +302,7 @@ F: include/linux/acpi.h
F: include/linux/fwnode.h
F: include/linux/fw_table.h
F: lib/fw_table.c
+F: rust/kernel/acpi.rs
F: tools/power/acpi/
ACPI APEI
diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
new file mode 100644
index 000000000000..2b25dc9e07ac
--- /dev/null
+++ b/rust/kernel/acpi.rs
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Advanced Configuration and Power Interface abstractions.
+
+use crate::{bindings, device_id::RawDeviceId, prelude::*};
+
+/// IdTable type for ACPI drivers.
+pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
+
+/// An ACPI device id.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct DeviceId(bindings::acpi_device_id);
+
+// SAFETY:
+// * `DeviceId` is a `#[repr(transparent)` wrapper of `struct acpi_device_id` and does not add
+// additional invariants, so it's safe to transmute to `RawType`.
+// * `DRIVER_DATA_OFFSET` is the offset to the `data` field.
+unsafe impl RawDeviceId for DeviceId {
+ type RawType = bindings::acpi_device_id;
+
+ const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::acpi_device_id, driver_data);
+
+ fn index(&self) -> usize {
+ self.0.driver_data as _
+ }
+}
+
+impl DeviceId {
+ const ACPI_ID_LEN: usize = 16;
+
+ /// Create a new device id from an ACPI 'id' string.
+ pub const fn new<const N: usize>(id: &[u8; N]) -> Self {
+ build_assert!(N <= Self::ACPI_ID_LEN, "ID exceeds 16 bytes");
+ // Replace with `bindings::acpi_device_id::default()` once stabilized for `const`.
+ // SAFETY: FFI type is valid to be zero-initialized.
+ let mut acpi: bindings::acpi_device_id = unsafe { core::mem::zeroed() };
+ let mut i = 0;
+ while i < N {
+ acpi.id[i] = id[i];
+ i += 1;
+ }
+
+ Self(acpi)
+ }
+}
+
+/// 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);
+ };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6b4774b2b1c3..5bbf3627212f 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -51,6 +51,7 @@
pub use ffi;
+pub mod acpi;
pub mod alloc;
#[cfg(CONFIG_AUXILIARY_BUS)]
pub mod auxiliary;
--
2.43.0On 6/20/25 5:24 PM, Igor Korotin wrote:
> +impl DeviceId {
> + const ACPI_ID_LEN: usize = 16;
> +
> + /// Create a new device id from an ACPI 'id' string.
> + pub const fn new<const N: usize>(id: &[u8; N]) -> Self {
Didn't notice before, but why was this silently changed from &CStr to &[u8; N]
from v6 to v7?
> + build_assert!(N <= Self::ACPI_ID_LEN, "ID exceeds 16 bytes");
> + // Replace with `bindings::acpi_device_id::default()` once stabilized for `const`.
> + // SAFETY: FFI type is valid to be zero-initialized.
> + let mut acpi: bindings::acpi_device_id = unsafe { core::mem::zeroed() };
> + let mut i = 0;
> + while i < N {
> + acpi.id[i] = id[i];
> + i += 1;
> + }
> +
> + Self(acpi)
> + }
> +}
On 6/26/25 16:25, Danilo Krummrich wrote:
> On 6/20/25 5:24 PM, Igor Korotin wrote:
>> +impl DeviceId {
>> + const ACPI_ID_LEN: usize = 16;
>> +
>> + /// Create a new device id from an ACPI 'id' string.
>> + pub const fn new<const N: usize>(id: &[u8; N]) -> Self {
>
> Didn't notice before, but why was this silently changed from &CStr to
> &[u8; N]
> from v6 to v7?
>
>> + build_assert!(N <= Self::ACPI_ID_LEN, "ID exceeds 16 bytes");
>> + // Replace with `bindings::acpi_device_id::default()` once
>> stabilized for `const`.
>> + // SAFETY: FFI type is valid to be zero-initialized.
>> + let mut acpi: bindings::acpi_device_id = unsafe
>> { core::mem::zeroed() };
>> + let mut i = 0;
>> + while i < N {
>> + acpi.id[i] = id[i];
>> + i += 1;
>> + }
>> +
>> + Self(acpi)
>> + }
>> +}
In v6 I was asked to change assert! (runtime) to build_assert! (build time)
It was as follows:
> + pub const fn new(id: &'static CStr) -> Self {
> + assert!(id.len() <= Self::ACPI_ID_LEN, "ID exceeds 16 bytes");
but id.len() breaks const context and so build_assert! triggers
assertion. If I needed to explicitly describe change from CStr to
[u8;20], then it's my bad.
Thanks,
Igor
On Thu, Jun 26, 2025 at 06:40:06PM +0100, Igor Korotin wrote:
>
>
> On 6/26/25 16:25, Danilo Krummrich wrote:
> > On 6/20/25 5:24 PM, Igor Korotin wrote:
> >> +impl DeviceId {
> >> + const ACPI_ID_LEN: usize = 16;
> >> +
> >> + /// Create a new device id from an ACPI 'id' string.
> >> + pub const fn new<const N: usize>(id: &[u8; N]) -> Self {
> >
> > Didn't notice before, but why was this silently changed from &CStr to
> > &[u8; N]
> > from v6 to v7?
> >
> >> + build_assert!(N <= Self::ACPI_ID_LEN, "ID exceeds 16 bytes");
> >> + // Replace with `bindings::acpi_device_id::default()` once
> >> stabilized for `const`.
> >> + // SAFETY: FFI type is valid to be zero-initialized.
> >> + let mut acpi: bindings::acpi_device_id = unsafe
> >> { core::mem::zeroed() };
> >> + let mut i = 0;
> >> + while i < N {
> >> + acpi.id[i] = id[i];
> >> + i += 1;
> >> + }
> >> +
> >> + Self(acpi)
> >> + }
> >> +}
>
> In v6 I was asked to change assert! (runtime) to build_assert! (build time)
> It was as follows:
>
> > + pub const fn new(id: &'static CStr) -> Self {
> > + assert!(id.len() <= Self::ACPI_ID_LEN, "ID exceeds 16 bytes");
>
> but id.len() breaks const context and so build_assert! triggers
> assertion.
It does indeed, but I'm not sure why it does...
> If I needed to explicitly describe change from CStr to
> [u8;20], then it's my bad.
Yes, that's usually better. Otherwise reviewers might skip the changed part. In
this case I think it actually introduced a bug:
Checking for N <= Self::ACPI_ID_LEN it can happen that acpi_device_id::id is not
NULL terminated anymore, whereas before this was ensured by
CStr::as_bytes_with_nul(). See also [1].
I think we can easily fix this by just checking N < Self::ACPI_ID_LEN.
However, I'd still like to check why build_assert!() does not work with
CStr::len() in this case first.
[1] https://elixir.bootlin.com/linux/v6.15.3/source/drivers/acpi/bus.c#L899
On Fri, Jun 20, 2025 at 04:24:25PM +0100, Igor Korotin wrote: > `acpi::DeviceId` is an abstraction around `struct acpi_device_id`. > > This is used by subsequent patches, in particular the i2c driver > abstractions, to create ACPI device ID tables. I think this should say something like "Enable drivers to build ACPI device ID tables, to be consumed by the corresponding bus abstractions, such as platform or I2C." instead. If we agree, I can change it when applying the patch -- no need to resend. > > Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com> @Rafael: Can I get an ACK for this one, such that I can take it together with all other patches through the driver-core tree?
On Sat, Jun 21, 2025 at 12:06 AM Danilo Krummrich <dakr@kernel.org> wrote: > > On Fri, Jun 20, 2025 at 04:24:25PM +0100, Igor Korotin wrote: > > `acpi::DeviceId` is an abstraction around `struct acpi_device_id`. > > > > This is used by subsequent patches, in particular the i2c driver > > abstractions, to create ACPI device ID tables. > > I think this should say something like > > "Enable drivers to build ACPI device ID tables, to be consumed by the > corresponding bus abstractions, such as platform or I2C." > > instead. > > If we agree, I can change it when applying the patch -- no need to resend. > > > > > Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com> > > @Rafael: Can I get an ACK for this one, such that I can take it together with > all other patches through the driver-core tree? I don't see anything objectionable in it, even though Rust code is still kind of outside my confidence zone ATM. Anyway, please feel free to add Acked-by: Rafael J. Wysocki <rafael@kernel.org> to this. Thanks!
On 6/20/25 23:06, Danilo Krummrich wrote: > On Fri, Jun 20, 2025 at 04:24:25PM +0100, Igor Korotin wrote: >> `acpi::DeviceId` is an abstraction around `struct acpi_device_id`. >> >> This is used by subsequent patches, in particular the i2c driver >> abstractions, to create ACPI device ID tables. > > I think this should say something like > > "Enable drivers to build ACPI device ID tables, to be consumed by the > corresponding bus abstractions, such as platform or I2C." > > instead. > > If we agree, I can change it when applying the patch -- no need to resend. > No objections. Thanks Igor
© 2016 - 2025 Red Hat, Inc.