RISC-V: Improve init_resources()
authorNick Kossifidis <mick@ics.forth.gr>
Mon, 19 Apr 2021 00:55:37 +0000 (03:55 +0300)
committerPalmer Dabbelt <palmerdabbelt@google.com>
Mon, 26 Apr 2021 15:25:22 +0000 (08:25 -0700)
The 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.

Additionally, we don't need to call memblock_free inside the loop, as 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.

Signed-off-by: Nick Kossifidis <mick@ics.forth.gr>
[Palmer: commit text]
Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
arch/riscv/kernel/setup.c

index d208abc..9193ba3 100644 (file)
@@ -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: