Re: kernel device reset support

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

Dong, Eddie wrote:
> Avi Kivity wrote:
>   
>> Dong, Eddie wrote:
>>     
>>> Avi Kivity wrote:
>>>
>>>       
>>>> But, for an ungraceful reset, nothing prevents an AP from
>>>> issuing a reset?
>>>>
>>>>         
>>> Mmm, Yes, but I think current architecture can't handle this.
>>> The thread where AP issues "RESET" will continue run, which
>>> means it becomes BSP now and wake up other APs later on.
>>> Or We can block that AP first and then inform BSP to do
>>> RESET job. Here we need to block the AP in kernel
>>> so that we can wake up.
>>>
>>>       
>> It should call vcpu_halt() immediately after reset.
>>     
>
> ?? Can user level be able to enter kernel HALT state.
>
>   

It's just like the guest kernel executing hlt.  Why is there a difference?

>>> It can be a future task which is not that high priority IMO.
>>> I will focus on SMP boot 1st. Your opnion?
>>>
>>>       
>> Agree.  But let's make it close to the complete solution.
>>
>>     
>
> Yes, halt all APs and let BSP do reset ops in user level. 
> Will post patch to Qemu to support SMP reboot some time later.
>
>   

Wait, that's a big change.  Need to think about this...

>> The test for vcpu->requests already exists (and is needed for tlb
>> flushes) so there is no additional performance hit.
>>     
>
> OK, that makes sense. changed. please verify.
> User level is split into libkvm and qemu.
>
>  /*
>   * Address types:
> diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
> index 0b2894a..33f16bd 100644
> --- a/drivers/kvm/kvm_main.c
> +++ b/drivers/kvm/kvm_main.c
> @@ -2189,9 +2189,17 @@ again:
>  
>  	vcpu->guest_mode = 1;
>  
> -	if (vcpu->requests)
> +	if (vcpu->requests) {
>  		if (test_and_clear_bit(KVM_TLB_FLUSH, &vcpu->requests))
>  			kvm_x86_ops->tlb_flush(vcpu);
> +		if (test_and_clear_bit(KVM_FROZEN, &vcpu->requests)) {
> +			local_irq_enable();
> +			preempt_enable();
> +			r = -EINTR;
> +			kvm_run->exit_reason = KVM_EXIT_FROZEN;
> +			goto out;
> +		}
> +	}
>   


Why not just call vcpu_reset() here, then call vcpu_halt() if we're an AP?

>  
> +/*
> + * Kernel side VM Reset.
> + * NOTE here: User level code must guarantee only the BSP
> + * thread can do this call.
> + *
> + */
> +int kvm_vm_reset(struct kvm *kvm)
> +{
> +	struct kvm_vcpu *vcpu;
> +	int i;
> +
> +	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;
> +			set_bit(KVM_FROZEN, &vcpu->requests);
> +			kvm_vcpu_kick(vcpu);
> +			/*
> +			 * Wait till the AP entered waiting for
> +			 * INIT/SIPI state
> +			 */
> +			while (test_bit(KVM_FROZEN, &vcpu->requests))
> +				schedule();
>   

I don't think we need to wait here.

> +		}
> +		else {
> +			vcpu->mp_state = VCPU_MP_STATE_RUNNABLE;
> +			kvm_lapic_reset(vcpu);
> +		}
> +	}
> +	/* Now only BSP is running... */
> +	kvm_reset_devices(kvm);
>   

But now you're reseting the devices while vcpu 0 may be running.  If in 
the first stage you halt all vcpus, and then restart vcpu 0 after the 
reset, you avoid the race.


-- 
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/