virtio: provide a rust interface of virtio_driver in rust #841
Conversation
Richardhongyu
force-pushed
the
rust_virtio
branch
from
5b482a9 to
1811c18
Compare
|
The new commit fixes the above errors in the comment. |
Richardhongyu
force-pushed
the
rust_virtio
branch
from
1811c18 to
9eaba0f
Compare
Richardhongyu
force-pushed
the
rust_virtio
branch
2 times, most recently
from
f936248 to
1fe931d
Compare
|
This looks great, but one question: shouldn't we use a dedicated crate for virtio support? First VIRTIO is not always enabled, we at least need to put some "cfg gate"s somewhere, second the virtio support is not a basic part of core kernel, we should separate it for maintainship. |
| vdrv.remove = Some(Self::remove_callback); | ||
| if let Some(t) = T::ID_TABLE { | ||
| vdrv.id_table = t.as_ref(); | ||
| } |
There was a problem hiding this comment.
If you want of ids to work here, you must set vdrv.driver.of_match_table -- see platform.rs for an example.
There was a problem hiding this comment.
I found that in the virtio drivers of C sides, the common way to match the driver with the device is to use 'id_table' rather than of. I write a macro define_virtio_id_table to implement this. It's more like define_amba_id_table in the amba.rs.
| // duration of this call, so it is guaranteed to remain alive for the lifetime of | ||
| // `vdev`. | ||
| let mut dev = unsafe { Device::from_ptr(vdev) }; | ||
| let data = T::probe(&mut dev)?; |
There was a problem hiding this comment.
of identifiers are allowed to carry some data with them. If the match happened through one of them, ideally you'll pass this data to the probe function. See get_id_info in platform.rs for an example of this. (Perhaps we should move this function to the driver module so all drivers that support of can use it.)
There was a problem hiding this comment.
The virtio drivers use id_table to match the driver with the device, which does not contain any data field. The probe function in the virtio Driver is more like the probe function in the amba Driver.
It's reasonable to move get_id_info into the driver module. I can open a new PR about it.
I don't think we support putting it a different crate just yet, although @ojeda has been thinking about this. As for a cfg gate, I agree we should have it. We normally put it in |
Richardhongyu
force-pushed
the
rust_virtio
branch
from
1fe931d to
054867b
Compare
Thanks for reviewing! I use '#[cfg(CONFIG_VIRTIO)]' in the new commit. |
Thanks for reviewing! It seems I confuse the |
|
@wedsonaf Hi, could you help have a look at this whenever you have time? Thanks in advance! |
Wrap the struct virtio_driver in rust. Signed-off-by: Li Hongyu <[email protected]>
Richardhongyu
force-pushed
the
rust_virtio
branch
from
054867b to
0980c9f
Compare
|
Now the code in the comments does not use |
Wrap the struct virtio_driver in rust.
Signed-off-by: Li Hongyu [email protected]