| Message ID | 20240308191204.819487-2-quic_obabatun@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Dynamic Allocation of the reserved_mem array | expand |
On Fri, Mar 08, 2024 at 11:12:01AM -0800, Oreoluwa Babatunde wrote: > The current implementation processes the reserved memory regions in two > stages which are done with two separate functions within the > early_init_fdt_scan_reserved_mem() function. > > Within the two stages of processing, the reserved memory regions are > broken up into two groups which are processed differently: > i) Statically-placed reserved memory regions > i.e. regions defined with a static start address and size using the > "reg" property in the DT. > ii) Dynamically-placed reserved memory regions. > i.e. regions defined by specifying a range of addresses where they can > be placed in memory using the "alloc_ranges" and "size" properties > in the DT. > > Stage 1: fdt_scan_reserved_mem() > This stage of the reserved memory processing is used to scan through the > reserved memory nodes defined in the devicetree and do the following on > each of the nodes: > > 1) If the node represents a statically-placed reserved memory region, > i.e. it is defined using the "reg" property: > - Call memblock_reserve() or memblock_mark_nomap() as needed. > - Add the information for the reserved region to the reserved_mem array. > eg: fdt_reserved_mem_save_node(node, name, base, size); > > 2) If the node represents a dynamically-placed reserved memory region, > i.e. it is defined using "alloc-ranges" and "size" properties: > - Add the information for the region to the reserved_mem array with > the starting address and size set to 0. > eg: fdt_reserved_mem_save_node(node, name, 0, 0); > > Stage 2: fdt_init_reserved_mem() > This stage of the reserved memory processing is used to iterate through > the reserved_mem array which was populated in stage 1 and do the > following on each of the entries: > > 1) If the entry represents a statically-placed reserved memory region: > - Call the region specific init function. > 2) If the entry represents a dynamically-placed reserved memory region: > - Call __reserved_mem_alloc_size() which is used to allocate memory > for the region using memblock_phys_alloc_range(), and call > memblock_mark_nomap() on the allocated region if the region is > specified as a no-map region. > - Call the region specific init function. > > On architectures such as arm64, the dynamic allocation of the > reserved_mem array needs to be done after the page tables have been > setup because memblock allocated memory is not writable until then. This > means that the reserved_mem array will not be available to store any > reserved memory information until after the page tables have been setup. > > It is possible to call memblock_reserve() and memblock_mark_nomap() on > the statically-placed reserved memory regions and not need to save them > to the reserved_mem array until later. This is because all the > information we need is present in the devicetree. > Dynamically-placed reserved memory regions on the other hand get assigned > a start address only at runtime, and since memblock_reserve() and > memblock_mark_nomap() need to be called before the memory mappings are > created, the allocation needs to happen before the page tables are setup. > > To make it easier to handle dynamically-placed reserved memory regions > before the page tables are setup, this patch makes changes to the steps > above to process the reserved memory regions in the following ways: > > Step 1: fdt_scan_reserved_mem() > This stage of the reserved memory processing is used to scan through the > reserved memory nodes defined in the devicetree and do the following on > each of the nodes: > > 1) If the node represents a statically-placed reserved memory region, > i.e. it is defined using the "reg" property: > - Call memblock_reserve() or memblock_mark_nomap() as needed. > > 2) If the node represents a dynamically-placed reserved memory region, > i.e. it is defined using "alloc-ranges" and "size" properties: > - Call __reserved_mem_alloc_size() which will: > i) Allocate memory for the reserved memory region. > ii) Call memblock_mark_nomap() as needed. > Note: There is no need to explicitly call memblock_reserve() here > because it is already called by memblock when the memory for the > region is being allocated. > iii) Save the information for the region in the reserved_mem array. > > Step 2: fdt_init_reserved_mem() > This stage of the reserved memory processing is used to: > > 1) Add the information for the statically-placed reserved memory into > the reserved_mem array. > > 2) Iterate through all the entries in the array and call the region > specific init function for each of them. > > Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com> > --- > drivers/of/fdt.c | 84 ++++++++++++++++++++++++++++++--- > drivers/of/of_private.h | 2 +- > drivers/of/of_reserved_mem.c | 54 +++++++++------------ > include/linux/of_fdt.h | 1 + > include/linux/of_reserved_mem.h | 9 ++++ > 5 files changed, 111 insertions(+), 39 deletions(-) > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > index bf502ba8da95..fe6c75c5a8c0 100644 > --- a/drivers/of/fdt.c > +++ b/drivers/of/fdt.c > @@ -504,7 +504,6 @@ static int __init __reserved_mem_reserve_reg(unsigned long node, > phys_addr_t base, size; > int len; > const __be32 *prop; > - int first = 1; > bool nomap; > > prop = of_get_flat_dt_prop(node, "reg", &len); > @@ -532,10 +531,6 @@ static int __init __reserved_mem_reserve_reg(unsigned long node, > uname, &base, (unsigned long)(size / SZ_1M)); > > len -= t_len; > - if (first) { > - fdt_reserved_mem_save_node(node, uname, base, size); > - first = 0; > - } > } > return 0; > } > @@ -563,12 +558,70 @@ static int __init __reserved_mem_check_root(unsigned long node) > return 0; > } > > +/** > + * fdt_scan_reserved_mem_reg_nodes() - Store info for the "reg" defined > + * reserved memory regions. > + * > + * This function is used to scan through the DT and store the > + * information for the reserved memory regions that are defined using > + * the "reg" property. The region node number, name, base address, and > + * size are all stored in the reserved_mem array by calling the > + * fdt_reserved_mem_save_node() function. > + */ > +void __init fdt_scan_reserved_mem_reg_nodes(void) > + > +{ > + int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32); > + const void *fdt = initial_boot_params; > + phys_addr_t base, size; > + const __be32 *prop; > + int node, child; > + int len; > + > + node = fdt_path_offset(fdt, "/reserved-memory"); > + if (node < 0) { > + pr_err("Reserved memory: Did not find reserved-memory node\n"); No reserved regions is perfectly valid. > + return; > + } > + > + if (__reserved_mem_check_root(node)) { > + pr_err("Reserved memory: unsupported node format, ignoring\n"); > + return; > + } > + > + fdt_for_each_subnode(child, fdt, node) { > + const char *uname; > + > + prop = of_get_flat_dt_prop(child, "reg", &len); > + if (!prop) > + continue; > + > + if (!of_fdt_device_is_available(fdt, child)) > + continue; > + > + uname = fdt_get_name(fdt, child, NULL); > + if (len && len % t_len != 0) { > + pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n", > + uname); > + continue; > + } > + > + base = dt_mem_next_cell(dt_root_addr_cells, &prop); > + size = dt_mem_next_cell(dt_root_size_cells, &prop); > + > + if (size) > + fdt_reserved_mem_save_node(child, uname, base, size); > + } > +} > + > /* > * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory > */ > static int __init fdt_scan_reserved_mem(void) > { > int node, child; > + int dynamic_nodes_cnt = 0; > + int dynamic_nodes[MAX_RESERVED_REGIONS]; > const void *fdt = initial_boot_params; > > node = fdt_path_offset(fdt, "/reserved-memory"); > @@ -590,8 +643,25 @@ static int __init fdt_scan_reserved_mem(void) > uname = fdt_get_name(fdt, child, NULL); > > err = __reserved_mem_reserve_reg(child, uname); > - if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) > - fdt_reserved_mem_save_node(child, uname, 0, 0); > + > + /* > + * Delay allocation of the dynamically-placed regions > + * until after all other statically-placed regions have > + * been reserved or marked as nomap > + */ > + if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) { > + dynamic_nodes[dynamic_nodes_cnt] = child; > + dynamic_nodes_cnt++; Can't you just call __reserved_mem_alloc_size() here instead of looping twice? > + } > + } > + > + for (int i = 0; i < dynamic_nodes_cnt; i++) { > + const char *uname; > + > + child = dynamic_nodes[i]; > + uname = fdt_get_name(fdt, child, NULL); > + > + __reserved_mem_alloc_size(child, uname); > } > return 0; > } > diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h > index f38397c7b582..542e37a37a24 100644 > --- a/drivers/of/of_private.h > +++ b/drivers/of/of_private.h > @@ -36,6 +36,7 @@ struct alias_prop { > #endif > > #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1 > +#define MAX_RESERVED_REGIONS 64 > > extern struct mutex of_mutex; > extern raw_spinlock_t devtree_lock; > @@ -175,7 +176,6 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node * > } > #endif > > -void fdt_init_reserved_mem(void); I don't see why this is moved. > void fdt_reserved_mem_save_node(unsigned long node, const char *uname, > phys_addr_t base, phys_addr_t size); > > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c > index 7ec94cfcbddb..d62f1956024c 100644 > --- a/drivers/of/of_reserved_mem.c > +++ b/drivers/of/of_reserved_mem.c > @@ -26,7 +26,6 @@ > > #include "of_private.h" > > -#define MAX_RESERVED_REGIONS 64 > static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS]; > static int reserved_mem_count; > > @@ -132,8 +131,7 @@ static int __init __reserved_mem_alloc_in_range(phys_addr_t size, > * __reserved_mem_alloc_size() - allocate reserved memory described by > * 'size', 'alignment' and 'alloc-ranges' properties. > */ > -static int __init __reserved_mem_alloc_size(unsigned long node, > - const char *uname, phys_addr_t *res_base, phys_addr_t *res_size) > +int __init __reserved_mem_alloc_size(unsigned long node, const char *uname) > { > int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32); > phys_addr_t start = 0, end = 0; > @@ -212,10 +210,7 @@ static int __init __reserved_mem_alloc_size(unsigned long node, > uname, (unsigned long)(size / SZ_1M)); > return -ENOMEM; > } > - > - *res_base = base; > - *res_size = size; > - > + fdt_reserved_mem_save_node(node, uname, base, size); > return 0; > } > > @@ -310,6 +305,8 @@ void __init fdt_init_reserved_mem(void) > { > int i; > > + fdt_scan_reserved_mem_reg_nodes(); > + > /* check for overlapping reserved regions */ > __rmem_check_for_overlap(); > > @@ -328,30 +325,25 @@ void __init fdt_init_reserved_mem(void) > if (prop) > rmem->phandle = of_read_number(prop, len/4); > > - if (rmem->size == 0) > - err = __reserved_mem_alloc_size(node, rmem->name, > - &rmem->base, &rmem->size); > - if (err == 0) { > - err = __reserved_mem_init_node(rmem); > - if (err != 0 && err != -ENOENT) { > - pr_info("node %s compatible matching fail\n", > - rmem->name); > - if (nomap) > - memblock_clear_nomap(rmem->base, rmem->size); > - else > - memblock_phys_free(rmem->base, > - rmem->size); > - } else { > - phys_addr_t end = rmem->base + rmem->size - 1; > - bool reusable = > - (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL; > - > - pr_info("%pa..%pa (%lu KiB) %s %s %s\n", > - &rmem->base, &end, (unsigned long)(rmem->size / SZ_1K), > - nomap ? "nomap" : "map", > - reusable ? "reusable" : "non-reusable", > - rmem->name ? rmem->name : "unknown"); > - } > + err = __reserved_mem_init_node(rmem); > + if (err != 0 && err != -ENOENT) { > + pr_info("node %s compatible matching fail\n", > + rmem->name); Can be 1 line now. > + if (nomap) > + memblock_clear_nomap(rmem->base, rmem->size); > + else > + memblock_phys_free(rmem->base, > + rmem->size); Can be 1 line now. > + } else { > + phys_addr_t end = rmem->base + rmem->size - 1; > + bool reusable = > + (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL; > + > + pr_info("%pa..%pa (%lu KiB) %s %s %s\n", > + &rmem->base, &end, (unsigned long)(rmem->size / SZ_1K), > + nomap ? "nomap" : "map", > + reusable ? "reusable" : "non-reusable", > + rmem->name ? rmem->name : "unknown"); > } > } > } > diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h > index d69ad5bb1eb1..7b2a5d93d719 100644 > --- a/include/linux/of_fdt.h > +++ b/include/linux/of_fdt.h > @@ -73,6 +73,7 @@ extern int early_init_dt_scan_root(void); > extern bool early_init_dt_scan(void *params); > extern bool early_init_dt_verify(void *params); > extern void early_init_dt_scan_nodes(void); > +extern void fdt_scan_reserved_mem_reg_nodes(void); This is internal to drivers/of/, so it goes in of_private.h > > extern const char *of_flat_dt_get_machine_name(void); > extern const void *of_flat_dt_match_machine(const void *default_match, > diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h > index 4de2a24cadc9..2a3178920bae 100644 > --- a/include/linux/of_reserved_mem.h > +++ b/include/linux/of_reserved_mem.h > @@ -32,12 +32,14 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem); > #define RESERVEDMEM_OF_DECLARE(name, compat, init) \ > _OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn) > > +void fdt_init_reserved_mem(void); > int of_reserved_mem_device_init_by_idx(struct device *dev, > struct device_node *np, int idx); > int of_reserved_mem_device_init_by_name(struct device *dev, > struct device_node *np, > const char *name); > void of_reserved_mem_device_release(struct device *dev); > +int __reserved_mem_alloc_size(unsigned long node, const char *uname); This is internal to drivers/of/, so it goes in of_private.h But really, I think fdt_scan_reserved_mem() should move to of_reserved_mem.c first. Then everything you add to fdt.c goes there too. Rob
On 3/11/2024 10:13 AM, Rob Herring wrote: > On Fri, Mar 08, 2024 at 11:12:01AM -0800, Oreoluwa Babatunde wrote: >> The current implementation processes the reserved memory regions in two >> stages which are done with two separate functions within the >> early_init_fdt_scan_reserved_mem() function. >> >> Within the two stages of processing, the reserved memory regions are >> broken up into two groups which are processed differently: >> i) Statically-placed reserved memory regions >> i.e. regions defined with a static start address and size using the >> "reg" property in the DT. >> ii) Dynamically-placed reserved memory regions. >> i.e. regions defined by specifying a range of addresses where they can >> be placed in memory using the "alloc_ranges" and "size" properties >> in the DT. >> >> Stage 1: fdt_scan_reserved_mem() >> This stage of the reserved memory processing is used to scan through the >> reserved memory nodes defined in the devicetree and do the following on >> each of the nodes: >> >> 1) If the node represents a statically-placed reserved memory region, >> i.e. it is defined using the "reg" property: >> - Call memblock_reserve() or memblock_mark_nomap() as needed. >> - Add the information for the reserved region to the reserved_mem array. >> eg: fdt_reserved_mem_save_node(node, name, base, size); >> >> 2) If the node represents a dynamically-placed reserved memory region, >> i.e. it is defined using "alloc-ranges" and "size" properties: >> - Add the information for the region to the reserved_mem array with >> the starting address and size set to 0. >> eg: fdt_reserved_mem_save_node(node, name, 0, 0); >> >> Stage 2: fdt_init_reserved_mem() >> This stage of the reserved memory processing is used to iterate through >> the reserved_mem array which was populated in stage 1 and do the >> following on each of the entries: >> >> 1) If the entry represents a statically-placed reserved memory region: >> - Call the region specific init function. >> 2) If the entry represents a dynamically-placed reserved memory region: >> - Call __reserved_mem_alloc_size() which is used to allocate memory >> for the region using memblock_phys_alloc_range(), and call >> memblock_mark_nomap() on the allocated region if the region is >> specified as a no-map region. >> - Call the region specific init function. >> >> On architectures such as arm64, the dynamic allocation of the >> reserved_mem array needs to be done after the page tables have been >> setup because memblock allocated memory is not writable until then. This >> means that the reserved_mem array will not be available to store any >> reserved memory information until after the page tables have been setup. >> >> It is possible to call memblock_reserve() and memblock_mark_nomap() on >> the statically-placed reserved memory regions and not need to save them >> to the reserved_mem array until later. This is because all the >> information we need is present in the devicetree. >> Dynamically-placed reserved memory regions on the other hand get assigned >> a start address only at runtime, and since memblock_reserve() and >> memblock_mark_nomap() need to be called before the memory mappings are >> created, the allocation needs to happen before the page tables are setup. >> >> To make it easier to handle dynamically-placed reserved memory regions >> before the page tables are setup, this patch makes changes to the steps >> above to process the reserved memory regions in the following ways: >> >> Step 1: fdt_scan_reserved_mem() >> This stage of the reserved memory processing is used to scan through the >> reserved memory nodes defined in the devicetree and do the following on >> each of the nodes: >> >> 1) If the node represents a statically-placed reserved memory region, >> i.e. it is defined using the "reg" property: >> - Call memblock_reserve() or memblock_mark_nomap() as needed. >> >> 2) If the node represents a dynamically-placed reserved memory region, >> i.e. it is defined using "alloc-ranges" and "size" properties: >> - Call __reserved_mem_alloc_size() which will: >> i) Allocate memory for the reserved memory region. >> ii) Call memblock_mark_nomap() as needed. >> Note: There is no need to explicitly call memblock_reserve() here >> because it is already called by memblock when the memory for the >> region is being allocated. >> iii) Save the information for the region in the reserved_mem array. >> >> Step 2: fdt_init_reserved_mem() >> This stage of the reserved memory processing is used to: >> >> 1) Add the information for the statically-placed reserved memory into >> the reserved_mem array. >> >> 2) Iterate through all the entries in the array and call the region >> specific init function for each of them. >> >> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com> >> --- >> drivers/of/fdt.c | 84 ++++++++++++++++++++++++++++++--- >> drivers/of/of_private.h | 2 +- >> drivers/of/of_reserved_mem.c | 54 +++++++++------------ >> include/linux/of_fdt.h | 1 + >> include/linux/of_reserved_mem.h | 9 ++++ >> 5 files changed, 111 insertions(+), 39 deletions(-) >> >> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c >> index bf502ba8da95..fe6c75c5a8c0 100644 >> --- a/drivers/of/fdt.c >> +++ b/drivers/of/fdt.c >> @@ -504,7 +504,6 @@ static int __init __reserved_mem_reserve_reg(unsigned long node, >> phys_addr_t base, size; >> int len; >> const __be32 *prop; >> - int first = 1; >> bool nomap; >> >> prop = of_get_flat_dt_prop(node, "reg", &len); >> @@ -532,10 +531,6 @@ static int __init __reserved_mem_reserve_reg(unsigned long node, >> uname, &base, (unsigned long)(size / SZ_1M)); >> >> len -= t_len; >> - if (first) { >> - fdt_reserved_mem_save_node(node, uname, base, size); >> - first = 0; >> - } >> } >> return 0; >> } >> @@ -563,12 +558,70 @@ static int __init __reserved_mem_check_root(unsigned long node) >> return 0; >> } >> >> +/** >> + * fdt_scan_reserved_mem_reg_nodes() - Store info for the "reg" defined >> + * reserved memory regions. >> + * >> + * This function is used to scan through the DT and store the >> + * information for the reserved memory regions that are defined using >> + * the "reg" property. The region node number, name, base address, and >> + * size are all stored in the reserved_mem array by calling the >> + * fdt_reserved_mem_save_node() function. >> + */ >> +void __init fdt_scan_reserved_mem_reg_nodes(void) >> + >> +{ >> + int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32); >> + const void *fdt = initial_boot_params; >> + phys_addr_t base, size; >> + const __be32 *prop; >> + int node, child; >> + int len; >> + >> + node = fdt_path_offset(fdt, "/reserved-memory"); >> + if (node < 0) { >> + pr_err("Reserved memory: Did not find reserved-memory node\n"); > No reserved regions is perfectly valid. ack >> + return; >> + } >> + >> + if (__reserved_mem_check_root(node)) { >> + pr_err("Reserved memory: unsupported node format, ignoring\n"); >> + return; >> + } >> + >> + fdt_for_each_subnode(child, fdt, node) { >> + const char *uname; >> + >> + prop = of_get_flat_dt_prop(child, "reg", &len); >> + if (!prop) >> + continue; >> + >> + if (!of_fdt_device_is_available(fdt, child)) >> + continue; >> + >> + uname = fdt_get_name(fdt, child, NULL); >> + if (len && len % t_len != 0) { >> + pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n", >> + uname); >> + continue; >> + } >> + >> + base = dt_mem_next_cell(dt_root_addr_cells, &prop); >> + size = dt_mem_next_cell(dt_root_size_cells, &prop); >> + >> + if (size) >> + fdt_reserved_mem_save_node(child, uname, base, size); >> + } >> +} >> + >> /* >> * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory >> */ >> static int __init fdt_scan_reserved_mem(void) >> { >> int node, child; >> + int dynamic_nodes_cnt = 0; >> + int dynamic_nodes[MAX_RESERVED_REGIONS]; >> const void *fdt = initial_boot_params; >> >> node = fdt_path_offset(fdt, "/reserved-memory"); >> @@ -590,8 +643,25 @@ static int __init fdt_scan_reserved_mem(void) >> uname = fdt_get_name(fdt, child, NULL); >> >> err = __reserved_mem_reserve_reg(child, uname); >> - if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) >> - fdt_reserved_mem_save_node(child, uname, 0, 0); >> + >> + /* >> + * Delay allocation of the dynamically-placed regions >> + * until after all other statically-placed regions have >> + * been reserved or marked as nomap >> + */ >> + if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) { >> + dynamic_nodes[dynamic_nodes_cnt] = child; >> + dynamic_nodes_cnt++; > Can't you just call __reserved_mem_alloc_size() here instead of looping > twice? The reason for looping twice here is in the comment right above the if-statement; "Delay allocation of the dynamically-placed regions until after all other statically-placed regions have been reserved or marked as nomap". If we call "__reserved_mem_alloc_size()" at this point then there is a possibility of allocating memory from one of the statically-placed reserved memory regions since not all of them have been marked as reserved or nomap yet. >> + } >> + } >> + >> + for (int i = 0; i < dynamic_nodes_cnt; i++) { >> + const char *uname; >> + >> + child = dynamic_nodes[i]; >> + uname = fdt_get_name(fdt, child, NULL); >> + >> + __reserved_mem_alloc_size(child, uname); >> } >> return 0; >> } >> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h >> index f38397c7b582..542e37a37a24 100644 >> --- a/drivers/of/of_private.h >> +++ b/drivers/of/of_private.h >> @@ -36,6 +36,7 @@ struct alias_prop { >> #endif >> >> #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1 >> +#define MAX_RESERVED_REGIONS 64 >> >> extern struct mutex of_mutex; >> extern raw_spinlock_t devtree_lock; >> @@ -175,7 +176,6 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node * >> } >> #endif >> >> -void fdt_init_reserved_mem(void); > I don't see why this is moved. True, there is no need for this anymore. Will revert it in the next revision. >> [...] >> >> @@ -328,30 +325,25 @@ void __init fdt_init_reserved_mem(void) >> if (prop) >> rmem->phandle = of_read_number(prop, len/4); >> >> - if (rmem->size == 0) >> - err = __reserved_mem_alloc_size(node, rmem->name, >> - &rmem->base, &rmem->size); >> - if (err == 0) { >> - err = __reserved_mem_init_node(rmem); >> - if (err != 0 && err != -ENOENT) { >> - pr_info("node %s compatible matching fail\n", >> - rmem->name); >> - if (nomap) >> - memblock_clear_nomap(rmem->base, rmem->size); >> - else >> - memblock_phys_free(rmem->base, >> - rmem->size); >> - } else { >> - phys_addr_t end = rmem->base + rmem->size - 1; >> - bool reusable = >> - (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL; >> - >> - pr_info("%pa..%pa (%lu KiB) %s %s %s\n", >> - &rmem->base, &end, (unsigned long)(rmem->size / SZ_1K), >> - nomap ? "nomap" : "map", >> - reusable ? "reusable" : "non-reusable", >> - rmem->name ? rmem->name : "unknown"); >> - } >> + err = __reserved_mem_init_node(rmem); >> + if (err != 0 && err != -ENOENT) { >> + pr_info("node %s compatible matching fail\n", >> + rmem->name); > Can be 1 line now. ack. >> + if (nomap) >> + memblock_clear_nomap(rmem->base, rmem->size); >> + else >> + memblock_phys_free(rmem->base, >> + rmem->size); > Can be 1 line now. ack. >> + } else { >> + phys_addr_t end = rmem->base + rmem->size - 1; >> + bool reusable = >> + (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL; >> + >> + pr_info("%pa..%pa (%lu KiB) %s %s %s\n", >> + &rmem->base, &end, (unsigned long)(rmem->size / SZ_1K), >> + nomap ? "nomap" : "map", >> + reusable ? "reusable" : "non-reusable", >> + rmem->name ? rmem->name : "unknown"); >> } >> } >> } >> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h >> index d69ad5bb1eb1..7b2a5d93d719 100644 >> --- a/include/linux/of_fdt.h >> +++ b/include/linux/of_fdt.h >> @@ -73,6 +73,7 @@ extern int early_init_dt_scan_root(void); >> extern bool early_init_dt_scan(void *params); >> extern bool early_init_dt_verify(void *params); >> extern void early_init_dt_scan_nodes(void); >> +extern void fdt_scan_reserved_mem_reg_nodes(void); > This is internal to drivers/of/, so it goes in of_private.h ack. >> >> extern const char *of_flat_dt_get_machine_name(void); >> extern const void *of_flat_dt_match_machine(const void *default_match, >> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h >> index 4de2a24cadc9..2a3178920bae 100644 >> --- a/include/linux/of_reserved_mem.h >> +++ b/include/linux/of_reserved_mem.h >> @@ -32,12 +32,14 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem); >> #define RESERVEDMEM_OF_DECLARE(name, compat, init) \ >> _OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn) >> >> +void fdt_init_reserved_mem(void); >> int of_reserved_mem_device_init_by_idx(struct device *dev, >> struct device_node *np, int idx); >> int of_reserved_mem_device_init_by_name(struct device *dev, >> struct device_node *np, >> const char *name); >> void of_reserved_mem_device_release(struct device *dev); >> +int __reserved_mem_alloc_size(unsigned long node, const char *uname); > This is internal to drivers/of/, so it goes in of_private.h ack. > > But really, I think fdt_scan_reserved_mem() should move to > of_reserved_mem.c first. Then everything you add to fdt.c goes there > too. ack. Regards, Oreoluwa
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index bf502ba8da95..fe6c75c5a8c0 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -504,7 +504,6 @@ static int __init __reserved_mem_reserve_reg(unsigned long node, phys_addr_t base, size; int len; const __be32 *prop; - int first = 1; bool nomap; prop = of_get_flat_dt_prop(node, "reg", &len); @@ -532,10 +531,6 @@ static int __init __reserved_mem_reserve_reg(unsigned long node, uname, &base, (unsigned long)(size / SZ_1M)); len -= t_len; - if (first) { - fdt_reserved_mem_save_node(node, uname, base, size); - first = 0; - } } return 0; } @@ -563,12 +558,70 @@ static int __init __reserved_mem_check_root(unsigned long node) return 0; } +/** + * fdt_scan_reserved_mem_reg_nodes() - Store info for the "reg" defined + * reserved memory regions. + * + * This function is used to scan through the DT and store the + * information for the reserved memory regions that are defined using + * the "reg" property. The region node number, name, base address, and + * size are all stored in the reserved_mem array by calling the + * fdt_reserved_mem_save_node() function. + */ +void __init fdt_scan_reserved_mem_reg_nodes(void) + +{ + int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32); + const void *fdt = initial_boot_params; + phys_addr_t base, size; + const __be32 *prop; + int node, child; + int len; + + node = fdt_path_offset(fdt, "/reserved-memory"); + if (node < 0) { + pr_err("Reserved memory: Did not find reserved-memory node\n"); + return; + } + + if (__reserved_mem_check_root(node)) { + pr_err("Reserved memory: unsupported node format, ignoring\n"); + return; + } + + fdt_for_each_subnode(child, fdt, node) { + const char *uname; + + prop = of_get_flat_dt_prop(child, "reg", &len); + if (!prop) + continue; + + if (!of_fdt_device_is_available(fdt, child)) + continue; + + uname = fdt_get_name(fdt, child, NULL); + if (len && len % t_len != 0) { + pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n", + uname); + continue; + } + + base = dt_mem_next_cell(dt_root_addr_cells, &prop); + size = dt_mem_next_cell(dt_root_size_cells, &prop); + + if (size) + fdt_reserved_mem_save_node(child, uname, base, size); + } +} + /* * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory */ static int __init fdt_scan_reserved_mem(void) { int node, child; + int dynamic_nodes_cnt = 0; + int dynamic_nodes[MAX_RESERVED_REGIONS]; const void *fdt = initial_boot_params; node = fdt_path_offset(fdt, "/reserved-memory"); @@ -590,8 +643,25 @@ static int __init fdt_scan_reserved_mem(void) uname = fdt_get_name(fdt, child, NULL); err = __reserved_mem_reserve_reg(child, uname); - if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) - fdt_reserved_mem_save_node(child, uname, 0, 0); + + /* + * Delay allocation of the dynamically-placed regions + * until after all other statically-placed regions have + * been reserved or marked as nomap + */ + if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) { + dynamic_nodes[dynamic_nodes_cnt] = child; + dynamic_nodes_cnt++; + } + } + + for (int i = 0; i < dynamic_nodes_cnt; i++) { + const char *uname; + + child = dynamic_nodes[i]; + uname = fdt_get_name(fdt, child, NULL); + + __reserved_mem_alloc_size(child, uname); } return 0; } diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index f38397c7b582..542e37a37a24 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -36,6 +36,7 @@ struct alias_prop { #endif #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1 +#define MAX_RESERVED_REGIONS 64 extern struct mutex of_mutex; extern raw_spinlock_t devtree_lock; @@ -175,7 +176,6 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node * } #endif -void fdt_init_reserved_mem(void); void fdt_reserved_mem_save_node(unsigned long node, const char *uname, phys_addr_t base, phys_addr_t size); diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 7ec94cfcbddb..d62f1956024c 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -26,7 +26,6 @@ #include "of_private.h" -#define MAX_RESERVED_REGIONS 64 static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS]; static int reserved_mem_count; @@ -132,8 +131,7 @@ static int __init __reserved_mem_alloc_in_range(phys_addr_t size, * __reserved_mem_alloc_size() - allocate reserved memory described by * 'size', 'alignment' and 'alloc-ranges' properties. */ -static int __init __reserved_mem_alloc_size(unsigned long node, - const char *uname, phys_addr_t *res_base, phys_addr_t *res_size) +int __init __reserved_mem_alloc_size(unsigned long node, const char *uname) { int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32); phys_addr_t start = 0, end = 0; @@ -212,10 +210,7 @@ static int __init __reserved_mem_alloc_size(unsigned long node, uname, (unsigned long)(size / SZ_1M)); return -ENOMEM; } - - *res_base = base; - *res_size = size; - + fdt_reserved_mem_save_node(node, uname, base, size); return 0; } @@ -310,6 +305,8 @@ void __init fdt_init_reserved_mem(void) { int i; + fdt_scan_reserved_mem_reg_nodes(); + /* check for overlapping reserved regions */ __rmem_check_for_overlap(); @@ -328,30 +325,25 @@ void __init fdt_init_reserved_mem(void) if (prop) rmem->phandle = of_read_number(prop, len/4); - if (rmem->size == 0) - err = __reserved_mem_alloc_size(node, rmem->name, - &rmem->base, &rmem->size); - if (err == 0) { - err = __reserved_mem_init_node(rmem); - if (err != 0 && err != -ENOENT) { - pr_info("node %s compatible matching fail\n", - rmem->name); - if (nomap) - memblock_clear_nomap(rmem->base, rmem->size); - else - memblock_phys_free(rmem->base, - rmem->size); - } else { - phys_addr_t end = rmem->base + rmem->size - 1; - bool reusable = - (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL; - - pr_info("%pa..%pa (%lu KiB) %s %s %s\n", - &rmem->base, &end, (unsigned long)(rmem->size / SZ_1K), - nomap ? "nomap" : "map", - reusable ? "reusable" : "non-reusable", - rmem->name ? rmem->name : "unknown"); - } + err = __reserved_mem_init_node(rmem); + if (err != 0 && err != -ENOENT) { + pr_info("node %s compatible matching fail\n", + rmem->name); + if (nomap) + memblock_clear_nomap(rmem->base, rmem->size); + else + memblock_phys_free(rmem->base, + rmem->size); + } else { + phys_addr_t end = rmem->base + rmem->size - 1; + bool reusable = + (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL; + + pr_info("%pa..%pa (%lu KiB) %s %s %s\n", + &rmem->base, &end, (unsigned long)(rmem->size / SZ_1K), + nomap ? "nomap" : "map", + reusable ? "reusable" : "non-reusable", + rmem->name ? rmem->name : "unknown"); } } } diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h index d69ad5bb1eb1..7b2a5d93d719 100644 --- a/include/linux/of_fdt.h +++ b/include/linux/of_fdt.h @@ -73,6 +73,7 @@ extern int early_init_dt_scan_root(void); extern bool early_init_dt_scan(void *params); extern bool early_init_dt_verify(void *params); extern void early_init_dt_scan_nodes(void); +extern void fdt_scan_reserved_mem_reg_nodes(void); extern const char *of_flat_dt_get_machine_name(void); extern const void *of_flat_dt_match_machine(const void *default_match, diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h index 4de2a24cadc9..2a3178920bae 100644 --- a/include/linux/of_reserved_mem.h +++ b/include/linux/of_reserved_mem.h @@ -32,12 +32,14 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem); #define RESERVEDMEM_OF_DECLARE(name, compat, init) \ _OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn) +void fdt_init_reserved_mem(void); int of_reserved_mem_device_init_by_idx(struct device *dev, struct device_node *np, int idx); int of_reserved_mem_device_init_by_name(struct device *dev, struct device_node *np, const char *name); void of_reserved_mem_device_release(struct device *dev); +int __reserved_mem_alloc_size(unsigned long node, const char *uname); struct reserved_mem *of_reserved_mem_lookup(struct device_node *np); #else @@ -45,6 +47,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np); #define RESERVEDMEM_OF_DECLARE(name, compat, init) \ _OF_DECLARE_STUB(reservedmem, name, compat, init, reservedmem_of_init_fn) +static inline void fdt_init_reserved_mem(void) { } + static inline int of_reserved_mem_device_init_by_idx(struct device *dev, struct device_node *np, int idx) { @@ -60,6 +64,11 @@ static inline int of_reserved_mem_device_init_by_name(struct device *dev, static inline void of_reserved_mem_device_release(struct device *pdev) { } +static inline int __reserved_mem_alloc_size(unsigned long node, const char *uname) +{ + return -ENOSYS; +} + static inline struct reserved_mem *of_reserved_mem_lookup(struct device_node *np) { return NULL;
The current implementation processes the reserved memory regions in two stages which are done with two separate functions within the early_init_fdt_scan_reserved_mem() function. Within the two stages of processing, the reserved memory regions are broken up into two groups which are processed differently: i) Statically-placed reserved memory regions i.e. regions defined with a static start address and size using the "reg" property in the DT. ii) Dynamically-placed reserved memory regions. i.e. regions defined by specifying a range of addresses where they can be placed in memory using the "alloc_ranges" and "size" properties in the DT. Stage 1: fdt_scan_reserved_mem() This stage of the reserved memory processing is used to scan through the reserved memory nodes defined in the devicetree and do the following on each of the nodes: 1) If the node represents a statically-placed reserved memory region, i.e. it is defined using the "reg" property: - Call memblock_reserve() or memblock_mark_nomap() as needed. - Add the information for the reserved region to the reserved_mem array. eg: fdt_reserved_mem_save_node(node, name, base, size); 2) If the node represents a dynamically-placed reserved memory region, i.e. it is defined using "alloc-ranges" and "size" properties: - Add the information for the region to the reserved_mem array with the starting address and size set to 0. eg: fdt_reserved_mem_save_node(node, name, 0, 0); Stage 2: fdt_init_reserved_mem() This stage of the reserved memory processing is used to iterate through the reserved_mem array which was populated in stage 1 and do the following on each of the entries: 1) If the entry represents a statically-placed reserved memory region: - Call the region specific init function. 2) If the entry represents a dynamically-placed reserved memory region: - Call __reserved_mem_alloc_size() which is used to allocate memory for the region using memblock_phys_alloc_range(), and call memblock_mark_nomap() on the allocated region if the region is specified as a no-map region. - Call the region specific init function. On architectures such as arm64, the dynamic allocation of the reserved_mem array needs to be done after the page tables have been setup because memblock allocated memory is not writable until then. This means that the reserved_mem array will not be available to store any reserved memory information until after the page tables have been setup. It is possible to call memblock_reserve() and memblock_mark_nomap() on the statically-placed reserved memory regions and not need to save them to the reserved_mem array until later. This is because all the information we need is present in the devicetree. Dynamically-placed reserved memory regions on the other hand get assigned a start address only at runtime, and since memblock_reserve() and memblock_mark_nomap() need to be called before the memory mappings are created, the allocation needs to happen before the page tables are setup. To make it easier to handle dynamically-placed reserved memory regions before the page tables are setup, this patch makes changes to the steps above to process the reserved memory regions in the following ways: Step 1: fdt_scan_reserved_mem() This stage of the reserved memory processing is used to scan through the reserved memory nodes defined in the devicetree and do the following on each of the nodes: 1) If the node represents a statically-placed reserved memory region, i.e. it is defined using the "reg" property: - Call memblock_reserve() or memblock_mark_nomap() as needed. 2) If the node represents a dynamically-placed reserved memory region, i.e. it is defined using "alloc-ranges" and "size" properties: - Call __reserved_mem_alloc_size() which will: i) Allocate memory for the reserved memory region. ii) Call memblock_mark_nomap() as needed. Note: There is no need to explicitly call memblock_reserve() here because it is already called by memblock when the memory for the region is being allocated. iii) Save the information for the region in the reserved_mem array. Step 2: fdt_init_reserved_mem() This stage of the reserved memory processing is used to: 1) Add the information for the statically-placed reserved memory into the reserved_mem array. 2) Iterate through all the entries in the array and call the region specific init function for each of them. Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com> --- drivers/of/fdt.c | 84 ++++++++++++++++++++++++++++++--- drivers/of/of_private.h | 2 +- drivers/of/of_reserved_mem.c | 54 +++++++++------------ include/linux/of_fdt.h | 1 + include/linux/of_reserved_mem.h | 9 ++++ 5 files changed, 111 insertions(+), 39 deletions(-)