Re: [PATCH] Activate Virtualization On Demand v2

[Alexander Graf <agraf@suse.de>]

Avi Kivity wrote:
> Alexander Graf wrote:
>
>  
>
>>> We'll be in a nice fix if we can only enable virtualization on some
>>> processors; that's the reason hardware_enable() was originally
>>> specified as returning void.
>>>
>>> I don't see an easy way out, but it's hardly a likely event.
>>>     
>>
>> I don't think there's any way we can circumvent that.
>>   
>
> No.  We can live with it though.
>
>> What I've wanted to ask for some time already: How does suspend/resume
>> work? 
>
> The question is important, even without the first word.
>
>> I only see one suspend/resume hook that disables virt on the
>> currently running CPU. Why don't we have to loop through the CPUs to
>> enable/disable all of them?
>> At least for suspend-to-disk this sounds pretty necessary.
>>
>>   
>
> Suspend first offlines all other cpus.

Ah, ok.

>>>>  {
>>>> -    hardware_enable(NULL);
>>>> +    if (atomic_read(&kvm_usage_count))
>>>> +        hardware_enable(NULL);
>>>>      return 0;
>>>>  }
>>>>         
>>> Move the test to hardware_enable()?  It's repeated too often.
>>>     
>>
>> What do we do about the on_each_cpu(hardware_enable) cases? We couldn't
>> tell when to activate/deactive virtualization then, as that's
>> semantically bound to "amount of VMs".
>>   
>  static int kvm_resume(struct sys_device *dev)
>
> I don't understand.  Moving the test to within the IPI shouldn't
> affect anything.

Oh, you only want the test to be in hardware_enable and
hardware_disable. Now I see what you mean: modify and lock
kvm_usage_count outside, but test inside of hardware_enable.