| Message ID | 1604387525-23400-6-git-send-email-yangtiezhu@loongson.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Modify some registers operations and move decode_cpucfg() to loongson_regs.h | expand |
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@loongson.cn> > Signed-off-by: Jun Yi <yijun@loongson.cn> > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > --- > > 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? 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"
在 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@loongson.cn> >> Signed-off-by: Jun Yi <yijun@loongson.cn> >> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> >> --- >> >> 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"
On 11/04/2020 03:04 PM, Jiaxun Yang wrote: > > > 在 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@loongson.cn> >>> Signed-off-by: Jun Yi <yijun@loongson.cn> >>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> >>> --- >>> >>> 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. OK, the current code looks better, please ignore my comment, sorry for the noise. Thanks, Jinyang > > 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"
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 */ + " 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"