[PATCH] arm64:kexec: Memstart should not be before the kernel start address

[PATCH] arm64:kexec: Memstart should not be before the kernel start address

[Sameer Goel]

Starting 4.6-rc4 the kernel memblock start is rounded down to a desirable
alignment. So, the kernel can see reserved memory regions before the kernel
start address in the iomem query.
Need to make sure that the right kernel start address is picked from the iomem
query.
---
 kexec/arch/arm64/kexec-arm64.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index 776596a..5e39f4f 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -30,6 +30,12 @@
 #include "kexec-syscall.h"
 #include "arch/options.h"
 
+#define SZ_2M 0x200000
+
+#define __round_mask(x, y) ((__typeof__(x))((y)-1))
+#define round_up(x, y) ((((x)-1) | __round_mask(x, y))+1)
+#define round_down(x, y) ((x) & ~__round_mask(x, y))
+
 /* Global varables the core kexec routines expect. */
 
 unsigned char reuse_initrd;
@@ -944,6 +950,10 @@ static int get_memory_ranges_iomem(struct memory_range *array,
 		str = line + consumed;
 		r.end++;
 
+		if (memcmp(str, "Kernel code\n", 12)==0) {
+			set_memstart(round_down(r.start,SZ_2M));
+		}
+
 		if (memcmp(str, "System RAM\n", 11)) {
 			dbgprintf("%s:%d: SKIP: %016Lx - %016Lx : %s", __func__,
 				__LINE__, r.start, r.end, str);
@@ -956,8 +966,6 @@ static int get_memory_ranges_iomem(struct memory_range *array,
 			__LINE__, r.start, r.end, str);
 
 		array[(*count)++] = r;
-
-		set_memstart(r.start);
 	}
 
 	fclose(fp);
-- 
Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] arm64:kexec: Memstart should not be before the kernel start address

[Geoff Levand]

Hi,

I already added a patch like this to my series.  Is this different?

-Geoff

On Tue, 2016-04-26 at 16:14 -0600, Sameer Goel wrote:
> Starting 4.6-rc4 the kernel memblock start is rounded down to a desirable
> alignment. So, the kernel can see reserved memory regions before the kernel
> start address in the iomem query.
> Need to make sure that the right kernel start address is picked from the iomem
> query.
> ---
>  kexec/arch/arm64/kexec-arm64.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 776596a..5e39f4f 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -30,6 +30,12 @@
>  #include "kexec-syscall.h"
>  #include "arch/options.h"
>  
> +#define SZ_2M 0x200000
> +
> +#define __round_mask(x, y) ((__typeof__(x))((y)-1))
> +#define round_up(x, y) ((((x)-1) | __round_mask(x, y))+1)
> +#define round_down(x, y) ((x) & ~__round_mask(x, y))
> +
>  /* Global varables the core kexec routines expect. */
>  
>  unsigned char reuse_initrd;
> @@ -944,6 +950,10 @@ static int get_memory_ranges_iomem(struct memory_range *array,
>  > 	> 	> str = line + consumed;
>  > 	> 	> r.end++;
>  
> +> 	> 	> if (memcmp(str, "Kernel code\n", 12)==0) {
> +> 	> 	> 	> set_memstart(round_down(r.start,SZ_2M));
> +> 	> 	> }
> +
>  > 	> 	> if (memcmp(str, "System RAM\n", 11)) {
>  > 	> 	> 	> dbgprintf("%s:%d: SKIP: %016Lx - %016Lx : %s", __func__,
>  > 	> 	> 	> 	> __LINE__, r.start, r.end, str);
> @@ -956,8 +966,6 @@ static int get_memory_ranges_iomem(struct memory_range *array,
>  > 	> 	> 	> __LINE__, r.start, r.end, str);
>  
>  > 	> 	> array[(*count)++] = r;
> -
> -> 	> 	> set_memstart(r.start);
>  > 	> }
>  
>  > 	> fclose(fp);

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] arm64:kexec: Memstart should not be before the kernel start address

[Goel, Sameer]

No, this is not. It was just a minor change to the commit message to 
clarify the change a bit. Appreciate your picking up the patch.

Regards,
Sameer

On 4/26/2016 5:58 PM, Geoff Levand wrote:
> Hi,
>
> I already added a patch like this to my series.  Is this different?
>
> -Geoff
>
> On Tue, 2016-04-26 at 16:14 -0600, Sameer Goel wrote:
>> Starting 4.6-rc4 the kernel memblock start is rounded down to a desirable
>> alignment. So, the kernel can see reserved memory regions before the kernel
>> start address in the iomem query.
>> Need to make sure that the right kernel start address is picked from the iomem
>> query.
>> ---
>>  kexec/arch/arm64/kexec-arm64.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
>> index 776596a..5e39f4f 100644
>> --- a/kexec/arch/arm64/kexec-arm64.c
>> +++ b/kexec/arch/arm64/kexec-arm64.c
>> @@ -30,6 +30,12 @@
>>  #include "kexec-syscall.h"
>>  #include "arch/options.h"
>>
>> +#define SZ_2M 0x200000
>> +
>> +#define __round_mask(x, y) ((__typeof__(x))((y)-1))
>> +#define round_up(x, y) ((((x)-1) | __round_mask(x, y))+1)
>> +#define round_down(x, y) ((x) & ~__round_mask(x, y))
>> +
>>  /* Global varables the core kexec routines expect. */
>>
>>  unsigned char reuse_initrd;
>> @@ -944,6 +950,10 @@ static int get_memory_ranges_iomem(struct memory_range *array,
>>  > 	> 	> str = line + consumed;
>>  > 	> 	> r.end++;
>>
>> +> 	> 	> if (memcmp(str, "Kernel code\n", 12)==0) {
>> +> 	> 	> 	> set_memstart(round_down(r.start,SZ_2M));
>> +> 	> 	> }
>> +
>>  > 	> 	> if (memcmp(str, "System RAM\n", 11)) {
>>  > 	> 	> 	> dbgprintf("%s:%d: SKIP: %016Lx - %016Lx : %s", __func__,
>>  > 	> 	> 	> 	> __LINE__, r.start, r.end, str);
>> @@ -956,8 +966,6 @@ static int get_memory_ranges_iomem(struct memory_range *array,
>>  > 	> 	> 	> __LINE__, r.start, r.end, str);
>>
>>  > 	> 	> array[(*count)++] = r;
>> -
>> -> 	> 	> set_memstart(r.start);
>>  > 	> }
>>
>>  > 	> fclose(fp);
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] arm64:kexec: Memstart should not be before the kernel start address

[Geoff Levand]

Hi Sameer,

> > > On Tue, 2016-04-26 at 16:14 -0600, Sameer Goel wrote:
> > > > Starting 4.6-rc4 the kernel memblock start is rounded down to a desirable
> > > > alignment. So, the kernel can see reserved memory regions before the kernel
> > > > start address in the iomem query.
> > > > Need to make sure that the right kernel start address is picked from the iomem
> > > > query.
> > > > ---

It is still in question whether or not your's is the correct
fix.  Could you provide a test case that hits this problem
so we can work on the solution?

Thanks.

-Geoff


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] arm64:kexec: Memstart should not be before the kernel start address

[Goel, Sameer]

Hi Geoff,
In our setup we cannot place the kernel starting at 0 address. So, it it 
placed at the first valid aligned address after placing the reserved 
regions in memory.

The code from the change (https://patchwork.kernel.org/patch/8325221/) 
adds the reserved regions to iomem ranges. So, when the kexec tool is 
run it picks up the first reserved region as the start address for 
physical ram. When the kernel is loaded at this address, it fails to boot.

So, I have put in a filter in the code that would pick up the right 
address of the primary kernel after this change was introduced.
Thanks,
Sameer

On 6/10/2016 10:41 AM, Geoff Levand wrote:
> Hi Sameer,
>
>>>> On Tue, 2016-04-26 at 16:14 -0600, Sameer Goel wrote:
>>>>> Starting 4.6-rc4 the kernel memblock start is rounded down to a desirable
>>>>> alignment. So, the kernel can see reserved memory regions before the kernel
>>>>> start address in the iomem query.
>>>>> Need to make sure that the right kernel start address is picked from the iomem
>>>>> query.
>>>>> ---
>
> It is still in question whether or not your's is the correct
> fix.  Could you provide a test case that hits this problem
> so we can work on the solution?
>
> Thanks.
>
> -Geoff
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] arm64:kexec: Memstart should not be before the kernel start address

[Geoff Levand]

Hi,

On Fri, 2016-07-29 at 16:58 -0600, Goel, Sameer wrote:
> In our setup we cannot place the kernel starting at 0 address. So, it it 
> placed at the first valid aligned address after placing the reserved 
> regions in memory.
> 
> The code from the change (https://patchwork.kernel.org/patch/8325221/) 
> adds the reserved regions to iomem ranges. So, when the kexec tool is 
> run it picks up the first reserved region as the start address for 
> physical ram. When the kernel is loaded at this address, it fails to boot.
> 
> So, I have put in a filter in the code that would pick up the right 
> address of the primary kernel after this change was introduced.
> Thanks,
> Sameer


Your patch no longer applies.  Please rebase it to my
for-merge-arm64-v2 branch and re-post.

-Geoff


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] arm64:kexec: Memstart should not be before the kernel start address

[AKASHI Takahiro]

On Mon, Aug 01, 2016 at 12:31:50PM -0700, Geoff Levand wrote:
> Hi,
> 
> On Fri, 2016-07-29 at 16:58 -0600, Goel, Sameer wrote:
> > In our setup we cannot place the kernel starting at 0 address. So, it it 
> > placed at the first valid aligned address after placing the reserved 
> > regions in memory.
> > 
> > The code from the change (https://patchwork.kernel.org/patch/8325221/) 
> > adds the reserved regions to iomem ranges. So, when the kexec tool is 
> > run it picks up the first reserved region as the start address for 
> > physical ram. When the kernel is loaded at this address, it fails to boot.
> > 
> > So, I have put in a filter in the code that would pick up the right 
> > address of the primary kernel after this change was introduced.

You're using Image, not vmlinux, right?
I don't think that it's a good idea to add this sort of restriction
because it is very system-specific.
You'd better use "--mem-min=" parameter in your case.
(In fact, this option doesn't work on the current arm64 port of kexec-tools,
though.)

Thanks,
-Takahiro AKASHI

> > Thanks,
> > Sameer
> 
> 
> Your patch no longer applies.  Please rebase it to my
> for-merge-arm64-v2 branch and re-post.
> 
> -Geoff
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] arm64:kexec: Memstart should not be before the kernel start address

[Goel, Sameer]

Hi,
I am using vmlinux in this case. Usually the kernel code start is at a 
2M alignment from the System RAM start address. Since, kexec default 
options were not loading the image at the right address (same as the 
original kernel) and the API used to get iomem listing does not 
differentiate between the reserved and memblock addresses, I recommended 
that we look for the Kernel Code segment and round this down to get the 
right start address.
The above should not impact any current functionality or ignore any ram 
regions when booting the patched kernel.
I will report the patch on V2.
Thanks,
Sameer


On 8/1/2016 7:25 PM, AKASHI Takahiro wrote:
> On Mon, Aug 01, 2016 at 12:31:50PM -0700, Geoff Levand wrote:
>> Hi,
>>
>> On Fri, 2016-07-29 at 16:58 -0600, Goel, Sameer wrote:
>>> In our setup we cannot place the kernel starting at 0 address. So, it it
>>> placed at the first valid aligned address after placing the reserved
>>> regions in memory.
>>>
>>> The code from the change (https://patchwork.kernel.org/patch/8325221/)
>>> adds the reserved regions to iomem ranges. So, when the kexec tool is
>>> run it picks up the first reserved region as the start address for
>>> physical ram. When the kernel is loaded at this address, it fails to boot.
>>>
>>> So, I have put in a filter in the code that would pick up the right
>>> address of the primary kernel after this change was introduced.
>
> You're using Image, not vmlinux, right?
> I don't think that it's a good idea to add this sort of restriction
> because it is very system-specific.
> You'd better use "--mem-min=" parameter in your case.
> (In fact, this option doesn't work on the current arm64 port of kexec-tools,
> though.)
>
> Thanks,
> -Takahiro AKASHI
>
>>> Thanks,
>>> Sameer
>>
>>
>> Your patch no longer applies.  Please rebase it to my
>> for-merge-arm64-v2 branch and re-post.
>>
>> -Geoff
>>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] arm64:kexec: Memstart should not be before the kernel start address

[AKASHI Takahiro]

On Wed, Aug 03, 2016 at 05:44:30PM -0600, Goel, Sameer wrote:
> Hi,
> I am using vmlinux in this case. Usually the kernel code start is at
> a 2M alignment from the System RAM start address. Since, kexec
> default options were not loading the image at the right address
> (same as the original kernel) and the API used to get iomem listing
> does not differentiate between the reserved and memblock addresses,

In your original email, you said that you cannot place the kernel
at the address 0x0. Is this your problem, isn't it?

If so, you can explicitly specify the start address with "crashkernel=X@Y"
parameter. Or we might better add "mem-min" support as I said.

> I recommended that we look for the Kernel Code segment and round
> this down to get the right start address.
> The above should not impact any current functionality or ignore any
> ram regions when booting the patched kernel.

no need to impose such a unnecessary restriction.

Thanks,
-Takahiro AKASHI

> I will report the patch on V2.
> Thanks,
> Sameer
> 
> 
> On 8/1/2016 7:25 PM, AKASHI Takahiro wrote:
> >On Mon, Aug 01, 2016 at 12:31:50PM -0700, Geoff Levand wrote:
> >>Hi,
> >>
> >>On Fri, 2016-07-29 at 16:58 -0600, Goel, Sameer wrote:
> >>>In our setup we cannot place the kernel starting at 0 address. So, it it
> >>>placed at the first valid aligned address after placing the reserved
> >>>regions in memory.
> >>>
> >>>The code from the change (https://patchwork.kernel.org/patch/8325221/)
> >>>adds the reserved regions to iomem ranges. So, when the kexec tool is
> >>>run it picks up the first reserved region as the start address for
> >>>physical ram. When the kernel is loaded at this address, it fails to boot.
> >>>
> >>>So, I have put in a filter in the code that would pick up the right
> >>>address of the primary kernel after this change was introduced.
> >
> >You're using Image, not vmlinux, right?
> >I don't think that it's a good idea to add this sort of restriction
> >because it is very system-specific.
> >You'd better use "--mem-min=" parameter in your case.
> >(In fact, this option doesn't work on the current arm64 port of kexec-tools,
> >though.)
> >
> >Thanks,
> >-Takahiro AKASHI
> >
> >>>Thanks,
> >>>Sameer
> >>
> >>
> >>Your patch no longer applies.  Please rebase it to my
> >>for-merge-arm64-v2 branch and re-post.
> >>
> >>-Geoff
> >>
> >
> >_______________________________________________
> >kexec mailing list
> >kexec@lists.infradead.org
> >http://lists.infradead.org/mailman/listinfo/kexec
> >
> 
> -- 
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] arm64:kexec: Memstart should not be before the kernel start address

[Goel, Sameer]

 > In your original email, you said that you cannot place the kernel
 > at the address 0x0. Is this your problem, isn't it?

Not being able to put the kernel at 0 should not be considered a  
restriction. The kernel is not mandated to start at 0 address. The tools  
code was working fine till arm linux developers released a new set of  
changes to get the alignment right for performance reasons.

This change makes sure that he kernel start is 1GB aligned. So, once the  
round doen happens the regions that are reserved memblocks are also seen  
using the iomem query API. The reserved regions should not be considered  
as a part of the kernel RAM.

 > If so, you can explicitly specify the start address with  
"crashkernel=X@Y"
 > parameter. Or we might better add "mem-min" support as I said.

I agree that the above options will solve this case.

As I mentioned before that not starting at address 0x0 should not be  
considered a limitation.So, default case of reuse-cmdline should work as  
is.
In this code we are trying to find where the kernel code start is. Based  
on this info we can always clearly determine the kernel ram start. This  
would not be a limitation.

Any system ram region located before kernel ram should not be used  
unless explicitly reclaimed or added.

Thanks,
Sameer


On 8/3/2016 7:16 PM, AKASHI Takahiro wrote:
> On Wed, Aug 03, 2016 at 05:44:30PM -0600, Goel, Sameer wrote:
>> Hi,
>> I am using vmlinux in this case. Usually the kernel code start is at
>> a 2M alignment from the System RAM start address. Since, kexec
>> default options were not loading the image at the right address
>> (same as the original kernel) and the API used to get iomem listing
>> does not differentiate between the reserved and memblock addresses,
>
> In your original email, you said that you cannot place the kernel
> at the address 0x0. Is this your problem, isn't it?
>
> If so, you can explicitly specify the start address with "crashkernel=X@Y"
> parameter. Or we might better add "mem-min" support as I said.
>
>> I recommended that we look for the Kernel Code segment and round
>> this down to get the right start address.
>> The above should not impact any current functionality or ignore any
>> ram regions when booting the patched kernel.
>
> no need to impose such a unnecessary restriction.
>
> Thanks,
> -Takahiro AKASHI
>
>> I will report the patch on V2.
>> Thanks,
>> Sameer
>>
>>
>> On 8/1/2016 7:25 PM, AKASHI Takahiro wrote:
>>> On Mon, Aug 01, 2016 at 12:31:50PM -0700, Geoff Levand wrote:
>>>> Hi,
>>>>
>>>> On Fri, 2016-07-29 at 16:58 -0600, Goel, Sameer wrote:
>>>>> In our setup we cannot place the kernel starting at 0 address. So, it it
>>>>> placed at the first valid aligned address after placing the reserved
>>>>> regions in memory.
>>>>>
>>>>> The code from the change (https://patchwork.kernel.org/patch/8325221/)
>>>>> adds the reserved regions to iomem ranges. So, when the kexec tool is
>>>>> run it picks up the first reserved region as the start address for
>>>>> physical ram. When the kernel is loaded at this address, it fails to boot.
>>>>>
>>>>> So, I have put in a filter in the code that would pick up the right
>>>>> address of the primary kernel after this change was introduced.
>>>
>>> You're using Image, not vmlinux, right?
>>> I don't think that it's a good idea to add this sort of restriction
>>> because it is very system-specific.
>>> You'd better use "--mem-min=" parameter in your case.
>>> (In fact, this option doesn't work on the current arm64 port of kexec-tools,
>>> though.)
>>>
>>> Thanks,
>>> -Takahiro AKASHI
>>>
>>>>> Thanks,
>>>>> Sameer
>>>>
>>>>
>>>> Your patch no longer applies.  Please rebase it to my
>>>> for-merge-arm64-v2 branch and re-post.
>>>>
>>>> -Geoff
>>>>
>>>
>>> _______________________________________________
>>> kexec mailing list
>>> kexec@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/kexec
>>>
>>
>> --
>> Qualcomm Innovation Center, Inc.
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project.
>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec