Clarification on necessary barriers before generating IPI

Clarification on necessary barriers before generating IPI

[Linu Cherian]

Hi,

How is it ensured that system register write using msr instruction(gic_send_sgi)
doesnt get reordered before the stores to IPI call processing
list(call_single_queue in kernel/smp.c), so that IPI is guaranteed to
be generated after the stores get completed.

CMIIW, Dont we need an isb() in addition to to the wmb() in the below code ?

Thanks.

static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
{
int cpu;

if (WARN_ON(irq >= 16))
return;

/*
* Ensure that stores to Normal memory are visible to the
* other CPUs before issuing the IPI.
*/
wmb();

for_each_cpu(cpu, mask) {
u64 cluster_id = MPIDR_TO_SGI_CLUSTER_ID(cpu_logical_map(cpu));
u16 tlist;

tlist = gic_compute_target_list(&cpu, mask, cluster_id);
gic_send_sgi(cluster_id, tlist, irq);
}

/* Force the above writes to ICC_SGI1R_EL1 to be executed */
isb();
}

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Clarification on necessary barriers before generating IPI

[Will Deacon]

On Wed, May 20, 2020 at 01:38:24PM +0530, Linu Cherian wrote:
> How is it ensured that system register write using msr instruction(gic_send_sgi)
> doesnt get reordered before the stores to IPI call processing
> list(call_single_queue in kernel/smp.c), so that IPI is guaranteed to
> be generated after the stores get completed.

I think the flow is:

	<store to memory>
	DSB	ST
	MSR	SGI1R
	ISB

and then on the receiver:

	<interrupt; implicit ISB/context sync>
	MRS	IAR
	DSB	SY
	<control dependency>
	MSR	EOIR
	ISB
	<handle IPI>

> CMIIW, Dont we need an isb() in addition to to the wmb() in the below code ?

There is an isb?() in the code, after the loop. Are you saying it should be
somewhere else? If so, why?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Clarification on necessary barriers before generating IPI

[Linu Cherian]

Hi Will,

On Wed, May 20, 2020 at 1:59 PM Will Deacon <will@kernel.org> wrote:
>
> On Wed, May 20, 2020 at 01:38:24PM +0530, Linu Cherian wrote:
> > How is it ensured that system register write using msr instruction(gic_send_sgi)
> > doesnt get reordered before the stores to IPI call processing
> > list(call_single_queue in kernel/smp.c), so that IPI is guaranteed to
> > be generated after the stores get completed.
>
> I think the flow is:
>
>         <store to memory>
>         DSB     ST

Dont we need an extra ISB here to ensure that the subsequent MSR SGI1R doesnt
get executed before <store to memory> and DSB ST ?

This is on the assumption that DSB ST doesnt enforce the ordering of MSR SGI1R.


>         MSR     SGI1R
>         ISB
>
> and then on the receiver:
>
>         <interrupt; implicit ISB/context sync>
>         MRS     IAR
>         DSB     SY
>         <control dependency>
>         MSR     EOIR
>         ISB
>         <handle IPI>
>
> > CMIIW, Dont we need an isb() in addition to to the wmb() in the below code ?
>
> There is an isb?() in the code, after the loop. Are you saying it should be
> somewhere else? If so, why?

Please see above.

>
> Will

Thanks.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Clarification on necessary barriers before generating IPI

[Will Deacon]

On Wed, May 20, 2020 at 02:23:25PM +0530, Linu Cherian wrote:
> On Wed, May 20, 2020 at 1:59 PM Will Deacon <will@kernel.org> wrote:
> >
> > On Wed, May 20, 2020 at 01:38:24PM +0530, Linu Cherian wrote:
> > > How is it ensured that system register write using msr instruction(gic_send_sgi)
> > > doesnt get reordered before the stores to IPI call processing
> > > list(call_single_queue in kernel/smp.c), so that IPI is guaranteed to
> > > be generated after the stores get completed.
> >
> > I think the flow is:
> >
> >         <store to memory>
> >         DSB     ST
> 
> Dont we need an extra ISB here to ensure that the subsequent MSR SGI1R doesnt
> get executed before <store to memory> and DSB ST ?
> 
> This is on the assumption that DSB ST doesnt enforce the ordering of MSR SGI1R.

I don't think that's a valid assumption. The architecture says:

  | A DSB instruction executed by a PE, PEe, completes when [...] all explicit
  | memory accesses of the required access types appearing in program order
  | before the DSB are complete for the set of observers in the required
  | shareability domain.

and:

  | In addition, no instruction that appears in program order after the DSB
  | instruction can alter any state of the system or perform any part of its
  | functionality until the DSB completes other than:
  |
  | * Being fetched from memory and decoded.
  | * Reading the general-purpose, SIMD and floating-point, Special-purpose, or
  |   System registers that are directly or indirectly read without causing
  |   side-effects.

Are you seeing a problem in practice?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Clarification on necessary barriers before generating IPI

[Linu Cherian]

Hi Will,

On Wed, May 20, 2020 at 2:33 PM Will Deacon <will@kernel.org> wrote:
>
> On Wed, May 20, 2020 at 02:23:25PM +0530, Linu Cherian wrote:
> > On Wed, May 20, 2020 at 1:59 PM Will Deacon <will@kernel.org> wrote:
> > >
> > > On Wed, May 20, 2020 at 01:38:24PM +0530, Linu Cherian wrote:
> > > > How is it ensured that system register write using msr instruction(gic_send_sgi)
> > > > doesnt get reordered before the stores to IPI call processing
> > > > list(call_single_queue in kernel/smp.c), so that IPI is guaranteed to
> > > > be generated after the stores get completed.
> > >
> > > I think the flow is:
> > >
> > >         <store to memory>
> > >         DSB     ST
> >
> > Dont we need an extra ISB here to ensure that the subsequent MSR SGI1R doesnt
> > get executed before <store to memory> and DSB ST ?
> >
> > This is on the assumption that DSB ST doesnt enforce the ordering of MSR SGI1R.
>
> I don't think that's a valid assumption. The architecture says:
>
>   | A DSB instruction executed by a PE, PEe, completes when [...] all explicit
>   | memory accesses of the required access types appearing in program order
>   | before the DSB are complete for the set of observers in the required
>   | shareability domain.
>
> and:
>
>   | In addition, no instruction that appears in program order after the DSB
>   | instruction can alter any state of the system or perform any part of its
>   | functionality until the DSB completes other than:
>   |
>   | * Being fetched from memory and decoded.
>   | * Reading the general-purpose, SIMD and floating-point, Special-purpose, or
>   |   System registers that are directly or indirectly read without causing
>   |   side-effects.
>

Thats clear. Thanks.


> Are you seeing a problem in practice?
>

Just came across this code, while trying to debug something related.
Will let you know if i have further followup.

> Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel