Avi Kivity wrote:
> On 10/05/2009 01:18 PM, Jan Kiszka wrote:
>> Avi Kivity wrote:
>>
>>> On 10/05/2009 09:43 AM, Jan Kiszka wrote:
>>>
>>>> Avi Kivity wrote:
>>>>
>>>>
>>>>> On 10/04/2009 09:07 PM, Jan Kiszka wrote:
>>>>>
>>>>>
>>>>>>> btw, instead of adding a new ioctl, perhaps it makes sense to define a
>>>>>>> new KVM_VCPU_STATE structure that holds all current and future state
>>>>>>> (with generous reserved space), instead of separating state over a
>>>>>>> dozen
>>>>>>> ioctls.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> OK, makes sense. With our without lapic state?
>>>>>>
>>>>>>
>>>>> I'm in two minds. I'm leaning towards including lapic but would welcome
>>>>> arguments either way.
>>>>>
>>>>>
>>>> The lapic is optional and, thus, typically handled in different code
>>>> modules by user space. QEMU even creates a separate device that holds
>>>> the state.
>>>>
>>> avx registers, nested vmx are optional as well.
>>>
>>>
>>>> I'm not sure user space will benefit from a unified query/set
>>>> interface with regard to this.
>>>>
>>>>
>>> The main benefit is to avoid creating an ioctl each time we find a
>>> missing bit.
>>>
>>>
>>>>> Note we have to be careful with timers such as the tsc and lapic timer.
>>>>> Maybe have a bitmask at the front specifying which elements are active.
>>>>>
>>>>>
>>>> ...and the lapic timers are another argument.
>>>>
>>>> Regarding TSC, which means MSRs: I tend to include only states into the
>>>> this meta state which have fixed sizes. Otherwise things will get very
>>>> hairy. And the GET/SET_MSRS interface is already fairly flexible, the
>>>> question would be again: What can we gain by unifying?
>>>>
>>>>
>>> For MSRs, not much.
>>>
>>> Note we can make it work, by storing an offset/length at a fixed
>>> location and letting userspace point it into the reserved area.
>>>
>> Hmm, pointers... That makes me think of a meta IOCTL like this:
>>
>> struct kvm_vcpu_state {
>> int substates;
>> void __user *substate[0];
>> };
>>
>>
>
> True pointers are no go since we have to deal with compat code (31/32
> bit userspace calling into a 64 bit kernel). Offsets into the state
> structure are easier.
So current KVM_GET_DIRTY_LOG is broken in the compat case...
>
>> #define KVM_VCPU_STATE_REGS 0 /* i.e. substate[0] points to kvm_regs */
>> #define KVM_VCPU_STATE_SREGS 1
>> #define KVM_VCPU_STATE_LAPIC 2
>> ...
>>
>> We could easily extend the call with more substates just by defining new
>> pointer slots. Moreover, user space could define which substates should
>> be get/set by simply passing NULL or a valid pointer for substate[n] (or
>> by limiting the substates field).
>>
>> The only ugliness I see is the missing type safety as we would have to
>> deal with void pointers to the substate structures here.
>>
>
> For fixed sized state a feature bitmap is sufficient I think.
>
We'll probably have to deal with both. Therefore, I'm looking for a
unified solution.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
