| 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: <20231025.090243.1437967503809186729.fujita.tomonori@gmail.com>
Date: Wed, 25 Oct 2023 09:02:43 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: benno.lossin@...ton.me
Cc: fujita.tomonori@...il.com, netdev@...r.kernel.org,
rust-for-linux@...r.kernel.org, andrew@...n.ch, tmgross@...ch.edu,
miguel.ojeda.sandonis@...il.com, wedsonaf@...il.com, greg@...ah.com
Subject: Re: [PATCH net-next v6 2/5] rust: net::phy add module_phy_driver
macro
On Tue, 24 Oct 2023 16:28:02 +0000
Benno Lossin <benno.lossin@...ton.me> wrote:
> On 24.10.23 02:58, FUJITA Tomonori wrote:
>> This macro creates an array of kernel's `struct phy_driver` and
>> registers it. This also corresponds to the kernel's
>> `MODULE_DEVICE_TABLE` macro, which embeds the information for module
>> loading into the module binary file.
>>
>> A PHY driver should use this macro.
>>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@...il.com>
>> ---
>> rust/kernel/net/phy.rs | 129 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 129 insertions(+)
>>
>> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
>> index 2d821c2475e1..f346b2b4d3cb 100644
>> --- a/rust/kernel/net/phy.rs
>> +++ b/rust/kernel/net/phy.rs
>> @@ -706,3 +706,132 @@ const fn as_int(&self) -> u32 {
>> }
>> }
>> }
>> +
>> +/// Declares a kernel module for PHYs drivers.
>> +///
>> +/// This creates a static array of kernel's `struct phy_driver` and registers it.
>> +/// This also corresponds to the kernel's `MODULE_DEVICE_TABLE` macro, which embeds the information
>> +/// for module loading into the module binary file. Every driver needs an entry in `device_table`.
>> +///
>> +/// # Examples
>> +///
>> +/// ```ignore
>
> Is this example ignored, because it does not compile?
The old version can't be compiled but the current version can so I'll
drop ignore.
> I think Wedson was wrapping his example with `module!` inside
> of a module, so maybe try that?
I'm not sure what you mean.
>> +/// use kernel::c_str;
>> +/// use kernel::net::phy::{self, DeviceId};
>> +/// use kernel::prelude::*;
>> +///
>> +/// kernel::module_phy_driver! {
>> +/// drivers: [PhyAX88772A],
>> +/// device_table: [
>> +/// DeviceId::new_with_driver::<PhyAX88772A>(),
>> +/// ],
>> +/// name: "rust_asix_phy",
>> +/// author: "Rust for Linux Contributors",
>> +/// description: "Rust Asix PHYs driver",
>> +/// license: "GPL",
>> +/// }
>> +///
>> +/// struct PhyAX88772A;
>> +///
>> +/// impl phy::Driver for PhyAX88772A {
>> +/// const NAME: &'static CStr = c_str!("Asix Electronics AX88772A");
>> +/// const PHY_DEVICE_ID: phy::DeviceId = phy::DeviceId::new_with_exact_mask(0x003b1861);
>> +/// }
>> +/// ```
>> +///
>> +/// This expands to the following code:
>> +///
>> +/// ```ignore
>> +/// use kernel::c_str;
>> +/// use kernel::net::phy::{self, DeviceId};
>> +/// use kernel::prelude::*;
>> +///
>> +/// struct Module {
>> +/// _reg: ::kernel::net::phy::Registration,
>> +/// }
>> +///
>> +/// module! {
>> +/// type: Module,
>> +/// name: "rust_asix_phy",
>> +/// author: "Rust for Linux Contributors",
>> +/// description: "Rust Asix PHYs driver",
>> +/// license: "GPL",
>> +/// }
>> +///
>> +/// const _: () = {
>> +/// static mut DRIVERS: [::kernel::net::phy::DriverVTable; 1] = [
>> +/// ::kernel::net::phy::create_phy_driver::<PhyAX88772A>::(),
>> +/// ];
>> +///
>> +/// impl ::kernel::Module for Module {
>> +/// fn init(module: &'static ThisModule) -> Result<Self> {
>> +/// let mut reg = unsafe { ::kernel::net::phy::Registration::register(module, Pin::static_mut(&mut DRIVERS)) }?;
>
> Can you please format this as well?
Done.
>> +/// Ok(Module { _reg: reg })
>> +/// }
>> +/// }
>> +/// }
>> +///
>> +/// static __mod_mdio__phydev_device_table: [::kernel::bindings::mdio_device_id; 2] = [
>> +/// ::kernel::bindings::mdio_device_id {
>> +/// phy_id: 0x003b1861,
>> +/// phy_id_mask: 0xffffffff,
>> +/// },
>> +/// ::kernel::bindings::mdio_device_id {
>> +/// phy_id: 0,
>> +/// phy_id_mask: 0,
>> +/// }
>> +/// ];
>> +/// ```
>> +#[macro_export]
>> +macro_rules! module_phy_driver {
>> + (@replace_expr $_t:tt $sub:expr) => {$sub};
>> +
>> + (@count_devices $($x:expr),*) => {
>> + 0usize $(+ $crate::module_phy_driver!(@replace_expr $x 1usize))*
>> + };
>> +
>> + (@device_table [$($dev:expr),+]) => {
>> + #[no_mangle]
>> + static __mod_mdio__phydev_device_table: [
>> + ::kernel::bindings::mdio_device_id;
>> + $crate::module_phy_driver!(@count_devices $($dev),+) + 1
>> + ] = [
>> + $($dev.mdio_device_id()),+,
>> + ::kernel::bindings::mdio_device_id {
>> + phy_id: 0,
>> + phy_id_mask: 0
>> + }
>> + ];
>> + };
>> +
>> + (drivers: [$($driver:ident),+], device_table: [$($dev:expr),+], $($f:tt)*) => {
>> + struct Module {
>> + _reg: ::kernel::net::phy::Registration,
>> + }
>> +
>> + $crate::prelude::module! {
>> + type: Module,
>> + $($f)*
>> + }
>> +
>> + const _: () = {
>> + static mut DRIVERS: [
>> + ::kernel::net::phy::DriverVTable;
>> + $crate::module_phy_driver!(@count_devices $($driver),+)
>> + ] = [
>> + $(::kernel::net::phy::create_phy_driver::<$driver>()),+
>> + ];
>> +
>> + impl ::kernel::Module for Module {
>> + fn init(module: &'static ThisModule) -> Result<Self> {
>> + // SAFETY: The anonymous constant guarantees that nobody else can access the `DRIVERS` static.
>> + // The array is used only in the C side.
>> + let mut reg = unsafe { ::kernel::net::phy::Registration::register(module, core::pin::Pin::static_mut(&mut DRIVERS)) }?;
>
> Can you put the safe operations outside of the `unsafe` block?
fn init(module: &'static ThisModule) -> Result<Self> {
// SAFETY: The anonymous constant guarantees that nobody else can access
// the `DRIVERS` static. The array is used only in the C side.
let mut reg = ::kernel::net::phy::Registration::register(
module,
core::pin::Pin::static_mut(unsafe { &mut DRIVERS }),
)?;
Ok(Module { _reg: reg })
}
> Since `rustfmt` does not work well within `macro_rules`, you
> have to manually format this code. One trick I use to do this
> is to replace all meta variables with normal variables and
> just put it in `rustfmt` and then revert the replacement in
> the output.
Understood, done.
Powered by blists - more mailing lists