Re: [PATCH 0/4] support reserving crashkernel above 4G on arm64 kdump

[James Morse <james.morse@arm.com>]

Hi!

On 07/05/2019 04:50, Chen Zhou wrote:
> We use crashkernel=X to reserve crashkernel below 4G, which will fail
> when there is no enough memory. Currently, crashkernel=Y@X can be used
> to reserve crashkernel above 4G, in this case, if swiotlb or DMA buffers
> are requierd, capture kernel will boot failure because of no low memory.

> When crashkernel is reserved above 4G in memory, kernel should reserve
> some amount of low memory for swiotlb and some DMA buffers. So there may
> be two crash kernel regions, one is below 4G, the other is above 4G.

This is a good argument for supporting the 'crashkernel=...,low' version.
What is the 'crashkernel=...,high' version for?

Wouldn't it be simpler to relax the ARCH_LOW_ADDRESS_LIMIT if we see 'crashkernel=...,low'
in the kernel cmdline?

I don't see what the 'crashkernel=...,high' variant is giving us, it just complicates the
flow of reserve_crashkernel().

If we called reserve_crashkernel_low() at the beginning of reserve_crashkernel() we could
use crashk_low_res.end to change some limit variable from ARCH_LOW_ADDRESS_LIMIT to
memblock_end_of_DRAM().
I think this is a simpler change that gives you what you want.


> Then
> Crash dump kernel reads more than one crash kernel regions via a dtb
> property under node /chosen,
> linux,usable-memory-range = <BASE1 SIZE1 [BASE2 SIZE2]>.

Won't this break if your kdump kernel doesn't know what the extra parameters are?
Or if it expects two ranges, but only gets one? These DT properties should be treated as
ABI between kernel versions, we can't really change it like this.

I think the 'low' region is an optional-extra, that is never mapped by the first kernel. I
think the simplest thing to do is to add an 'linux,low-memory-range' that we
memblock_add() after memblock_cap_memory_range() has been called.
If its missing, or the new kernel doesn't know what its for, everything keeps working.


> Besides, we need to modify kexec-tools:
>   arm64: support more than one crash kernel regions(see [1])

> I post this patch series about one month ago. The previous changes and
> discussions can be retrived from:

Ah, this wasn't obvious as you've stopped numbering the series. Please label the next one
'v6' so that we can describe this as 'v5'. (duplicate numbering would be even more confusing!)


Thanks,

James