Re: [PATCH v3 5/6] MIPS: Loongson64: SMP: Fix up play_dead jump indicator

[Jiaxun Yang]

在 2020/11/4 14:31, Jinyang He 写道:
> Hi, all,
>
> On 11/03/2020 03:12 PM, Tiezhu Yang wrote:
> > In play_dead function, the whole 64-bit PC mailbox was used as a 
> > indicator
> > to determine if the master core had written boot jump information.
> >
> > However, after we introduced CSR mailsend, the hardware will not 
> > guarante
> > an atomic write for the 64-bit PC mailbox. Thus we have to use the lower
> > 32-bit which is written at the last as the jump indicator instead.
> >
> > Signed-off-by: Lu Zeng <zenglu@xxxxxxxxxxx>
> > Signed-off-by: Jun Yi <yijun@xxxxxxxxxxx>
> > Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>
> > ---
> >
> > v2: No changes
> > v3: Update the commit message and comment
> >
> >   arch/mips/loongson64/smp.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/mips/loongson64/smp.c b/arch/mips/loongson64/smp.c
> > index 736e98d..aa0cd72 100644
> > --- a/arch/mips/loongson64/smp.c
> > +++ b/arch/mips/loongson64/smp.c
> > @@ -764,9 +764,10 @@ static void loongson3_type3_play_dead(int 
> > *state_addr)
> >           "1: li    %[count], 0x100             \n" /* wait for init 
> > loop */
> >           "2: bnez  %[count], 2b                \n" /* limit mailbox 
> > access */
> >           "   addiu %[count], -1                \n"
> > -        "   ld    %[initfunc], 0x20(%[base])  \n" /* get PC via 
> > mailbox */
> I have some confusion here. Play_dead CPUs is always brought up by 
> cpu_up().
> On Loongson64, it calls loongson3_boot_secondary(). The value of 
> startargs[0]
> is the address of smp_bootstrap() which is in CKSEG0 and a constant 
> after the
> kernel is compiled. That means its value likes 0xffffffff8... and only 
> the low
> 32bit is useful. As "lw" is sign-extended, could we replace "ld" with 
> "lw" simply?

Hi Jinyang,

I'd prefer not to do so. In future we may have kernel running in other 
spaces,
(e.g. xkphys), and there is no reason to add a barrier on that without 
actual benefit.
I had check PMON firmware and it's also loading the full 64-bit address.

Also to keep consistent, mailbox writing part needs to be refined to 
match the
behavior of reading. Otherwise other readers will be confused.

Thus leaving it as is looks much more reasonable.

Thanks.

- Jiaxun

> Thanks,
> Jinyang
> > +        "   lw    %[initfunc], 0x20(%[base])  \n" /* check lower 
> > 32-bit as jump indicator */
> >           "   beqz  %[initfunc], 1b             \n"
> >           "   nop                               \n"
> > +        "   ld    %[initfunc], 0x20(%[base])  \n" /* get PC (whole 
> > 64-bit) via mailbox */
> >           "   ld    $sp, 0x28(%[base])          \n" /* get SP via 
> > mailbox */
> >           "   ld    $gp, 0x30(%[base])          \n" /* get GP via 
> > mailbox */
> >           "   ld    $a1, 0x38(%[base])          \n"