Page MenuHomeFreeBSD

arm64: Make local stores observable before sending IPIs
ClosedPublic

Authored by scottph on Jul 25 2020, 4:57 AM.

Details

Summary

Add a synchronizing instruction to flush and wait until the local
CPU's writes are observable to other CPUs before sending IPIs.

This fixes an issue where recipient CPUs doing a rendezvous could
enter the rendezvous handling code before the initiator's writes
to the smp_rv_* variables were visible. This manifested as a
system hang, where a single CPU's increment of smp_rv_waiters[0]
actually happened "before" the initiator's zeroing of that field,
so all CPUs were stuck with the field appearing to be at
ncpus - 1.

MFC after: 1 week
Sponsored by: Ampere Computing, Inc.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This revision is now accepted and ready to land.Jul 25 2020, 6:36 AM

smp_rendezvous_cpus() uses a store-release to update smp_rv_waiters before sending the IPI - why isn't that sufficient?

sys/arm64/arm64/gic_v3.c
920 ↗(On Diff #74903)

Typo, "CPU's".

smp_rendezvous_cpus() uses a store-release to update smp_rv_waiters before
sending the IPI - why isn't that sufficient?

store-release controls the order of visibility of stores done by the initiator,
so if another CPU observes the store to smp_rv_waiters[0] (with load-acquire),
then it must also observe the previous stores. But no guarantees are made about
*when* that store will become visible, and in the particular case I was seeing,
the IPI to another CPU races ahead of the visibility of the smp_rv_waiters[0]
store.

I think ARM gic writes are not ordered with regards to STLR. It is typical in the sense that for instance Intel LAPIC ICR writes in x2APIC mode are also not ordered with normal accesses. We issue MFENCE before ICR write to avoid similar issue there.

Sorry, I missed the reply somehow.

Don't other PIC drivers require a similar change then? i.e., why isn't the change in pic_ipi_send() instead?

scottph retitled this revision from gic_v3: Make local stores observable before sending IPIs to arm64: Make local stores observable before sending IPIs.
scottph edited the summary of this revision. (Show Details)
This revision now requires review to proceed.Aug 24 2020, 11:27 PM

Sorry, I missed the reply somehow.

Don't other PIC drivers require a similar change then? i.e., why isn't the change in pic_ipi_send() instead?

Ah, that definitely seems like it should be the case.

This revision is now accepted and ready to land.Aug 25 2020, 2:21 PM