Add self pause ability, which is required by vcpu0/dom0 when
running on a AP. This can''t be satisfied by existing interface,
since the new flag also serves as a sync point.
Signed-off-by Kevin Tian <kevin.tian@intel.com>
diff -r d5315422dbc8 xen/common/domain.c
--- a/xen/common/domain.c Mon May 14 18:35:31 2007 -0400
+++ b/xen/common/domain.c Mon May 14 20:21:04 2007 -0400
@@ -530,6 +530,17 @@ void vcpu_pause_nosync(struct vcpu *v)
vcpu_sleep_nosync(v);
}
+/* _VPF_need_sync serves not only as flag for sync pause, but also
+ * as hint for other cpu waiting for this pause.
+ */
+void vcpu_pause_self(void)
+{
+ struct vcpu *v = current;
+
+ set_bit(_VPF_need_sync, &v->pause_flags);
+ vcpu_pause_nosync(v);
+}
+
void vcpu_unpause(struct vcpu *v)
{
if ( atomic_dec_and_test(&v->pause_count) )
diff -r d5315422dbc8 xen/common/schedule.c
--- a/xen/common/schedule.c Mon May 14 18:35:31 2007 -0400
+++ b/xen/common/schedule.c Mon May 14 18:54:28 2007 -0400
@@ -691,6 +691,13 @@ void context_saved(struct vcpu *prev)
if ( unlikely(test_bit(_VPF_migrating, &prev->pause_flags)) )
vcpu_migrate(prev);
+
+ if ( unlikely(test_bit(_VPF_need_sync, &prev->pause_flags)) )
+ {
+ sync_vcpu_execstate(prev);
+ /* test and clear can be split, since here is the only clear
point */
+ clear_bit(_VPF_need_sync, &prev->pause_flags);
+ }
}
/* The scheduler timer: force a run through the scheduler */
diff -r d5315422dbc8 xen/include/xen/sched.h
--- a/xen/include/xen/sched.h Mon May 14 18:35:31 2007 -0400
+++ b/xen/include/xen/sched.h Mon May 14 20:21:30 2007 -0400
@@ -457,6 +457,9 @@ extern struct domain *domain_list;
/* VCPU affinity has changed: migrating to a new CPU. */
#define _VPF_migrating 3
#define VPF_migrating (1UL<<_VPF_migrating)
+ /* VCPU needs full context sync once switched out */
+#define _VPF_need_sync 4
+#define VPF_need_sync (1UL<<_VPF_need_sync)
static inline int vcpu_runnable(struct vcpu *v)
{
@@ -467,6 +470,7 @@ static inline int vcpu_runnable(struct v
void vcpu_pause(struct vcpu *v);
void vcpu_pause_nosync(struct vcpu *v);
+void vcpu_pause_self(void);
void domain_pause(struct domain *d);
void vcpu_unpause(struct vcpu *v);
void domain_unpause(struct domain *d);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On 27/6/07 14:37, "Tian, Kevin" <kevin.tian@intel.com> wrote:> Add self pause ability, which is required by vcpu0/dom0 when > running on a AP. This can''t be satisfied by existing interface, > since the new flag also serves as a sync point. > > Signed-off-by Kevin Tian <kevin.tian@intel.com>I think this should not be needed. Why is dom0/vcpu0 special at all? If you are doing the final work from a softirq context, can''t dom0/vcpu0 simply be paused like all others at that point? If not then we''ll need to make some arrangement using vcpu_set_affinity() - I won''t add another flag on the context-switch path. So currently patches 6,7,9,10 are not applied. Patches 6 & 7 because they need more iteration, as commented above. Patches 9 & 10 will likely change when the platform_op hypercall interface is slimmed down, so I''m leaving them out for now. All other patches are in (although the platform_op interface part of patch 2 is disabled). -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>From: Keir Fraser [mailto:keir@xensource.com] >Sent: 2007年7月12日 1:02 > >On 27/6/07 14:37, "Tian, Kevin" <kevin.tian@intel.com> wrote: > >> Add self pause ability, which is required by vcpu0/dom0 when >> running on a AP. This can''t be satisfied by existing interface, >> since the new flag also serves as a sync point. >> >> Signed-off-by Kevin Tian <kevin.tian@intel.com> > >I think this should not be needed. Why is dom0/vcpu0 special at all? If >you >are doing the final work from a softirq context, can''t dom0/vcpu0 simply >be >paused like all others at that point? If not then we''ll need to make some >arrangement using vcpu_set_affinity() - I won''t add another flag on the >context-switch path.I tried to recall the reason for adding this flag. The major reason is that sleep hypercall happens on dom0/vcpu0''s context, while actual enter_state may happen in softirq on idle vcpu context. As a result, we need to update rax as return value to dom0/vcpu0 which means lazy state required flush into per-vcpu guest context before updating. However existing vcpu_pause doesn''t work on self context and vcpu_pause_nosync leaves lazy state there. That''s why a new flag is added to allow lazy context sync-ed after switching out. But after a further thinking, based on the fact that enter_state will force a lazy context flush on all CPUs now, this interface can be abandoned then.> >So currently patches 6,7,9,10 are not applied. Patches 6 & 7 because >they >need more iteration, as commented above. Patches 9 & 10 will likely >change >when the platform_op hypercall interface is slimmed down, so I''m >leaving >them out for now.I''ll resend later.> >All other patches are in (although the platform_op interface part of patch >2 >is disabled). >Thanks so much, Kevin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>From: Tian, Kevin >Sent: 2007年7月12日 10:37 >> >>I think this should not be needed. Why is dom0/vcpu0 special at all? If >>you >>are doing the final work from a softirq context, can''t dom0/vcpu0 simply >>be >>paused like all others at that point? If not then we''ll need to make some >>arrangement using vcpu_set_affinity() - I won''t add another flag on the >>context-switch path. > >I tried to recall the reason for adding this flag. The major reason is that >sleep hypercall happens on dom0/vcpu0''s context, while actual >enter_state may happen in softirq on idle vcpu context. As a result, we >need to update rax as return value to dom0/vcpu0 which means lazy >state required flush into per-vcpu guest context before updating. >However existing vcpu_pause doesn''t work on self context and >vcpu_pause_nosync leaves lazy state there. That''s why a new flag is >added to allow lazy context sync-ed after switching out. > >But after a further thinking, based on the fact that enter_state will force >a lazy context flush on all CPUs now, this interface can be abandoned >then. >Seems issue still existing. It''s possible that force lazy context flush in enter_state is done before dom0/vcpu0 enters context switch, since softirq is sent out before pause. How to find a safe point where we know that dom0/vcpu0 is definitely switched out? Vcpu_set_affinity doesn''t solve the problem, since migrated vcpu won''t continue with previous flow. Or do you mean forcing user to set such affinity explicitly before requesting suspend? Thanks, Kevin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Oh, I can check is_running flag bit of dom0/vcpu0 as a sync point before requesting lazy context flush on all CPUs. :-) Thanks, Kevin>From: Tian, Kevin >Sent: 2007年7月12日 13:06 > >>From: Tian, Kevin >>Sent: 2007年7月12日 10:37 >>> >>>I think this should not be needed. Why is dom0/vcpu0 special at all? If >>>you >>>are doing the final work from a softirq context, can''t dom0/vcpu0 >simply >>>be >>>paused like all others at that point? If not then we''ll need to make >some >>>arrangement using vcpu_set_affinity() - I won''t add another flag on the >>>context-switch path. >> >>I tried to recall the reason for adding this flag. The major reason is that >>sleep hypercall happens on dom0/vcpu0''s context, while actual >>enter_state may happen in softirq on idle vcpu context. As a result, we >>need to update rax as return value to dom0/vcpu0 which means lazy >>state required flush into per-vcpu guest context before updating. >>However existing vcpu_pause doesn''t work on self context and >>vcpu_pause_nosync leaves lazy state there. That''s why a new flag is >>added to allow lazy context sync-ed after switching out. >> >>But after a further thinking, based on the fact that enter_state will force >>a lazy context flush on all CPUs now, this interface can be abandoned >>then. >> > >Seems issue still existing. It''s possible that force lazy context flush >in enter_state is done before dom0/vcpu0 enters context switch, >since softirq is sent out before pause. How to find a safe point where >we know that dom0/vcpu0 is definitely switched out? > >Vcpu_set_affinity doesn''t solve the problem, since migrated vcpu >won''t continue with previous flow. Or do you mean forcing user to set >such affinity explicitly before requesting suspend? > >Thanks, >Kevin_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Oh, by the way, shouldn''t the lazy context flush be driven via cpu hot-unplug? That would seem the more general place to do the flush. -- Keir On 12/7/07 07:02, "Tian, Kevin" <kevin.tian@intel.com> wrote:> Oh, I can check is_running flag bit of dom0/vcpu0 as a sync point > before requesting lazy context flush on all CPUs. :-) > > Thanks, > Kevin > >> From: Tian, Kevin >> Sent: 2007年7月12日 13:06 >> >>> From: Tian, Kevin >>> Sent: 2007年7月12日 10:37 >>>> >>>> I think this should not be needed. Why is dom0/vcpu0 special at all? If >>>> you >>>> are doing the final work from a softirq context, can''t dom0/vcpu0 >> simply >>>> be >>>> paused like all others at that point? If not then we''ll need to make >> some >>>> arrangement using vcpu_set_affinity() - I won''t add another flag on the >>>> context-switch path. >>> >>> I tried to recall the reason for adding this flag. The major reason is that >>> sleep hypercall happens on dom0/vcpu0''s context, while actual >>> enter_state may happen in softirq on idle vcpu context. As a result, we >>> need to update rax as return value to dom0/vcpu0 which means lazy >>> state required flush into per-vcpu guest context before updating. >>> However existing vcpu_pause doesn''t work on self context and >>> vcpu_pause_nosync leaves lazy state there. That''s why a new flag is >>> added to allow lazy context sync-ed after switching out. >>> >>> But after a further thinking, based on the fact that enter_state will force >>> a lazy context flush on all CPUs now, this interface can be abandoned >>> then. >>> >> >> Seems issue still existing. It''s possible that force lazy context flush >> in enter_state is done before dom0/vcpu0 enters context switch, >> since softirq is sent out before pause. How to find a safe point where >> we know that dom0/vcpu0 is definitely switched out? >> >> Vcpu_set_affinity doesn''t solve the problem, since migrated vcpu >> won''t continue with previous flow. Or do you mean forcing user to set >> such affinity explicitly before requesting suspend? >> >> Thanks, >> Kevin > > _______________________________________________ > 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 12/7/07 06:05, "Tian, Kevin" <kevin.tian@intel.com> wrote:>> But after a further thinking, based on the fact that enter_state will force >> a lazy context flush on all CPUs now, this interface can be abandoned >> then. >> > > Seems issue still existing. It''s possible that force lazy context flush > in enter_state is done before dom0/vcpu0 enters context switch, > since softirq is sent out before pause. How to find a safe point where > we know that dom0/vcpu0 is definitely switched out?How about doing the whole suspend/resume in dom0/vcpu0 context? Why switch to a softirq at all? You can force dom0/vcpu0 onto cpu0 temporarily by wrapping the suspend/resume in a pair of calls to vcpu_set_affinity(). If your register save/restore across the low-level S3 entry/exit is comprehensive, then it should be fine to do it in dom0/vcpu0 context. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 12/7/07 08:44, "Keir Fraser" <keir@xensource.com> wrote:> How about doing the whole suspend/resume in dom0/vcpu0 context? Why switch > to a softirq at all? You can force dom0/vcpu0 onto cpu0 temporarily by > wrapping the suspend/resume in a pair of calls to vcpu_set_affinity().You may need to temporarily rewrite ->schedule_tail as well, to regain control after the scheduling softirq. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>From: Keir Fraser [mailto:keir@xensource.com] >Sent: 2007年7月12日 15:41 > >Oh, by the way, shouldn''t the lazy context flush be driven via cpu >hot-unplug? That would seem the more general place to do the flush. > > -- KeirAgree. Thanks, Kevin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>From: Keir Fraser [mailto:keir@xensource.com] >Sent: 2007年7月12日 16:01 > >On 12/7/07 08:44, "Keir Fraser" <keir@xensource.com> wrote: > >> How about doing the whole suspend/resume in dom0/vcpu0 context? >Why switch >> to a softirq at all? You can force dom0/vcpu0 onto cpu0 temporarily by >> wrapping the suspend/resume in a pair of calls to vcpu_set_affinity(). > >You may need to temporarily rewrite ->schedule_tail as well, to regain >control after the scheduling softirq. > > -- KeirYes, schedule_tail has to be overridden with a special stub to continue with previous suspend flow. Maybe we can make it generic, for example, to create a new vcpu_migrate_and_continue, which basically: - save continue point to vcpu structure - override schedule_tail with a helper function - switch out Then that helper function jumps to continue point on new processor. At finish, reset stack and recover schedule_tail. Is it sounds like the way? Thanks, Kevin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 12/7/07 09:23, "Tian, Kevin" <kevin.tian@intel.com> wrote:> Yes, schedule_tail has to be overridden with a special stub to continue > with previous suspend flow. Maybe we can make it generic, for example, > to create a new vcpu_migrate_and_continue, which basically: > - save continue point to vcpu structure > - override schedule_tail with a helper function > - switch out > Then that helper function jumps to continue point on new processor. At > finish, reset stack and recover schedule_tail. > > Is it sounds like the way?I don''t mind whether you opn code this in acpi/power.c, or add a new interface function. Whichever works out best. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel