From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: kernel device reset support Date: Wed, 10 Oct 2007 12:23:16 +0200 Message-ID: <470CA814.9050907@qumranet.com> References: <10EA09EFD8728347A513008B6B0DA77A0231BB36@pdsmsx411.ccr.corp.intel.com> <470A0556.80903@qumranet.com> <10EA09EFD8728347A513008B6B0DA77A0231BD83@pdsmsx411.ccr.corp.intel.com> <470B4B2E.1000500@qumranet.com> <10EA09EFD8728347A513008B6B0DA77A0231C1AA@pdsmsx411.ccr.corp.intel.com> <470B5528.2010605@qumranet.com> <10EA09EFD8728347A513008B6B0DA77A0231C1B2@pdsmsx411.ccr.corp.intel.com> <470B5E3B.4060006@qumranet.com> <10EA09EFD8728347A513008B6B0DA77A02364242@pdsmsx411.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel To: "Dong, Eddie" Return-path: In-Reply-To: <10EA09EFD8728347A513008B6B0DA77A02364242-wq7ZOvIWXbNpB2pF5aRoyrfspsVTdybXVpNB7YpNyf8@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.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/