| Message ID | 20200922192824.19619-1-julien@xen.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: sched: Ensure the vCPU context is seen before vcpu_pause() returns | expand |
Doh, I forgot to CC Stefano on the e-mail. I will resend it. Sorry for the inconvenience. On 22/09/2020 20:28, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Some callers of vcpu_pause() will expect to access the latest vcpu > context when the function returns (see XENDOMCTL_{set,get}vcpucontext}. > > However, the latest vCPU context can only be observed after > v->is_running has been observed to be false. > > As there is no memory barrier instruction generated, a processor could > try to speculatively access the vCPU context before it was observed. > > To prevent the corruption of the vCPU context, we need to insert a > memory barrier instruction after v->is_running is observed and before > the context is accessed. This barrier is added in sync_vcpu_execstate() > as it seems to be the place where we expect the synchronization to > happen. > > Signed-off-by: Julien Grall <jgrall@amazon.com> > > --- > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: George Dunlap <george.dunlap@citrix.com> > Cc: Dario Faggioli <dfaggioli@suse.com> > Cc: Bertrand Marquis <Bertrand.Marquis@arm.com> > > I am also adding the x86 and scheduler maintainers because I am not sure > whether this barrier should be part of the common code instead. > --- > xen/arch/arm/domain.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 9258f6d3faa2..3b37f899b9da 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -371,7 +371,20 @@ void sync_local_execstate(void) > > void sync_vcpu_execstate(struct vcpu *v) > { > - /* Nothing to do -- no lazy switching */ > + /* > + * We don't support lazy switching. > + * > + * However the context may have been saved from a remote pCPU so we > + * need a barrier to ensure it is observed before continuing. > + * > + * Per vcpu_context_saved(), the context can be observed when > + * v->is_running is false (the caller should check it before calling > + * this function). > + * > + * Note this is a full barrier to also prevent update of the context > + * to happen before it was observed. > + */ > + smp_mb(); > } > > #define NEXT_ARG(fmt, args) \ >
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 9258f6d3faa2..3b37f899b9da 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -371,7 +371,20 @@ void sync_local_execstate(void) void sync_vcpu_execstate(struct vcpu *v) { - /* Nothing to do -- no lazy switching */ + /* + * We don't support lazy switching. + * + * However the context may have been saved from a remote pCPU so we + * need a barrier to ensure it is observed before continuing. + * + * Per vcpu_context_saved(), the context can be observed when + * v->is_running is false (the caller should check it before calling + * this function). + * + * Note this is a full barrier to also prevent update of the context + * to happen before it was observed. + */ + smp_mb(); } #define NEXT_ARG(fmt, args) \