Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a "Rockusb" driver #117

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

CFSworks
Copy link
Contributor

To further our mission of being able to image modules from the BMC itself, I am offering here a kernel driver that translates SCSI commands into Rockusb commands. This facilitates access to the RK1's eMMC the same way we do with CM4.

Rockchip devices in USB recovery mode generally speak a protocol called "Rockusb." This protocol is USB-MSC, but with the SCSI layer ripped out and replaced with a custom command set. There are commands for LBA read/write, however, so it is pretty straightforward to have a kernel module perform the translation, allowing access to an RK1's eMMC through a /dev/sda device.

This driver is built by default, but not loaded by default. It is necessary on the BMC to run modprobe rockusb to enable the driver. We should decide how we want to use this (if at all) before loading it automatically.

When testing on a real RK1, I found that an apparent bootloader bug(?) prevents access beyond 32MiB (reads return an all-0xCC bit pattern; I did not dare try write). I have had to use the xrock tool to reload the RK1 to firmware that doesn't have this problem:

# xrock reset maskrom
# xrock maskrom rk3588_ddr_lp4_2112MHz_lp5_2736MHz_v1.12.bin rk3588_usbplug_v1.10.bin --rc4-off

We will have to automate this operation (similar to what we do with CM4) unless TMs can ensure that any RK1s shipped to users are preloaded with firmware without this bug.

@svenrademakers
Copy link
Collaborator

We need to see what is the best way forward. Currently, the same functionality is in the master branch of the "new bmc daemon" repository. see rockusb_fwupdate.rs

When testing on a real RK1, I found that an apparent bootloader bug(?) prevents access beyond 32MiB (reads return an all-0xCC bit pattern; I did not dare try write). I have had to use the xrock tool to reload the RK1 to firmware that doesn't have this problem:

I haven't seen this problem myself. I do notice with testing that the rockusb is quite sensitive to anything other than the happy flow. I found some issues sniffing USB traffic. Perhaps that's worth a shot.

Does xrock reset maskrom work for you?! Which RK1 sample do you have? I reckon one of the latest

@CFSworks
Copy link
Contributor Author

We need to see what is the best way forward. Currently, the same functionality is in the master branch of the "new bmc daemon" repository. see rockusb_fwupdate.rs

That looks like it's complementary to the driver here: the majority of code in rockusb_fwudate.rs looks like it's for getting a maskrom-booted RK3588 to usbplug mode. So the only code that would need to be replaced is this:

    Ok(StdTransportWrapper::new(
        transport.into_io().map_err_into_logged_io(logging)?,
    ))

...with instead some code that (re)binds the rockusb.ko driver and does a hunt for the correct /dev/sdX node (mirroring Raspberry Pi).

That's of course assuming this driver is of any value -- but I do like being able to mount the RK1 rootfs directly on the BMC, and it may be cleaner in tpi_rs to remove the "transport" abstraction and rely on Linux block device I/O in both cases. (Here's hoping I can figure out something equally clean for accessing Jetsons!)

I haven't seen this problem myself. I do notice with testing that the rockusb is quite sensitive to anything other than the happy flow. I found some issues sniffing USB traffic. Perhaps that's worth a shot.

Are you booting from maskrom? The .bin you're loading probably doesn't have the 32MiB bug. The bootloader that came on my RK1 sample does have this bug, though, and I need to go out to maskrom and back in order to avoid using it.

Does xrock reset maskrom work for you?! Which RK1 sample do you have? I reckon one of the latest

It does (I can try to find an equivalent rockchiprs snippet if you need it on your end). I don't know how to identify which revision of the RK1 I have, though.

This is a driver that allows accessing the RK1 from the BMC
as /dev/sdX.

There are some conditions that prevent the driver from binding to a
Rockchip device in "maskrom" mode, with the goal of only binding if the
device is truly in Rockusb mode.
@svenrademakers
Copy link
Collaborator

I like the possibility of mounting rockchip devices as block devices. I do want to challenge the idea of writing the driver ourself:

  • rockchip protocol has some areas that are left to interpretation (see the above conversation for example)
  • We are the ones that need to maintain it. There exist already various libs on the internet that can do the same. it would be nice if we could leverage these projects. We can get fixes and improvements "for cheap" this way.

Im not saying i reject this PR, i would like to have a better understanding of the pro's and con's.

@CFSworks
Copy link
Contributor Author

CFSworks commented Oct 4, 2023

I like the possibility of mounting rockchip devices as block devices. I do want to challenge the idea of writing the driver ourself:

* rockchip protocol has some areas that are left to interpretation (see the above conversation for example)

* We are the ones that need to maintain it. There exist already various libs on the internet that can do the same. it would be nice if we could leverage these projects.  We can get fixes and improvements "for cheap" this way.

Im not saying i reject this PR, i would like to have a better understanding of the pro's and con's.

Bah, yeah, you're very right: it's easy to forget that there's going to be a maintenance burden when the shiny new toy is still shiny and new. An out-of-tree kernel module is something of a commitment and, even though I think this one's simple enough that there won't be much maintenance attention each time, it's yet one more thing that will have to be handled whenever the kernel is updated. We would need to pro/con that.

If it comes to it, it might be worthwhile to contribute this driver to the kernel. The maintenance is "free" at that point, and we'd instead only pay the upfront cost of making the driver pass review. (We would probably also want to make the driver handle loading the usbplug blob, likely treating it as device firmware.) But let's explore other options first.

The big downside to the existing libs is that they aren't meant to run in kernelspace. From a runtime/usability perspective, it's very nice having the /dev/sda block device show up as soon as the RK3588 is connected (and running usbplug), without needing a userspace daemon running to handle the I/O. But it may be inevitable that some node with a really specific USB<->eMMC access protocol (cough, Tegra, cough) makes the userspace I/O daemon necessary. Perhaps we shouldn't be getting comfortable with kernelspace solutions, and look for a way to make bmcd handle it.

The main problem to solve is getting the kernel to register a block device that wraps userspace I/O, but there are some good options for this: Linux 6.0 introduces the ublk userspace block device, which is exactly what I've just described (it is to block devices what FUSE is to filesystems or tun/tap is to network interfaces). We could also pick up the pace on #14, and have the bmcd localhost-bind the NBD server and then attach the in-kernel NBD client to localhost -- which means we can use the same logic for both "real" NBD and local attachments alike. The only thing I wonder about here is how we would handle the CM4 (and other nodes that speak true USB-MSC). Do we have bmcd claim /dev/sda exclusively (and hide the latter so it doesn't create confusion among users)? Or do we not bother with the loopback attachment and tell users that their node is locally-attached as /dev/sda? (Or do we rename /dev/sda, /dev/nbd0, ... to /dev/nodeN in all cases?)

But, either way, I'm now thinking we should start with the user story and work our way back to the software solution, rather than the other way around. Thoughts?

@svenrademakers
Copy link
Collaborator

@CFSworks You have some intresting angles to this problem which could be worth exploring. My recommendation would be to first make sure we have "something" and try to improve on this situation iteratively. Meaning, i think having the rockusb driver in its current form, which exposes a block device, is an excellent starting point. (even though it has some loose ends). Its most important that it provides a stable experience for the RK1.

We can spin-off efforts later on to see if we can improve experience with having a NBD server. Even though my initial reaction is somewhat concervative on this matter as it involves introducing more moving components.

Copy link
Collaborator

@svenrademakers svenrademakers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you feel comfortable, i would suggest we merge this. I would require some time to make the needed changes to let the bmcd make use of this driver

@CFSworks
Copy link
Contributor Author

All I care about is that the RK1 eMMC is available as a (mountable/shareable/etc) block device. Exactly how that happens is of little concern to me.

So, sounds good to me that we merge this driver for now, with an eye toward either mainlining it or replacing it in the future!

@dgoodlad
Copy link

Just wanted to note that I think this might be useful beyond the RK1: there are a number of CM4-pin-compatible modules, like the Orange Pi CM4, which are based on Rockchip SOCs. I have been testing one on the CM4 carrier board on the tpi2 and it seems to work just fine, but obviously flashing its eMMC is pretty cumbersome!

@bhuism
Copy link

bhuism commented Oct 11, 2023

+1

@svenrademakers svenrademakers merged commit 2c9e63a into turing-machines:master Oct 16, 2023
1 check passed
@CFSworks CFSworks deleted the rockusb-driver branch February 13, 2024 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants