linux-kernel - Re: [PATCH v3 5/6] MIPS: Loongson64: SMP: Fix up play

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <85ce1b63-1c39-d567-1bb1-8a6431c9c895@flygoat.com>
Date:   Wed, 4 Nov 2020 15:04:18 +0800
From:   Jiaxun Yang <jiaxun.yang@...goat.com>
To:     Jinyang He <hejinyang@...ngson.cn>,
        Tiezhu Yang <yangtiezhu@...ngson.cn>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Huacai Chen <chenhc@...ote.com>
Cc:     linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org,
        Xuefeng Li <lixuefeng@...ngson.cn>,
        Lu Zeng <zenglu@...ngson.cn>, Jun Yi <yijun@...ngson.cn>
Subject: Re: [PATCH v3 5/6] MIPS: Loongson64: SMP: Fix up play_dead jump
 indicator



在 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@...ngson.cn>
>> Signed-off-by: Jun Yi <yijun@...ngson.cn>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@...ngson.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"