Message-ID: <42ff66fb-a0bf-937f-8415-5d407fb93e03@flygoat.com>
Date:   Thu, 17 Sep 2020 22:21:10 +0800
From:   Jiaxun Yang <jiaxun.yang@...goat.com>
To:     Jinyang He <hejinyang@...ngson.cn>, 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



在 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.
>
> 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.

Thanks.

- Jiaxun

>
> Thanks,
>
> - Jinyang.
>

Powered by blists - more mailing lists