| Message ID | 1590655604-13704-2-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | [v2,1/3] riscv: Avoid the reserved memory fixup if src and dst point to the same place | expand |
On Thu, May 28, 2020 at 1:47 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > From: Bin Meng <bin.meng@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@andestech.com> > Signed-off-by: Bin Meng <bin.meng@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 ? > + 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); > > -- > 2.7.4 > -- Regards, Atish
Hi Atish, On Wed, Jun 3, 2020 at 2:23 AM Atish Patra <atishp@atishpatra.org> wrote: > > On Thu, May 28, 2020 at 1:47 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > From: Bin Meng <bin.meng@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@andestech.com> > > Signed-off-by: Bin Meng <bin.meng@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 ?? > > > + 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
On Wed, Jun 10, 2020 at 2:05 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Atish, > > On Wed, Jun 3, 2020 at 2:23 AM Atish Patra <atishp@atishpatra.org> wrote: > > > > On Thu, May 28, 2020 at 1:47 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > From: Bin Meng <bin.meng@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@andestech.com> > > > Signed-off-by: Bin Meng <bin.meng@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
On Fri, Jun 12, 2020 at 3:18 AM Atish Patra <atishp@atishpatra.org> wrote: > > On Wed, Jun 10, 2020 at 2:05 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Atish, > > > > On Wed, Jun 3, 2020 at 2:23 AM Atish Patra <atishp@atishpatra.org> wrote: > > > > > > On Thu, May 28, 2020 at 1:47 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > From: Bin Meng <bin.meng@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@andestech.com> > > > > Signed-off-by: Bin Meng <bin.meng@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 ? > Sure, will do. Regards, Bin
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); + 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);