Re: [PATCH v18 02/17] x86/setup: Move xen_pv_domain() check and insert_resource() to setup_arch()

From: "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, <x86@kernel.org>,
	"H . Peter Anvin" <hpa@zytor.com>, <linux-kernel@vger.kernel.org>,
	Dave Young <dyoung@redhat.com>, Baoquan He <bhe@redhat.com>,
	Vivek Goyal <vgoyal@redhat.com>,
	Eric Biederman <ebiederm@xmission.com>,
	<kexec@lists.infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	<devicetree@vger.kernel.org>, "Jonathan Corbet" <corbet@lwn.net>,
	<linux-doc@vger.kernel.org>, Randy Dunlap <rdunlap@infradead.org>,
	Feng Zhou <zhoufeng.zf@bytedance.com>,
	Kefeng Wang <wangkefeng.wang@huawei.com>,
	Chen Zhou <dingguo.cz@antgroup.com>,
	"John Donnelly" <John.p.donnelly@oracle.com>
Subject: Re: [PATCH v18 02/17] x86/setup: Move xen_pv_domain() check and insert_resource() to setup_arch()
Date: Fri, 7 Jan 2022 21:09:26 +0800	[thread overview]
Message-ID: <681a2403-1e1c-1b06-54d0-ca92a7f07e71@huawei.com> (raw)
In-Reply-To: <7e7c8d93-e745-2bfd-b93d-aecb3b70bf33@huawei.com>

On 2022/1/7 16:13, Leizhen (ThunderTown) wrote:
> 
> 
> On 2021/12/25 9:53, Leizhen (ThunderTown) wrote:
>>
>>
>> On 2021/12/24 14:36, Leizhen (ThunderTown) wrote:
>>>
>>>
>>> On 2021/12/24 1:26, Borislav Petkov wrote:
>>>> On Wed, Dec 22, 2021 at 09:08:05PM +0800, Zhen Lei wrote:
>>>>> From: Chen Zhou <chenzhou10@huawei.com>
>>>>>
>>>>> We will make the functions reserve_crashkernel() as generic, the
>>>>> xen_pv_domain() check in reserve_crashkernel() is relevant only to
>>>>> x86,
>>>>
>>>> Why is that so? Is Xen-PV x86-only?
>>>>
>>>>> the same as insert_resource() in reserve_crashkernel[_low]().
>>>>
>>>> Why?
>>>>
>>>> Looking at
>>>>
>>>>   0212f9159694 ("x86: Add Crash kernel low reservation")
>>>>
>>>> it *surprisingly* explains why that resources thing is being added:
>>>>
>>>>     We need to add another range in /proc/iomem like "Crash kernel low",
>>>>     so kexec-tools could find that info and append to kdump kernel
>>>>     command line.
>>>>
>>>> Then,
>>>>
>>>>   157752d84f5d ("kexec: use Crash kernel for Crash kernel low")
>>>>
>>>> renamed it because, as it states, kexec-tools was taught to handle
>>>> multiple resources of the same name.
>>>>
>>>> So why does kexec-tools on arm *not* need those iomem resources? How
>>>> does it parse the ranges there? Questions over questions...
> 
> Hi Borislav:
>   The reason why insert_resource() cannot be used in reserve_crashkernel[_low]()
> on arm64 is clear. The parent resource node of crashk[_low]_res is added by
> request_resource() in request_standard_resources(), so that it will be conflicted.
> All request_resource() in request_standard_resources() should be changed to
> insert_resource(), to make insert_resource() can be used in reserve_crashkernel[_low]().
> 
>   I found commit e25e6e7593ca ("kdump, x86: Process multiple Crash kernel in /proc/iomem")
> in kexec-tools. I'm trying to port it to arm64, or make it generic.

Chen Zhou's done it before. But the "Crash kernel (low)" can really be eliminated. Chen
Zhou just used it to distinguish whether the crashkernel memory range is crashkernel load
range or not. We can use get_crash_kernel_load_range() to get and check the load range.

> 
>   Thanks.
> 
>>
>> It's a good question worth figuring out. I'm going to dig into this.
>> I admire your rigorous style and sharp vision.
>>
> 
> 

-- 
Regards,
  Zhen Lei