[RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Jinjie Ruan]

As ARM LPAE feature support accessing memory beyond the 4G limit, define
HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to support reserving crash
memory above 4G for ARM32 LPAE.

No test because there is no LPAE ARM32 hardware.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 arch/arm/include/asm/crash_reserve.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/crash_reserve.h b/arch/arm/include/asm/crash_reserve.h
index 85c9298bd3b7..33a2f18b0ec1 100644
--- a/arch/arm/include/asm/crash_reserve.h
+++ b/arch/arm/include/asm/crash_reserve.h
@@ -19,6 +19,10 @@ static inline unsigned long crash_addr_low_max(void)
 	return (crash_max > lowmem_max) ? lowmem_max : crash_max;
 }
 
-
 #define HAVE_ARCH_ADD_CRASH_RES_TO_IOMEM_EARLY
+
+#if defined(CONFIG_ARM_LPAE) && defined(CONFIG_HIGHMEM)
+#define HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH
+#endif
+
 #endif
-- 
2.34.1

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Russell King (Oracle)]

On Fri, Aug 02, 2024 at 05:25:10PM +0800, Jinjie Ruan wrote:
> As ARM LPAE feature support accessing memory beyond the 4G limit, define
> HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to support reserving crash
> memory above 4G for ARM32 LPAE.
> 
> No test because there is no LPAE ARM32 hardware.

Why are you submitting patches for features you can't test?

I'm not going to apply this without it being properly tested, because I
don't believe that this will work in the generic case.

If the crash kernel is located in memory outside of the lower 4GiB of
address space, and there is no alias within physical address space
for that memory, then there is *no* *way* for such a kernel to boot.

So, right now I believe this patch to be *fundamentally* wrong.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Jinjie Ruan]

On 2024/8/2 19:01, Russell King (Oracle) wrote:
> On Fri, Aug 02, 2024 at 05:25:10PM +0800, Jinjie Ruan wrote:
>> As ARM LPAE feature support accessing memory beyond the 4G limit, define
>> HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to support reserving crash
>> memory above 4G for ARM32 LPAE.
>>
>> No test because there is no LPAE ARM32 hardware.
> 
> Why are you submitting patches for features you can't test?
> 
> I'm not going to apply this without it being properly tested, because I
> don't believe that this will work in the generic case.
> 
> If the crash kernel is located in memory outside of the lower 4GiB of
> address space, and there is no alias within physical address space
> for that memory, then there is *no* *way* for such a kernel to boot.

I'm sorry that I released this patch without testing it. I actually
intended to bring up this issue for discussion. If anyone has the
environment to test it, that would be great. In the meantime, we could
have a discussion on the significance and relevance of this approach.

> 
> So, right now I believe this patch to be *fundamentally* wrong.
> 

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Baoquan He]

On 08/05/24 at 09:23am, Jinjie Ruan wrote:
> 
> 
> On 2024/8/2 19:01, Russell King (Oracle) wrote:
> > On Fri, Aug 02, 2024 at 05:25:10PM +0800, Jinjie Ruan wrote:
> >> As ARM LPAE feature support accessing memory beyond the 4G limit, define
> >> HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to support reserving crash
> >> memory above 4G for ARM32 LPAE.
> >>
> >> No test because there is no LPAE ARM32 hardware.
> > 
> > Why are you submitting patches for features you can't test?
> > 
> > I'm not going to apply this without it being properly tested, because I
> > don't believe that this will work in the generic case.
> > 
> > If the crash kernel is located in memory outside of the lower 4GiB of
> > address space, and there is no alias within physical address space
> > for that memory, then there is *no* *way* for such a kernel to boot.
> 
> I'm sorry that I released this patch without testing it. I actually
> intended to bring up this issue for discussion. If anyone has the
> environment to test it, that would be great. In the meantime, we could
> have a discussion on the significance and relevance of this approach.

I don't know arm32 and its LPAE. I know a little about x86_32 where
crashkernel can only be reserved below 896M because of the virtual
memory layout, and all memory above that is high memory which can't be
used as kernel memory directly. So from this patch, arm32 is different
than x86_32.

> 
> > 
> > So, right now I believe this patch to be *fundamentally* wrong.
> > 
> 

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Jinjie Ruan]

On 2024/8/5 10:56, Baoquan He wrote:
> On 08/05/24 at 09:23am, Jinjie Ruan wrote:
>>
>>
>> On 2024/8/2 19:01, Russell King (Oracle) wrote:
>>> On Fri, Aug 02, 2024 at 05:25:10PM +0800, Jinjie Ruan wrote:
>>>> As ARM LPAE feature support accessing memory beyond the 4G limit, define
>>>> HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to support reserving crash
>>>> memory above 4G for ARM32 LPAE.
>>>>
>>>> No test because there is no LPAE ARM32 hardware.
>>>
>>> Why are you submitting patches for features you can't test?
>>>
>>> I'm not going to apply this without it being properly tested, because I
>>> don't believe that this will work in the generic case.
>>>
>>> If the crash kernel is located in memory outside of the lower 4GiB of
>>> address space, and there is no alias within physical address space
>>> for that memory, then there is *no* *way* for such a kernel to boot.
>>
>> I'm sorry that I released this patch without testing it. I actually
>> intended to bring up this issue for discussion. If anyone has the
>> environment to test it, that would be great. In the meantime, we could
>> have a discussion on the significance and relevance of this approach.
> 
> I don't know arm32 and its LPAE. I know a little about x86_32 where
> crashkernel can only be reserved below 896M because of the virtual
> memory layout, and all memory above that is high memory which can't be
> used as kernel memory directly. So from this patch, arm32 is different
> than x86_32.

Hi，Baoquan

Does the following code make sense? Now parse_crashkernel() use
HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to parse "high", but use
CONFIG_64BIT when reserving "low" memory in reserve_crashkernel_low().

And when LPAE is enabled in ARM32, and "high" is reserved,
reserve_crashkernel_low() need also function ok.

--- a/kernel/crash_reserve.c
+++ b/kernel/crash_reserve.c
@@ -354,7 +354,7 @@ early_param("crashkernel", parse_crashkernel_dummy);
 #ifdef CONFIG_ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
 static int __init reserve_crashkernel_low(unsigned long long low_size)
 {
-#ifdef CONFIG_64BIT
+#ifdef HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH
        unsigned long long low_base;


> 
>>
>>>
>>> So, right now I believe this patch to be *fundamentally* wrong.
>>>
>>
> 
> 

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Baoquan He]

On 08/06/24 at 10:46am, Jinjie Ruan wrote:
> 
> 
> On 2024/8/5 10:56, Baoquan He wrote:
> > On 08/05/24 at 09:23am, Jinjie Ruan wrote:
> >>
> >>
> >> On 2024/8/2 19:01, Russell King (Oracle) wrote:
> >>> On Fri, Aug 02, 2024 at 05:25:10PM +0800, Jinjie Ruan wrote:
> >>>> As ARM LPAE feature support accessing memory beyond the 4G limit, define
> >>>> HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to support reserving crash
> >>>> memory above 4G for ARM32 LPAE.
> >>>>
> >>>> No test because there is no LPAE ARM32 hardware.
> >>>
> >>> Why are you submitting patches for features you can't test?
> >>>
> >>> I'm not going to apply this without it being properly tested, because I
> >>> don't believe that this will work in the generic case.
> >>>
> >>> If the crash kernel is located in memory outside of the lower 4GiB of
> >>> address space, and there is no alias within physical address space
> >>> for that memory, then there is *no* *way* for such a kernel to boot.
> >>
> >> I'm sorry that I released this patch without testing it. I actually
> >> intended to bring up this issue for discussion. If anyone has the
> >> environment to test it, that would be great. In the meantime, we could
> >> have a discussion on the significance and relevance of this approach.
> > 
> > I don't know arm32 and its LPAE. I know a little about x86_32 where
> > crashkernel can only be reserved below 896M because of the virtual
> > memory layout, and all memory above that is high memory which can't be
> > used as kernel memory directly. So from this patch, arm32 is different
> > than x86_32.
> 
> Hi，Baoquan
> 
> Does the following code make sense? Now parse_crashkernel() use
> HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to parse "high", but use
> CONFIG_64BIT when reserving "low" memory in reserve_crashkernel_low().

I am fine with it. BUT have you addressed Russell's concern, e.g how to
test it actually?

> 
> And when LPAE is enabled in ARM32, and "high" is reserved,
> reserve_crashkernel_low() need also function ok.
> 
> --- a/kernel/crash_reserve.c
> +++ b/kernel/crash_reserve.c
> @@ -354,7 +354,7 @@ early_param("crashkernel", parse_crashkernel_dummy);
>  #ifdef CONFIG_ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
>  static int __init reserve_crashkernel_low(unsigned long long low_size)
>  {
> -#ifdef CONFIG_64BIT
> +#ifdef HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH
>         unsigned long long low_base;
> 
> 
> > 
> >>
> >>>
> >>> So, right now I believe this patch to be *fundamentally* wrong.
> >>>
> >>
> > 
> > 
> 

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Russell King (Oracle)]

On Tue, Aug 06, 2024 at 04:11:30PM +0800, Baoquan He wrote:
> I am fine with it. BUT have you addressed Russell's concern, e.g how to
> test it actually?

Thanks for bringing that up.

Let me reinforce my position on this. I will _not_ be accepting a patch
that allows the crash kernel to be placed into high memory on 32-bit
ARM unless it has been thoroughly tested to prove that it can actually
work.

Right now, I don't believe it can work as placing the kernel in highmem
likely means it will be located *outside* of the lower 4GiB of physical
memory which is all that will be accessible when the MMU is turned off.
This is a pre-condition to boot a kernel - the kernel image _must_ be
located within a region of memory which is exposed to the CPU when the
MMU is turned off.

Unless it can be proven that placing the kernel in highmem means that
the kernel will be located in the lower 4GiB of physical memory space
with the MMU off, then further work on this patch is a waste of time.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Baoquan He]

On 08/06/24 at 09:34am, Russell King (Oracle) wrote:
> On Tue, Aug 06, 2024 at 04:11:30PM +0800, Baoquan He wrote:
> > I am fine with it. BUT have you addressed Russell's concern, e.g how to
> > test it actually?
> 
> Thanks for bringing that up.
> 
> Let me reinforce my position on this. I will _not_ be accepting a patch
> that allows the crash kernel to be placed into high memory on 32-bit
> ARM unless it has been thoroughly tested to prove that it can actually
> work.
> 
> Right now, I don't believe it can work as placing the kernel in highmem
> likely means it will be located *outside* of the lower 4GiB of physical
> memory which is all that will be accessible when the MMU is turned off.
> This is a pre-condition to boot a kernel - the kernel image _must_ be
> located within a region of memory which is exposed to the CPU when the
> MMU is turned off.
> 
> Unless it can be proven that placing the kernel in highmem means that
> the kernel will be located in the lower 4GiB of physical memory space
> with the MMU off, then further work on this patch is a waste of time.

Yeah, totally agree.

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Jinjie Ruan]

On 2024/8/6 16:34, Russell King (Oracle) wrote:
> On Tue, Aug 06, 2024 at 04:11:30PM +0800, Baoquan He wrote:
>> I am fine with it. BUT have you addressed Russell's concern, e.g how to
>> test it actually?
> 
> Thanks for bringing that up.
> 
> Let me reinforce my position on this. I will _not_ be accepting a patch
> that allows the crash kernel to be placed into high memory on 32-bit
> ARM unless it has been thoroughly tested to prove that it can actually
> work.

Thank you! I'm looking for an available environment to test, otherwise,
the patch is invalid.

> 
> Right now, I don't believe it can work as placing the kernel in highmem
> likely means it will be located *outside* of the lower 4GiB of physical
> memory which is all that will be accessible when the MMU is turned off.
> This is a pre-condition to boot a kernel - the kernel image _must_ be
> located within a region of memory which is exposed to the CPU when the
> MMU is turned off.
> 
> Unless it can be proven that placing the kernel in highmem means that
> the kernel will be located in the lower 4GiB of physical memory space
> with the MMU off, then further work on this patch is a waste of time.
> 

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Jinjie Ruan]

On 2024/8/6 16:11, Baoquan He wrote:
> On 08/06/24 at 10:46am, Jinjie Ruan wrote:
>>
>>
>> On 2024/8/5 10:56, Baoquan He wrote:
>>> On 08/05/24 at 09:23am, Jinjie Ruan wrote:
>>>>
>>>>
>>>> On 2024/8/2 19:01, Russell King (Oracle) wrote:
>>>>> On Fri, Aug 02, 2024 at 05:25:10PM +0800, Jinjie Ruan wrote:
>>>>>> As ARM LPAE feature support accessing memory beyond the 4G limit, define
>>>>>> HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to support reserving crash
>>>>>> memory above 4G for ARM32 LPAE.
>>>>>>
>>>>>> No test because there is no LPAE ARM32 hardware.
>>>>>
>>>>> Why are you submitting patches for features you can't test?
>>>>>
>>>>> I'm not going to apply this without it being properly tested, because I
>>>>> don't believe that this will work in the generic case.
>>>>>
>>>>> If the crash kernel is located in memory outside of the lower 4GiB of
>>>>> address space, and there is no alias within physical address space
>>>>> for that memory, then there is *no* *way* for such a kernel to boot.
>>>>
>>>> I'm sorry that I released this patch without testing it. I actually
>>>> intended to bring up this issue for discussion. If anyone has the
>>>> environment to test it, that would be great. In the meantime, we could
>>>> have a discussion on the significance and relevance of this approach.
>>>
>>> I don't know arm32 and its LPAE. I know a little about x86_32 where
>>> crashkernel can only be reserved below 896M because of the virtual
>>> memory layout, and all memory above that is high memory which can't be
>>> used as kernel memory directly. So from this patch, arm32 is different
>>> than x86_32.
>>
>> Hi，Baoquan
>>
>> Does the following code make sense? Now parse_crashkernel() use
>> HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to parse "high", but use
>> CONFIG_64BIT when reserving "low" memory in reserve_crashkernel_low().
> 
> I am fine with it. BUT have you addressed Russell's concern, e.g how to
> test it actually?

Thank you! Let me find the test environment.

> 
>>
>> And when LPAE is enabled in ARM32, and "high" is reserved,
>> reserve_crashkernel_low() need also function ok.
>>
>> --- a/kernel/crash_reserve.c
>> +++ b/kernel/crash_reserve.c
>> @@ -354,7 +354,7 @@ early_param("crashkernel", parse_crashkernel_dummy);
>>  #ifdef CONFIG_ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
>>  static int __init reserve_crashkernel_low(unsigned long long low_size)
>>  {
>> -#ifdef CONFIG_64BIT
>> +#ifdef HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH
>>         unsigned long long low_base;
>>
>>
>>>
>>>>
>>>>>
>>>>> So, right now I believe this patch to be *fundamentally* wrong.
>>>>>
>>>>
>>>
>>>
>>
> 
> 

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Catalin Marinas]

On Fri, Aug 02, 2024 at 12:01:43PM +0100, Russell King wrote:
> On Fri, Aug 02, 2024 at 05:25:10PM +0800, Jinjie Ruan wrote:
> > As ARM LPAE feature support accessing memory beyond the 4G limit, define
> > HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to support reserving crash

At least in 6.11-rc1, there's no trace of such macro anywhere. So not
sure this patch has any effect (I haven't checked linux-next though).

> > memory above 4G for ARM32 LPAE.
> > 
> > No test because there is no LPAE ARM32 hardware.
> 
> Why are you submitting patches for features you can't test?
> 
> I'm not going to apply this without it being properly tested, because I
> don't believe that this will work in the generic case.
> 
> If the crash kernel is located in memory outside of the lower 4GiB of
> address space, and there is no alias within physical address space
> for that memory, then there is *no* *way* for such a kernel to boot.
> 
> So, right now I believe this patch to be *fundamentally* wrong.

Indeed. Even on arm64, we keep some crashkernel reservations in the
lower parts of the memory for ZONE_DMA allocations.

On arch/arm with LPAE, we could do something similar like forcing some
lowmem reservation and allowing explicit allocation in the higher ranges
with crashkernel=<size>,high. We should, of course, force the kdump
image placement in the lower memory. The user kexec tools must be taught
to interpret this information and provide a DT accordingly to the crash
kernel.

-- 
Catalin

Re: [RFC PATCH] ARM: Support allocating crashkernel above 4G for LPAE

[Jinjie Ruan]

On 2024/8/5 21:18, Catalin Marinas wrote:
> On Fri, Aug 02, 2024 at 12:01:43PM +0100, Russell King wrote:
>> On Fri, Aug 02, 2024 at 05:25:10PM +0800, Jinjie Ruan wrote:
>>> As ARM LPAE feature support accessing memory beyond the 4G limit, define
>>> HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro to support reserving crash
> 
> At least in 6.11-rc1, there's no trace of such macro anywhere. So not
> sure this patch has any effect (I haven't checked linux-next though).

Sorry, this macro is introduced in linux-next, the -next subject has
been missed.

> 
>>> memory above 4G for ARM32 LPAE.
>>>
>>> No test because there is no LPAE ARM32 hardware.
>>
>> Why are you submitting patches for features you can't test?
>>
>> I'm not going to apply this without it being properly tested, because I
>> don't believe that this will work in the generic case.
>>
>> If the crash kernel is located in memory outside of the lower 4GiB of
>> address space, and there is no alias within physical address space
>> for that memory, then there is *no* *way* for such a kernel to boot.
>>
>> So, right now I believe this patch to be *fundamentally* wrong.
> 
> Indeed. Even on arm64, we keep some crashkernel reservations in the
> lower parts of the memory for ZONE_DMA allocations.

Indeed, it is.

> 
> On arch/arm with LPAE, we could do something similar like forcing some
> lowmem reservation and allowing explicit allocation in the higher ranges
> with crashkernel=<size>,high. We should, of course, force the kdump

In linux-next, with the HAVE_ARCH_CRASHKERNEL_RESERVATION_HIGH macro
defined, it is ok.

> image placement in the lower memory. The user kexec tools must be taught
> to interpret this information and provide a DT accordingly to the crash
> kernel.
>