Re: kernel device reset support

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Dong, Eddie wrote:
>>>
>>> +
>>> +/*
>>> + * Reset VM.
>>> + *
>>> + */
>>> +int kvm_vm_reset(struct kvm *kvm)
>>> +{
>>> +	struct kvm_vcpu *vcpu;
>>> +	int i;
>>> +
>>> +	kvm_reset_devices(kvm);
>>> +	for (i = 0; i < KVM_MAX_VCPUS; i++) {
>>> +		vcpu = kvm->vcpus[i];
>>> +		if (!vcpu)
>>> +			continue;
>>> +		/* active VCPU */
>>> +		if (vcpu->vcpu_id)
>>> +			vcpu->mp_state = VCPU_MP_STATE_UNINITIALIZED;
>>> +		else {
>>> +			vcpu->mp_state = VCPU_MP_STATE_RUNNABLE;
>>> +			kvm_lapic_reset(vcpu);
>>>
>>>       
>> Why is the handling here different?
>>     
>
> In native, RESET signal force every processor enter "RESET" status, and
> then immediately after RESET signal is removed, all CPUs will compete
> for BSP role, the winner continue execution, and failor will be blocked
> till INIT/SIPI/SIPI.
>   

I meant, you could set both to VCPU_MP_STATE_UNINITIALIZED and let the 
vcpu code do the reset differently depending on vcpu_id.  This way you 
don't run into locking issues (kvm_lapic_reset() needs the vcpu lock?)

>
>>>  {
>>>  	++vcpu->stat.irq_exits;
>>> +	if (vcpu->force_to_quit) {
>>> +		vcpu->force_to_quit = 0;
>>> +		return -EINTR;
>>> +	}
>>>  	return 1;
>>>  }
>>>
>>> 	
>> Why is this needed?
>>     
>
> For a graceful reboot, this one is not needed since every APs are
> already brought to HALT status before BSP issue RESET signal. But in
> case of non-graceful reboot, it is possible the VCPUs are still
> executing guest instruction, so we kick the VCPU and then use this logic
> to force the exception handler to be a heavy VM Exit and execute
> following code at beginning of kvm_vcpu_ioctl_run. (Let
> VCPU_MP_STATE_UNINITIALIZED take effective)
>
>         if (unlikely(vcpu->mp_state == VCPU_MP_STATE_UNINITIALIZED)) {
>                 if (irqchip_in_kernel(vcpu->kvm) && vcpu->apic)
>                         kvm_lapic_reset(vcpu);
>                 kvm_vcpu_block(vcpu);
>                 vcpu_put(vcpu);
>                 return -EAGAIN;
>         }
>
> Whether we need to handle those un-graceful reboot case is up to you :-)
> If not, then those code can be removed.
>   

We do need to support ungraceful resets.  But this could easily be done 
via vcpu->requests.

To reset a vcpu:

    set_bit(KVM_REQ_RESET, &vcpu->requests)
    kvm_vcpu_kick(vcpu);

And in __vcpu_run():

   if (vcpu_requests) {
         if (test_and_reset_bit(KVM_REQ_RESET, &vcpu->requests))
            ....
   }

>   
>> Userspace can always force an exit by sending a signal.
>>
>>     
>>> +/*
>>> + * Reset VM.
>>> + *
>>> + */
>>> +int kvm_vm_reset(struct kvm *kvm)
>>> +{
>>> +	struct kvm_vcpu *vcpu;
>>> +	int i;
>>> +
>>> +	kvm_reset_devices(kvm);
>>>
>>>       
>> Need to take kvm->lock around this.
>>     
>
> Mmm, I will move this to be after VCPU reset.
> If a VCPU is still accessing (write)  devices register, we always have
> problem.
> So move after all the processors enetring frozen state will be simple
> and safer.
>
> Any opnion? Will post after your new comments.
>
>   

Sounds good.  But the BSP starts executing immediately, no?

So maybe two stages for vcpu reset:  first to reset and halt it, then 
start it.  pic and ioapic reset can be performed in between.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/