rust-vmm / vm-memory

Virtual machine's guest memory crate
Apache License 2.0
295 stars 97 forks source link

Introduce custom derive for ByteValued/replace ByteValued with zerocopy #246

Open Manishearth opened 12 months ago

Manishearth commented 12 months ago

ByteValued is an unsafe trait and implementing it is tricky with the padding requirement. But I think it can be autoderived.

Basically, a custom derive that does two things:

This might significantly reduce the amount of unsafe needed by crate users.

Ablu commented 7 months ago

https://github.com/rust-vmm/vhost/issues/215 and https://github.com/rust-vmm/vhost/pull/208 are an examples where this would have helped.

The helper should probably also check that the types of the fields are only integeral types that can be safely assigning from byte slices. Checking repr(C) would also help.

Ablu commented 7 months ago

As @roypat suggested on https://github.com/rust-vmm/vm-memory/pull/274, there is https://docs.rs/zerocopy/latest/zerocopy that may be useful here. However, it only allows (de)serializing from and to &[u8]. That does not work for our volatile memory scenarios. But maybe one could contribute a pointer based API towards that crate? :thinking:

roypat commented 6 months ago

I don't think we'd need a pointer based API for zerocopy, our ByteValued trait only operates on slices (its Bytes that uses pointers, but this trait is about containers that allow byte-level access to its contents, such as guest memory, not about PODs. So the Bytes trait would have its ByteValued bounds updated to whatever the zerocopy equivalent is)

roypat commented 6 months ago

It seems that rust-vmm/acpi_tables has done the switch from ByteValued to zerocopy a while ago, and it seemed to have gone smoothly: https://github.com/rust-vmm/acpi_tables

I think doing this can be a good first step towards reducing our confusing jungle of traits

Ablu commented 6 months ago

Ah. You are right. I misunderstood the API. I thought that it would require an additional copy from the slice to the object. But it just turns a reference to a slice into a reference to a struct instance. So we can just copy to a slice and then use zerocopy.

DemiMarie commented 3 weeks ago

I have a crate that solves this without requiring any procedural macros.