On 28/01/21 17:49, David Woodhouse wrote:
On Thu, 2021-01-28 at 13:18 +0100, Paolo Bonzini wrote:
My only qualm is really that the userspace API is really ugly.
Can you just have both a VM and a VCPU ioctl (so no vcpu_id to pass!),
Sure, that seems like a sensible thing to do.
add a generous padding to the struct,
I think I added *some* padding to the struct kvm_xen_hvm_attr which
wasn't there in Joao's original. I can add more, certainly.
and just get everything out with a
single ioctl without having to pass in a type?
Honestly, I don't even care about reading it out except for long_mode
which the kernel *does* infer for itself when the MSR is used to fill
in the hypercall page.
What about VM migration?
I quite like keeping them separate; they *do* get set separately, in
response to different hypercalls from the guest. And the capabilities
translate naturally to a given field existing or not existing; having
another mapping of that to fields in a binary structure would be
additional complexity.
Does it make sense to reuse the bits that you return from
KVM_CHECK_EXTENSION as a bitset for both the get and set ioctls? The
struct then would be
struct kvm_xen_attr {
uint32_t valid;
uint32_t lm;
struct {
} ...;
uint8_t pad[nnn /* to 128 bytes */];
};
The get ioctl would return a constant value in "valid" (different for
the VM and VCPU ioctls of course), the set ioctl would look only at the
fields mentioned in "valid" and error out if they're unsupported or
invalid for VM/VCPU. Basically the "switch" you have becomes a series
of "if (attr->valid & ...)" statements.
Paolo
