| Message ID | 20210310102013.91253-1-vladimir.murzin@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: NOMMU: Fix conversion for_each_membock() to for_each_mem_range() | expand |
On Wed, Mar 10, 2021 at 10:20:13AM +0000, Vladimir Murzin wrote: > for_each_mem_range() uses a loop variable, yet looking into code it is > not just iteration counter but more complex entity which encodes > information about memblock. Thus condition i == 0 looks fragile. > Indeed, it broke boot of R-class platforms since it never took i == 0 > path (due to i was set to 1). Oops, sorry about that. > Fix that with restoring original flag > check. > > Fixes: b10d6bca8720 ("arch, drivers: replace for_each_membock() with for_each_mem_range()") > Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> Acked-by: Mike Rapoport <rppt@linux.ibm.com> > --- > arch/arm/mm/pmsa-v7.c | 4 +++- > arch/arm/mm/pmsa-v8.c | 4 +++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mm/pmsa-v7.c b/arch/arm/mm/pmsa-v7.c > index 88950e41a3a9..59d916ccdf25 100644 > --- a/arch/arm/mm/pmsa-v7.c > +++ b/arch/arm/mm/pmsa-v7.c > @@ -235,6 +235,7 @@ void __init pmsav7_adjust_lowmem_bounds(void) > phys_addr_t mem_end; > phys_addr_t reg_start, reg_end; > unsigned int mem_max_regions; > + bool first = true; > int num; > u64 i; > > @@ -263,7 +264,7 @@ void __init pmsav7_adjust_lowmem_bounds(void) > #endif > > for_each_mem_range(i, ®_start, ®_end) { > - if (i == 0) { > + if (first) { > phys_addr_t phys_offset = PHYS_OFFSET; > > /* > @@ -275,6 +276,7 @@ void __init pmsav7_adjust_lowmem_bounds(void) > mem_start = reg_start; > mem_end = reg_end; > specified_mem_size = mem_end - mem_start; > + first = false; > } else { > /* > * memblock auto merges contiguous blocks, remove > diff --git a/arch/arm/mm/pmsa-v8.c b/arch/arm/mm/pmsa-v8.c > index 2de019f7503e..8359748a19a1 100644 > --- a/arch/arm/mm/pmsa-v8.c > +++ b/arch/arm/mm/pmsa-v8.c > @@ -95,10 +95,11 @@ void __init pmsav8_adjust_lowmem_bounds(void) > { > phys_addr_t mem_end; > phys_addr_t reg_start, reg_end; > + bool first = true; > u64 i; > > for_each_mem_range(i, ®_start, ®_end) { > - if (i == 0) { > + if (first) { > phys_addr_t phys_offset = PHYS_OFFSET; > > /* > @@ -107,6 +108,7 @@ void __init pmsav8_adjust_lowmem_bounds(void) > if (reg_start != phys_offset) > panic("First memory bank must be contiguous from PHYS_OFFSET"); > mem_end = reg_end; > + first = false; > } else { > /* > * memblock auto merges contiguous blocks, remove > -- > 2.24.0 >
On 3/10/21 1:23 PM, Mike Rapoport wrote: > On Wed, Mar 10, 2021 at 10:20:13AM +0000, Vladimir Murzin wrote: >> for_each_mem_range() uses a loop variable, yet looking into code it is >> not just iteration counter but more complex entity which encodes >> information about memblock. Thus condition i == 0 looks fragile. >> Indeed, it broke boot of R-class platforms since it never took i == 0 >> path (due to i was set to 1). > > Oops, sorry about that. > >> Fix that with restoring original flag >> check. >> >> Fixes: b10d6bca8720 ("arch, drivers: replace for_each_membock() with for_each_mem_range()") >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > > Acked-by: Mike Rapoport <rppt@linux.ibm.com> > Thanks! I've just put it into Russell's patch system as patch 9069/1 Vladimir >> --- >> arch/arm/mm/pmsa-v7.c | 4 +++- >> arch/arm/mm/pmsa-v8.c | 4 +++- >> 2 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/mm/pmsa-v7.c b/arch/arm/mm/pmsa-v7.c >> index 88950e41a3a9..59d916ccdf25 100644 >> --- a/arch/arm/mm/pmsa-v7.c >> +++ b/arch/arm/mm/pmsa-v7.c >> @@ -235,6 +235,7 @@ void __init pmsav7_adjust_lowmem_bounds(void) >> phys_addr_t mem_end; >> phys_addr_t reg_start, reg_end; >> unsigned int mem_max_regions; >> + bool first = true; >> int num; >> u64 i; >> >> @@ -263,7 +264,7 @@ void __init pmsav7_adjust_lowmem_bounds(void) >> #endif >> >> for_each_mem_range(i, ®_start, ®_end) { >> - if (i == 0) { >> + if (first) { >> phys_addr_t phys_offset = PHYS_OFFSET; >> >> /* >> @@ -275,6 +276,7 @@ void __init pmsav7_adjust_lowmem_bounds(void) >> mem_start = reg_start; >> mem_end = reg_end; >> specified_mem_size = mem_end - mem_start; >> + first = false; >> } else { >> /* >> * memblock auto merges contiguous blocks, remove >> diff --git a/arch/arm/mm/pmsa-v8.c b/arch/arm/mm/pmsa-v8.c >> index 2de019f7503e..8359748a19a1 100644 >> --- a/arch/arm/mm/pmsa-v8.c >> +++ b/arch/arm/mm/pmsa-v8.c >> @@ -95,10 +95,11 @@ void __init pmsav8_adjust_lowmem_bounds(void) >> { >> phys_addr_t mem_end; >> phys_addr_t reg_start, reg_end; >> + bool first = true; >> u64 i; >> >> for_each_mem_range(i, ®_start, ®_end) { >> - if (i == 0) { >> + if (first) { >> phys_addr_t phys_offset = PHYS_OFFSET; >> >> /* >> @@ -107,6 +108,7 @@ void __init pmsav8_adjust_lowmem_bounds(void) >> if (reg_start != phys_offset) >> panic("First memory bank must be contiguous from PHYS_OFFSET"); >> mem_end = reg_end; >> + first = false; >> } else { >> /* >> * memblock auto merges contiguous blocks, remove >> -- >> 2.24.0 >> >
diff --git a/arch/arm/mm/pmsa-v7.c b/arch/arm/mm/pmsa-v7.c index 88950e41a3a9..59d916ccdf25 100644 --- a/arch/arm/mm/pmsa-v7.c +++ b/arch/arm/mm/pmsa-v7.c @@ -235,6 +235,7 @@ void __init pmsav7_adjust_lowmem_bounds(void) phys_addr_t mem_end; phys_addr_t reg_start, reg_end; unsigned int mem_max_regions; + bool first = true; int num; u64 i; @@ -263,7 +264,7 @@ void __init pmsav7_adjust_lowmem_bounds(void) #endif for_each_mem_range(i, ®_start, ®_end) { - if (i == 0) { + if (first) { phys_addr_t phys_offset = PHYS_OFFSET; /* @@ -275,6 +276,7 @@ void __init pmsav7_adjust_lowmem_bounds(void) mem_start = reg_start; mem_end = reg_end; specified_mem_size = mem_end - mem_start; + first = false; } else { /* * memblock auto merges contiguous blocks, remove diff --git a/arch/arm/mm/pmsa-v8.c b/arch/arm/mm/pmsa-v8.c index 2de019f7503e..8359748a19a1 100644 --- a/arch/arm/mm/pmsa-v8.c +++ b/arch/arm/mm/pmsa-v8.c @@ -95,10 +95,11 @@ void __init pmsav8_adjust_lowmem_bounds(void) { phys_addr_t mem_end; phys_addr_t reg_start, reg_end; + bool first = true; u64 i; for_each_mem_range(i, ®_start, ®_end) { - if (i == 0) { + if (first) { phys_addr_t phys_offset = PHYS_OFFSET; /* @@ -107,6 +108,7 @@ void __init pmsav8_adjust_lowmem_bounds(void) if (reg_start != phys_offset) panic("First memory bank must be contiguous from PHYS_OFFSET"); mem_end = reg_end; + first = false; } else { /* * memblock auto merges contiguous blocks, remove
for_each_mem_range() uses a loop variable, yet looking into code it is not just iteration counter but more complex entity which encodes information about memblock. Thus condition i == 0 looks fragile. Indeed, it broke boot of R-class platforms since it never took i == 0 path (due to i was set to 1). Fix that with restoring original flag check. Fixes: b10d6bca8720 ("arch, drivers: replace for_each_membock() with for_each_mem_range()") Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> --- arch/arm/mm/pmsa-v7.c | 4 +++- arch/arm/mm/pmsa-v8.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)