| Message ID | 20210405085712.1953848-4-mick@ics.forth.gr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V: Add kexec/kdump support | expand |
Hi Nick, Thanks for your patch! On Mon, Apr 5, 2021 at 10:57 AM Nick Kossifidis <mick@ics.forth.gr> wrote: > * Kernel region is always present and we know where it is, no > need to look for it inside the loop, just ignore it like the > rest of the reserved regions within system's memory. > > * Don't call memblock_free inside the loop, if called it'll split > the region of pre-allocated resources in two parts, messing things > up, just re-use the previous pre-allocated resource and free any > unused resources after both loops finish. > > * memblock_alloc may add a region when called, so increase the > number of pre-allocated regions by one to be on the safe side > (reported and patched by Geert Uytterhoeven) > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> Where does this SoB come from? > Signed-off-by: Nick Kossifidis <mick@ics.forth.gr> > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -129,53 +139,42 @@ static void __init init_resources(void) > struct resource *res = NULL; > struct resource *mem_res = NULL; > size_t mem_res_sz = 0; > - int ret = 0, i = 0; > - > - code_res.start = __pa_symbol(_text); > - code_res.end = __pa_symbol(_etext) - 1; > - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > - > - rodata_res.start = __pa_symbol(__start_rodata); > - rodata_res.end = __pa_symbol(__end_rodata) - 1; > - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > - > - data_res.start = __pa_symbol(_data); > - data_res.end = __pa_symbol(_edata) - 1; > - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > + int num_resources = 0, res_idx = 0; > + int ret = 0; > > - bss_res.start = __pa_symbol(__bss_start); > - bss_res.end = __pa_symbol(__bss_stop) - 1; > - bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > + /* + 1 as memblock_alloc() might increase memblock.reserved.cnt */ > + num_resources = memblock.memory.cnt + memblock.reserved.cnt + 1; > + res_idx = num_resources - 1; > > - mem_res_sz = (memblock.memory.cnt + memblock.reserved.cnt) * sizeof(*mem_res); Oh, you incorporated my commit ce989f1472ae350e ("RISC-V: Fix out-of-bounds accesses in init_resources()") (from v5.12-rc4) into your patch. Why? This means your patch does not apply against upstream. > + mem_res_sz = num_resources * sizeof(*mem_res); > mem_res = memblock_alloc(mem_res_sz, SMP_CACHE_BYTES); > if (!mem_res) > panic("%s: Failed to allocate %zu bytes\n", __func__, mem_res_sz); Gr{oetje,eeting}s, Geert
Hello Geert, Στις 2021-04-06 10:19, Geert Uytterhoeven έγραψε: > Hi Nick, > > Thanks for your patch! > > On Mon, Apr 5, 2021 at 10:57 AM Nick Kossifidis <mick@ics.forth.gr> > wrote: >> * Kernel region is always present and we know where it is, no >> need to look for it inside the loop, just ignore it like the >> rest of the reserved regions within system's memory. >> >> * Don't call memblock_free inside the loop, if called it'll split >> the region of pre-allocated resources in two parts, messing things >> up, just re-use the previous pre-allocated resource and free any >> unused resources after both loops finish. >> >> * memblock_alloc may add a region when called, so increase the >> number of pre-allocated regions by one to be on the safe side >> (reported and patched by Geert Uytterhoeven) >> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > > Where does this SoB come from? > >> Signed-off-by: Nick Kossifidis <mick@ics.forth.gr> > >> --- a/arch/riscv/kernel/setup.c >> +++ b/arch/riscv/kernel/setup.c > >> @@ -129,53 +139,42 @@ static void __init init_resources(void) >> struct resource *res = NULL; >> struct resource *mem_res = NULL; >> size_t mem_res_sz = 0; >> - int ret = 0, i = 0; >> - >> - code_res.start = __pa_symbol(_text); >> - code_res.end = __pa_symbol(_etext) - 1; >> - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> - >> - rodata_res.start = __pa_symbol(__start_rodata); >> - rodata_res.end = __pa_symbol(__end_rodata) - 1; >> - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> - >> - data_res.start = __pa_symbol(_data); >> - data_res.end = __pa_symbol(_edata) - 1; >> - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> + int num_resources = 0, res_idx = 0; >> + int ret = 0; >> >> - bss_res.start = __pa_symbol(__bss_start); >> - bss_res.end = __pa_symbol(__bss_stop) - 1; >> - bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> + /* + 1 as memblock_alloc() might increase >> memblock.reserved.cnt */ >> + num_resources = memblock.memory.cnt + memblock.reserved.cnt + >> 1; >> + res_idx = num_resources - 1; >> >> - mem_res_sz = (memblock.memory.cnt + memblock.reserved.cnt) * >> sizeof(*mem_res); > > Oh, you incorporated my commit ce989f1472ae350e ("RISC-V: Fix > out-of-bounds > accesses in init_resources()") (from v5.12-rc4) into your patch. > Why? This means your patch does not apply against upstream. > Sorry if this looks awkward, I'm under the impression that new features go on for-next instead of fixes and your patch hasn't been merged on for-next yet. I thought it would be cleaner to have one patch to merge for init_resources instead of two, and simpler for people to test the series. I can rebase this on top of fixes if that works better for you or Palmer. Regards, Nick
Hi Nick, On Tue, Apr 6, 2021 at 10:11 AM Nick Kossifidis <mick@ics.forth.gr> wrote: > Hello Geert, > Στις 2021-04-06 10:19, Geert Uytterhoeven έγραψε: > > On Mon, Apr 5, 2021 at 10:57 AM Nick Kossifidis <mick@ics.forth.gr> > > wrote: > >> * Kernel region is always present and we know where it is, no > >> need to look for it inside the loop, just ignore it like the > >> rest of the reserved regions within system's memory. > >> > >> * Don't call memblock_free inside the loop, if called it'll split > >> the region of pre-allocated resources in two parts, messing things > >> up, just re-use the previous pre-allocated resource and free any > >> unused resources after both loops finish. > >> > >> * memblock_alloc may add a region when called, so increase the > >> number of pre-allocated regions by one to be on the safe side > >> (reported and patched by Geert Uytterhoeven) > >> > >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > > > > Where does this SoB come from? > > > >> Signed-off-by: Nick Kossifidis <mick@ics.forth.gr> > > > >> --- a/arch/riscv/kernel/setup.c > >> +++ b/arch/riscv/kernel/setup.c > > > >> @@ -129,53 +139,42 @@ static void __init init_resources(void) > >> struct resource *res = NULL; > >> struct resource *mem_res = NULL; > >> size_t mem_res_sz = 0; > >> - int ret = 0, i = 0; > >> - > >> - code_res.start = __pa_symbol(_text); > >> - code_res.end = __pa_symbol(_etext) - 1; > >> - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > >> - > >> - rodata_res.start = __pa_symbol(__start_rodata); > >> - rodata_res.end = __pa_symbol(__end_rodata) - 1; > >> - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > >> - > >> - data_res.start = __pa_symbol(_data); > >> - data_res.end = __pa_symbol(_edata) - 1; > >> - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > >> + int num_resources = 0, res_idx = 0; > >> + int ret = 0; > >> > >> - bss_res.start = __pa_symbol(__bss_start); > >> - bss_res.end = __pa_symbol(__bss_stop) - 1; > >> - bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > >> + /* + 1 as memblock_alloc() might increase > >> memblock.reserved.cnt */ > >> + num_resources = memblock.memory.cnt + memblock.reserved.cnt + > >> 1; > >> + res_idx = num_resources - 1; > >> > >> - mem_res_sz = (memblock.memory.cnt + memblock.reserved.cnt) * > >> sizeof(*mem_res); > > > > Oh, you incorporated my commit ce989f1472ae350e ("RISC-V: Fix > > out-of-bounds > > accesses in init_resources()") (from v5.12-rc4) into your patch. > > Why? This means your patch does not apply against upstream. > > > > Sorry if this looks awkward, I'm under the impression that new features > go on for-next instead of fixes and your patch hasn't been merged on > for-next yet. I thought it would be cleaner to have one patch to merge > for init_resources instead of two, and simpler for people to test the > series. I can rebase this on top of fixes if that works better for you > or Palmer. Ideally the fixes branch is part of the next branch. That also helps to avoid other people having to fix conflicts when merging both. Gr{oetje,eeting}s, Geert
Στις 2021-04-06 11:22, Geert Uytterhoeven έγραψε: > Hi Nick, > > On Tue, Apr 6, 2021 at 10:11 AM Nick Kossifidis <mick@ics.forth.gr> > wrote: >> Hello Geert, >> Στις 2021-04-06 10:19, Geert Uytterhoeven έγραψε: >> > On Mon, Apr 5, 2021 at 10:57 AM Nick Kossifidis <mick@ics.forth.gr> >> > wrote: >> >> * Kernel region is always present and we know where it is, no >> >> need to look for it inside the loop, just ignore it like the >> >> rest of the reserved regions within system's memory. >> >> >> >> * Don't call memblock_free inside the loop, if called it'll split >> >> the region of pre-allocated resources in two parts, messing things >> >> up, just re-use the previous pre-allocated resource and free any >> >> unused resources after both loops finish. >> >> >> >> * memblock_alloc may add a region when called, so increase the >> >> number of pre-allocated regions by one to be on the safe side >> >> (reported and patched by Geert Uytterhoeven) >> >> >> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> >> > >> > Where does this SoB come from? >> > >> >> Signed-off-by: Nick Kossifidis <mick@ics.forth.gr> >> > >> >> --- a/arch/riscv/kernel/setup.c >> >> +++ b/arch/riscv/kernel/setup.c >> > >> >> @@ -129,53 +139,42 @@ static void __init init_resources(void) >> >> struct resource *res = NULL; >> >> struct resource *mem_res = NULL; >> >> size_t mem_res_sz = 0; >> >> - int ret = 0, i = 0; >> >> - >> >> - code_res.start = __pa_symbol(_text); >> >> - code_res.end = __pa_symbol(_etext) - 1; >> >> - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> >> - >> >> - rodata_res.start = __pa_symbol(__start_rodata); >> >> - rodata_res.end = __pa_symbol(__end_rodata) - 1; >> >> - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> >> - >> >> - data_res.start = __pa_symbol(_data); >> >> - data_res.end = __pa_symbol(_edata) - 1; >> >> - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> >> + int num_resources = 0, res_idx = 0; >> >> + int ret = 0; >> >> >> >> - bss_res.start = __pa_symbol(__bss_start); >> >> - bss_res.end = __pa_symbol(__bss_stop) - 1; >> >> - bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> >> + /* + 1 as memblock_alloc() might increase >> >> memblock.reserved.cnt */ >> >> + num_resources = memblock.memory.cnt + memblock.reserved.cnt + >> >> 1; >> >> + res_idx = num_resources - 1; >> >> >> >> - mem_res_sz = (memblock.memory.cnt + memblock.reserved.cnt) * >> >> sizeof(*mem_res); >> > >> > Oh, you incorporated my commit ce989f1472ae350e ("RISC-V: Fix >> > out-of-bounds >> > accesses in init_resources()") (from v5.12-rc4) into your patch. >> > Why? This means your patch does not apply against upstream. >> > >> >> Sorry if this looks awkward, I'm under the impression that new >> features >> go on for-next instead of fixes and your patch hasn't been merged on >> for-next yet. I thought it would be cleaner to have one patch to merge >> for init_resources instead of two, and simpler for people to test the >> series. I can rebase this on top of fixes if that works better for you >> or Palmer. > > Ideally the fixes branch is part of the next branch. That also helps > to avoid other people having to fix conflicts when merging both. > OK I'll re-base this on top of fixes instead.
On Mon, 05 Apr 2021 01:57:10 PDT (-0700), mick@ics.forth.gr wrote: > * Kernel region is always present and we know where it is, no > need to look for it inside the loop, just ignore it like the > rest of the reserved regions within system's memory. > > * Don't call memblock_free inside the loop, if called it'll split > the region of pre-allocated resources in two parts, messing things > up, just re-use the previous pre-allocated resource and free any > unused resources after both loops finish. > > * memblock_alloc may add a region when called, so increase the > number of pre-allocated regions by one to be on the safe side > (reported and patched by Geert Uytterhoeven) IIUC this one has already been fixen on for-next. Either way, it caused a merge conflict. I think I've fixed it up, LMK if something went wrong. Also: I cleaned up the commit text a bit, as this is an odd way to do it. It's probably best to just have split this into two commits. > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Nick Kossifidis <mick@ics.forth.gr> > --- > arch/riscv/kernel/setup.c | 90 ++++++++++++++++++++------------------- > 1 file changed, 46 insertions(+), 44 deletions(-) > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index e85bacff1..030554bab 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -60,6 +60,7 @@ static DEFINE_PER_CPU(struct cpu, cpu_devices); > * also add "System RAM" regions for compatibility with other > * archs, and the rest of the known regions for completeness. > */ > +static struct resource kimage_res = { .name = "Kernel image", }; > static struct resource code_res = { .name = "Kernel code", }; > static struct resource data_res = { .name = "Kernel data", }; > static struct resource rodata_res = { .name = "Kernel rodata", }; > @@ -80,45 +81,54 @@ static int __init add_resource(struct resource *parent, > return 1; > } > > -static int __init add_kernel_resources(struct resource *res) > +static int __init add_kernel_resources(void) > { > int ret = 0; > > /* > * The memory region of the kernel image is continuous and > - * was reserved on setup_bootmem, find it here and register > - * it as a resource, then register the various segments of > - * the image as child nodes > + * was reserved on setup_bootmem, register it here as a > + * resource, with the various segments of the image as > + * child nodes. > */ > - if (!(res->start <= code_res.start && res->end >= data_res.end)) > - return 0; > > - res->name = "Kernel image"; > - res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > + code_res.start = __pa_symbol(_text); > + code_res.end = __pa_symbol(_etext) - 1; > + code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > > - /* > - * We removed a part of this region on setup_bootmem so > - * we need to expand the resource for the bss to fit in. > - */ > - res->end = bss_res.end; > + rodata_res.start = __pa_symbol(__start_rodata); > + rodata_res.end = __pa_symbol(__end_rodata) - 1; > + rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > > - ret = add_resource(&iomem_resource, res); > + data_res.start = __pa_symbol(_data); > + data_res.end = __pa_symbol(_edata) - 1; > + data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > + > + bss_res.start = __pa_symbol(__bss_start); > + bss_res.end = __pa_symbol(__bss_stop) - 1; > + bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > + > + kimage_res.start = code_res.start; > + kimage_res.end = bss_res.end; > + kimage_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > + > + ret = add_resource(&iomem_resource, &kimage_res); > if (ret < 0) > return ret; > > - ret = add_resource(res, &code_res); > + ret = add_resource(&kimage_res, &code_res); > if (ret < 0) > return ret; > > - ret = add_resource(res, &rodata_res); > + ret = add_resource(&kimage_res, &rodata_res); > if (ret < 0) > return ret; > > - ret = add_resource(res, &data_res); > + ret = add_resource(&kimage_res, &data_res); > if (ret < 0) > return ret; > > - ret = add_resource(res, &bss_res); > + ret = add_resource(&kimage_res, &bss_res); > > return ret; > } > @@ -129,53 +139,42 @@ static void __init init_resources(void) > struct resource *res = NULL; > struct resource *mem_res = NULL; > size_t mem_res_sz = 0; > - int ret = 0, i = 0; > - > - code_res.start = __pa_symbol(_text); > - code_res.end = __pa_symbol(_etext) - 1; > - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > - > - rodata_res.start = __pa_symbol(__start_rodata); > - rodata_res.end = __pa_symbol(__end_rodata) - 1; > - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > - > - data_res.start = __pa_symbol(_data); > - data_res.end = __pa_symbol(_edata) - 1; > - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > + int num_resources = 0, res_idx = 0; > + int ret = 0; > > - bss_res.start = __pa_symbol(__bss_start); > - bss_res.end = __pa_symbol(__bss_stop) - 1; > - bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > + /* + 1 as memblock_alloc() might increase memblock.reserved.cnt */ > + num_resources = memblock.memory.cnt + memblock.reserved.cnt + 1; > + res_idx = num_resources - 1; > > - mem_res_sz = (memblock.memory.cnt + memblock.reserved.cnt) * sizeof(*mem_res); > + mem_res_sz = num_resources * sizeof(*mem_res); > mem_res = memblock_alloc(mem_res_sz, SMP_CACHE_BYTES); > if (!mem_res) > panic("%s: Failed to allocate %zu bytes\n", __func__, mem_res_sz); > + > /* > * Start by adding the reserved regions, if they overlap > * with /memory regions, insert_resource later on will take > * care of it. > */ > + ret = add_kernel_resources(); > + if (ret < 0) > + goto error; > + > for_each_reserved_mem_region(region) { > - res = &mem_res[i++]; > + res = &mem_res[res_idx--]; > > res->name = "Reserved"; > res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; > res->start = __pfn_to_phys(memblock_region_reserved_base_pfn(region)); > res->end = __pfn_to_phys(memblock_region_reserved_end_pfn(region)) - 1; > > - ret = add_kernel_resources(res); > - if (ret < 0) > - goto error; > - else if (ret) > - continue; > - > /* > * Ignore any other reserved regions within > * system memory. > */ > if (memblock_is_memory(res->start)) { > - memblock_free((phys_addr_t) res, sizeof(struct resource)); > + /* Re-use this pre-allocated resource */ > + res_idx++; > continue; > } > > @@ -186,7 +185,7 @@ static void __init init_resources(void) > > /* Add /memory regions to the resource tree */ > for_each_mem_region(region) { > - res = &mem_res[i++]; > + res = &mem_res[res_idx--]; > > if (unlikely(memblock_is_nomap(region))) { > res->name = "Reserved"; > @@ -204,6 +203,9 @@ static void __init init_resources(void) > goto error; > } > > + /* Clean-up any unused pre-allocated resources */ > + mem_res_sz = (num_resources - res_idx + 1) * sizeof(*mem_res); > + memblock_free((phys_addr_t) mem_res, mem_res_sz); > return; > > error:
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index e85bacff1..030554bab 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -60,6 +60,7 @@ static DEFINE_PER_CPU(struct cpu, cpu_devices); * also add "System RAM" regions for compatibility with other * archs, and the rest of the known regions for completeness. */ +static struct resource kimage_res = { .name = "Kernel image", }; static struct resource code_res = { .name = "Kernel code", }; static struct resource data_res = { .name = "Kernel data", }; static struct resource rodata_res = { .name = "Kernel rodata", }; @@ -80,45 +81,54 @@ static int __init add_resource(struct resource *parent, return 1; } -static int __init add_kernel_resources(struct resource *res) +static int __init add_kernel_resources(void) { int ret = 0; /* * The memory region of the kernel image is continuous and - * was reserved on setup_bootmem, find it here and register - * it as a resource, then register the various segments of - * the image as child nodes + * was reserved on setup_bootmem, register it here as a + * resource, with the various segments of the image as + * child nodes. */ - if (!(res->start <= code_res.start && res->end >= data_res.end)) - return 0; - res->name = "Kernel image"; - res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + code_res.start = __pa_symbol(_text); + code_res.end = __pa_symbol(_etext) - 1; + code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - /* - * We removed a part of this region on setup_bootmem so - * we need to expand the resource for the bss to fit in. - */ - res->end = bss_res.end; + rodata_res.start = __pa_symbol(__start_rodata); + rodata_res.end = __pa_symbol(__end_rodata) - 1; + rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - ret = add_resource(&iomem_resource, res); + data_res.start = __pa_symbol(_data); + data_res.end = __pa_symbol(_edata) - 1; + data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + + bss_res.start = __pa_symbol(__bss_start); + bss_res.end = __pa_symbol(__bss_stop) - 1; + bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + + kimage_res.start = code_res.start; + kimage_res.end = bss_res.end; + kimage_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + + ret = add_resource(&iomem_resource, &kimage_res); if (ret < 0) return ret; - ret = add_resource(res, &code_res); + ret = add_resource(&kimage_res, &code_res); if (ret < 0) return ret; - ret = add_resource(res, &rodata_res); + ret = add_resource(&kimage_res, &rodata_res); if (ret < 0) return ret; - ret = add_resource(res, &data_res); + ret = add_resource(&kimage_res, &data_res); if (ret < 0) return ret; - ret = add_resource(res, &bss_res); + ret = add_resource(&kimage_res, &bss_res); return ret; } @@ -129,53 +139,42 @@ static void __init init_resources(void) struct resource *res = NULL; struct resource *mem_res = NULL; size_t mem_res_sz = 0; - int ret = 0, i = 0; - - code_res.start = __pa_symbol(_text); - code_res.end = __pa_symbol(_etext) - 1; - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - - rodata_res.start = __pa_symbol(__start_rodata); - rodata_res.end = __pa_symbol(__end_rodata) - 1; - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - - data_res.start = __pa_symbol(_data); - data_res.end = __pa_symbol(_edata) - 1; - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + int num_resources = 0, res_idx = 0; + int ret = 0; - bss_res.start = __pa_symbol(__bss_start); - bss_res.end = __pa_symbol(__bss_stop) - 1; - bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + /* + 1 as memblock_alloc() might increase memblock.reserved.cnt */ + num_resources = memblock.memory.cnt + memblock.reserved.cnt + 1; + res_idx = num_resources - 1; - mem_res_sz = (memblock.memory.cnt + memblock.reserved.cnt) * sizeof(*mem_res); + mem_res_sz = num_resources * sizeof(*mem_res); mem_res = memblock_alloc(mem_res_sz, SMP_CACHE_BYTES); if (!mem_res) panic("%s: Failed to allocate %zu bytes\n", __func__, mem_res_sz); + /* * Start by adding the reserved regions, if they overlap * with /memory regions, insert_resource later on will take * care of it. */ + ret = add_kernel_resources(); + if (ret < 0) + goto error; + for_each_reserved_mem_region(region) { - res = &mem_res[i++]; + res = &mem_res[res_idx--]; res->name = "Reserved"; res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; res->start = __pfn_to_phys(memblock_region_reserved_base_pfn(region)); res->end = __pfn_to_phys(memblock_region_reserved_end_pfn(region)) - 1; - ret = add_kernel_resources(res); - if (ret < 0) - goto error; - else if (ret) - continue; - /* * Ignore any other reserved regions within * system memory. */ if (memblock_is_memory(res->start)) { - memblock_free((phys_addr_t) res, sizeof(struct resource)); + /* Re-use this pre-allocated resource */ + res_idx++; continue; } @@ -186,7 +185,7 @@ static void __init init_resources(void) /* Add /memory regions to the resource tree */ for_each_mem_region(region) { - res = &mem_res[i++]; + res = &mem_res[res_idx--]; if (unlikely(memblock_is_nomap(region))) { res->name = "Reserved"; @@ -204,6 +203,9 @@ static void __init init_resources(void) goto error; } + /* Clean-up any unused pre-allocated resources */ + mem_res_sz = (num_resources - res_idx + 1) * sizeof(*mem_res); + memblock_free((phys_addr_t) mem_res, mem_res_sz); return; error: