linux-kernel - Re: [PATCH v7 1/2] of: reserved_mem: Restruture how the reserved memory regions are processed

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3e6e8921-6d52-40e2-9124-7ab550566716@quicinc.com>
Date: Wed, 21 Aug 2024 16:41:10 -0700
From: Oreoluwa Babatunde <quic_obabatun@...cinc.com>
To: Rob Herring <robh@...nel.org>
CC: <saravanak@...gle.com>, <klarasmodin@...il.com>, <aisheng.dong@....com>,
        <hch@....de>, <m.szyprowski@...sung.com>, <robin.murphy@....com>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <iommu@...ts.linux.dev>, <will@...nel.org>, <catalin.marinas@....com>,
        <kernel@...cinc.com>
Subject: Re: [PATCH v7 1/2] of: reserved_mem: Restruture how the reserved
 memory regions are processed


On 8/19/2024 3:04 PM, Rob Herring wrote:
> On Fri, Aug 9, 2024 at 1:48 PM Oreoluwa Babatunde
> <quic_obabatun@...cinc.com> 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.
>>
>> [...]
>>
>> 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 | 172 +++++++++++++++++++++++++----------
>>  3 files changed, 131 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 68103ad230ee..d4b7aaa70e31 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();
>>  }
>>
>>  /**
>> @@ -1239,6 +1237,9 @@ void __init unflatten_device_tree(void)
>>         of_alias_scan(early_init_dt_alloc_memory_arch);
>>
>>         unittest_unflatten_overlay_base();
>> +
>> +       /* Save the statically-placed regions in the reserved_mem array */
>> +       fdt_scan_reserved_mem_reg_nodes();
Hi Rob,
> I'm still not understanding why the unflatttened API doesn't work
> here? It was just used in of_alias_scan() above here.
The main reason why the unflatten_devicetree APIs does not work here is
because a reference to fdt_node needs to be stored for the reserved
regions, and it can only be gotten by using the fdt APIs.

The fdt_node is needed by rmem_dma_setup(), rmem_cma_setup(), and
rmem_swiotlb_setup(). All of which are used to configure the reserved
memory regions during early bootup.

In my previous versions, I replaced fdt_node with device_node in struct
reserved_mem in order to leverage the unflatten_devicetree APIs, and the
above functions were being called after the page tables were setup.

As we found out later, those functions need to be called before the page
tables are setup in order for the reserved regions to be configured
correctly[1]. But since the unflatten_devicetree APIs are not available
before the page tables are setup, I had to switch back to using the
fdt_node which can only be gotten by using the fdt APIs.

[1] https://lore.kernel.org/all/002b6176-41b3-4888-abb1-978399d108b8@arm.com/

The only way I see that we can avoid using the fdt APIs here is if
we just don't store an fdt_node reference for the reserved regions
in resvered_mem.  But I'm not sure if we want to do that.
> The problem reported is this function uses initial_boot_params, but
> that's NULL for x86.
ack
Thank you,
Oreoluwa.