| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <fd9f5a0e-b2d4-4b72-9f34-9d8fcc74c00c@de.bosch.com>
Date: Tue, 29 Oct 2024 08:20:55 +0100
From: Dirk Behme <dirk.behme@...bosch.com>
To: Danilo Krummrich <dakr@...nel.org>
CC: <gregkh@...uxfoundation.org>, <rafael@...nel.org>, <bhelgaas@...gle.com>,
<ojeda@...nel.org>, <alex.gaynor@...il.com>, <boqun.feng@...il.com>,
<gary@...yguo.net>, <bjorn3_gh@...tonmail.com>, <benno.lossin@...ton.me>,
<tmgross@...ch.edu>, <a.hindborg@...sung.com>, <aliceryhl@...gle.com>,
<airlied@...il.com>, <fujita.tomonori@...il.com>, <lina@...hilina.net>,
<pstanner@...hat.com>, <ajanulgu@...hat.com>, <lyude@...hat.com>,
<robh@...nel.org>, <daniel.almeida@...labora.com>, <saravanak@...gle.com>,
<rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-pci@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v3 15/16] rust: platform: add basic platform device /
driver abstractions
On 28.10.2024 11:19, Danilo Krummrich wrote:
> On Thu, Oct 24, 2024 at 11:11:50AM +0200, Dirk Behme wrote:
>>> +/// IdTable type for platform drivers.
>>> +pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<of::DeviceId, T>;
>>> +
>>> +/// The platform driver trait.
>>> +///
>>> +/// # Example
>>> +///
>>> +///```
>>> +/// # use kernel::{bindings, c_str, of, platform};
>>> +///
>>> +/// struct MyDriver;
>>> +///
>>> +/// kernel::of_device_table!(
>>> +/// OF_TABLE,
>>> +/// MODULE_OF_TABLE,
>>
>> It looks to me that OF_TABLE and MODULE_OF_TABLE are quite generic names
>> used here. Shouldn't they be somehow driver specific, e.g. OF_TABLE_MYDRIVER
>> and MODULE_OF_TABLE_MYDRIVER or whatever? Same for the other
>> examples/samples in this patch series. Found that while using the *same*
>> somewhere else ;)
>
> I think the names by themselves are fine. They're local to the module. However,
> we stringify `OF_TABLE` in `module_device_table` to build the export name, i.e.
> "__mod_of__OF_TABLE_device_table". Hence the potential duplicate symbols.
>
> I think we somehow need to build the module name into the symbol name as well.
Something like this?
Subject: [PATCH] rust: device: Add the module name to the symbol name
Make the symbol name unique by adding the module name to avoid
duplicate symbol errors like
ld.lld: error: duplicate symbol: __mod_of__OF_TABLE_device_table
>>> defined at doctests_kernel_generated.ff18649a828ae8c4-cgu.0
>>>
rust/doctests_kernel_generated.o:(__mod_of__OF_TABLE_device_table) in
archive vmlinux.a
>>> defined at rust_driver_platform.2308c4225c4e08b3-cgu.0
>>> samples/rust/rust_driver_platform.o:(.rodata+0x5A8) in
archive vmlinux.a
make[2]: *** [scripts/Makefile.vmlinux_o:65: vmlinux.o] Error 1
make[1]: *** [Makefile:1154: vmlinux_o] Error 2
__mod_of__OF_TABLE_device_table is too generic. Add the module name.
Proposed-by: Danilo Krummrich <dakr@...nel.org>
Link: https://lore.kernel.org/rust-for-linux/Zx9lFG1XKnC_WaG0@pollux/
Signed-off-by: Dirk Behme <dirk.behme@...bosch.com>
---
rust/kernel/device_id.rs | 4 ++--
rust/kernel/of.rs | 4 ++--
rust/kernel/pci.rs | 5 +++--
rust/kernel/platform.rs | 1 +
samples/rust/rust_driver_pci.rs | 1 +
samples/rust/rust_driver_platform.rs | 1 +
6 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 5b1329fba528..231f34362da9 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -151,10 +151,10 @@ 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_name: literal,
$module_table_name:ident, $table_name:ident) => {
#[rustfmt::skip]
#[export_name =
- concat!("__mod_", $table_type, "__",
stringify!($table_name), "_device_table")
+ concat!("__mod_", $table_type, "__",
stringify!($table_name), "_", $device_name, "_device_table")
]
static $module_table_name: [core::mem::MaybeUninit<u8>;
$table_name.raw_ids().size()] =
unsafe { core::mem::transmute_copy($table_name.raw_ids()) };
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index a37629997974..77679c30638c 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -51,13 +51,13 @@ pub fn compatible<'a>(&self) -> &'a CStr {
/// 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) => {
+ ($device_name: literal, $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);
+ $crate::module_device_table!("of", $device_name,
$module_table_name, $table_name);
};
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 58f7d9c0045b..806d192b9600 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -176,14 +176,14 @@ 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) => {
+ ($device_name: literal, $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);
+ $crate::module_device_table!("pci", $device_name,
$module_table_name, $table_name);
};
}
@@ -197,6 +197,7 @@ macro_rules! pci_device_table {
/// struct MyDriver;
///
/// kernel::pci_device_table!(
+/// "MyDriver",
/// PCI_TABLE,
/// MODULE_PCI_TABLE,
/// <MyDriver as pci::Driver>::IdInfo,
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index a926233a789f..fcdd3c5da0e5 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -118,6 +118,7 @@ macro_rules! module_platform_driver {
/// struct MyDriver;
///
/// kernel::of_device_table!(
+/// "MyDriver",
/// OF_TABLE,
/// MODULE_OF_TABLE,
/// <MyDriver as platform::Driver>::IdInfo,
diff --git a/samples/rust/rust_driver_pci.rs
b/samples/rust/rust_driver_pci.rs
index d24dc1fde9e8..6ee570b59233 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -31,6 +31,7 @@ struct SampleDriver {
}
kernel::pci_device_table!(
+ "SampleDriver",
PCI_TABLE,
MODULE_PCI_TABLE,
<SampleDriver as pci::Driver>::IdInfo,
diff --git a/samples/rust/rust_driver_platform.rs
b/samples/rust/rust_driver_platform.rs
index fd7a5ad669fe..9dfbe3b9932b 100644
--- a/samples/rust/rust_driver_platform.rs
+++ b/samples/rust/rust_driver_platform.rs
@@ -11,6 +11,7 @@ struct SampleDriver {
struct Info(u32);
kernel::of_device_table!(
+ "SampleDriver",
OF_TABLE,
MODULE_OF_TABLE,
<SampleDriver as platform::Driver>::IdInfo,
--
2.46.2
Powered by blists - more mailing lists