| Message ID | 20200907181659.92449-6-seanga2@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | riscv: Correctly handle IPIs already pending upon boot | expand |
Hi Sean > Without these fences, it is perfectly valid for an out-of-order core to > re-order memory accesses to outside of the available_harts_lock critical > section. > > Signed-off-by: Sean Anderson <seanga2@gmail.com> > --- > > arch/riscv/cpu/start.S | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > index e3222b1ea7..ad18e746b6 100644 > --- a/arch/riscv/cpu/start.S > +++ b/arch/riscv/cpu/start.S > @@ -126,12 +126,12 @@ call_board_init_f_0: > #ifndef CONFIG_XIP > la t0, available_harts_lock > fence rw, w > - amoswap.w zero, zero, 0(t0) > + amoswap.w.rl zero, zero, 0(t0) fence rw, w can also be removed. > > wait_for_gd_init: > la t0, available_harts_lock > li t1, 1 > -1: amoswap.w t1, t1, 0(t0) > +1: amoswap.w.aq t1, t1, 0(t0) > fence r, rw fence r, rw can also be removed. > bnez t1, 1b > > @@ -143,7 +143,7 @@ wait_for_gd_init: > SREG t2, GD_AVAILABLE_HARTS(gp) > > fence rw, w fence rw, w can also be removed. Thanks, Rick > - amoswap.w zero, zero, 0(t0) > + amoswap.w.rl zero, zero, 0(t0) > > /* > * Continue on hart lottery winner, others branch to > -- > 2.28.0 >
On 9/9/20 11:26 PM, Rick Chen wrote: > Hi Sean > >> Without these fences, it is perfectly valid for an out-of-order core to >> re-order memory accesses to outside of the available_harts_lock critical >> section. >> >> Signed-off-by: Sean Anderson <seanga2@gmail.com> >> --- >> >> arch/riscv/cpu/start.S | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S >> index e3222b1ea7..ad18e746b6 100644 >> --- a/arch/riscv/cpu/start.S >> +++ b/arch/riscv/cpu/start.S >> @@ -126,12 +126,12 @@ call_board_init_f_0: >> #ifndef CONFIG_XIP >> la t0, available_harts_lock >> fence rw, w >> - amoswap.w zero, zero, 0(t0) >> + amoswap.w.rl zero, zero, 0(t0) > > fence rw, w can also be removed. Hmm. Actually, upon reading the spec I noticed this paragraph > To help implement multiprocessor synchronization, the AMOs optionally > provide release consistency semantics. If the aq bit is set, then no > later memory operations in this RISC-V hart can be observed to take > place before the AMO. Conversely, if the rl bit is set, then other > RISC-V harts will not observe the AMO before memory accesses preceding > the AMO in this RISC-V hart. Setting both the aq and the rl bit on an > AMO makes the sequence sequentially consistent, meaning that it cannot > be reordered with earlier or later memory operations from the same > hart. The key word being optionally. However, I could not find any information on how to detect if this option is enabled. Perhaps a separate fence was used to prevent having to try and detect this feature? >> >> wait_for_gd_init: >> la t0, available_harts_lock >> li t1, 1 >> -1: amoswap.w t1, t1, 0(t0) >> +1: amoswap.w.aq t1, t1, 0(t0) >> fence r, rw > > fence r, rw can also be removed. > >> bnez t1, 1b >> >> @@ -143,7 +143,7 @@ wait_for_gd_init: >> SREG t2, GD_AVAILABLE_HARTS(gp) >> >> fence rw, w > > fence rw, w can also be removed. > > Thanks, > Rick > >> - amoswap.w zero, zero, 0(t0) >> + amoswap.w.rl zero, zero, 0(t0) >> >> /* >> * Continue on hart lottery winner, others branch to >> -- >> 2.28.0 >>
On Tue, Sep 8, 2020 at 2:17 AM Sean Anderson <seanga2@gmail.com> wrote: > > Without these fences, it is perfectly valid for an out-of-order core to > re-order memory accesses to outside of the available_harts_lock critical > section. The commit message should be reworded, because current codes do nothing wrong as "fence" instruction is used. What was done in this patch is using .aq / .rl to replace "fence". > > Signed-off-by: Sean Anderson <seanga2@gmail.com> > --- > > arch/riscv/cpu/start.S | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Regards, Bin
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index e3222b1ea7..ad18e746b6 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -126,12 +126,12 @@ call_board_init_f_0: #ifndef CONFIG_XIP la t0, available_harts_lock fence rw, w - amoswap.w zero, zero, 0(t0) + amoswap.w.rl zero, zero, 0(t0) wait_for_gd_init: la t0, available_harts_lock li t1, 1 -1: amoswap.w t1, t1, 0(t0) +1: amoswap.w.aq t1, t1, 0(t0) fence r, rw bnez t1, 1b @@ -143,7 +143,7 @@ wait_for_gd_init: SREG t2, GD_AVAILABLE_HARTS(gp) fence rw, w - amoswap.w zero, zero, 0(t0) + amoswap.w.rl zero, zero, 0(t0) /* * Continue on hart lottery winner, others branch to
Without these fences, it is perfectly valid for an out-of-order core to re-order memory accesses to outside of the available_harts_lock critical section. Signed-off-by: Sean Anderson <seanga2@gmail.com> --- arch/riscv/cpu/start.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)