Vring addresses are not inside guest memory range if placed behind a vIOMMU

[sboeuf]

I think there's a limitation for supporting vIOMMU use case because of the following lines:

• https://github.com/rust-vmm/vhost/blob/main/src/vhost_kern/mod.rs#L80
• https://github.com/rust-vmm/vhost/blob/main/src/vhost_kern/mod.rs#L86
• https://github.com/rust-vmm/vhost/blob/main/src/vhost_kern/mod.rs#L92

When an IOTLB is involved, vhost in the kernel considers desc_table_addr, used_ring_addr and avail_ring_addr as IOVAs. That means it could be either a GPA or a GVA. In case it's a GPA, that's fine to check if the address is in range, but if that's a GVA, it doesn't make any sense.

Could we introduce a boolean or flag to let VhostKernBackend know about this use case, which would avoid the incorrect check?

@stefano-garzarella I've identified this issue while implementing vDPA for Cloud Hypervisor, so I was thinking maybe an alternative would be to implement is_valid() from VhostKernVdpa, which would override the default implementation. WDYT?

/cc @jiangliu

[jiangliu]

That's because we have a mechanism to handle IOMMU. We have some draft proposal to extend GuestMemoryMmap by
providing GuestMemoryManager, which provides a mechanism to support IOMMU and virtio-fs DAX window.
https://github.com/openanolis/dragonball-sandbox/blob/main/crates/db-address-space/src/memory/mod.rs#L85

[jiangliu]

When I was designing the vm-memory crate, I proposed to add an GuestAddressSpace abstraction over GuestMemory, so we have an extensible mechanism to deal with guest memory other than DRAM. But that proposal was rejected, so we maintained our own version.

[jiangliu]

And this is a prerequisite to support our GuestMemoryManager, rust-vmm/vm-memory#192

[sboeuf]

I understand and hopefully the improvements that are needed in vm-memory will eventually get accepted, but in the meantime, we need some alternative solution since the upstream GuestMemory doesn't provide what you described.

[stefano-garzarella]

@stefano-garzarella I've identified this issue while implementing vDPA for Cloud Hypervisor, so I was thinking maybe an alternative would be to implement is_valid() from VhostKernVdpa, which would override the default implementation. WDYT?

Yes, I was thinking of doing the same thing. I'm currently implementing a driver in the host so I was going to redefine is_valid() because I had an empty GuestMemory.

Then Jiang suggested how to set a GuestMemory to represent the entire memory of the host process and I didn't need it anymore.

I understand and hopefully the improvements that are needed in vm-memory will eventually get accepted, but in the meantime, we need some alternative solution since the upstream GuestMemory doesn't provide what you described.

I'm in favor of a temporary solution until we have the final one (Jiang's one) accepted.

[sboeuf]

@jiangliu are you okay with a temporary solution for vDPA?
@stefano-garzarella do you want to take care of it, or do you want me to submit a PR for it?

[stefano-garzarella]

@stefano-garzarella do you want to take care of it, or do you want me to submit a PR for it?

@sboeuf If you have time, please go ahead, I'm a little busy this week :-)

[sboeuf]

Sounds good, I'll try to submit something tomorrow.

[sboeuf]

@stefano-garzarella #109

[stefano-garzarella]

Fixed by #109