Add support for RTC (Real Time Clock)

[andreeaflorescu]

The RTC is needed for booting an aarch64 machine.

We can use the RTC implementation from Firecracker as a starting point: https://github.com/firecracker-microvm/firecracker/blob/master/src/devices/src/legacy/rtc_pl031.rs

Similarly to the serial console implementation, the RTC in vm-device should only handle the emulation part (event handling and Bus implementation should not be included as they're specific to the VMM consuming vm-superio).

Nice to have:

• metrics (these can also be defined later)

[andreeaflorescu]

@dodsonmg do you want to take a look at this one?

[dodsonmg]

Sure! Thanks!

[dodsonmg]

I have a working RTC implementation that is passing unit tests; however...

It currently depends on Firecracker's utils wrapper around the vmm-sys-utils crate. They have some time utilities and functions for converting between u32 and byte arrays that are very convenient, and it seems silly to reimplement them as local helper functions.

Thoughts on the best way to proceed here?

I'm also a bit unsure about the right interface to present to the bus. I've tried to match the function signatures from Firecracker for the read() and write() methods.

If you want to have a look, I can submit an [RFC] PR or you can find it here. I haven't updated the README.md yet (since I wanted to get some guidance on the interface first).

[lauralt]

Hmm, it's not very clear to me if it's okay to accept data len != 4. Seems a bit weird, but I'll check this out. Anyway, in case data.len() == 4, you can use the following replacements:

let val = byte_order::read_le_u32(&data[..]); -> let v = u32::from_le_bytes(data.try_into().unwrap()); (or for example let v = u32::from_le_bytes(data[..4].try_into().unwrap()); if data len > 4)

byte_order::write_le_u32(data, v); -> data.copy_from_slice(&v.to_le_bytes());

If it turns out it makes sense to have data len != 4, I'm sure we can work around the above substitutions (even with some tiny helper functions) so that we don't need to add or import macros.

I think it would be easier to review if you open it as an RFC.

[dodsonmg]

Thanks @lauralt. I was trying to approximate what Firecracker does on the bus, which seems to assume that the data argument provided to read or write is a slice, and then checks that it's sufficiently sized to read from or write to.

read() https://github.com/firecracker-microvm/firecracker/blob/67cbafbc51e41a3f63b4481a0409206b623c08c0/src/devices/src/legacy/rtc_pl031.rs#L138

write() https://github.com/firecracker-microvm/firecracker/blob/67cbafbc51e41a3f63b4481a0409206b623c08c0/src/devices/src/legacy/rtc_pl031.rs#L176

The easiest solution (from the myopic view of this module) is just to assume that data is u32 and let the bus implementation handle the conversion.

I'll bring the minimal functionality for the byte array conversion and time into the module and submit an RFC PR this evening.

[lauralt]

Sorry, didn't notice the utils::time dependency until now. It would be great if we could simplify things there too :D .

[andreeaflorescu]

Would it make sense to add utils::time and utils::byte_order to vmm-sys-util as well? And then consume the dependency from there? As a shortcut we can also first import them locally (in this crate), and then work on upstreaming them to vmm-sys-util where they could be potentially reused.

Are you thinking about another way of solving the problem?

[lauralt]

utils::byte_order is not needed and for the utils::time we were hoping to simplify things (it looks like crosvm does those things in a simpler way for example). @dodsonmg do you have any update on this?

[dodsonmg]

I've been chatting with @lauralt on this a bit. Looking at crosvm, I think I can use std time functions rather than bringing in utils::time, which strikes me as something written because there weren't other options available at the time (though feel free to correct that opinion).

I only need one of the functions from utils::byte_order (or, rather, one of the functions generated by the macros in utils::byte_order). In fact, if we decide that the argument to read() and write() should be a 4 element array of bytes rather than a slice, then I don't need any functions from utils::byte_order.

While utils::byte_order seems to be a tightly written and useful bit of code, I don't think it's worth adding to vmm-sys-util just for the RTC.

[lauralt]

4 element array of bytes should be the right thing.

[andreeaflorescu]

Fixed by #31