| Message ID | 20230126135219.1054658-2-dbarboza@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv_load_fdt() semantics change | expand |
Hi Daniel, On Thu, Jan 26, 2023 at 9:53 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > fdt_pack() can change the fdt size, meaning that fdt_totalsize() can > contain a now deprecated (bigger) value. The commit message is a bit confusing. The original code in this patch does not call fdt_pack(). So not sure where the issue of "deprecated (bigger) value" happens? > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > hw/riscv/boot.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index 3172a76220..a563b7482a 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -287,8 +287,13 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) > { > uint64_t temp, fdt_addr; > hwaddr dram_end = dram_base + mem_size; > - int ret, fdtsize = fdt_totalsize(fdt); > + int ret = fdt_pack(fdt); > + int fdtsize; > > + /* Should only fail if we've built a corrupted tree */ > + g_assert(ret == 0); > + > + fdtsize = fdt_totalsize(fdt); > if (fdtsize <= 0) { > error_report("invalid device-tree"); > exit(1); Regards, Bin
On Sun, Jan 29, 2023 at 10:20 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Daniel, > > On Thu, Jan 26, 2023 at 9:53 PM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: > > > > fdt_pack() can change the fdt size, meaning that fdt_totalsize() can > > contain a now deprecated (bigger) value. > > The commit message is a bit confusing. > > The original code in this patch does not call fdt_pack(). So not sure > where the issue of "deprecated (bigger) value" happens? I see where the call to fdt_pack() happens. I think you should move the following changes in patch#2 of this series to this commit. - ret = fdt_pack(fdt); - /* Should only fail if we've built a corrupted tree */ - g_assert(ret == 0); After that, your commit message makes sense, as it describes the problem and how your patch fixes the problem. > > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > > --- > > hw/riscv/boot.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > > index 3172a76220..a563b7482a 100644 > > --- a/hw/riscv/boot.c > > +++ b/hw/riscv/boot.c > > @@ -287,8 +287,13 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) > > { > > uint64_t temp, fdt_addr; > > hwaddr dram_end = dram_base + mem_size; > > - int ret, fdtsize = fdt_totalsize(fdt); > > + int ret = fdt_pack(fdt); > > + int fdtsize; > > > > + /* Should only fail if we've built a corrupted tree */ > > + g_assert(ret == 0); > > + > > + fdtsize = fdt_totalsize(fdt); > > if (fdtsize <= 0) { > > error_report("invalid device-tree"); > > exit(1); > Regards, Bin
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c index 3172a76220..a563b7482a 100644 --- a/hw/riscv/boot.c +++ b/hw/riscv/boot.c @@ -287,8 +287,13 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) { uint64_t temp, fdt_addr; hwaddr dram_end = dram_base + mem_size; - int ret, fdtsize = fdt_totalsize(fdt); + int ret = fdt_pack(fdt); + int fdtsize; + /* Should only fail if we've built a corrupted tree */ + g_assert(ret == 0); + + fdtsize = fdt_totalsize(fdt); if (fdtsize <= 0) { error_report("invalid device-tree"); exit(1);