The patch fixed pin-vcpu issue for VMX domain. The __vmpclear should be done on the last_launch LP. It also migrate the timers for vmx domain. Signed-off-by: Yunhong Jiang yunhong.jiang@intel.com <mailto:yunhong.jiang@intel.com> Signed-off-by: Eddie Dong eddie.dong@intel.com For SVM domain ======The operation for timers may be needed. Thanks Yunhong Jiang _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 8 Feb 2006, at 09:28, Jiang, Yunhong wrote:> The patch fixed pin-vcpu issue for VMX domain. > The __vmpclear should be done on the last_launch LP. It also migrate > the timers for vmx domain. > > > Signed-off-by: Yunhong Jiang yunhong.jiang@intel.com > Signed-off-by: Eddie Dong eddie.dong@intel.comYou don''t need to __vmpclear at the time the affinity is changed. You can still do it from within arch_vmx_do_resume() -- it is valid to call smp_call_function() there. That avoids having yet another HVM function, and avoids calling yet another arch_* function from common code. Rather than calling vmx_remove_timers() from your SMP call function, can you not just call it from vmx_reinstall_timers()? You explain why the pit and hlt timers do not need to be re-activated in vmx_reinstall_timer() -- what about the APIC timer? There''s no need to check active_timer() before calling stop_timer() -- stop_timer does the check for you. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir: Just some curious question. Keir Fraser wrote:> On 8 Feb 2006, at 09:28, Jiang, Yunhong wrote: > > You don''t need to __vmpclear at the time the affinity is changed. You > can still do it from within arch_vmx_do_resume() -- it is valid to > call smp_call_function() there. That avoids having yet another HVM > function, and avoids calling yet another arch_* function from common > code.Good point. Do u mean we check v->arch.hvm_vmx.launch_cpu with v->processor in arch_vmx_do_resume and send IPI if they are not equal? If yes, is it worth to exchage common code change with the frequent check at every resume?> > Rather than calling vmx_remove_timers() from your SMP call function, > can you not just call it from vmx_reinstall_timers()? You explain whySure will look into details. Do u mean removing ac_timer on remote physical processor? Just a little bit worry about the corner case.> the pit and hlt timers do not need to be re-activated in > vmx_reinstall_timer() -- what about the APIC timer?Oo, thanks, a mistake here.> There''s no need to check active_timer() before calling stop_timer() -- > stop_timer does the check for you.Yes, sir!>Eddie _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 8 Feb 2006, at 10:25, Dong, Eddie wrote:>> You don''t need to __vmpclear at the time the affinity is changed. You >> can still do it from within arch_vmx_do_resume() -- it is valid to >> call smp_call_function() there. That avoids having yet another HVM >> function, and avoids calling yet another arch_* function from common >> code. > Good point. Do u mean we check v->arch.hvm_vmx.launch_cpu with > v->processor in arch_vmx_do_resume and send IPI if they are not equal? > If yes, is it worth to exchage common code change with the frequent > check > at every resume?It''s not every return to the guest --- it''s on every context switch to the guest, which will usually be a lot less frequent. It''s worth doing the check there to have tidier interfaces. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>Rather than calling vmx_remove_timers() from your SMP call function, >can you not just call it from vmx_reinstall_timers()?I''m not sure if it will has some corner case, and hope your clarification for it. considering following situation, assume the VMX switch from cpu 0->cpu1 1) on cpu0, the timer interrupt happened, so timer_softirq_action is called, the pit_timer_fn is called, and in pit_timer_fn, it will try to set the pit timer again. 2) before pit_timer_fn set the pit timer on cpu0, the stop_timer is called on cpu1 on vmx_reinstall_timers. 3) after stop_timer finished, the pit_timer_fn on cpu0 set the ac_timer on cpu0. on this situation, the stop_timer on cpu1 will has no effect. And then the init_timer may cause error situation. In fact, I think the old patch itself is not safe on this situation, since the stop_timer is called on IPI interrupt , which may run when the pit_timer_fn is running.>You explain why >the pit and hlt timers do not need to be re-activated in >vmx_reinstall_timer() -- what about the APIC timer? >The APIC timer should be same to PIT timer in future and a patch is needed for this, I had a comments for it and deleted when clean coding style. Thanks Yunhong Jiang>There''s no need to check active_timer() before calling stop_timer() -- >stop_timer does the check for you. > > -- Keir >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 8 Feb 2006, at 12:58, Jiang, Yunhong wrote:> I''m not sure if it will has some corner case, and hope your > clarification for it. > considering following situation, assume the VMX switch from cpu 0->cpu1 > 1) on cpu0, the timer interrupt happened, so timer_softirq_action is > called, the pit_timer_fn is called, and in pit_timer_fn, it will try to > set the pit timer again. > 2) before pit_timer_fn set the pit timer on cpu0, the stop_timer is > called on cpu1 on vmx_reinstall_timers. > 3) after stop_timer finished, the pit_timer_fn on cpu0 set the ac_timer > on cpu0. > > on this situation, the stop_timer on cpu1 will has no effect. And then > the init_timer may cause error situation. > > In fact, I think the old patch itself is not safe on this situation, > since the stop_timer is called on IPI interrupt , which may run when > the > pit_timer_fn is running.Good point. I''ll have to add a migrate_timer() function I think, that does the migration atomically. That''ll save you the hassle of stop/init/reactivate as well. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hi, Keir Attached is updated patch. It use migrate_timer() function and do __vmpclear on arcj_vmx_do_resume. There is still one corner case. Currently the PIT timer is initialized on pit_hook. So when migrate the timer, it maybe possible that the pit timer is still not initialized. However, this situation may happen also on vmx_relinquish_resources. So we plat to provide a patch to init the pit timer on hvm_setup_platform, which is called from vmx_do_launch. Thanks Yunhong Jiang>-----Original Message----- >From: xen-devel-bounces@lists.xensource.com >[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser >Sent: Wednesday, February 08, 2006 11:14 PM >To: Jiang, Yunhong >Cc: xen-devel@lists.xensource.com >Subject: Re: [Xen-devel] [PATCH] Pin vcpu for VMX domain > > >On 8 Feb 2006, at 12:58, Jiang, Yunhong wrote: > >> I''m not sure if it will has some corner case, and hope your >> clarification for it. >> considering following situation, assume the VMX switch from >cpu 0->cpu1 >> 1) on cpu0, the timer interrupt happened, so timer_softirq_action is >> called, the pit_timer_fn is called, and in pit_timer_fn, it >will try to >> set the pit timer again. >> 2) before pit_timer_fn set the pit timer on cpu0, the stop_timer is >> called on cpu1 on vmx_reinstall_timers. >> 3) after stop_timer finished, the pit_timer_fn on cpu0 set >the ac_timer >> on cpu0. >> >> on this situation, the stop_timer on cpu1 will has no >effect. And then >> the init_timer may cause error situation. >> >> In fact, I think the old patch itself is not safe on this situation, >> since the stop_timer is called on IPI interrupt , which may run when >> the >> pit_timer_fn is running. > >Good point. I''ll have to add a migrate_timer() function I think, that >does the migration atomically. >That''ll save you the hassle of stop/init/reactivate as well. > > -- Keir > > >_______________________________________________ >Xen-devel mailing list >Xen-devel@lists.xensource.com >http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 9 Feb 2006, at 08:46, Jiang, Yunhong wrote:> Attached is updated patch. It use migrate_timer() function and > do __vmpclear on arcj_vmx_do_resume. > There is still one corner case. Currently the PIT timer is > initialized on pit_hook. So when migrate the timer, it maybe possible > that the pit timer is still not initialized. However, this situation > may > happen also on vmx_relinquish_resources. So we plat to provide a patch > to init the pit timer on hvm_setup_platform, which is called from > vmx_do_launch.Even vmx_do_launch may be too late, for vmx_relinquish_resources. Consider a VMX domain that is destroyed without ever running -- it will never call vmx_do_launch. However, actually you will be okay because the timer structures are initialised to zeroes during domain creation. That is sufficient for stop_timer/kill_timer/migrate_timer to work properly, so long as they *never* execute concurrently with init_timer. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser wrote:> You don''t need to __vmpclear at the time the affinity is changed. You > can still do it from within arch_vmx_do_resume() -- it is valid to > call smp_call_function() there. That avoids having yet another HVM > function, and avoids calling yet another arch_* function from common > code. >Keir: The latest patch is doing smp_call_function() at time of arch_vmx_do_resume :-) But I get another issue recently that need it to be done earlier. In VMX domain save/restore case, it pauses the VMX domain and then do states save (and destroy). In this case the arch_vmx_do_resume is not invoked, so it looks like doing smp_call_function earlier has other benefits, any suggestion? thx,eddie _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 13 Feb 2006, at 03:49, Dong, Eddie wrote:> But I get another issue recently that need it to be done > earlier. In VMX domain save/restore case, it pauses the VMX domain and > then do states save (and destroy). In this case the arch_vmx_do_resume > is not invoked, so it looks like doing smp_call_function earlier has > other benefits, any suggestion?You should request_clear_vmcs() from within sync_vcpu_execstate(). That''ll need a new ''sync'' VMX hook function. As well as fixing save, it will also mean you no longer need to VMCLEAR in vmx_relinquish_resources() as it will be guaranteed to already be done. If you''re implementing save/restore, one thing I notice is that your state save/restore functions assume the current VMCS references the domain of interest -- they will need to be wrapped in VMPTRST/VMPTRLD if the given vcpu is not current. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser wrote:> On 13 Feb 2006, at 03:49, Dong, Eddie wrote: > >> But I get another issue recently that need it to be done >> earlier. In VMX domain save/restore case, it pauses the VMX domain >> and then do states save (and destroy). In this case the >> arch_vmx_do_resume is not invoked, so it looks like doing >> smp_call_function earlier has other benefits, any suggestion? > > You should request_clear_vmcs() from within sync_vcpu_execstate(). > That''ll need a new ''sync'' VMX hook function. As well as fixing save, > it will also mean you no longer need to VMCLEAR in > vmx_relinquish_resources() as it will be guaranteed to already be > done.Yes, my prefer too. thanks! Eddie _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 13 Feb 2006, at 15:32, Dong, Eddie wrote:>> You should request_clear_vmcs() from within sync_vcpu_execstate(). >> That''ll need a new ''sync'' VMX hook function. As well as fixing save, >> it will also mean you no longer need to VMCLEAR in >> vmx_relinquish_resources() as it will be guaranteed to already be >> done. > Yes, my prefer too. thanks!You''ll also need to retrigger the ''do_relaunch'' path in arch_vmx_do_resume after a sync. Could do that by setting launch_cpu to -1 after sync''ing. That''ll take you down the else path in arch_vmx_do_resume, and all you need to do is have vmx_request_clear_vmcs() detect and ignore launch_cpu==-1. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel