From: Zhen Lei <[email protected]>
Unlike in the past, the low memory allocation direction of the crashkernel is
changed from top-down to bottom-up. As long as the DMA zone has sufficient
continuous free memory, the allocated crashkernel low memory must meet the
requirements. The allocation direction of crashkernel high memory remains
unchanged, that is, top-down. As long as the high memory(above DMA zone) has
sufficient continuous free memory, the allocated crashkernel high memory must
meet the requirements. In this way, with the restoration of the original
page-level mapping and the implementation of the arch_kexec_protect_crashkres()
function, write protection for the crashkernel memory region can be supported.
Of course, if the high memory or low memory cannot meet the initial requirements,
that is, fall back is required. In this case, write protection is not supported
because the newly allocated memory is not page-level mapped.
Because the original retry process is eliminated, the new process looks clearer
and is a simple sequential flow.
Zhen Lei (3):
arm64: kdump: Allocate crash low memory in the bottom-up direction
arm64: kdump: use page-level mapping for crashkernel region
arm64: kdump: add support access protection for crashkernel region
arch/arm64/include/asm/kexec.h | 8 ++
arch/arm64/kernel/machine_kexec.c | 26 ++++
arch/arm64/mm/init.c | 216 +++++++++++++++++++++++-------
arch/arm64/mm/mmu.c | 21 +++
4 files changed, 219 insertions(+), 52 deletions(-)
--
2.25.1
Hi,
On 07/21/23 at 04:17pm, [email protected] wrote:
> From: Zhen Lei <[email protected]>
>
> Unlike in the past, the low memory allocation direction of the crashkernel is
> changed from top-down to bottom-up. As long as the DMA zone has sufficient
> continuous free memory, the allocated crashkernel low memory must meet the
> requirements. The allocation direction of crashkernel high memory remains
> unchanged, that is, top-down. As long as the high memory(above DMA zone) has
> sufficient continuous free memory, the allocated crashkernel high memory must
> meet the requirements. In this way, with the restoration of the original
> page-level mapping and the implementation of the arch_kexec_protect_crashkres()
> function, write protection for the crashkernel memory region can be supported.
>
> Of course, if the high memory or low memory cannot meet the initial requirements,
> that is, fall back is required. In this case, write protection is not supported
> because the newly allocated memory is not page-level mapped.
>
> Because the original retry process is eliminated, the new process looks clearer
> and is a simple sequential flow.
Thanks, but no.
The pure semantics and the corresponding implementation have been
complicated, it's not worth adding so much more complication to it
just because of one inessential feature.
If stomp really happened and destroy the loaded kdump kernel, the write
protection truly can save kdump to make vmcore dumping succeed. While
without write protection, we at least know that stomp happened by the
later checksum verifycation. That's an advantage over write protection
which silently ignores the stomp, right?
So, due to the low cost performance, from people maintaining and
understanding the code point of view, I would like to NACK this series.
BUT since all these code changes are added into arm64 arch, I won't
object if arm64 maintainers wants to pikc them up.
By the way, as we have talked before, arm64 lacks the loaded kernel
checksum storing and verifying, would you like to add that?
Thanks
Baoquan
On 2023/7/24 21:34, Baoquan He wrote:
> Hi,
>
> On 07/21/23 at 04:17pm, [email protected] wrote:
>> From: Zhen Lei <[email protected]>
>>
>> Unlike in the past, the low memory allocation direction of the crashkernel is
>> changed from top-down to bottom-up. As long as the DMA zone has sufficient
>> continuous free memory, the allocated crashkernel low memory must meet the
>> requirements. The allocation direction of crashkernel high memory remains
>> unchanged, that is, top-down. As long as the high memory(above DMA zone) has
>> sufficient continuous free memory, the allocated crashkernel high memory must
>> meet the requirements. In this way, with the restoration of the original
>> page-level mapping and the implementation of the arch_kexec_protect_crashkres()
>> function, write protection for the crashkernel memory region can be supported.
>>
>> Of course, if the high memory or low memory cannot meet the initial requirements,
>> that is, fall back is required. In this case, write protection is not supported
>> because the newly allocated memory is not page-level mapped.
>>
>> Because the original retry process is eliminated, the new process looks clearer
>> and is a simple sequential flow.
>
> Thanks, but no.
>
> The pure semantics and the corresponding implementation have been
> complicated, it's not worth adding so much more complication to it
> just because of one inessential feature.
It's just that the code looks like if..else branches are a little more, but the
idea is not complicated.
1. Reserve low memory bottom-up(start from 0), reserve high memory top-down(start from CRASH_ADDR_HIGH_MAX)
2. zone_sizes_init() update arm64_dma_phys_limit, now CRASH_ADDR_LOW_MAX is known.
3. Use CRASH_ADDR_LOW_MAX to verify the preserved low memory and high memory, OK or fall back.
To be honest, the code can be simplified a lot if we don't support the following:
-----
If reservation from the high memory failed, the kernel falls back to
searching the low memory with the specified size in crashkernel=,high.
If it succeeds, no further reservation for low memory is needed.
>
> If stomp really happened and destroy the loaded kdump kernel, the write
> protection truly can save kdump to make vmcore dumping succeed. While
> without write protection, we at least know that stomp happened by the
> later checksum verifycation. That's an advantage over write protection
> which silently ignores the stomp, right?
Theoretically, write protection can catch exceptions in a timely manner
and ensure that kdump is successful. If the problem is easy to reproduce,
it doesn't matter if it fails once. However, for versions that have been
delivered for commercial use, the customer may not give the second chance.
>
> So, due to the low cost performance, from people maintaining and
> understanding the code point of view, I would like to NACK this series.
> BUT since all these code changes are added into arm64 arch, I won't
> object if arm64 maintainers wants to pikc them up.
This new idea is not bad. After all, before commercial use, "fall back"
can be avoided by adjusting crashkernel size in command line. So the
problem is pretty much solved.
>
> By the way, as we have talked before, arm64 lacks the loaded kernel
> checksum storing and verifying, would you like to add that?
OKay.
>
> Thanks
> Baoquan
>
> .
>
--
Regards,
Zhen Lei