linux-kernel - Re: Question regarding "reserved-memory"

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] [day] [month] [year] [list]
Date:   Thu, 24 Oct 2019 15:19:52 +0000
From:   Ayan Halder <Ayan.Halder@....com>
To:     Rob Herring <robh+dt@...nel.org>
CC:     Mark Rutland <Mark.Rutland@....com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Liviu Dudau <Liviu.Dudau@....com>,
        Mihail Atanassov <Mihail.Atanassov@....com>,
        "james qian wang (Arm Technology China)" <james.qian.wang@....com>,
        Brian Starkey <Brian.Starkey@....com>, nd <nd@....com>
Subject: Re: Question regarding "reserved-memory"

On Thu, Oct 24, 2019 at 09:51:04AM -0500, Rob Herring wrote:
> On Thu, Oct 24, 2019 at 9:22 AM Ayan Halder <Ayan.Halder@....com> wrote:
Hi Bob,

Thanks for your quick response.
> >
> >
> > Hi Folks,
> >
> > I have a question regarding "reserved-memory". I am using an Arm Juno
> > platform which has a chunk of ram in its fpga. I intend to make this
> > memory as reserved so that it can be shared between various devices
> > for passing framebuffer.
> >
> > My dts looks like the following:-
> >
> > / {
> >         .... // some nodes
> >
> >         tlx@...00000 {
> >                 compatible = "simple-bus";
> >                 ...
> >
> >                 juno_wrapper {
> >
> >                         ... /* here we have all the nodes */
> >                             /* corresponding to the devices in the fpga */
> >
> >                         memory@...0000 {
> >                                device_type = "memory";
> >                                reg = <0x00 0x60000000 0x00 0x8000000>;
> >                         };
> >
> >                         reserved-memory {
> >                                #address-cells = <0x01>;
> >                                #size-cells = <0x01>;
> >                                ranges;
> >
> >                                framebuffer@...0000 {
> >                                         compatible = "shared-dma-pool";
> >                                         linux,cma-default;
> >                                         reusable;
> >                                         reg = <0x00 0x60000000 0x00 0x8000000>;
> >                                         phandle = <0x44>;
> >                                 };
> >                         };
> >                         ...
> >                 }
> >         }
> > ...
> > }
> >
> > Note that the depth of the "reserved-memory" node is 3.
> >
> > Refer __fdt_scan_reserved_mem() :-
> >
> >         if (!found && depth == 1 && strcmp(uname, "reserved-memory") == 0) {
> >
> >                 if (__reserved_mem_check_root(node) != 0) {
> >                         pr_err("Reserved memory: unsupported node
> > format, ignoring\n");
> >                         /* break scan */
> >                         return 1;
> >                 }
> >                 found = 1;
> >
> >                 /* scan next node */
> >                 return 0;
> >         }
> >
> > It expects the "reserved-memory" node to be at depth == 1 and so it
> > does not probe it in our case.
> >
> > Niether from the
> > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> >  nor from commit - e8d9d1f5485b52ec3c4d7af839e6914438f6c285,
> > I could understand the reason for such restriction.
> >
> > So, I seek the community's advice as to whether I should fix up
> > __fdt_scan_reserved_mem() so as to do away with the restriction or
> > put the "reserved-memory" node outside of 'tlx@...00000' (which looks
> >  logically incorrect as the memory is on the fpga platform).
> 
> For now, I'm going to say it must be at the root level. 
Can you mention it in the Documentation/.../reserved-memory.txt,
please?

> I'd guess the
> memory@...0000 node is also just ignored (wrong unit-address BTW).
I would request you to ignore the address for the time being. In
juno_wrapper{}, we have a complex remapping of addresses of all the
sub-devices. I deliberately did not put that in the snippet, so as to
prevent any distraction from the core issue.

> 
> I think you're also forgetting the later unflattened parsing of
> /reserved-memory.
Are you talking about the remaining part of the
__fdt_scan_reserved_mem() ie

       ....
        } else if (found && depth < 2) {
                /* scanning of /reserved-memory has been finished */
                return 1;
        }

        if (!of_fdt_device_is_available(initial_boot_params, node))
                return 0;

        err = __reserved_mem_reserve_reg(node, uname);
        if (err == -ENOENT && of_get_flat_dt_prop(node, "size", NULL))
                fdt_reserved_mem_save_node(node, uname, 0, 0);

        /* scan next node */
        return 0;

If so, I agree with you that it needs to be changed as well (if we
were to do away with the restriction).

> The other complication IIRC is booting with UEFI
> does it's own reserved memory table which often uses the DT table as
> its source.
I have no knowledge of UEFI booting. So if UEFI expects
"reserved-memory" nodes to be at root level, then we must adhere to
the restriction. :)

Ayan
> 
> Rob