* Re: [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints
2022-06-08 6:20 [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints Alistair Francis
@ 2022-06-08 6:41 ` Bin Meng
2022-06-23 4:14 ` Alistair Francis
2022-06-27 7:01 ` Atish Patra
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Bin Meng @ 2022-06-08 6:41 UTC (permalink / raw)
To: Alistair Francis, Atish Patra
Cc: open list:RISC-V, qemu-devel@nongnu.org Developers, Bin Meng,
Alistair Francis, Palmer Dabbelt, Alistair Francis
+Atish
On Wed, Jun 8, 2022 at 2:20 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> We previously stored the device tree at a 16MB alignment from the end of
> memory (or 3GB). This means we need at least 16MB of memory to be able
> to do this. We don't actually need the FDT to be 16MB aligned, so let's
> drop it down to 2MB so that we can support systems with less memory,
> while also allowing FDT size expansion.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/992
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> hw/riscv/boot.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 57a41df8e9..e476d8f491 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -226,11 +226,11 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
> /*
> * We should put fdt as far as possible to avoid kernel/initrd overwriting
> * its content. But it should be addressable by 32 bit system as well.
> - * Thus, put it at an 16MB aligned address that less than fdt size from the
> + * Thus, put it at an 2MB aligned address that less than fdt size from the
> * end of dram or 3GB whichever is lesser.
> */
> temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end;
> - fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 16 * MiB);
> + fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
>
@Atish Patra may have some pointers about the 16MiB alignment requirement.
Regards,
Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints
2022-06-08 6:41 ` Bin Meng
@ 2022-06-23 4:14 ` Alistair Francis
2022-06-23 5:45 ` Atish Kumar Patra
0 siblings, 1 reply; 8+ messages in thread
From: Alistair Francis @ 2022-06-23 4:14 UTC (permalink / raw)
To: Bin Meng
Cc: Alistair Francis, Atish Patra, open list:RISC-V,
qemu-devel@nongnu.org Developers, Bin Meng, Alistair Francis,
Palmer Dabbelt
On Wed, Jun 8, 2022 at 4:41 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> +Atish
>
> On Wed, Jun 8, 2022 at 2:20 PM Alistair Francis
> <alistair.francis@opensource.wdc.com> wrote:
> >
> > From: Alistair Francis <alistair.francis@wdc.com>
> >
> > We previously stored the device tree at a 16MB alignment from the end of
> > memory (or 3GB). This means we need at least 16MB of memory to be able
> > to do this. We don't actually need the FDT to be 16MB aligned, so let's
> > drop it down to 2MB so that we can support systems with less memory,
> > while also allowing FDT size expansion.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/992
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> > hw/riscv/boot.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > index 57a41df8e9..e476d8f491 100644
> > --- a/hw/riscv/boot.c
> > +++ b/hw/riscv/boot.c
> > @@ -226,11 +226,11 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
> > /*
> > * We should put fdt as far as possible to avoid kernel/initrd overwriting
> > * its content. But it should be addressable by 32 bit system as well.
> > - * Thus, put it at an 16MB aligned address that less than fdt size from the
> > + * Thus, put it at an 2MB aligned address that less than fdt size from the
> > * end of dram or 3GB whichever is lesser.
> > */
> > temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end;
> > - fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 16 * MiB);
> > + fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> >
>
> @Atish Patra may have some pointers about the 16MiB alignment requirement.
Any thoughts?
Alistair
>
> Regards,
> Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints
2022-06-23 4:14 ` Alistair Francis
@ 2022-06-23 5:45 ` Atish Kumar Patra
2022-06-27 0:49 ` Alistair Francis
0 siblings, 1 reply; 8+ messages in thread
From: Atish Kumar Patra @ 2022-06-23 5:45 UTC (permalink / raw)
To: Alistair Francis
Cc: Bin Meng, Alistair Francis, open list:RISC-V,
qemu-devel@nongnu.org Developers, Bin Meng, Alistair Francis,
Palmer Dabbelt
On Wed, Jun 22, 2022 at 9:15 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Jun 8, 2022 at 4:41 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > +Atish
> >
> > On Wed, Jun 8, 2022 at 2:20 PM Alistair Francis
> > <alistair.francis@opensource.wdc.com> wrote:
> > >
> > > From: Alistair Francis <alistair.francis@wdc.com>
> > >
> > > We previously stored the device tree at a 16MB alignment from the end of
> > > memory (or 3GB). This means we need at least 16MB of memory to be able
> > > to do this. We don't actually need the FDT to be 16MB aligned, so let's
> > > drop it down to 2MB so that we can support systems with less memory,
> > > while also allowing FDT size expansion.
> > >
> > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/992
> > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > > ---
> > > hw/riscv/boot.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > > index 57a41df8e9..e476d8f491 100644
> > > --- a/hw/riscv/boot.c
> > > +++ b/hw/riscv/boot.c
> > > @@ -226,11 +226,11 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
> > > /*
> > > * We should put fdt as far as possible to avoid kernel/initrd overwriting
> > > * its content. But it should be addressable by 32 bit system as well.
> > > - * Thus, put it at an 16MB aligned address that less than fdt size from the
> > > + * Thus, put it at an 2MB aligned address that less than fdt size from the
> > > * end of dram or 3GB whichever is lesser.
> > > */
> > > temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end;
> > > - fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 16 * MiB);
> > > + fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> > >
> >
> > @Atish Patra may have some pointers about the 16MiB alignment requirement.
>
Sorry. I missed this patch for some reason. 16MiB alignment was just
chosen as a safe option.
We couldn't put it at the end of DRAM and I just wanted to place it at
a reasonable distance.
2MB should be totally okay.
> Any thoughts?
>
> Alistair
>
> >
> > Regards,
> > Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints
2022-06-23 5:45 ` Atish Kumar Patra
@ 2022-06-27 0:49 ` Alistair Francis
0 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2022-06-27 0:49 UTC (permalink / raw)
To: Atish Kumar Patra
Cc: Bin Meng, Alistair Francis, open list:RISC-V,
qemu-devel@nongnu.org Developers, Bin Meng, Alistair Francis,
Palmer Dabbelt
On Thu, Jun 23, 2022 at 3:45 PM Atish Kumar Patra <atishp@rivosinc.com> wrote:
>
> On Wed, Jun 22, 2022 at 9:15 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Wed, Jun 8, 2022 at 4:41 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > +Atish
> > >
> > > On Wed, Jun 8, 2022 at 2:20 PM Alistair Francis
> > > <alistair.francis@opensource.wdc.com> wrote:
> > > >
> > > > From: Alistair Francis <alistair.francis@wdc.com>
> > > >
> > > > We previously stored the device tree at a 16MB alignment from the end of
> > > > memory (or 3GB). This means we need at least 16MB of memory to be able
> > > > to do this. We don't actually need the FDT to be 16MB aligned, so let's
> > > > drop it down to 2MB so that we can support systems with less memory,
> > > > while also allowing FDT size expansion.
> > > >
> > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/992
> > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > > > ---
> > > > hw/riscv/boot.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > > > index 57a41df8e9..e476d8f491 100644
> > > > --- a/hw/riscv/boot.c
> > > > +++ b/hw/riscv/boot.c
> > > > @@ -226,11 +226,11 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
> > > > /*
> > > > * We should put fdt as far as possible to avoid kernel/initrd overwriting
> > > > * its content. But it should be addressable by 32 bit system as well.
> > > > - * Thus, put it at an 16MB aligned address that less than fdt size from the
> > > > + * Thus, put it at an 2MB aligned address that less than fdt size from the
> > > > * end of dram or 3GB whichever is lesser.
> > > > */
> > > > temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end;
> > > > - fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 16 * MiB);
> > > > + fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> > > >
> > >
> > > @Atish Patra may have some pointers about the 16MiB alignment requirement.
> >
>
> Sorry. I missed this patch for some reason. 16MiB alignment was just
> chosen as a safe option.
> We couldn't put it at the end of DRAM and I just wanted to place it at
> a reasonable distance.
>
> 2MB should be totally okay.
Can I get a review or Ack :)
Alistair
>
> > Any thoughts?
> >
> > Alistair
> >
> > >
> > > Regards,
> > > Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints
2022-06-08 6:20 [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints Alistair Francis
2022-06-08 6:41 ` Bin Meng
@ 2022-06-27 7:01 ` Atish Patra
2022-06-27 14:03 ` Bin Meng
2022-06-27 23:23 ` Alistair Francis
3 siblings, 0 replies; 8+ messages in thread
From: Atish Patra @ 2022-06-27 7:01 UTC (permalink / raw)
To: Alistair Francis
Cc: open list:RISC-V, qemu-devel@nongnu.org Developers, Bin Meng,
Alistair Francis, Palmer Dabbelt, Bin Meng, Alistair Francis
On Tue, Jun 7, 2022 at 11:21 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> We previously stored the device tree at a 16MB alignment from the end of
> memory (or 3GB). This means we need at least 16MB of memory to be able
> to do this. We don't actually need the FDT to be 16MB aligned, so let's
> drop it down to 2MB so that we can support systems with less memory,
> while also allowing FDT size expansion.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/992
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> hw/riscv/boot.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 57a41df8e9..e476d8f491 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -226,11 +226,11 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
> /*
> * We should put fdt as far as possible to avoid kernel/initrd overwriting
> * its content. But it should be addressable by 32 bit system as well.
> - * Thus, put it at an 16MB aligned address that less than fdt size from the
> + * Thus, put it at an 2MB aligned address that less than fdt size from the
> * end of dram or 3GB whichever is lesser.
> */
> temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end;
> - fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 16 * MiB);
> + fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
>
> ret = fdt_pack(fdt);
> /* Should only fail if we've built a corrupted tree */
> --
> 2.36.1
>
>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints
2022-06-08 6:20 [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints Alistair Francis
2022-06-08 6:41 ` Bin Meng
2022-06-27 7:01 ` Atish Patra
@ 2022-06-27 14:03 ` Bin Meng
2022-06-27 23:23 ` Alistair Francis
3 siblings, 0 replies; 8+ messages in thread
From: Bin Meng @ 2022-06-27 14:03 UTC (permalink / raw)
To: Alistair Francis
Cc: open list:RISC-V, qemu-devel@nongnu.org Developers, Bin Meng,
Alistair Francis, Palmer Dabbelt, Alistair Francis
On Wed, Jun 8, 2022 at 2:20 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> We previously stored the device tree at a 16MB alignment from the end of
> memory (or 3GB). This means we need at least 16MB of memory to be able
> to do this. We don't actually need the FDT to be 16MB aligned, so let's
> drop it down to 2MB so that we can support systems with less memory,
> while also allowing FDT size expansion.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/992
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> hw/riscv/boot.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested booting Linux 64-bit/32-bit v5.18 kernel, VxWorks 7
64-bit/32-bit 22.06 release kernel
Tested-by: Bin Meng <bin.meng@windriver.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints
2022-06-08 6:20 [PATCH] hw/riscv: boot: Reduce FDT address alignment constraints Alistair Francis
` (2 preceding siblings ...)
2022-06-27 14:03 ` Bin Meng
@ 2022-06-27 23:23 ` Alistair Francis
3 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2022-06-27 23:23 UTC (permalink / raw)
To: Alistair Francis
Cc: open list:RISC-V, qemu-devel@nongnu.org Developers, Bin Meng,
Alistair Francis, Palmer Dabbelt, Bin Meng
On Wed, Jun 8, 2022 at 4:20 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> We previously stored the device tree at a 16MB alignment from the end of
> memory (or 3GB). This means we need at least 16MB of memory to be able
> to do this. We don't actually need the FDT to be 16MB aligned, so let's
> drop it down to 2MB so that we can support systems with less memory,
> while also allowing FDT size expansion.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/992
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> hw/riscv/boot.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 57a41df8e9..e476d8f491 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -226,11 +226,11 @@ uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
> /*
> * We should put fdt as far as possible to avoid kernel/initrd overwriting
> * its content. But it should be addressable by 32 bit system as well.
> - * Thus, put it at an 16MB aligned address that less than fdt size from the
> + * Thus, put it at an 2MB aligned address that less than fdt size from the
> * end of dram or 3GB whichever is lesser.
> */
> temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end;
> - fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 16 * MiB);
> + fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
>
> ret = fdt_pack(fdt);
> /* Should only fail if we've built a corrupted tree */
> --
> 2.36.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread