GuestRegionMmap: extend for supporting guest_memfd

[MatiasVara]

Probably I am missing a lot of things here, but I was wondering if it would be worth to extend GuestRegionMmap (see

vm-memory/src/mmap.rs

Line 107 in 3a0b31d

pub struct GuestRegionMmap<B = ()> {

) to contain the fd resulting from kvm_create_guest_memfd(). The problem I have is that I keep a separated Vec<> to store the fd for each region created by using kvm_userspace_memory_region2(). I would like to have everything in the same structure. The mapping between fd and region is required when switching regions from private to shared and vice-versa.

Any thoughts?

Thanks, Matias.

[roypat]

Hi Matias! Yeah, I've thought about how/if vm-memory should deal with guest_memfds before, because I ran into the same awkwardness of having to store my guest_memfds separately from my guest memory regions.

The idea I came up with was #312, which would allow the definition of some struct KvmGuestRegion { region: GuestRegionMmap, guest_memfd: Option<File> } wrapper type implementing GuestMemoryRegion that can be managed with GuestMemoryMmap/GuestRegionCollection in #312 (personally, I'd love simply doing impl GuestMemoryRegion for kvm_userspace_memory_region2, but the bitmap tracking stuff ruins these dreams. EDIT: we cant do that anyway because nothing guarantees us that the userspace_addr is actually valid memory, and GuestMemoryRegion offers safe interfaces). Does that sounds compatible with what you want to do?

I'm not a huge fan of adding KVM/guest_memfd specific fields to GuestRegionMmap, for the same reason I'm not a huge fan of merging the Xen stuff into GuestRegionMmap, because it'll just end up as some hodge-podge god-object that tries to support a million different things at once and just becomes unmaintainable :/ - and I wish for vm-memory to have APIs that are extensible without strictly requiring changes in vm-memory itself (there's some related discussion earlier this week in #314).

[MatiasVara]

Hi Matias! Yeah, I've thought about how/if vm-memory should deal with guest_memfds before, because I ran into the same awkwardness of having to store my guest_memfds separately from my guest memory regions.

Hello Patrick and thanks for the response.

The idea I came up with was #312, which would allow the definition of some struct KvmGuestRegion { region: GuestRegionMmap, guest_memfd: Option<File> } wrapper type implementing GuestMemoryRegion that can be managed with GuestMemoryMmap/GuestRegionCollection in #312 (personally, I'd love simply doing impl GuestMemoryRegion for kvm_userspace_memory_region2, but the bitmap tracking stuff ruins these dreams). Does that sounds compatible with what you want to do?

Yes, I think so. If I understand correctly, I could just use KvmGuestRegion. Do you mean that I need to define KvmGuestRegion in my code as a wrapper type implementing GuestMemoryRegion ?

I'm not a huge fan of adding KVM/guest_memfd specific fields to GuestRegionMmap, for the same reason I'm not a huge fan of merging the Xen stuff into GuestRegionMmap, because it'll just end up as some hodge-podge god-object that tries to support a million different things at once and just becomes unmaintainable :/ - and I wish for vm-memory to have APIs that are extensible without strictly requiring changes in vm-memory itself (there's some related discussion earlier this week in #314).

That sounds good for me.

[Xander-C]

Hi Matias, I encountered the same issue with you.

Do you mean that I need to define KvmGuestRegion in my code as a wrapper type implementing GuestMemoryRegion ?

I think it is yes. Because in the scenario of using guest_memfd, we allocate the mmap memory region and guest_memfd separately. It is better to save only mmap memory information in GuestRegionMmap.

But now #312 is not merged yet. The implementation of GuestMemory, GuestMemoryMmap only accept instance of GuestRegionMmap so we could not insert a KvmGuestRegion instance.

Thanks to roypat for #312 , hope it can be merged soon.

[tylerfanelli]

The idea I came up with was #312, which would allow the definition of some struct KvmGuestRegion { region: GuestRegionMmap, guest_memfd: Option<File> } wrapper type implementing GuestMemoryRegion that can be managed with GuestMemoryMmap/GuestRegionCollection in #312 (personally, I'd love simply doing impl GuestMemoryRegion for kvm_userspace_memory_region2, but the bitmap tracking stuff ruins these dreams). Does that sounds compatible with what you want to do?

Just adding some thoughts, this sounds reasonable to me. Since it can be managed through GuestMemoryMmap, it can almost become a drop-in "replacement" (although its functionally just a wrapper over) GuestRegionMmap.

[MatiasVara]

Hi Matias, I encountered the same issue with you.

Do you mean that I need to define KvmGuestRegion in my code as a wrapper type implementing GuestMemoryRegion ?

I think it is yes. Because in the scenario of using guest_memfd, we allocate the mmap memory region and guest_memfd separately. It is better to save only mmap memory information in GuestRegionMmap.

But now #312 is not merged yet. The implementation of GuestMemory, GuestMemoryMmap only accept instance of GuestRegionMmap so we could not insert a KvmGuestRegion instance.

I see, thanks for the clarification.

Thanks to roypat for #312 , hope it can be merged soon.

Yes!

Matias

[roypat]

#312 has been merged 🎉

[MatiasVara]

#312 has been merged 🎉

Just saw this! Thanks!