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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Arnd, Greg,

On 11/4/2022 9:19 PM, Trilok Soni wrote:
On 11/4/2022 3:38 PM, Elliot Berman wrote:


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.

I agree w/ Elliot. We would like to keep Gunyah independent and not rely on the existing KVM ioctls space. We should allow new hypervisor drivers interfaces addition in Linux kernel without them relying on KVM.


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.

Agree, tools can be updated and that is the easy part as we grow the s/w stack around Gunyah in userspace, like we already do w/ CrosVM (Virtual Machine Manager) and QEMU will be next followed by rust-vmm. All of them can be done without Gunyah ioctls relying anything on the KVM ioctls. Elliot has also explained very well that we don't to go to KVM maintainers for any of our additions and we also don't want them to come to us, since there is no interoperability testing. It is best that both Hypervisors and their Linux interfaces evolve independently.

Are above explanations reasonable to not re-use KVM ioctl numbers?

Thanks,
Elliot



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux