Re: Heads up: More user-unaccessible x86 states?

[Jan Kiszka <jan.kiszka@siemens.com>]

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