Re: [PATCH v6 13/21] gunyah: vm_mgr: Introduce basic VM Manager

From: Elliot Berman
Date: Fri Nov 04 2022 - 18:38:22 EST




On 11/4/2022 1:10 AM, Arnd Bergmann wrote:
On Fri, Nov 4, 2022, at 01:11, Elliot Berman wrote:
On 11/2/2022 5:24 PM, Greg Kroah-Hartman wrote:
On Wed, Nov 02, 2022 at 11:45:12AM -0700, Elliot Berman wrote:

Even if you don't support it 1:1, at least for the ones that are the
same thing, pick the same numbers as that's a nicer thing to do, right?


Does same thing == interpretation of arguments is the same? For
instance, GH_CREATE_VM and KVM_CREATE_VM interpret the arguments
differently. Same for KVM_SET_USERSPACE_MEMORY. The high level
functionality should be similar for most all hypervisors since they will
all support creating a VM and probably sharing memory with that VM. The
arguments for that will necessarily look similar, but they will probably
be subtly different because the hypervisors support different features.

I think in the ideal case, you should make the arguments and the
command codes the same for any command where that is possible. If
you come across a command that is shared with KVM but just needs
another flag, that would involve coordinating with the KVM maintainers
about sharing the definition so the same flag does not get reused
in an incompatible way.


I think the converse also needs to be true; KVM would need to check that
new flags don't get used in some incompatible way with Gunyah, even if
one of us is just -EINVAL'ing. I don't think Gunyah and KVM should be
reliant on the other reviewing shared ioctls.

The problem is a bit worse because "machine type" is architecture-
dependent whereas the planned Gunyah flags are architecture-independent.
KVM within itself re-uses flags between architectures so Gunyah would
need to reserve some flags from all architectures that KVM supports.

For commands that cannot fit into the existing definition, there
should be a different command code, using your own namespace and
not the 0xAE block that KVM has. It still makes sense to follow
the argument structure roughly here, unless there is a technical
reason for making it different.

I don't think userspace that supports both KVM and Gunyah will benefit
much from re-using the same numbers since those re-used ioctl calls
still need to sit within the context of a Gunyah VM.

One immediate benefit is for tools that work on running processes,
such as strace, gdb or qemu-user. If they encounter a known command,
they can correctly display the arguments etc.


We can update these tools and anyway there will be different ioctls to
get started. There are important ioctls that wouldn't be correctly
displayed off the bat anyway; work would need to be done to support the
Gunyah ioctls either way. Whereas tooling update is temporary, the
coupling of KVM and Gunyah ioctls would be permanent.

Another benefit is for sharing portions of the VMM that live in
outside processes like vhost-user based device emulation that
interacts with irqfd, memfd etc. The more similar the command
interface is, the easier it gets to keep these tools portable.


Hypervisor interfaces already have different ioctls for equivalent
functionality [1], so VMMs that want to scale to multiple hypervisors
already abstract out ioctl-level interfaces so there wouldn't be any
code-reuse even if Gunyah and KVM shared the same ioctl number. Between
hypervisors, the best case is there is design similarity for userspace,
which makes it easier to add new hypervisor support for VMMs and that's
what we are aiming for.

[1]: e.g. compare KVM, acrn, xen for implementing virtual interrupts.
KVM and acrn use independently implemented irqfd interfaces, xen has
totally different implementation called event channels.

Thanks,
Elliot