Re: [RFC PATCH 17/17] KVM: PPC: Add an ioctl for userspace to select

[Alexander Graf <agraf@suse.de>]

On 06/30/2011 05:16 PM, Avi Kivity wrote:
> On 06/30/2011 06:04 PM, Alexander Graf wrote:
>>> +4.64 KVM_PPC_SET_PLATFORM
>>> +
>>> +Capability: none
>>> +Architectures: powerpc
>>> +Type: vm ioctl
>>> +Parameters: struct kvm_ppc_set_platform (in)
>>> +Returns: 0, or -1 on error
>>> +
>>> +This is used by userspace to tell KVM what sort of platform it should
>>> +emulate.  The return value of the ioctl tells userspace whether the
>>> +emulation it is requesting is supported by KVM.
>>> +
>>> +struct kvm_ppc_set_platform {
>>> +    __u16 platform;        /* defines the OS/hypervisor ABI */
>>> +    __u16 guest_arch;    /* e.g. decimal 206 for v2.06 */
>>> +    __u32 flags;
>>
>>
>> Please add some padding so we can extend it later if necessary.
>
> Regarding that.  There's another option - the ioctl code embeds the 
> structure size.  So if we extend the ioctl parsing to pad up (or 
> truncate down) from the user's size to our size, and similarly in the 
> other direction, we can get away from this ugliness.
>
> Some years ago I posted a generic helper that did this (and also 
> kmalloc'ed and kfree'd the data itself), but it wasn't received 
> favourably.  Maybe I should try again (and we can possibly use it in 
> kvm even if it is rejected for general use, though that's against our 
> principles of pushing all generic infrastructure to the wider kernel).

That does sound interesting, but requires a lot more thought to be put 
into the actual code, as we basically need to read out the feature 
bitmap, then provide a minimum size for the chosen features and then 
decide if they fit in.

But yes, that'd still be a lot better than shoving around useless data 
all the time :)


Alex