From 1d0cf0d27a463f004b95bf88da04540786c75d9d Mon Sep 17 00:00:00 2001 From: Finn Behrens Date: Wed, 28 Apr 2021 15:28:45 +0200 Subject: [PATCH 1/2] make module! arguments optional Signed-off-by: Finn Behrens --- rust/module.rs | 479 +++++++++++++++++++++--------------- samples/rust/rust_chrdev.rs | 2 - 2 files changed, 287 insertions(+), 194 deletions(-) diff --git a/rust/module.rs b/rust/module.rs index bddb90d4822d42..73adeb847e93ee 100644 --- a/rust/module.rs +++ b/rust/module.rs @@ -103,14 +103,6 @@ fn expect_end(it: &mut token_stream::IntoIter) { } } -fn get_ident(it: &mut token_stream::IntoIter, expected_name: &str) -> String { - assert_eq!(expect_ident(it), expected_name); - assert_eq!(expect_punct(it), ':'); - let ident = expect_ident(it); - assert_eq!(expect_punct(it), ','); - ident -} - fn get_literal(it: &mut token_stream::IntoIter, expected_name: &str) -> String { assert_eq!(expect_ident(it), expected_name); assert_eq!(expect_punct(it), ':'); @@ -119,14 +111,6 @@ fn get_literal(it: &mut token_stream::IntoIter, expected_name: &str) -> String { literal } -fn get_group(it: &mut token_stream::IntoIter, expected_name: &str) -> Group { - assert_eq!(expect_ident(it), expected_name); - assert_eq!(expect_punct(it), ':'); - let group = expect_group(it); - assert_eq!(expect_punct(it), ','); - group -} - fn get_byte_string(it: &mut token_stream::IntoIter, expected_name: &str) -> String { assert_eq!(expect_ident(it), expected_name); assert_eq!(expect_punct(it), ':'); @@ -202,6 +186,14 @@ fn build_modinfo_string(module: &str, field: &str, content: &str) -> String { + &build_modinfo_string_only_loadable(module, field, content) } +fn build_modinfo_string_optional(module: &str, field: &str, content: Option<&str>) -> String { + if let Some(content) = content { + build_modinfo_string(module, field, content) + } else { + "".to_string() + } +} + fn build_modinfo_string_param(module: &str, field: &str, param: &str, content: &str) -> String { let variable = format!( "__{module}_{field}_{param}", @@ -315,6 +307,97 @@ fn generated_array_ops_name(vals: &str, max_length: usize) -> String { ) } +#[derive(Debug, Default)] +struct ModuleInfo { + type_: String, + license: String, + name: String, + author: Option, + description: Option, + alias: Option, + params: Option, +} + +impl ModuleInfo { + fn parse(it: &mut token_stream::IntoIter) -> Self { + let mut info = ModuleInfo::default(); + // The first allowed position is 1, to prevent setting something twice. + let mut pos = 0u8; + + loop { + let name = match it.next() { + Some(TokenTree::Ident(ident)) => ident.to_string(), + Some(_) => panic!("Expected Ident or end"), + None => break, + }; + + assert_eq!(expect_punct(it), ':'); + + if name == "type" { + Self::allowed_pos(&mut pos, 1, &name); + let type_ = expect_ident(it); + info.type_ = type_; + } else if name == "params" { + Self::allowed_pos(&mut pos, 7, &name); + let params = expect_group(it); + info.params = Some(params); + } else { + let value = expect_byte_string(it); + + match name.as_str() { + "name" => { + Self::allowed_pos(&mut pos, 2, &name); + info.name = value; + } + "author" => { + Self::allowed_pos(&mut pos, 3, &name); + info.author = Some(value); + } + "description" => { + Self::allowed_pos(&mut pos, 4, &name); + info.description = Some(value); + } + "license" => { + Self::allowed_pos(&mut pos, 5, &name); + info.license = value; + } + "alias" => { + Self::allowed_pos(&mut pos, 6, &name); + info.alias = Some(value); + } + "alias_rtnl_link" => { + Self::allowed_pos(&mut pos, 6, &name); + info.alias = Some(format!("rtnl-link-{}", value)); + } + _ => panic!("field '{}' is not supported by module", name), + } + } + assert_eq!(expect_punct(it), ','); + } + + expect_end(it); + + if info.type_.is_empty() { + panic!("'type' not specified in module macro"); + } + if info.license.is_empty() { + panic!("'license' not specified in module macro"); + } + if info.name.is_empty() { + panic!("'name' not specified in module macro"); + } + + info + } + + fn allowed_pos(pos: &mut u8, allowed_pos: u8, name: &str) { + if *pos > allowed_pos { + panic!("'{}' is not allowed at this position", name); + } + *pos = allowed_pos; + } +} + /// Declares a kernel module. /// /// The `type` argument should be a type which implements the [`KernelModule`] @@ -365,6 +448,16 @@ fn generated_array_ops_name(vals: &str, max_length: usize) -> String { /// } /// ``` /// +/// # Supported argument types +/// - `type`: type which implements the [`KernelModule`] trait (required). +/// - `name`: byte array of the name of the kernel module (required). +/// - `author`: byte array of the author of the kernel module. +/// - `description`: byte array of the description of the kernel module. +/// - `license`: byte array of the license of the kernel module (required). +/// - `alias`: byte array of alias name of the kernel module. +/// - `alias_rtnl_link`: byte array of the `rtnl_link_alias` of the kernel module (mutually exclusive with `alias`). +/// - `params`: parameters for the kernel module, as described below. +/// /// # Supported parameter types /// /// - `bool`: Corresponds to C `bool` param type. @@ -388,178 +481,173 @@ fn generated_array_ops_name(vals: &str, max_length: usize) -> String { pub fn module(ts: TokenStream) -> TokenStream { let mut it = ts.into_iter(); - let type_ = get_ident(&mut it, "type"); - let name = get_byte_string(&mut it, "name"); - let author = get_byte_string(&mut it, "author"); - let description = get_byte_string(&mut it, "description"); - let license = get_byte_string(&mut it, "license"); - let params = get_group(&mut it, "params"); - - expect_end(&mut it); - - assert_eq!(params.delimiter(), Delimiter::Brace); + let info = ModuleInfo::parse(&mut it); - let mut it = params.stream().into_iter(); - - let mut params_modinfo = String::new(); + let name = info.name.clone(); let mut array_types_to_generate = Vec::new(); - - loop { - let param_name = match it.next() { - Some(TokenTree::Ident(ident)) => ident.to_string(), - Some(_) => panic!("Expected Ident or end"), - None => break, - }; - - assert_eq!(expect_punct(&mut it), ':'); - let param_type = expect_type(&mut it); - let group = expect_group(&mut it); - assert_eq!(expect_punct(&mut it), ','); - - assert_eq!(group.delimiter(), Delimiter::Brace); - - let mut param_it = group.stream().into_iter(); - let param_default = get_default(¶m_type, &mut param_it); - let param_permissions = get_literal(&mut param_it, "permissions"); - let param_description = get_byte_string(&mut param_it, "description"); - expect_end(&mut param_it); - - // TODO: more primitive types - // TODO: other kinds: unsafes, etc. - let (param_kernel_type, ops): (String, _) = match param_type { - ParamType::Ident(ref param_type) => ( - param_type.to_string(), - param_ops_path(¶m_type).to_string(), - ), - ParamType::Array { - ref vals, - max_length, - } => { - array_types_to_generate.push((vals.clone(), max_length)); - ( - format!("__rust_array_param_{}_{}", vals, max_length), - generated_array_ops_name(vals, max_length), + let mut params_modinfo = String::new(); + if let Some(params) = info.params { + assert_eq!(params.delimiter(), Delimiter::Brace); + + let mut it = params.stream().into_iter(); + + loop { + let param_name = match it.next() { + Some(TokenTree::Ident(ident)) => ident.to_string(), + Some(_) => panic!("Expected Ident or end"), + None => break, + }; + + assert_eq!(expect_punct(&mut it), ':'); + let param_type = expect_type(&mut it); + let group = expect_group(&mut it); + assert_eq!(expect_punct(&mut it), ','); + + assert_eq!(group.delimiter(), Delimiter::Brace); + + let mut param_it = group.stream().into_iter(); + let param_default = get_default(¶m_type, &mut param_it); + let param_permissions = get_literal(&mut param_it, "permissions"); + let param_description = get_byte_string(&mut param_it, "description"); + expect_end(&mut param_it); + + // TODO: more primitive types + // TODO: other kinds: unsafes, etc. + let (param_kernel_type, ops): (String, _) = match param_type { + ParamType::Ident(ref param_type) => ( + param_type.to_string(), + param_ops_path(¶m_type).to_string(), + ), + ParamType::Array { + ref vals, + max_length, + } => { + array_types_to_generate.push((vals.clone(), max_length)); + ( + format!("__rust_array_param_{}_{}", vals, max_length), + generated_array_ops_name(vals, max_length), + ) + } + }; + + params_modinfo.push_str(&build_modinfo_string_param( + &name, + "parmtype", + ¶m_name, + ¶m_kernel_type, + )); + params_modinfo.push_str(&build_modinfo_string_param( + &name, + "parm", + ¶m_name, + ¶m_description, + )); + let param_type_internal = match param_type { + ParamType::Ident(ref param_type) => match param_type.as_ref() { + "str" => "kernel::module_param::StringParam".to_string(), + other => other.to_string(), + }, + ParamType::Array { + ref vals, + max_length, + } => format!( + "kernel::module_param::ArrayParam<{vals}, {max_length}>", + vals = vals, + max_length = max_length + ), + }; + let read_func = if permissions_are_readonly(¶m_permissions) { + format!( + " + fn read(&self) -> &<{param_type_internal} as kernel::module_param::ModuleParam>::Value {{ + // SAFETY: Parameters do not need to be locked because they are read only or sysfs is not enabled. + unsafe {{ <{param_type_internal} as kernel::module_param::ModuleParam>::value(&__{name}_{param_name}_value) }} + }} + ", + name = name, + param_name = param_name, + param_type_internal = param_type_internal, ) - } - }; - - params_modinfo.push_str(&build_modinfo_string_param( - &name, - "parmtype", - ¶m_name, - ¶m_kernel_type, - )); - params_modinfo.push_str(&build_modinfo_string_param( - &name, - "parm", - ¶m_name, - ¶m_description, - )); - let param_type_internal = match param_type { - ParamType::Ident(ref param_type) => match param_type.as_ref() { - "str" => "kernel::module_param::StringParam".to_string(), - other => other.to_string(), - }, - ParamType::Array { - ref vals, - max_length, - } => format!( - "kernel::module_param::ArrayParam<{vals}, {max_length}>", - vals = vals, - max_length = max_length - ), - }; - let read_func = if permissions_are_readonly(¶m_permissions) { - format!( - " - fn read(&self) -> &<{param_type_internal} as kernel::module_param::ModuleParam>::Value {{ - // SAFETY: Parameters do not need to be locked because they are read only or sysfs is not enabled. - unsafe {{ <{param_type_internal} as kernel::module_param::ModuleParam>::value(&__{name}_{param_name}_value) }} - }} - ", - name = name, - param_name = param_name, - param_type_internal = param_type_internal, - ) - } else { - format!( + } else { + format!( + " + fn read<'lck>(&self, lock: &'lck kernel::KParamGuard) -> &'lck <{param_type_internal} as kernel::module_param::ModuleParam>::Value {{ + // SAFETY: Parameters are locked by `KParamGuard`. + unsafe {{ <{param_type_internal} as kernel::module_param::ModuleParam>::value(&__{name}_{param_name}_value) }} + }} + ", + name = name, + param_name = param_name, + param_type_internal = param_type_internal, + ) + }; + let kparam = format!( " - fn read<'lck>(&self, lock: &'lck kernel::KParamGuard) -> &'lck <{param_type_internal} as kernel::module_param::ModuleParam>::Value {{ - // SAFETY: Parameters are locked by `KParamGuard`. - unsafe {{ <{param_type_internal} as kernel::module_param::ModuleParam>::value(&__{name}_{param_name}_value) }} - }} + kernel::bindings::kernel_param__bindgen_ty_1 {{ + arg: unsafe {{ &__{name}_{param_name}_value }} as *const _ as *mut kernel::c_types::c_void, + }}, ", name = name, param_name = param_name, - param_type_internal = param_type_internal, - ) - }; - let kparam = format!( - " - kernel::bindings::kernel_param__bindgen_ty_1 {{ - arg: unsafe {{ &__{name}_{param_name}_value }} as *const _ as *mut kernel::c_types::c_void, - }}, - ", - name = name, - param_name = param_name, - ); - params_modinfo.push_str( - &format!( - " - static mut __{name}_{param_name}_value: {param_type_internal} = {param_default}; - - struct __{name}_{param_name}; + ); + params_modinfo.push_str( + &format!( + " + static mut __{name}_{param_name}_value: {param_type_internal} = {param_default}; - impl __{name}_{param_name} {{ {read_func} }} + struct __{name}_{param_name}; - const {param_name}: __{name}_{param_name} = __{name}_{param_name}; + impl __{name}_{param_name} {{ {read_func} }} - // Note: the C macro that generates the static structs for the `__param` section - // asks for them to be `aligned(sizeof(void *))`. However, that was put in place - // in 2003 in commit 38d5b085d2 (\"[PATCH] Fix over-alignment problem on x86-64\") - // to undo GCC over-alignment of static structs of >32 bytes. It seems that is - // not the case anymore, so we simplify to a transparent representation here - // in the expectation that it is not needed anymore. - // TODO: revisit this to confirm the above comment and remove it if it happened - #[repr(transparent)] - struct __{name}_{param_name}_RacyKernelParam(kernel::bindings::kernel_param); + const {param_name}: __{name}_{param_name} = __{name}_{param_name}; - unsafe impl Sync for __{name}_{param_name}_RacyKernelParam {{ - }} + // Note: the C macro that generates the static structs for the `__param` section + // asks for them to be `aligned(sizeof(void *))`. However, that was put in place + // in 2003 in commit 38d5b085d2 (\"[PATCH] Fix over-alignment problem on x86-64\") + // to undo GCC over-alignment of static structs of >32 bytes. It seems that is + // not the case anymore, so we simplify to a transparent representation here + // in the expectation that it is not needed anymore. + // TODO: revisit this to confirm the above comment and remove it if it happened + #[repr(transparent)] + struct __{name}_{param_name}_RacyKernelParam(kernel::bindings::kernel_param); - #[cfg(not(MODULE))] - const __{name}_{param_name}_name: *const kernel::c_types::c_char = b\"{name}.{param_name}\\0\" as *const _ as *const kernel::c_types::c_char; + unsafe impl Sync for __{name}_{param_name}_RacyKernelParam {{ + }} - #[cfg(MODULE)] - const __{name}_{param_name}_name: *const kernel::c_types::c_char = b\"{param_name}\\0\" as *const _ as *const kernel::c_types::c_char; + #[cfg(not(MODULE))] + const __{name}_{param_name}_name: *const kernel::c_types::c_char = b\"{name}.{param_name}\\0\" as *const _ as *const kernel::c_types::c_char; - #[link_section = \"__param\"] - #[used] - static __{name}_{param_name}_struct: __{name}_{param_name}_RacyKernelParam = __{name}_{param_name}_RacyKernelParam(kernel::bindings::kernel_param {{ - name: __{name}_{param_name}_name, - // SAFETY: `__this_module` is constructed by the kernel at load time and will not be freed until the module is unloaded. #[cfg(MODULE)] - mod_: unsafe {{ &kernel::bindings::__this_module as *const _ as *mut _ }}, - #[cfg(not(MODULE))] - mod_: core::ptr::null_mut(), - ops: unsafe {{ &{ops} }} as *const kernel::bindings::kernel_param_ops, - perm: {permissions}, - level: -1, - flags: 0, - __bindgen_anon_1: {kparam} - }}); - ", - name = name, - param_type_internal = param_type_internal, - read_func = read_func, - param_default = param_default, - param_name = param_name, - ops = ops, - permissions = param_permissions, - kparam = kparam, - ) - ); + const __{name}_{param_name}_name: *const kernel::c_types::c_char = b\"{param_name}\\0\" as *const _ as *const kernel::c_types::c_char; + + #[link_section = \"__param\"] + #[used] + static __{name}_{param_name}_struct: __{name}_{param_name}_RacyKernelParam = __{name}_{param_name}_RacyKernelParam(kernel::bindings::kernel_param {{ + name: __{name}_{param_name}_name, + // SAFETY: `__this_module` is constructed by the kernel at load time and will not be freed until the module is unloaded. + #[cfg(MODULE)] + mod_: unsafe {{ &kernel::bindings::__this_module as *const _ as *mut _ }}, + #[cfg(not(MODULE))] + mod_: core::ptr::null_mut(), + ops: unsafe {{ &{ops} }} as *const kernel::bindings::kernel_param_ops, + perm: {permissions}, + level: -1, + flags: 0, + __bindgen_anon_1: {kparam} + }}); + ", + name = name, + param_type_internal = param_type_internal, + read_func = read_func, + param_default = param_default, + param_name = param_name, + ops = ops, + permissions = param_permissions, + kparam = kparam, + ) + ); + } } let mut generated_array_types = String::new(); @@ -664,6 +752,7 @@ pub fn module(ts: TokenStream) -> TokenStream { {author} {description} {license} + {alias} // Built-in modules also export the `file` modinfo string {file} @@ -672,11 +761,12 @@ pub fn module(ts: TokenStream) -> TokenStream { {generated_array_types} ", - type_ = type_, - name = name, - author = &build_modinfo_string(&name, "author", &author), - description = &build_modinfo_string(&name, "description", &description), - license = &build_modinfo_string(&name, "license", &license), + type_ = info.type_, + name = info.name, + author = &build_modinfo_string_optional(&name, "author", info.author.as_deref()), + description = &build_modinfo_string_optional(&name, "description", info.description.as_deref()), + license = &build_modinfo_string(&name, "license", &info.license), + alias = &build_modinfo_string_optional(&name, "alias", info.alias.as_deref()), file = &build_modinfo_string_only_builtin(&name, "file", &file), params_modinfo = params_modinfo, generated_array_types = generated_array_types, @@ -715,14 +805,9 @@ pub fn module(ts: TokenStream) -> TokenStream { pub fn module_misc_device(ts: TokenStream) -> TokenStream { let mut it = ts.into_iter(); - let type_ = get_ident(&mut it, "type"); - let name = get_byte_string(&mut it, "name"); - let author = get_byte_string(&mut it, "author"); - let description = get_byte_string(&mut it, "description"); - let license = get_byte_string(&mut it, "license"); - expect_end(&mut it); + let info = ModuleInfo::parse(&mut it); - let module = format!("__internal_ModuleFor{}", type_); + let module = format!("__internal_ModuleFor{}", info.type_); format!( " @@ -746,18 +831,28 @@ pub fn module_misc_device(ts: TokenStream) -> TokenStream { kernel::prelude::module! {{ type: {module}, name: b\"{name}\", - author: b\"{author}\", - description: b\"{description}\", + {author} + {description} license: b\"{license}\", - params: {{}}, + {alias} }} ", module = module, - type_ = type_, - name = name, - author = author, - description = description, - license = license + type_ = info.type_, + name = info.name, + author = info + .author + .map(|v| format!("author: b\"{}\",", v)) + .unwrap_or_else(|| "".to_string()), + description = info + .description + .map(|v| format!("description: b\"{}\",", v)) + .unwrap_or_else(|| "".to_string()), + alias = info + .alias + .map(|v| format!("alias: b\"{}\",", v)) + .unwrap_or_else(|| "".to_string()), + license = info.license ) .parse() .expect("Error parsing formatted string into token stream.") diff --git a/samples/rust/rust_chrdev.rs b/samples/rust/rust_chrdev.rs index 78423b1e3d116f..ddc6ad145531d4 100644 --- a/samples/rust/rust_chrdev.rs +++ b/samples/rust/rust_chrdev.rs @@ -16,8 +16,6 @@ module! { author: b"Rust for Linux Contributors", description: b"Rust character device sample", license: b"GPL v2", - params: { - }, } #[derive(Default)] From 1825e2f39f9ff82655510abcb4e0944949f04b4a Mon Sep 17 00:00:00 2001 From: Finn Behrens Date: Wed, 28 Apr 2021 16:59:50 +0200 Subject: [PATCH 2/2] Remove unused params from module! macro calls Signed-off-by: Finn Behrens --- drivers/android/rust_binder.rs | 1 - samples/rust/rust_minimal.rs | 2 -- samples/rust/rust_miscdev.rs | 2 -- samples/rust/rust_print.rs | 2 -- samples/rust/rust_semaphore.rs | 1 - samples/rust/rust_stack_probing.rs | 2 -- samples/rust/rust_sync.rs | 2 -- 7 files changed, 12 deletions(-) diff --git a/drivers/android/rust_binder.rs b/drivers/android/rust_binder.rs index a5297651008cb5..75640fc3f81c97 100644 --- a/drivers/android/rust_binder.rs +++ b/drivers/android/rust_binder.rs @@ -35,7 +35,6 @@ module! { author: b"Wedson Almeida Filho", description: b"Android Binder", license: b"GPL v2", - params: {}, } enum Either { diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs index 21627ce5656e2a..8238bf28578a2c 100644 --- a/samples/rust/rust_minimal.rs +++ b/samples/rust/rust_minimal.rs @@ -13,8 +13,6 @@ module! { author: b"Rust for Linux Contributors", description: b"Rust minimal sample", license: b"GPL v2", - params: { - }, } struct RustMinimal { diff --git a/samples/rust/rust_miscdev.rs b/samples/rust/rust_miscdev.rs index 944da811f4c164..bd4b35ab5bdb44 100644 --- a/samples/rust/rust_miscdev.rs +++ b/samples/rust/rust_miscdev.rs @@ -23,8 +23,6 @@ module! { author: b"Rust for Linux Contributors", description: b"Rust miscellaneous device sample", license: b"GPL v2", - params: { - }, } const MAX_TOKENS: usize = 3; diff --git a/samples/rust/rust_print.rs b/samples/rust/rust_print.rs index ddfac800f425d6..5717b398b4348f 100644 --- a/samples/rust/rust_print.rs +++ b/samples/rust/rust_print.rs @@ -13,8 +13,6 @@ module! { author: b"Rust for Linux Contributors", description: b"Rust printing macros sample", license: b"GPL v2", - params: { - }, } struct RustPrint; diff --git a/samples/rust/rust_semaphore.rs b/samples/rust/rust_semaphore.rs index 83b163f647f20c..dea6c3460e96d7 100644 --- a/samples/rust/rust_semaphore.rs +++ b/samples/rust/rust_semaphore.rs @@ -39,7 +39,6 @@ module! { author: b"Rust for Linux Contributors", description: b"Rust semaphore sample", license: b"GPL v2", - params: {}, } struct SemaphoreInner { diff --git a/samples/rust/rust_stack_probing.rs b/samples/rust/rust_stack_probing.rs index f992773545658d..aeabb3d1361e1f 100644 --- a/samples/rust/rust_stack_probing.rs +++ b/samples/rust/rust_stack_probing.rs @@ -14,8 +14,6 @@ module! { author: b"Rust for Linux Contributors", description: b"Rust stack probing sample", license: b"GPL v2", - params: { - }, } struct RustStackProbing; diff --git a/samples/rust/rust_sync.rs b/samples/rust/rust_sync.rs index a921bfd7d55fb5..14796286b0f087 100644 --- a/samples/rust/rust_sync.rs +++ b/samples/rust/rust_sync.rs @@ -19,8 +19,6 @@ module! { author: b"Rust for Linux Contributors", description: b"Rust synchronisation primitives sample", license: b"GPL v2", - params: { - }, } struct RustSync;