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