Device / Driver PCI / Platform Rust abstractions

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Or does the Rust side not have KBUILD_MODNAME?

Quoted 2 lines · Greg KH

Or does the Rust side not have KBUILD_MODNAME?

We can definitely give access to it at compile-time with e.g. env!:

    pr_info!("{}\n", env!("KBUILD_MODNAME"));

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

thanks,

greg k-h

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Quoted 5 lines

thanks,

greg k-h

Quoted 49 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

Quoted 1 line

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Quoted 55 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Quoted 3 lines

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

thanks,

greg k-h

Quoted 56 lines · Greg KH

Quoted 55 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Quoted 6 lines

Quoted 3 lines

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

I think we could avoid having an additional name parameter in Module::init, but it would still need to be the driver resolving env!(KBUILD_MODNAME) passing it into the bus abstraction.

However, similar to what Alice suggested in another thread, we could include this step in the module_*_driver! macros.

Modules that don't use this convenience macro would need to do it by hand though. But that's probably not that big a deal.

Quoted 73 lines · Danilo Krummrich

Quoted 56 lines · Greg KH

Quoted 55 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Quoted 6 lines

Quoted 3 lines

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

I think we could avoid having an additional name parameter in Module::init, but it would still need to be the driver resolving env!(KBUILD_MODNAME) passing it into the bus abstraction.

Quoted 5 lines

However, similar to what Alice suggested in another thread, we could include this step in the module_*_driver! macros.

Modules that don't use this convenience macro would need to do it by hand though. But that's probably not that big a deal.

I think we can do it in the core module! macro that everyone has to use.

Alice

Quoted 77 lines · Alice Ryhl

Quoted 76 lines · Danilo Krummrich

Quoted 56 lines · Greg KH

Quoted 55 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Quoted 6 lines

Quoted 3 lines

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

I think we could avoid having an additional name parameter in Module::init, but it would still need to be the driver resolving env!(KBUILD_MODNAME) passing it into the bus abstraction.

However, similar to what Alice suggested in another thread, we could include this step in the module_*_driver! macros.

Quoted 4 lines

Quoted 2 lines

Modules that don't use this convenience macro would need to do it by hand though. But that's probably not that big a deal.

I think we can do it in the core module! macro that everyone has to use.

How? The module! macro does not know about the registration instances within the module structure.

Quoted 79 lines · Danilo Krummrich

Quoted 77 lines · Alice Ryhl

Quoted 76 lines · Danilo Krummrich

Quoted 56 lines · Greg KH

Quoted 55 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Quoted 6 lines

Quoted 3 lines

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

I think we could avoid having an additional name parameter in Module::init, but it would still need to be the driver resolving env!(KBUILD_MODNAME) passing it into the bus abstraction.

However, similar to what Alice suggested in another thread, we could include this step in the module_*_driver! macros.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Modules that don't use this convenience macro would need to do it by hand though. But that's probably not that big a deal.

I think we can do it in the core module! macro that everyone has to use.

How? The module! macro does not know about the registration instances within the module structure.

You could have the module! macro emit something along these lines:

impl ModuleName for {type_} {
    const NAME: &'static CStr = c_str!(env!("KBUILD_MODNAME"));
}

Then you can do <Self as ModuleName>::NAME to obtain the name elsewhere.

Alice

Quoted 88 lines · Alice Ryhl

Quoted 79 lines · Danilo Krummrich

Quoted 77 lines · Alice Ryhl

Quoted 76 lines · Danilo Krummrich

Quoted 56 lines · Greg KH

Quoted 55 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the `Registration` abstraction used
to register driver structures. Some subsystems require the module name on
driver registration (e.g. PCI in __pci_register_driver()), hence pass
the module name to `Module::init`.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Quoted 6 lines

Quoted 3 lines

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

I think we could avoid having an additional name parameter in Module::init, but it would still need to be the driver resolving env!(KBUILD_MODNAME) passing it into the bus abstraction.

However, similar to what Alice suggested in another thread, we could include this step in the module_*_driver! macros.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Modules that don't use this convenience macro would need to do it by hand though. But that's probably not that big a deal.

I think we can do it in the core module! macro that everyone has to use.

How? The module! macro does not know about the registration instances within the module structure.

Quoted 7 lines

You could have the module! macro emit something along these lines:

impl ModuleName for {type_} {
    const NAME: &'static CStr = c_str!(env!("KBUILD_MODNAME"));
}

Then you can do <Self as ModuleName>::NAME to obtain the name elsewhere.

Where {type_} would need to be the driver's Driver structure?

We'd then need to define the bus adapter as:

pub struct Adapter<T: Driver + ModuleName>(T)

But the question stands I guess, how would the module macro know {type_}?

Quoted 102 lines · Danilo Krummrich

Quoted 88 lines · Alice Ryhl

Quoted 79 lines · Danilo Krummrich

Quoted 77 lines · Alice Ryhl

Quoted 76 lines · Danilo Krummrich

Quoted 56 lines · Greg KH

Quoted 55 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

On Tue, Dec 10, 2024 at 11:46:28PM +0100, Danilo Krummrich wrote:
In a subsequent patch we introduce the `Registration` abstraction used
to register driver structures. Some subsystems require the module name on
driver registration (e.g. PCI in __pci_register_driver()), hence pass
the module name to `Module::init`.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but
that's not the real thing), we want the pointer to the module structure
in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the
"name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Quoted 6 lines

Quoted 3 lines

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

I think we could avoid having an additional name parameter in Module::init, but it would still need to be the driver resolving env!(KBUILD_MODNAME) passing it into the bus abstraction.

However, similar to what Alice suggested in another thread, we could include this step in the module_*_driver! macros.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Modules that don't use this convenience macro would need to do it by hand though. But that's probably not that big a deal.

I think we can do it in the core module! macro that everyone has to use.

How? The module! macro does not know about the registration instances within the module structure.

Quoted 7 lines

You could have the module! macro emit something along these lines:

impl ModuleName for {type_} {
    const NAME: &'static CStr = c_str!(env!("KBUILD_MODNAME"));
}

Then you can do <Self as ModuleName>::NAME to obtain the name elsewhere.

Where {type_} would need to be the driver's Driver structure?

We'd then need to define the bus adapter as:

Quoted 3 lines

pub struct Adapter<T: Driver + ModuleName>(T)

But the question stands I guess, how would the module macro know {type_}?

If you look at the macro implementation in rust/macros/module.rs you will find many uses of {type_} throughout the expansion. It's whatever is passed to the macro using the type: argument.

Alice

Quoted 107 lines · Alice Ryhl

Quoted 105 lines · Danilo Krummrich

Quoted 88 lines · Alice Ryhl

Quoted 79 lines · Danilo Krummrich

Quoted 77 lines · Alice Ryhl

Quoted 76 lines · Danilo Krummrich

Quoted 56 lines · Greg KH

Quoted 55 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 19 lines · Greg KH

On Wed, Dec 11, 2024 at 11:45:20AM +0100, Greg KH wrote:
On Tue, Dec 10, 2024 at 11:46:28PM +0100, Danilo Krummrich wrote:
In a subsequent patch we introduce the `Registration` abstraction used
to register driver structures. Some subsystems require the module name on
driver registration (e.g. PCI in __pci_register_driver()), hence pass
the module name to `Module::init`.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but
that's not the real thing), we want the pointer to the module structure
in the register_driver call.

Does this provide for that? I'm thinking it does, but it's not the
"name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to
"module.0" to get the module structure pointer (if I'm reading the code
right), but as you have that pointer already, why can't you just use
module->name there as well as you have a pointer to a valid module
structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Quoted 6 lines

Quoted 3 lines

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

I think we could avoid having an additional name parameter in Module::init, but it would still need to be the driver resolving env!(KBUILD_MODNAME) passing it into the bus abstraction.

However, similar to what Alice suggested in another thread, we could include this step in the module_*_driver! macros.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Modules that don't use this convenience macro would need to do it by hand though. But that's probably not that big a deal.

I think we can do it in the core module! macro that everyone has to use.

How? The module! macro does not know about the registration instances within the module structure.

Quoted 7 lines

You could have the module! macro emit something along these lines:

impl ModuleName for {type_} {
    const NAME: &'static CStr = c_str!(env!("KBUILD_MODNAME"));
}

Then you can do <Self as ModuleName>::NAME to obtain the name elsewhere.

Where {type_} would need to be the driver's Driver structure?

We'd then need to define the bus adapter as:

pub struct Adapter<T: Driver + ModuleName>(T)

But the question stands I guess, how would the module macro know {type_}?

Quoted 3 lines

If you look at the macro implementation in rust/macros/module.rs you will find many uses of {type_} throughout the expansion. It's whatever is passed to the macro using the type: argument.

Oh, I see. So, this means that module / driver author would still need to create the "connection" by listing the correspong driver types in the module! macro, right?

If so, I think it'd be better to do it in the module_*_driver! macro and let people implement the trait by hand for modules with multiple drivers (which should be pretty rare).

The reason is that I think that otherwise we're probably encoding too much semantics into the module! macro that isn't obvious and people need to understand.

Quoted 3 lines

