On 2023/4/8 10:00, Leizhen (ThunderTown) wrote:
On 2023/4/7 20:58, Leizhen (ThunderTown) wrote:
On 2023/4/7 20:03, Simon Horman wrote:
On Fri, Apr 07, 2023 at 06:02:05AM +0800, Chen Jiahao wrote:
On riscv, the current crash kernel allocation logic is trying to
allocate within 32bit addressible memory region by default, if
failed, try to allocate without 4G restriction.
In need of saving DMA zone memory while allocating a relatively large
crash kernel region, allocating the reserved memory top down in
high memory, without overlapping the DMA zone, is a mature solution.
Here introduce the parameter option crashkernel=X,[high,low].
One can reserve the crash kernel from high memory above DMA zone range
by explicitly passing "crashkernel=X,high"; or reserve a memory range
below 4G with "crashkernel=X,low".
Signed-off-by: Chen Jiahao <chenjiahao16@xxxxxxxxxx>
...
@@ -1180,14 +1206,37 @@ static void __init reserve_crashkernel(void)
return;
}
- ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
+ ret = parse_crashkernel(cmdline, memblock_phys_mem_size(),
&crash_size, &crash_base);
- if (ret || !crash_size)
+ if (ret == -ENOENT) {
+ /*
+ * crashkernel=X,[high,low] can be specified or not, but
+ * invalid value is not allowed.
nit: Perhaps something like this would be easier to correlate with the
code that follows:
/* Fallback to crashkernel=X,[high,low] */
The description "crashkernel=X,[high,low] can be specified or not" is not
correct, because crashkernel=X,high must be specified when walking into this
branch. So use Simon's comments or copy arm64's comments(it's written for
parse_crashkernel_low()).
I rethink it a little bit, if it's relative to crashkernel=X[@offset],
that's also true.
Reviewed-by: Zhen Lei <thunder.leizhen@xxxxxxxxxx>
Sure, The commit should not be ambiguous like this, Simon's comment above is
a better option.
+ */
+ ret = parse_crashkernel_high(cmdline, 0, &crash_size, &crash_base);
+ if (ret || !crash_size)
+ return;
+
+ /*
+ * crashkernel=Y,low is valid only when crashkernel=X,high
+ * is passed and high memory is reserved successful.
nit: s/successful/successfully/
Seems like the whole "and high memory is reserved successful" needs to be deleted.
Only the dependency between the two boot options should be described here,
regardless of whether their memory is successfully allocated.
The comment here is imprecise, since there is absolutely no check whether
the allocation is successful before "parse_crashkernel_low"
+ */
+ ret = parse_crashkernel_low(cmdline, 0, &crash_low_size, &crash_base);
+ if (ret == -ENOENT)
+ crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
+ else if (ret)
+ return;
+
+ search_start = search_low_max;
+ } else if (ret || !crash_size) {
+ /* Invalid argument value specified */
return;
+ }
...
.
BR,
Jiahao
