[PATCH v2 2/3] riscv: Expand the DT size before copy reserved memory node

[Atish Patra]

On Wed, Jun 10, 2020 at 2:05 AM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Atish,
>
> On Wed, Jun 3, 2020 at 2:23 AM Atish Patra <atishp at atishpatra.org> wrote:
> >
> > On Thu, May 28, 2020 at 1:47 AM Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > From: Bin Meng <bin.meng at windriver.com>
> > >
> > > The FDT blob might not have sufficient space to hold a copy of
> > > reserved memory node. Expand it before the copy.
> > >
> > > Reported-by: Rick Chen <rick at andestech.com>
> > > Signed-off-by: Bin Meng <bin.meng at windriver.com>
> > > ---
> > >
> > >  arch/riscv/lib/fdt_fixup.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c
> > > index 5f523f0..1290a64 100644
> > > --- a/arch/riscv/lib/fdt_fixup.c
> > > +++ b/arch/riscv/lib/fdt_fixup.c
> > > @@ -41,6 +41,12 @@ int riscv_fdt_copy_resv_mem_node(const void *src, void *dst)
> > >                 return 0;
> > >         }
> > >
> > > +       err = fdt_open_into(dst, dst, fdt_totalsize(dst) + 32);
> >
> > The size may be bigger than 32 bytes in future given that we may have
> > multiple reserved pmp regions.
> > How about calculating the size from the source and using that instead
> > of a fixed size ?
> >
>
> This looks way too complicated. That means the codes here need to
> understand the FDT blob binary representation to know exactly how much
> additional size is needed. By looking at existing calls to
> fdt_open_into() none of these do such things.
>
> I guess we will have to give an estimated size to fit all possible
> situations. Say there are 16 PMP regions, and one occupies 64 bytes,
> so we can extend the FDT by 64 * 16 = 1024 bytes. Thoughts ??
>

That works too. I see we only allocate 256 bytes in OpenSBI.
Can you send a patch to OpenSBI as well with the above reasoning ?

> >
> > > +       if (err < 0) {
> > > +               printf("Device Tree can't be expanded to accommodate new node");
> > > +               return err;
> > > +       }
> > > +
> > >         fdt_for_each_subnode(node, src, offset) {
> > >                 name = fdt_get_name(src, node, NULL);
> > >
>
> Regards,
> Bin



-- 
Regards,
Atish