| 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: <ca5873fb-3139-4198-8ff8-c3abc6c40347@proton.me>
Date: Tue, 24 Oct 2023 16:28:02 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org
Cc: 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 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?
I think Wedson was wrapping his example with `module!` inside
of a module, so maybe try that?
> +/// 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?
> +/// 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?
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.
--
Cheers,
Benno
> + Ok(Module { _reg: reg })
> + }
> + }
> + };
> +
> + $crate::module_phy_driver!(@device_table [$($dev),+]);
> + }
> +}
> --
> 2.34.1
>
Powered by blists - more mailing lists