lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0eede1f8-cbbc-4694-a91f-92002def33b7@quicinc.com>
Date: Fri, 27 Jun 2025 11:01:51 -0700
From: Oreoluwa Babatunde <quic_obabatun@...cinc.com>
To: William Zhang <william.zhang@...adcom.com>, <robh@...nel.org>
CC: <aisheng.dong@....com>, <andy@...ck.fi.intel.com>,
        <catalin.marinas@....com>, <devicetree@...r.kernel.org>, <hch@....de>,
        <iommu@...ts.linux.dev>, <kernel@...cinc.com>, <klarasmodin@...il.com>,
        <linux-kernel@...r.kernel.org>, <m.szyprowski@...sung.com>,
        <quic_ninanaik@...cinc.com>, <robin.murphy@....com>,
        <saravanak@...gle.com>, <will@...nel.org>,
        <oreoluwa.babatunde@....qualcomm.com>
Subject: Re: [PATCH v10 1/2] of: reserved_mem: Restruture how the reserved
 memory regions are processed



On 6/22/2025 6:24 PM, William Zhang wrote:
> On Tue, Jun 17, 2025 at 10:15 AM William Zhang
> <william.zhang@...adcom.com> wrote:
>>
>> Hi Oreoluwa,
>>
>> On 10/8/2024 3:06 PM, Oreoluwa Babatunde wrote:
>>> Reserved memory regions defined in the devicetree can be broken up into
>>> two groups:
>>> i) Statically-placed reserved memory regions
>>> i.e. regions defined with a static start address and size using the
>>>       "reg" property.
>>> ii) Dynamically-placed reserved memory regions.
>>> i.e. regions defined by specifying an address range where they can be
>>>       placed in memory using the "alloc_ranges" and "size" properties.
>>>
>>> These regions are processed and set aside at boot time.
>>> This is done in two stages as seen below:
>>>
>>> Stage 1:
>>> At this stage, fdt_scan_reserved_mem() scans through the child nodes of
>>> the reserved_memory node using the flattened devicetree and does the
>>> following:
>>>
>>> 1) If the node represents a statically-placed reserved memory region,
>>>     i.e. if it is defined using the "reg" property:
>>>     - Call memblock_reserve() or memblock_mark_nomap() as needed.
>>>     - Add the information for that region into the reserved_mem array
>>>       using fdt_reserved_mem_save_node().
>>>       i.e. fdt_reserved_mem_save_node(node, name, base, size).
>>>
>>> 2) If the node represents a dynamically-placed reserved memory region,
>>>     i.e. if it is defined using "alloc-ranges" and "size" properties:
>>>     - Add the information for that region to the reserved_mem array with
>>>       the starting address and size set to 0.
>>>       i.e. fdt_reserved_mem_save_node(node, name, 0, 0).
>>>     Note: This region is saved to the array with a starting address of 0
>>>     because a starting address is not yet allocated for it.
>>>
>>> Stage 2:
>>> After iterating through all the reserved memory nodes and storing their
>>> relevant information in the reserved_mem array,fdt_init_reserved_mem() is
>>> called and does the following:
>>>
>>> 1) For statically-placed reserved memory regions:
>>>     - Call the region specific init function using
>>>       __reserved_mem_init_node().
>>> 2) For dynamically-placed reserved memory regions:
>>>     - Call __reserved_mem_alloc_size() which is used to allocate memory
>>>       for each of these regions, and mark them as nomap if they have the
>>>       nomap property specified in the DT.
>>>     - Call the region specific init function.
>>>
>>> The current size of the resvered_mem array is 64 as is defined by
>>> MAX_RESERVED_REGIONS. This means that there is a limitation of 64 for
>>> how many reserved memory regions can be specified on a system.
>>> As systems continue to grow more and more complex, the number of
>>> reserved memory regions needed are also growing and are starting to hit
>>> this 64 count limit, hence the need to make the reserved_mem array
>>> dynamically sized (i.e. dynamically allocating memory for the
>>> reserved_mem array using membock_alloc_*).
>>>
>>> On architectures such as arm64, memory allocated using memblock is
>>> writable only after the page tables have been setup. This means that if
>>> the reserved_mem array is going to be dynamically allocated, it needs to
>>> happen after the page tables have been setup, not before.
>>>
>>> Since the reserved memory regions are currently being processed and
>>> added to the array before the page tables are setup, there is a need to
>>> change the order in which some of the processing is done to allow for
>>> the reserved_mem array to be dynamically sized.
>>>
>>> It is possible to process the statically-placed reserved memory regions
>>> without needing to store them in the reserved_mem array until after the
>>> page tables have been setup because all the information stored in the
>>> array is readily available in the devicetree and can be referenced at
>>> any time.
>>> Dynamically-placed reserved memory regions on the other hand get
>>> assigned a start address only at runtime, and hence need a place to be
>>> stored once they are allocated since there is no other referrence to the
>>> start address for these regions.
>>>
>>> Hence this patch changes the processing order of the reserved memory
>>> regions in the following ways:
>>>
>>> Step 1:
>>> fdt_scan_reserved_mem() scans through the child nodes of
>>> the reserved_memory node using the flattened devicetree and does the
>>> following:
>>>
>>> 1) If the node represents a statically-placed reserved memory region,
>>>     i.e. if 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. if it is defined using "alloc-ranges" and "size" properties:
>>>     - Call __reserved_mem_alloc_size() which will:
>>>       i) Allocate memory for the reserved region and call
>>>       memblock_mark_nomap() as needed.
>>>       ii) Call the region specific initialization function using
>>>       fdt_init_reserved_mem_node().
>>>       iii) Save the region information in the reserved_mem array using
>>>       fdt_reserved_mem_save_node().
>>>
>>> Step 2:
>>> 1) This stage of the reserved memory processing is now only used to add
>>>     the statically-placed reserved memory regions into the reserved_mem
>>>     array using fdt_scan_reserved_mem_reg_nodes(), as well as call their
>>>     region specific initialization functions.
>>>
>>> 2) This step has also been moved to be after the page tables are
>>>     setup. Moving this will allow us to replace the reserved_mem
>>>     array with a dynamically sized array before storing the rest of
>>>     these regions.
>> I am running into a call trace with this order change on armv7 chip when
>> I tried to allocate dma coherent memory from the device reserved memory.
>> The issue does not happen on armv8 chips.
>>
>> [    0.000000] Reserved memory: created CMA memory pool at 0x1e000000,
>> size 32 MiB
>> [    0.000000] OF: reserved mem: initialized node dt_reserved_cma,
>> compatible id shared-dma-pool
>> [    0.000000] OF: reserved mem: 0x1e000000..0x1fffffff (32768 KiB) map
>> reusable dt_reserved_cma
>> ....
>>
>> [    0.445322] ------------[ cut here ]------------
>> [    0.445353] WARNING: CPU: 0 PID: 1 at mm/memory.c:3069
>> __apply_to_page_range+0x380/0x388
>> [    0.488911] Modules linked in:
>> [    0.492027] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted
>> 6.16.0-rc1-g27605c8c0f69-dirty #3 NONE
>> [    0.501174] Hardware name: Generic DT based system
>> [    0.505965] Call trace:
>> [    0.505985]  unwind_backtrace from show_stack+0x10/0x14
>> [    0.513764]  show_stack from dump_stack_lvl+0x54/0x68
>> [    0.518834]  dump_stack_lvl from __warn+0x7c/0x128
>> [    0.523639]  __warn from warn_slowpath_fmt+0x184/0x18c
>> [    0.527676] Freeing initrd memory: 65536K
>> [    0.532788]  warn_slowpath_fmt from __apply_to_page_range+0x380/0x388
>> [    0.539242]  __apply_to_page_range from apply_to_page_range+0x1c/0x24
>> [    0.545689]  apply_to_page_range from __alloc_from_contiguous+0xc0/0x14c
>> [    0.552398]  __alloc_from_contiguous from cma_allocator_alloc+0x34/0x3c
>> [    0.559016]  cma_allocator_alloc from arch_dma_alloc+0x11c/0x2ac
>> [    0.565025]  arch_dma_alloc from dma_alloc_attrs+0x90/0x2e8
>> [    0.570603]  dma_alloc_attrs from dmydev_probe+0x8c/0xe8
>> [    0.575919]  dmydev_probe from platform_probe+0x5c/0xb0
>> [    0.581152]  platform_probe from really_probe+0xc8/0x2c8
>> [    0.586467]  really_probe from __driver_probe_device+0x88/0x19c
>> [    0.592387]  __driver_probe_device from driver_probe_device+0x30/0x104
>> [    0.598915]  driver_probe_device from __driver_attach+0x90/0x178
>> [    0.604921]  __driver_attach from bus_for_each_dev+0x7c/0xcc
>> [    0.610582]  bus_for_each_dev from bus_add_driver+0xcc/0x1ec
>> [    0.616241]  bus_add_driver from driver_register+0x7c/0x114
>> [    0.621814]  driver_register from dmydev_init+0x20/0x28
>> [    0.627045]  dmydev_init from do_one_initcall+0x58/0x200
>> [    0.632363]  do_one_initcall from kernel_init_freeable+0x1cc/0x228
>> [    0.638550]  kernel_init_freeable from kernel_init+0x1c/0x12c
>> [    0.644299]  kernel_init from ret_from_fork+0x14/0x28
>> [    0.649351] Exception stack(0xe0819fb0 to 0xe0819ff8)
>> [    0.654401] 9fa0:                                     00000000
>> 00000000 00000000 00000000
>> [    0.662575] 9fc0: 00000000 00000000 00000000 00000000 00000000
>> 00000000 00000000 00000000
>> [    0.670747] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>> [    0.677403] ---[ end trace 0000000000000000 ]---
>> [    0.682083] dmydev dmy_device: Allocate dma memory at 0xde000000 dma
>> addr 0x1e000000
>>
>> The reason is that now reserved memory's fixup function
>> dma_contiguous_early_fixup is called after the page table is
>> initialized. This fixup function increases the dma_mmu_remap count for
>> each reserved memory. And the dma_contiguous_remap function depends on
>> it to properly set up the reserved memory mmu table entry. Before this
>> change, the paging_init function calls dma_contiguous_remap and it
>> founds the reserved memory and set it up properly.  After the change,
>> this function found there is no reserved memory so skip any
>> initialization hence causes the crash later on when my driver tries to
>> allocate dma memory from the reserved memory.
>>
>> My workaround below is to move the dma_contiguous_remap out from the
>> paging_init function to the place right after unflatten_device_tree
>> where the dma_mmu_remap count is correctly set. But this is not ideal
>> solution and would like to see if you have any better way to solve the
>> issue.
>>
>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
>> index a41c93988d2c..535d1bf44529 100644
>> --- a/arch/arm/kernel/setup.c
>> +++ b/arch/arm/kernel/setup.c
>> @@ -1079,6 +1079,7 @@ void __init hyp_mode_check(void)
>>   #endif
>>   }
>>
>> +void __init dma_contiguous_remap(void);
>>   static void (*__arm_pm_restart)(enum reboot_mode reboot_mode, const
>> char *cmd);
>>
>>   static int arm_restart(struct notifier_block *nb, unsigned long action,
>> @@ -1164,6 +1165,7 @@ void __init setup_arch(char **cmdline_p)
>>          }
>>
>>          unflatten_device_tree();
>> +       dma_contiguous_remap();
>>
>>          arm_dt_init_cpu_maps();
>>          psci_dt_init();
>> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
>> index edb7f56b7c91..1828c8737d70 100644
>> --- a/arch/arm/mm/mmu.c
>> +++ b/arch/arm/mm/mmu.c
>> @@ -1773,7 +1773,6 @@ void __init paging_init(const struct machine_desc
>> *mdesc)
>>           * be used
>>           */
>>          map_kernel();
>> -       dma_contiguous_remap();
>>          early_fixmap_shutdown();
>>          devicemaps_init(mdesc);
>>          kmap_init();
>>
>> You can reproduce the issue on any v7 devices by adding these nodes to
>> the device tree
>> +       reserved-memory {
>> +               #address-cells = <1>;
>> +               #size-cells = <1>;
>> +               ranges;
>> +
>> +               dt_reserved_cma: dt_reserved_cma {
>> +                       compatible = "shared-dma-pool";
>> +                       reusable;
>> +
>> +                       reg = <0x1e000000 0x2000000>;
>> +               };
>> +       };
>> +
>> +       dmy_device {
>> +               compatible = "xyz,dmydev";
>> +               memory-region = <&dt_reserved_cma>;
>> +       };
>>
>> And use this test driver to trigger the call stack:
>> diff --git a/drivers/char/dmydev.c b/drivers/char/dmydev.c
>> new file mode 100644
>> index 000000000000..1dd52ec492eb
>> --- /dev/null
>> +++ b/drivers/char/dmydev.c
>> @@ -0,0 +1,67 @@
>> +#include<linux/module.h>
>> +#include<linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of.h>
>> +#include <linux/of_reserved_mem.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +static int dmydev_probe(struct platform_device *pdev)
>> +{
>> +       void* virt_addr;
>> +       dma_addr_t dma_addr;
>> +       int ret;
>> +
>> +       printk(KERN_ALERT "dmydev_probe called\n");
>> +
>> +       ret = of_reserved_mem_device_init(&pdev->dev);
>> +       if (ret && ret != -ENODEV) {
>> +               dev_err(&pdev->dev, "Couldn't assign reserve memory to
>> device ret = %d\n", ret);
>> +                       return ret;
>> +       }
>> +
>> +       virt_addr = dma_alloc_coherent(&pdev->dev, 0x800000, &dma_addr,
>> GFP_KERNEL);
>> +       if (virt_addr == NULL) {
>> +               dev_err(&pdev->dev,"Failed to allocated cma memory\n");
>> +               ret = -ENOMEM;
>> +       }
>> +       else
>> +               dev_err(&pdev->dev,"Allocate dma memory at 0x%px dma
>> addr %pad\n", virt_addr, &dma_addr);
>> +
>> +       return ret;
>> +}
>> +
>> +static void dmydev_remove(struct platform_device *pdev)
>> +{
>> +}
>> +
>> +static const struct of_device_id dmydev_of_match[] = {
>> +       {.compatible = "xyz,dmydev"},
>> +       {}
>> +};
>> +MODULE_DEVICE_TABLE(of, dmydev_of_match);
>>
>> Let me know if you need more info.
>>
>>>
>>> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@...cinc.com>
>>> ---
>>>   drivers/of/fdt.c             |   5 +-
>>>   drivers/of/of_private.h      |   3 +-
>>>   drivers/of/of_reserved_mem.c | 168 ++++++++++++++++++++++++-----------
>>>   3 files changed, 122 insertions(+), 54 deletions(-)
>>>
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 4d528c10df3a..d0dbc8183ac4 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -511,8 +511,6 @@ void __init early_init_fdt_scan_reserved_mem(void)
>>>                       break;
>>>               memblock_reserve(base, size);
>>>       }
>>> -
>>> -     fdt_init_reserved_mem();
>>>   }
>>>
>>>   /**
>>> @@ -1212,6 +1210,9 @@ void __init unflatten_device_tree(void)
>>>   {
>>>       void *fdt = initial_boot_params;
>>>
>>> +     /* Save the statically-placed regions in the reserved_mem array */
>>> +     fdt_scan_reserved_mem_reg_nodes();
>>> +
>>>       /* Don't use the bootloader provided DTB if ACPI is enabled */
>>>       if (!acpi_disabled)
>>>               fdt = NULL;
>>> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
>>> index 04aa2a91f851..29525c0b9939 100644
>>> --- a/drivers/of/of_private.h
>>> +++ b/drivers/of/of_private.h
>>> @@ -9,6 +9,7 @@
>>>    */
>>>
>>>   #define FDT_ALIGN_SIZE 8
>>> +#define MAX_RESERVED_REGIONS    64
>>>
>>>   /**
>>>    * struct alias_prop - Alias property in 'aliases' node
>>> @@ -180,7 +181,7 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node *
>>>   #endif
>>>
>>>   int fdt_scan_reserved_mem(void);
>>> -void fdt_init_reserved_mem(void);
>>> +void __init fdt_scan_reserved_mem_reg_nodes(void);
>>>
>>>   bool of_fdt_device_is_available(const void *blob, unsigned long node);
>>>
>>> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
>>> index 46e1c3fbc769..2011174211f9 100644
>>> --- a/drivers/of/of_reserved_mem.c
>>> +++ b/drivers/of/of_reserved_mem.c
>>> @@ -27,7 +27,6 @@
>>>
>>>   #include "of_private.h"
>>>
>>> -#define MAX_RESERVED_REGIONS 64
>>>   static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
>>>   static int reserved_mem_count;
>>>
>>> @@ -56,6 +55,7 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>>>       return err;
>>>   }
>>>
>>> +static void __init fdt_init_reserved_mem_node(struct reserved_mem *rmem);
>>>   /*
>>>    * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
>>>    */
>>> @@ -74,6 +74,9 @@ static void __init fdt_reserved_mem_save_node(unsigned long node, const char *un
>>>       rmem->base = base;
>>>       rmem->size = size;
>>>
>>> +     /* Call the region specific initialization function */
>>> +     fdt_init_reserved_mem_node(rmem);
>>> +
>>>       reserved_mem_count++;
>>>       return;
>>>   }
>>> @@ -106,7 +109,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);
>>> @@ -134,10 +136,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;
>>>   }
>>> @@ -165,12 +163,77 @@ static int __init __reserved_mem_check_root(unsigned long node)
>>>       return 0;
>>>   }
>>>
>>> +static void __init __rmem_check_for_overlap(void);
>>> +
>>> +/**
>>> + * 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;
>>> +
>>> +     if (!fdt)
>>> +             return;
>>> +
>>> +     node = fdt_path_offset(fdt, "/reserved-memory");
>>> +     if (node < 0) {
>>> +             pr_info("Reserved memory: No reserved-memory node in the DT\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);
>>> +     }
>>> +
>>> +     /* check for overlapping reserved regions */
>>> +     __rmem_check_for_overlap();
>>> +}
>>> +
>>> +static int __init __reserved_mem_alloc_size(unsigned long node, const char *uname);
>>> +
>>>   /*
>>>    * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
>>>    */
>>>   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");
>>> @@ -192,8 +255,24 @@ 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);
>>> +             /*
>>> +              * Save the nodes for the dynamically-placed regions
>>> +              * into an array which will be used for allocation right
>>> +              * after all the statically-placed regions are reserved
>>> +              * or marked as no-map. This is done to avoid dynamically
>>> +              * allocating from one of the statically-placed regions.
>>> +              */
>>> +             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;
>>>   }
>>> @@ -253,8 +332,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)
>>> +static 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;
>>> @@ -334,9 +412,8 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
>>>               return -ENOMEM;
>>>       }
>>>
>>> -     *res_base = base;
>>> -     *res_size = size;
>>> -
>>> +     /* Save region in the reserved_mem array */
>>> +     fdt_reserved_mem_save_node(node, uname, base, size);
>>>       return 0;
>>>   }
>>>
>>> @@ -425,48 +502,37 @@ static void __init __rmem_check_for_overlap(void)
>>>   }
>>>
>>>   /**
>>> - * fdt_init_reserved_mem() - allocate and init all saved reserved memory regions
>>> + * fdt_init_reserved_mem_node() - Initialize a reserved memory region
>>> + * @rmem: reserved_mem struct of the memory region to be initialized.
>>> + *
>>> + * This function is used to call the region specific initialization
>>> + * function for a reserved memory region.
>>>    */
>>> -void __init fdt_init_reserved_mem(void)
>>> +static void __init fdt_init_reserved_mem_node(struct reserved_mem *rmem)
>>>   {
>>> -     int i;
>>> -
>>> -     /* check for overlapping reserved regions */
>>> -     __rmem_check_for_overlap();
>>> -
>>> -     for (i = 0; i < reserved_mem_count; i++) {
>>> -             struct reserved_mem *rmem = &reserved_mem[i];
>>> -             unsigned long node = rmem->fdt_node;
>>> -             int err = 0;
>>> -             bool nomap;
>>> +     unsigned long node = rmem->fdt_node;
>>> +     int err = 0;
>>> +     bool nomap;
>>>
>>> -             nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
>>> +     nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
>>>
>>> -             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");
>>>       }
>>>   }
>>>
>>
> Just want to follow up on this issue.  Do you need any further detail
> or clarification?
> Any ARM memory manage guru on this thread can comment?
> Or is my workaround acceptable as a patch?