Alice

Quoted 113 lines · Danilo Krummrich

Quoted 110 lines · Alice Ryhl

Quoted 98 lines · Danilo Krummrich

Quoted 88 lines · Alice Ryhl

Quoted 79 lines · Danilo Krummrich

Quoted 77 lines · Alice Ryhl

Quoted 76 lines · Danilo Krummrich

Quoted 56 lines · Greg KH

Quoted 55 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 29 lines · Greg KH

On Wed, Dec 11, 2024 at 11:48:23AM +0100, Greg KH wrote:
On Wed, Dec 11, 2024 at 11:45:20AM +0100, Greg KH wrote:
On Tue, Dec 10, 2024 at 11:46:28PM +0100, Danilo Krummrich wrote:
In a subsequent patch we introduce the `Registration` abstraction used
to register driver structures. Some subsystems require the module name on
driver registration (e.g. PCI in __pci_register_driver()), hence pass
the module name to `Module::init`.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but
that's not the real thing), we want the pointer to the module structure
in the register_driver call.

Does this provide for that? I'm thinking it does, but it's not the
"name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to
"module.0" to get the module structure pointer (if I'm reading the code
right), but as you have that pointer already, why can't you just use
module->name there as well as you have a pointer to a valid module
structure that has the name already embedded in it.

In digging further, it's used by the pci code to call into lower layers,
but why it's using a different string other than the module name string
is beyond me. Looks like this goes way back before git was around, and
odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all,
and pull it from the module which would make all of this go away in the
end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Quoted 6 lines

Quoted 3 lines

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

I think we could avoid having an additional name parameter in Module::init, but it would still need to be the driver resolving env!(KBUILD_MODNAME) passing it into the bus abstraction.

However, similar to what Alice suggested in another thread, we could include this step in the module_*_driver! macros.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Modules that don't use this convenience macro would need to do it by hand though. But that's probably not that big a deal.

I think we can do it in the core module! macro that everyone has to use.

How? The module! macro does not know about the registration instances within the module structure.

Quoted 7 lines

You could have the module! macro emit something along these lines:

impl ModuleName for {type_} {
    const NAME: &'static CStr = c_str!(env!("KBUILD_MODNAME"));
}

Then you can do <Self as ModuleName>::NAME to obtain the name elsewhere.

Quoted 7 lines

Where {type_} would need to be the driver's Driver structure?

We'd then need to define the bus adapter as:

pub struct Adapter<T: Driver + ModuleName>(T)

But the question stands I guess, how would the module macro know {type_}?

If you look at the macro implementation in rust/macros/module.rs you will find many uses of {type_} throughout the expansion. It's whatever is passed to the macro using the type: argument.

Quoted 3 lines

Oh, I see. So, this means that module / driver author would still need to create the "connection" by listing the correspong driver types in the module! macro, right?

I'm not sure what you mean. I'm *not* suggesting any changes to the interface of module! or module_*_driver!.

Quoted 12 lines

If so, I think it'd be better to do it in the module_*_driver! macro and let people implement the trait by hand for modules with multiple drivers (which should be pretty rare).

The reason is that I think that otherwise we're probably encoding too much semantics into the module! macro that isn't obvious and people need to understand.

Quoted 3 lines

Alice

Quoted 118 lines · Alice Ryhl

Quoted 116 lines · Danilo Krummrich

Quoted 103 lines · Alice Ryhl

Quoted 102 lines · Danilo Krummrich

Quoted 88 lines · Alice Ryhl

Quoted 79 lines · Danilo Krummrich

Quoted 77 lines · Alice Ryhl

Quoted 76 lines · Danilo Krummrich

Quoted 56 lines · Greg KH

Quoted 55 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 36 lines · Greg KH

On Wed, Dec 11, 2024 at 11:59:54AM +0100, Greg KH wrote:
On Wed, Dec 11, 2024 at 11:48:23AM +0100, Greg KH wrote:
On Wed, Dec 11, 2024 at 11:45:20AM +0100, Greg KH wrote:
On Tue, Dec 10, 2024 at 11:46:28PM +0100, Danilo Krummrich wrote:
In a subsequent patch we introduce the `Registration` abstraction used
to register driver structures. Some subsystems require the module name on
driver registration (e.g. PCI in __pci_register_driver()), hence pass
the module name to `Module::init`.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but
that's not the real thing), we want the pointer to the module structure
in the register_driver call.

Does this provide for that? I'm thinking it does, but it's not the
"name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to
"module.0" to get the module structure pointer (if I'm reading the code
right), but as you have that pointer already, why can't you just use
module->name there as well as you have a pointer to a valid module
structure that has the name already embedded in it.

In digging further, it's used by the pci code to call into lower layers,
but why it's using a different string other than the module name string
is beyond me. Looks like this goes way back before git was around, and
odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all,
and pull it from the module which would make all of this go away in the
end. Odds are something will break but who knows...

Nope, things break, the "name" is there to handle built-in modules (as
the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that
will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here,
just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Quoted 6 lines

Quoted 3 lines

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

I think we could avoid having an additional name parameter in Module::init, but it would still need to be the driver resolving env!(KBUILD_MODNAME) passing it into the bus abstraction.

However, similar to what Alice suggested in another thread, we could include this step in the module_*_driver! macros.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Modules that don't use this convenience macro would need to do it by hand though. But that's probably not that big a deal.

I think we can do it in the core module! macro that everyone has to use.

How? The module! macro does not know about the registration instances within the module structure.

Quoted 7 lines

You could have the module! macro emit something along these lines:

impl ModuleName for {type_} {
    const NAME: &'static CStr = c_str!(env!("KBUILD_MODNAME"));
}

Then you can do <Self as ModuleName>::NAME to obtain the name elsewhere.

Where {type_} would need to be the driver's Driver structure?

We'd then need to define the bus adapter as:

Quoted 7 lines

Quoted 3 lines

pub struct Adapter<T: Driver + ModuleName>(T)

But the question stands I guess, how would the module macro know {type_}?

If you look at the macro implementation in rust/macros/module.rs you will find many uses of {type_} throughout the expansion. It's whatever is passed to the macro using the type: argument.

Oh, I see. So, this means that module / driver author would still need to create the "connection" by listing the correspong driver types in the module! macro, right?

Quoted 2 lines

I'm not sure what you mean. I'm *not* suggesting any changes to the interface of module! or module_*_driver!.

Huh! Seems like we're talking past each other than. Maybe we can briefly discuss it in today's call?

Quoted 13 lines

Quoted 13 lines

If so, I think it'd be better to do it in the module_*_driver! macro and let people implement the trait by hand for modules with multiple drivers (which should be pretty rare).

The reason is that I think that otherwise we're probably encoding too much semantics into the module! macro that isn't obvious and people need to understand.

Quoted 3 lines

Alice

Quoted 60 lines · Greg KH

Quoted 58 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Quoted 2 lines

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

Quoted 61 lines · Danilo Krummrich

Quoted 60 lines · Greg KH

Quoted 58 lines · Danilo Krummrich

Quoted 42 lines · Danilo Krummrich

Quoted 31 lines · Greg KH

Quoted 21 lines · Greg KH

Quoted 11 lines · Greg KH

Quoted 10 lines · Greg KH

Quoted 4 lines · Danilo Krummrich

In a subsequent patch we introduce the Registration abstraction used to register driver structures. Some subsystems require the module name on driver registration (e.g. PCI in __pci_register_driver()), hence pass the module name to Module::init.

Nit, we don't need the NAME of the PCI driver (well, we do like it, but that's not the real thing), we want the pointer to the module structure in the register_driver call.

Quoted 8 lines

Quoted 2 lines

Does this provide for that? I'm thinking it does, but it's not the "name" that is the issue here.

Wait, no, you really do want the name, don't you. You refer to "module.0" to get the module structure pointer (if I'm reading the code right), but as you have that pointer already, why can't you just use module->name there as well as you have a pointer to a valid module structure that has the name already embedded in it.

Quoted 8 lines

In digging further, it's used by the pci code to call into lower layers, but why it's using a different string other than the module name string is beyond me. Looks like this goes way back before git was around, and odds are it's my fault for something I wrote a long time ago.

I'll see if I can just change the driver core to not need a name at all, and pull it from the module which would make all of this go away in the end. Odds are something will break but who knows...

Quoted 5 lines

Nope, things break, the "name" is there to handle built-in modules (as the module pointer will be NULL.)

So what you really want is not the module->name (as I don't think that will be set), but you want KBUILD_MODNAME which the build system sets.

That's correct, and the reason why I pass through this name argument.

Sorry I wasn't able to reply earlier to save you some time.

Quoted 8 lines

Quoted 4 lines

You shouldn't need to pass the name through all of the subsystems here, just rely on the build system instead.

Or does the Rust side not have KBUILD_MODNAME?

AFAIK, it doesn't (or didn't have at the time I wrote the patch).

@Miguel: Can we access KBUILD_MODNAME conveniently?

Actually, I now remember there was another reason why I pass it through in Module::init.

Even if we had env!(KBUILD_MODNAME) already, I'd want to use it from the bus abstraction code, e.g. rust/kernel/pci.rs. But since this is generic code, it won't get the KBUILD_MODNAME from the module that is using the bus abstraction.

Quoted 6 lines

Quoted 2 lines

Rust can't do that in a macro somehow that all pci rust drivers can pull from?

The problem is that register / unregister is encapsulated within methods of the abstraction types. So the C macro trick (while generally possible) isn't applicable.

Really? You can't have something in a required "register()" type function? Something for when the driver "instance" is created as part of pci::Driver? You do that today in your sample driver for the id table:

	const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;

Something else called DRIVER_NAME that you could then set:

	const DRIVER_NAME: env!(KBUILD_MODNAME);

Also, I think you will want this for when a single module registers multiple drivers which I think can happen at times, so you could manually override the DRIVER_NAME field.

And if DRIVER_NAME doesn't get set, well, you just don't get the module symlink in sysfs, just like what happens today if you don't provide that field (but for PCI drivers, the .h file does it automatically for you.)

Anyway, this is a driver issue, NOT a module issue, so having to "plumb" the module name all the way down through this really isn't the best abstraction to do here from what I can tell.

thanks,

greg k-h

Quoted 7 lines

Really? You can't have something in a required "register()" type function? Something for when the driver "instance" is created as part of pci::Driver? You do that today in your sample driver for the id table:

	const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;

Something else called DRIVER_NAME that you could then set:

	const DRIVER_NAME: env!(KBUILD_MODNAME);

Sure, that's possible. But that means that the driver has to set it explicitly -- even when e.g. module_pci_driver! is used.

In C you don't have that, because there it's implicit within the pci_register_driver() macro. (Regardless of whether it's a single module for a single driver or multiple drivers per module.)

Anyways, like I mentioned, given that we have env!(KBUILD_MODNAME) (which we still need to add), there are other options to make it work similarly, e.g. add a parameter to pci::Adapter and bake this into module_pci_driver!.

For this particular option, it would mean that for modules registering multiple drivers a corresponding name would need to be passed explicitly.

Quoted 3 lines

Also, I think you will want this for when a single module registers multiple drivers which I think can happen at times, so you could manually override the DRIVER_NAME field.

My proposal above would provide this option, but do we care? In C no one ever changes the name. There is zero users of __pci_register_driver(), everyone uses pci_register_driver() where the name is just KBUILD_MODNAME. Same for __platform_driver_register().

From: Wedson Almeida Filho <[email protected]>

Add a simple abstraction to guard critical code sections with an rcu read lock.

Signed-off-by Wedson Almeida Filho <[email protected]>

Co-developed-by Danilo Krummrich <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/helpers/helpers.c  |  1 +
 rust/helpers/rcu.c      | 13 ++++++++++++
 rust/kernel/sync.rs     |  1 +
 rust/kernel/sync/rcu.rs | 47 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 62 insertions(+)
 create mode 100644 rust/helpers/rcu.c
 create mode 100644 rust/kernel/sync/rcu.rs

rust/helpers/helpers.c +1 -0

index dcf827a61b52..060750af6524 100644
@@ -20,6 +20,7 @@
 #include "page.c"
 #include "pid_namespace.c"
 #include "rbtree.c"
+#include "rcu.c"
 #include "refcount.c"
 #include "security.c"
 #include "signal.c"

rust/helpers/rcu.c new file +13 -0

new file mode 100644
index 000000000000..f1cec6583513
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/rcupdate.h>
+
+void rust_helper_rcu_read_lock(void)
+{
+	rcu_read_lock();
+}
+
+void rust_helper_rcu_read_unlock(void)
+{
+	rcu_read_unlock();
+}

rust/kernel/sync.rs +1 -0

index 1eab7ebf25fd..0654008198b2 100644
@@ -12,6 +12,7 @@
 pub mod lock;
 mod locked_by;
 pub mod poll;
+pub mod rcu;
 
 pub use arc::{Arc, ArcBorrow, UniqueArc};
 pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult};

rust/kernel/sync/rcu.rs new file +29 -0

new file mode 100644
index 000000000000..3beef70d5f34
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! RCU support.
+//!
+//! C header: [`include/linux/rcupdate.h`](srctree/include/linux/rcupdate.h)
+
+use crate::{bindings, types::NotThreadSafe};
+
+/// Evidence that the RCU read side lock is held on the current thread/CPU.
+///
+/// The type is explicitly not `Send` because this property is per-thread/CPU.
+///
+/// # Invariants
+///
+/// The RCU read side lock is actually held while instances of this guard exist.
+pub struct Guard(NotThreadSafe);
+
+impl Guard {
+    /// Acquires the RCU read side lock and returns a guard.
+    pub fn new() -> Self {
+        // SAFETY: An FFI call with no additional requirements.
+        unsafe { bindings::rcu_read_lock() };
+        // INVARIANT: The RCU read side lock was just acquired above.
+        Self(NotThreadSafe)
+    }
+
+    /// Explicitly releases the RCU read side lock.
+    pub fn unlock(self) {}
+}

rust/kernel/sync/rcu.rs new file +8 -0

+        unsafe { bindings::rcu_read_unlock() };
+    }
+}
+
+/// Acquires the RCU read side lock.
+pub fn read_lock() -> Guard {
+    Guard::new()
+}

Signature · 1 line

2.47.0

Analogous to Opaque::new add Opaque::pin_init, which instead of a value T takes a PinInit<T> and returns a PinInit<Opaque<T>>.

Suggested-by Alice Ryhl <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

From: Wedson Almeida Filho <[email protected]>

Revocable allows access to objects to be safely revoked at run time.

This is useful, for example, for resources allocated during device probe; when the device is removed, the driver should stop accessing the device resources even if another state is kept in memory due to existing references (i.e., device context data is ref-counted and has a non-zero refcount after removal of the device).

Signed-off-by Wedson Almeida Filho <[email protected]>

Co-developed-by Danilo Krummrich <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/kernel/lib.rs       |   1 +
 rust/kernel/revocable.rs | 223 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 224 insertions(+)
 create mode 100644 rust/kernel/revocable.rs

rust/kernel/lib.rs +1 -0

index b5da7c520eb8..200c5f99a805 100644
@@ -60,6 +60,7 @@
 pub mod prelude;
 pub mod print;
 pub mod rbtree;
+pub mod revocable;
 pub mod security;
 pub mod seq_file;
 pub mod sizes;

rust/kernel/revocable.rs new file +123 -0

new file mode 100644
index 000000000000..e464d59eb6b5
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Revocable objects.
+//!
+//! The [`Revocable`] type wraps other types and allows access to them to be revoked. The existence
+//! of a [`RevocableGuard`] ensures that objects remain valid.
+
+use crate::{bindings, prelude::*, sync::rcu, types::Opaque};
+use core::{
+    marker::PhantomData,
+    ops::Deref,
+    ptr::drop_in_place,
+    sync::atomic::{AtomicBool, Ordering},
+};
+
+/// An object that can become inaccessible at runtime.
+///
+/// Once access is revoked and all concurrent users complete (i.e., all existing instances of
+/// [`RevocableGuard`] are dropped), the wrapped object is also dropped.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::revocable::Revocable;
+///
+/// struct Example {
+///     a: u32,
+///     b: u32,
+/// }
+///
+/// fn add_two(v: &Revocable<Example>) -> Option<u32> {
+///     let guard = v.try_access()?;
+///     Some(guard.a + guard.b)
+/// }
+///
+/// let v = KBox::pin_init(Revocable::new(Example { a: 10, b: 20 }), GFP_KERNEL).unwrap();
+/// assert_eq!(add_two(&v), Some(30));
+/// v.revoke();
+/// assert_eq!(add_two(&v), None);
+/// ```
+///
+/// Sample example as above, but explicitly using the rcu read side lock.
+///
+/// ```
+/// # use kernel::revocable::Revocable;
+/// use kernel::sync::rcu;
+///
+/// struct Example {
+///     a: u32,
+///     b: u32,
+/// }
+///
+/// fn add_two(v: &Revocable<Example>) -> Option<u32> {
+///     let guard = rcu::read_lock();
+///     let e = v.try_access_with_guard(&guard)?;
+///     Some(e.a + e.b)
+/// }
+///
+/// let v = KBox::pin_init(Revocable::new(Example { a: 10, b: 20 }), GFP_KERNEL).unwrap();
+/// assert_eq!(add_two(&v), Some(30));
+/// v.revoke();
+/// assert_eq!(add_two(&v), None);
+/// ```
+#[pin_data(PinnedDrop)]
+pub struct Revocable<T> {
+    is_available: AtomicBool,
+    #[pin]
+    data: Opaque<T>,
+}
+
+// SAFETY: `Revocable` is `Send` if the wrapped object is also `Send`. This is because while the
+// functionality exposed by `Revocable` can be accessed from any thread/CPU, it is possible that
+// this isn't supported by the wrapped object.
+unsafe impl<T: Send> Send for Revocable<T> {}
+
+// SAFETY: `Revocable` is `Sync` if the wrapped object is both `Send` and `Sync`. We require `Send`
+// from the wrapped object as well because  of `Revocable::revoke`, which can trigger the `Drop`
+// implementation of the wrapped object from an arbitrary thread.
+unsafe impl<T: Sync + Send> Sync for Revocable<T> {}
+
+impl<T> Revocable<T> {
+    /// Creates a new revocable instance of the given data.
+    pub fn new(data: impl PinInit<T>) -> impl PinInit<Self> {
+        pin_init!(Self {
+            is_available: AtomicBool::new(true),
+            data <- Opaque::pin_init(data),
+        })
+    }
+
+    /// Tries to access the revocable wrapped object.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible.
+    ///
+    /// Returns a guard that gives access to the object otherwise; the object is guaranteed to
+    /// remain accessible while the guard is alive. In such cases, callers are not allowed to sleep
+    /// because another CPU may be waiting to complete the revocation of this object.
+    pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
+        let guard = rcu::read_lock();
+        if self.is_available.load(Ordering::Relaxed) {
+            // Since `self.is_available` is true, data is initialised and has to remain valid
+            // because the RCU read side lock prevents it from being dropped.
+            Some(RevocableGuard::new(self.data.get(), guard))
+        } else {
+            None
+        }
+    }
+
+    /// Tries to access the revocable wrapped object.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible.
+    ///
+    /// Returns a shared reference to the object otherwise; the object is guaranteed to
+    /// remain accessible while the rcu read side guard is alive. In such cases, callers are not
+    /// allowed to sleep because another CPU may be waiting to complete the revocation of this
+    /// object.
+    pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a T> {
+        if self.is_available.load(Ordering::Relaxed) {
+            // SAFETY: Since `self.is_available` is true, data is initialised and has to remain
+            // valid because the RCU read side lock prevents it from being dropped.
+            Some(unsafe { &*self.data.get() })
+        } else {
+            None
+        }

rust/kernel/revocable.rs new file +60 -0

+        {
+            if SYNC {
+                // SAFETY: Just an FFI call, there are no further requirements.
+                unsafe { bindings::synchronize_rcu() };
+            }
+
+            // SAFETY: We know `self.data` is valid because only one CPU can succeed the
+            // `compare_exchange` above that takes `is_available` from `true` to `false`.
+            unsafe { drop_in_place(self.data.get()) };
+        }
+    }
+
+    /// Revokes access to and drops the wrapped object.
+    ///
+    /// Access to the object is revoked immediately to new callers of [`Revocable::try_access`],
+    /// expecting that there are no concurrent users of the object.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that there are no more concurrent users of the revocable object.
+    pub unsafe fn revoke_nosync(&self) {
+        // SAFETY: By the safety requirement of this function, the caller ensures that nobody is
+        // accessing the data anymore and hence we don't have to wait for the grace period to
+        // finish.
+        unsafe { self.revoke_internal::<false>() }
+    }
+
+    /// Revokes access to and drops the wrapped object.
+    ///
+    /// Access to the object is revoked immediately to new callers of [`Revocable::try_access`].
+    ///
+    /// If there are concurrent users of the object (i.e., ones that called
+    /// [`Revocable::try_access`] beforehand and still haven't dropped the returned guard), this
+    /// function waits for the concurrent access to complete before dropping the wrapped object.
+    pub fn revoke(&self) {
+        // SAFETY: By passing `true` we ask `revoke_internal` to wait for the grace period to
+        // finish.
+        unsafe { self.revoke_internal::<true>() }
+    }
+}
+
+#[pinned_drop]
+impl<T> PinnedDrop for Revocable<T> {
+    fn drop(self: Pin<&mut Self>) {
+        // Drop only if the data hasn't been revoked yet (in which case it has already been
+        // dropped).
+        // SAFETY: We are not moving out of `p`, only dropping in place
+        let p = unsafe { self.get_unchecked_mut() };
+        if *p.is_available.get_mut() {
+            // SAFETY: We know `self.data` is valid because no other CPU has changed
+            // `is_available` to `false` yet, and no other CPU can do it anymore because this CPU
+            // holds the only reference (mutable) to `self` now.
+            unsafe { drop_in_place(p.data.get()) };
+        }
+    }
+}
+
+/// A guard that allows access to a revocable object and keeps it alive.
+///
+/// CPUs may not sleep while holding on to [`RevocableGuard`] because it's in atomic context

rust/kernel/revocable.rs new file +20 -1

+
+impl<T> RevocableGuard<'_, T> {
+    fn new(data_ref: *const T, rcu_guard: rcu::Guard) -> Self {
+        Self {
+            data_ref,
+            _rcu_guard: rcu_guard,
+            _p: PhantomData,
+        }
+    }
+}
+
+impl<T> Deref for RevocableGuard<'_, T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: By the type invariants, we hold the rcu read-side lock, so the object is
+        // guaranteed to remain valid.
+        unsafe { &*self.data_ref }
+    }
+}
--

2.47.0

rust/kernel/revocable.rs line 203

Quoted 13 lines · On Tue, 10 Dec 2024 23:46:33 +0100 Danilo Krummrich <[email protected]> wrote:

rust/kernel/revocable.rs new file +13 -0

+/// A guard that allows access to a revocable object and keeps it alive.
+///
+/// CPUs may not sleep while holding on to [`RevocableGuard`] because it's in atomic context
+/// holding the RCU read-side lock.
+///
+/// # Invariants
+///
+/// The RCU read-side lock is held while the guard is alive.
+pub struct RevocableGuard<'a, T> {
+    data_ref: *const T,
+    _rcu_guard: rcu::Guard,
+    _p: PhantomData<&'a ()>,
+}

Shouldn't this type hold a &'a T directly instead of a raw pointer ?

In order to not duplicate code in bus specific implementations (e.g. platform), implement a generic driver::Adapter to represent the connection of matched drivers and devices.

Bus specific Adapter implementations can simply implement this trait to inherit generic functionality, such as matching OF or ACPI device IDs and ID table entries.

Suggested-by Rob Herring (Arm) <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/kernel/driver.rs | 59 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

rust/kernel/driver.rs +6 -1

index ab0bb46fe2cc..d169899a5da1 100644
@@ -6,7 +6,9 @@
 //! register using the [`Registration`] class.
 
 use crate::error::{Error, Result};
-use crate::{init::PinInit, str::CStr, try_pin_init, types::Opaque, ThisModule};
+use crate::{
+    device, device_id, init::PinInit, of, str::CStr, try_pin_init, types::Opaque, ThisModule,
+};
 use core::pin::Pin;
 use macros::{pin_data, pinned_drop};
 

@@ -114,3 +116,58 @@ macro_rules! module_driver {
         }
     }
 }
+
+/// The bus independent adapter to match a drivers and a devices.
+///

rust/kernel/driver.rs +42 -1

+
+    /// Returns the driver's private data from the matching entry in the [`of::IdTable`], if any.
+    ///
+    /// If this returns `None`, it means there is no match with an entry in the [`of::IdTable`].
+    #[cfg(CONFIG_OF)]
+    fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
+        let table = Self::of_id_table();
+
+        // SAFETY:
+        // - `table` has static lifetime, hence it's valid for read,
+        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
+        let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), dev.as_raw()) };
+
+        if raw_id.is_null() {
+            None
+        } else {
+            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
+            // does not add additional invariants, so it's safe to transmute.
+            let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
+
+            Some(table.info(<of::DeviceId as device_id::RawDeviceId>::index(id)))
+        }
+    }
+
+    #[cfg(not(CONFIG_OF))]
+    fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
+        None
+    }
+
+    /// Returns the driver's private data from the matching entry of any of the ID tables, if any.
+    ///
+    /// If this returns `None`, it means that there is no match in any of the ID tables directly
+    /// associated with a [`device::Device`].
+    fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
+        let id = Self::of_id_info(dev);
+        if id.is_some() {
+            return id;
+        }
+
+        None
+    }
+}
--

2.47.0

Implement the basic platform bus abstractions required to write a basic platform driver. This includes the following data structures:

The platform::Driver trait represents the interface to the driver and provides platform::Driver::probe for the driver to implement.

The platform::Device abstraction represents a struct platform_device.

In order to provide the platform bus specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by platform::Adapter.

Signed-off-by Danilo Krummrich <[email protected]>

---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   2 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/platform.c         |  13 +++
 rust/kernel/lib.rs              |   1 +
 rust/kernel/platform.rs         | 193 ++++++++++++++++++++++++++++++++
 6 files changed, 211 insertions(+)
 create mode 100644 rust/helpers/platform.c
 create mode 100644 rust/kernel/platform.rs

Add a sample Rust platform driver illustrating the usage of the platform bus abstractions.

This driver probes through either a match of device / driver name or a match within the OF ID table.

Signed-off-by Danilo Krummrich <[email protected]>

---
 MAINTAINERS                                  |  1 +
 drivers/of/unittest-data/tests-platform.dtsi |  5 ++
 samples/rust/Kconfig                         | 10 ++++
 samples/rust/Makefile                        |  1 +
 samples/rust/rust_driver_platform.rs         | 49 ++++++++++++++++++++
 5 files changed, 66 insertions(+)
 create mode 100644 samples/rust/rust_driver_platform.rs