Message-ID: <97e7b66c-b1cc-4810-7431-ba302abaabe0@loongson.cn>
Date:   Fri, 18 Sep 2020 14:20:33 +0800
From:   Jinyang He <hejinyang@...ngson.cn>
To:     Jiaxun Yang <jiaxun.yang@...goat.com>,
        Huacai Chen <chenhc@...ote.com>
Cc:     Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Youling Tang <tangyouling@...ngson.cn>,
        "open list:MIPS" <linux-mips@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, kexec@...ts.infradead.org,
        Xuefeng Li <lixuefeng@...ngson.cn>
Subject: Re: [PATCH] MIPS: Loongson64: Add kexec/kdump support

On 09/17/2020 10:21 PM, Jiaxun Yang wrote:
>
>
> 在 2020/9/17 20:41, Jinyang He 写道:
>> Hi, Huacai,
>>
>>
>> On 09/16/2020 01:39 PM, Huacai Chen wrote:
>>> Hi, Jinyang,
>>>
>>> On Tue, Sep 15, 2020 at 10:17 PM Jinyang He <hejinyang@...ngson.cn> 
>>> wrote:
>>>>
>>>>
>>>> On 09/16/2020 09:33 AM, Jiaxun Yang wrote:
>>>>> 于 2020年9月15日 GMT+08:00 下午9:07:43, Jinyang He 
>>>>> <hejinyang@...ngson.cn> 写到:
>>>>>> Add loongson_kexec_prepare(), loongson_kexec_shutdown() and
>>>>>> loongson_kexec_crashdown() for passing the parameters of kexec_args.
>>>>>>
>>>>>> To start loongson64, CPU0 needs 3 parameters:
>>>>>> fw_arg0: the number of cmd.
>>>>>> fw_arg1: cmd structure which seems strange, the cmd array[index]'s
>>>>>>           value is cmd string's address, index >= 1.
>>>>>> fw_arg2: environment.
>>>>>>
>>>>>> Secondary CPUs do not need parameter at once. They query their
>>>>>> mailbox to get PC, SP and GP in a loop before CPU0 brings them up
>>>>>> and passes these parameters via mailbox.
>>>>>>
>>>>>> loongson_kexec_prepare(): Alloc new memory to save cmd for kexec.
>>>>>> Combine the kexec append option string as cmd structure, and the cmd
>>>>>> struct will be parsed in fw_init_cmdline() of 
>>>>>> arch/mips/fw/lib/cmdline.c.
>>>>>> image->control_code_page need pointing to a safe memory page. In 
>>>>>> order to
>>>>>> maintain compatibility for the old firmware the low 2MB is reserverd
>>>>>> and safe for Loongson. So let it points here.
>>>>>>
>>>>>> loongson_kexec_shutdown(): Wake up all present CPUs and let them go
>>>>>> to reboot_code_buffer. Pass the kexec parameters to kexec_args.
>>>>>>
>>>>>> loongson_crash_shutdown(): Pass the kdump parameters to kexec_args.
>>>>>>
>>>>>> The assembly part provide a way like BIOS doing to keep secondary
>>>>>> CPUs in a querying loop.
>>>>>>
>>>>>> This patch referenced [1][2][3].
>>>>>>
>>>>>> [1] arch/mips/cavium-octeon/setup.c
>>>>>> [2] https://patchwork.kernel.org/patch/10799217/
>>>>>> [3] 
>>>>>> https://gitee.com/loongsonlab/qemu/blob/master/hw/mips/loongson3a_rom.h 
>>>>>>
>>>>>>
>>>>>> Co-developed-by: Youling Tang <tangyouling@...ngson.cn>
>>>>>> Signed-off-by: Youling Tang <tangyouling@...ngson.cn>
>>>>>> Signed-off-by: Jinyang He <hejinyang@...ngson.cn>
>>>>>> ---
>>>>>> arch/mips/kernel/relocate_kernel.S | 19 ++++++++
>>>>>> arch/mips/loongson64/reset.c       | 88 
>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>> 2 files changed, 107 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/mips/kernel/relocate_kernel.S 
>>>>>> b/arch/mips/kernel/relocate_kernel.S
>>>>>> index ac87089..061cbfb 100644
>>>>>> --- a/arch/mips/kernel/relocate_kernel.S
>>>>>> +++ b/arch/mips/kernel/relocate_kernel.S
>>>>>> @@ -133,6 +133,25 @@ LEAF(kexec_smp_wait)
>>>>>> #else
>>>>>>       sync
>>>>>> #endif
>>>>>> +
>>>>>> +#ifdef CONFIG_CPU_LOONGSON64
>>>>>> +#define MAILBOX_BASE 0x900000003ff01000
>>>>> Please avoid hardcoded SMP information. You're breaking Loongson 
>>>>> 3B support.
>>>>>
>>>> Ok, I see. Since my machine is Loongson 3A. I'll send v2
>>>> after I test it in 3B.
>>> 1, My original version can work on both Loongson-3A and Loongson-3B,
>>> why you modify my patch and hadn't discuss with me?
>>>
>>> 2, With this single patch both kexec and kdump cannot work reliably,
>>> because kexec need this patch:
>>>     https://patchwork.kernel.org/patch/11695929/
>>>
>>>     and kdump need my first patch in my original version:
>>>     https://patchwork.kernel.org/patch/10799215/
>>>
>>>     You may argue that you have tested. Yes, I believe that, I'm not
>>> saying that you haven't test, and I'm not saying that your patch
>>> cannot work, I'm just saying that your patch is not robust.
>>>
>>> 3, I'm the original author and paying attention to kexec/kdump
>>> continuosly, I will send a new version once the above two patches be
>>> accepted. But you re-send my patch without any communication with me,
>>> why you so impatient?
>>>
>>> Huacai
>>>
>>
>> 1, Your original version:
>>    https://patchwork.kernel.org/patch/10799217/
>>
>> This patch can work on Loongson-3A, I tested it.
>>
>> But it works wrong after the follow behaviors,
>>    kexec -l vmlinux --append=cmdline_kexec
>>    kexec -p vmlinux --append=cmdline_kdump
>>    kexec -e
>>
>> It works but cmdline_kdump merged cmdline_kexec.
>>
>> And this patch memcpy from fw_arg2 to kexec_envp and later memcpy from
>> kexec_envp to fw_arg2 when fw_arg2 was not changed, it's redundant.
>>
>> However, I have not Loongson-3B now, and did not test it. For this 
>> patch,
>> does it work well on Loongson-3B3000/Loongson-3B4000?
>
> Hi Jingyang,
>
> Well for Loongson-3B I meant 3B1000/3B1500, which have different layout
> of SMP registers. For 3B3000/3B4000 everyone know they're just different
> branding to the same silicon.

OK, I see. Thank you.

>>
>> 3, I try to fix Loongson64 kexec function since I joined the community.
>> I fell sorry to not do enough research on Loongson64 kexec. My first 
>> patch:
>>    https://patchwork.kernel.org/patch/11684849/
>
> I'm glad to see Loongson staff joining the community and enhance our 
> code.
> It is common practice to investigate historical changes before do 
> something
> to the upstream code :-)
>
>>
>> It fixed problem about "Crash kernel" which can be traced back to 
>> linux-5.4.
>> At that time, I thought there is no developer work on Kexec. Thus, I 
>> did a
>> lot on Kexec. Are you really continuosly paying attention to 
>> kexec/kdump?
>> With the exploring and developing deep, I found your patch several 
>> days ago
>> after I did a draft patch witch referenced:
>>    arch/mips/cavium-octeon/setup.c
>> https://gitee.com/loongsonlab/qemu/blob/master/hw/mips/loongson3a_rom.h
>>
>> There is no doubt that your patch gives me confidence and suggestion 
>> while
>> it gives me worry. As a newcomer, I do not know if should communicate 
>> with
>> you since your patch was committed one year ago. And now it may be a 
>> good
>> chance to do some communication.
>
> You should *always* try to *credit* others properly, that's the first 
> thing.
> Communication is optional but highly recommanded.
>
> Recently I can smell there are some tensions raised between the 
> community and
> the Loongson company, mainly about GPL violation and Loongson's attitude
> towards the community. Personaly I don't hold any hostility to Loongosn's
> practice in the community but I'd also like to see Loongson show their 
> respect
> to the community.

I have replied to Yanjie:
https://lore.kernel.org/patchwork/patch/1306401/#1502870

>
> Thanks.
>
> - Jiaxun
>
>>
>> Thanks,
>>
>> - Jinyang.
>>

Powered by blists - more mailing lists