Hi William,

Sorry about the delay in getting back to you.

Instead of moving dma_contiguous_remap(), I suggest moving  dma_contiguous_early_fixup()
to the function that parses the reserved regions so that it is done before paging_init.

Here is what that could look like. Can you please give this a try?

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 77016c0cc296..132d2c66cafc 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -25,6 +25,7 @@
 #include <linux/memblock.h>
 #include <linux/kmemleak.h>
 #include <linux/cma.h>
+#include <linux/dma-map-ops.h>

 #include "of_private.h"

@@ -175,13 +176,17 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
                base = dt_mem_next_cell(dt_root_addr_cells, &prop);
                size = dt_mem_next_cell(dt_root_size_cells, &prop);

-               if (size &&
-                   early_init_dt_reserve_memory(base, size, nomap) == 0)
+               if (size && early_init_dt_reserve_memory(base, size, nomap) == 0) {
+                       /* Architecture specific contiguous memory fixup. */
+                       if (of_flat_dt_is_compatible(node, "shared-dma-pool"))
+                               dma_contiguous_early_fixup(base, size);
+
                        pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
                                uname, &base, (unsigned long)(size / SZ_1M));
-               else
+               } else {
                        pr_err("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
                               uname, &base, (unsigned long)(size / SZ_1M));
+               }

                len -= t_len;
        }
@@ -472,6 +477,9 @@ static int __init __reserved_mem_alloc_size(unsigned long node, const char *unam
                       uname, (unsigned long)(size / SZ_1M));
                return -ENOMEM;
        }
+       /* Architecture specific contiguous memory fixup. */
+       if (of_flat_dt_is_compatible(node, "shared-dma-pool"))
+               dma_contiguous_early_fixup(base, size);
        /* Save region in the reserved_mem array */
        fdt_reserved_mem_save_node(node, uname, base, size);
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 8df0dfaaca18..9e5d63efe7c5 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -480,8 +480,6 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)
                pr_err("Reserved memory: unable to setup CMA region\n");
                return err;
        }
-       /* Architecture specific contiguous memory fixup. */
-       dma_contiguous_early_fixup(rmem->base, rmem->size);

        if (default_cma)
                dma_contiguous_default_area = cma;

Regards,
Oreoluwa

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