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/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
