| Message ID | 20210719213826.39041-1-xypron.glpk@gmx.de |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [RFC,1/1] board: sifive: unmatched: use zero copy for initrd | expand |
On 19.07.21 23:38, Heinrich Schuchardt wrote: > Booting Ubuntu Impish showed the following output: > > relocaddr = 0x00000000fff60000 > > Loading Ramdisk to fa118000, end fffff19d ... > > The initrd is overwriting the U-Boot binary. Booting fails. > > There is no need to copy the initrd from $ramdisk_addr_r. > Set init_high = ~0UL to use zero copy. > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > --- > Generally copying to another memory location than $ramdisk_addr_r provides > no benefit whatsoever. > > But we still should find out why the initrd relocation fails so badly. > --- > include/configs/sifive-unmatched.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/configs/sifive-unmatched.h b/include/configs/sifive-unmatched.h > index d63a5f62fb..8dcfffedbe 100644 > --- a/include/configs/sifive-unmatched.h > +++ b/include/configs/sifive-unmatched.h > @@ -67,6 +67,7 @@ > "scriptaddr=0x88100000\0" \ > "pxefile_addr_r=0x88200000\0" \ > "ramdisk_addr_r=0x88300000\0" \ > + "initrd_high=0xffffffffffffffff\0" \ > "kernel_comp_addr_r=0x90000000\0" \ Tom remarked that kernel_comp_addr_r is poorly chosen as it will overlap with initrd if initrd exceeds 125 MiB. Current RISC-V Linux thinks initrd should fit into the first 128 MiB of RAM but that is an unnecessary restriction in Linux. Best regards Heinrich > "kernel_comp_size=0x4000000\0" \ > "type_guid_gpt_loader1=" TYPE_GUID_LOADER1 "\0" \ > -- > 2.30.2 >
On Mon, Jul 19, 2021 at 11:52:09PM +0200, Heinrich Schuchardt wrote: > > > On 19.07.21 23:38, Heinrich Schuchardt wrote: > > Booting Ubuntu Impish showed the following output: > > > > relocaddr = 0x00000000fff60000 > > > > Loading Ramdisk to fa118000, end fffff19d ... > > > > The initrd is overwriting the U-Boot binary. Booting fails. > > > > There is no need to copy the initrd from $ramdisk_addr_r. > > Set init_high = ~0UL to use zero copy. > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > --- > > Generally copying to another memory location than $ramdisk_addr_r provides > > no benefit whatsoever. > > > > But we still should find out why the initrd relocation fails so badly. > > --- > > include/configs/sifive-unmatched.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/include/configs/sifive-unmatched.h b/include/configs/sifive-unmatched.h > > index d63a5f62fb..8dcfffedbe 100644 > > --- a/include/configs/sifive-unmatched.h > > +++ b/include/configs/sifive-unmatched.h > > @@ -67,6 +67,7 @@ > > "scriptaddr=0x88100000\0" \ > > "pxefile_addr_r=0x88200000\0" \ > > "ramdisk_addr_r=0x88300000\0" \ > > + "initrd_high=0xffffffffffffffff\0" \ > > "kernel_comp_addr_r=0x90000000\0" \ > > Tom remarked that kernel_comp_addr_r is poorly chosen as it will overlap > with initrd if initrd exceeds 125 MiB. Current RISC-V Linux thinks > initrd should fit into the first 128 MiB of RAM but that is an > unnecessary restriction in Linux. I'll repeat what I said on IRC as well. I lament that there does not seem to be as detailed "booting" requirements in Linux for RISC-V as there is for arm/aarch64 because we REALLY need something like: https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/ti_armv7_common.h#L32 but for one or more RISC-V platforms (and well, MIPS and everyone else using device trees) that can be referenced when bringing up a new board to get good and always safe addresses to use. And I'll even say now that the ti_armv7_common.h example could be improved upon because I'm not super thrilled with the DTBO storage spot, but I don't have a better answer off the top of my head.
On Tue, Jul 20, 2021 at 1:06 AM Tom Rini <trini@konsulko.com> wrote: > > On Mon, Jul 19, 2021 at 11:52:09PM +0200, Heinrich Schuchardt wrote: > > > > > > On 19.07.21 23:38, Heinrich Schuchardt wrote: > > > Booting Ubuntu Impish showed the following output: > > > > > > relocaddr = 0x00000000fff60000 > > > > > > Loading Ramdisk to fa118000, end fffff19d ... > > > > > > The initrd is overwriting the U-Boot binary. Booting fails. > > > > > > There is no need to copy the initrd from $ramdisk_addr_r. > > > Set init_high = ~0UL to use zero copy. > > > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > > --- > > > Generally copying to another memory location than $ramdisk_addr_r provides > > > no benefit whatsoever. > > > > > > But we still should find out why the initrd relocation fails so badly. > > > --- > > > include/configs/sifive-unmatched.h | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/include/configs/sifive-unmatched.h b/include/configs/sifive-unmatched.h > > > index d63a5f62fb..8dcfffedbe 100644 > > > --- a/include/configs/sifive-unmatched.h > > > +++ b/include/configs/sifive-unmatched.h > > > @@ -67,6 +67,7 @@ > > > "scriptaddr=0x88100000\0" \ > > > "pxefile_addr_r=0x88200000\0" \ > > > "ramdisk_addr_r=0x88300000\0" \ > > > + "initrd_high=0xffffffffffffffff\0" \ > > > "kernel_comp_addr_r=0x90000000\0" \ > > > > Tom remarked that kernel_comp_addr_r is poorly chosen as it will overlap > > with initrd if initrd exceeds 125 MiB. Current RISC-V Linux thinks > > initrd should fit into the first 128 MiB of RAM but that is an > > unnecessary restriction in Linux. While I don't expect this to be a major issue we should definitely fix it. When I originally picked the value I wasn't thinking about it and initrd images were small. david > > I'll repeat what I said on IRC as well. I lament that there does not > seem to be as detailed "booting" requirements in Linux for RISC-V as > there is for arm/aarch64 because we REALLY need something like: > https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/ti_armv7_common.h#L32 > but for one or more RISC-V platforms (and well, MIPS and everyone else > using device trees) that can be referenced when bringing up a new board > to get good and always safe addresses to use. > > And I'll even say now that the ti_armv7_common.h example could be > improved upon because I'm not super thrilled with the DTBO storage spot, > but I don't have a better answer off the top of my head. > > -- > Tom
On Tue, Jul 20, 2021 at 04:46:23PM +0300, David Abdurachmanov wrote: > On Tue, Jul 20, 2021 at 1:06 AM Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Jul 19, 2021 at 11:52:09PM +0200, Heinrich Schuchardt wrote: > > > > > > > > > On 19.07.21 23:38, Heinrich Schuchardt wrote: > > > > Booting Ubuntu Impish showed the following output: > > > > > > > > relocaddr = 0x00000000fff60000 > > > > > > > > Loading Ramdisk to fa118000, end fffff19d ... > > > > > > > > The initrd is overwriting the U-Boot binary. Booting fails. > > > > > > > > There is no need to copy the initrd from $ramdisk_addr_r. > > > > Set init_high = ~0UL to use zero copy. > > > > > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > > > --- > > > > Generally copying to another memory location than $ramdisk_addr_r provides > > > > no benefit whatsoever. > > > > > > > > But we still should find out why the initrd relocation fails so badly. > > > > --- > > > > include/configs/sifive-unmatched.h | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/include/configs/sifive-unmatched.h b/include/configs/sifive-unmatched.h > > > > index d63a5f62fb..8dcfffedbe 100644 > > > > --- a/include/configs/sifive-unmatched.h > > > > +++ b/include/configs/sifive-unmatched.h > > > > @@ -67,6 +67,7 @@ > > > > "scriptaddr=0x88100000\0" \ > > > > "pxefile_addr_r=0x88200000\0" \ > > > > "ramdisk_addr_r=0x88300000\0" \ > > > > + "initrd_high=0xffffffffffffffff\0" \ > > > > "kernel_comp_addr_r=0x90000000\0" \ > > > > > > Tom remarked that kernel_comp_addr_r is poorly chosen as it will overlap > > > with initrd if initrd exceeds 125 MiB. Current RISC-V Linux thinks > > > initrd should fit into the first 128 MiB of RAM but that is an > > > unnecessary restriction in Linux. > > While I don't expect this to be a major issue we should definitely fix it. > When I originally picked the value I wasn't thinking about it and > initrd images were small. Yeah, honestly, I think it should be a high priority thing to fix / document. Distro initrds tend to not be small and these overlaps end up being a huge problem for end users. A good and documented example of where to put everything relative to start of DDR so that nothing should overlap by default makes things so much easier for everyone.
On 7/20/21 4:06 PM, Tom Rini wrote: > On Tue, Jul 20, 2021 at 04:46:23PM +0300, David Abdurachmanov wrote: >> On Tue, Jul 20, 2021 at 1:06 AM Tom Rini <trini@konsulko.com> wrote: >>> >>> On Mon, Jul 19, 2021 at 11:52:09PM +0200, Heinrich Schuchardt wrote: >>>> >>>> >>>> On 19.07.21 23:38, Heinrich Schuchardt wrote: >>>>> Booting Ubuntu Impish showed the following output: >>>>> >>>>> relocaddr = 0x00000000fff60000 >>>>> >>>>> Loading Ramdisk to fa118000, end fffff19d ... >>>>> >>>>> The initrd is overwriting the U-Boot binary. Booting fails. >>>>> >>>>> There is no need to copy the initrd from $ramdisk_addr_r. >>>>> Set init_high = ~0UL to use zero copy. >>>>> >>>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >>>>> --- >>>>> Generally copying to another memory location than $ramdisk_addr_r provides >>>>> no benefit whatsoever. >>>>> >>>>> But we still should find out why the initrd relocation fails so badly. >>>>> --- >>>>> include/configs/sifive-unmatched.h | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/include/configs/sifive-unmatched.h b/include/configs/sifive-unmatched.h >>>>> index d63a5f62fb..8dcfffedbe 100644 >>>>> --- a/include/configs/sifive-unmatched.h >>>>> +++ b/include/configs/sifive-unmatched.h >>>>> @@ -67,6 +67,7 @@ >>>>> "scriptaddr=0x88100000\0" \ >>>>> "pxefile_addr_r=0x88200000\0" \ >>>>> "ramdisk_addr_r=0x88300000\0" \ >>>>> + "initrd_high=0xffffffffffffffff\0" \ >>>>> "kernel_comp_addr_r=0x90000000\0" \ >>>> >>>> Tom remarked that kernel_comp_addr_r is poorly chosen as it will overlap >>>> with initrd if initrd exceeds 125 MiB. Current RISC-V Linux thinks >>>> initrd should fit into the first 128 MiB of RAM but that is an >>>> unnecessary restriction in Linux. >> >> While I don't expect this to be a major issue we should definitely fix it. >> When I originally picked the value I wasn't thinking about it and >> initrd images were small. > > Yeah, honestly, I think it should be a high priority thing to fix / > document. Distro initrds tend to not be small and these overlaps end up > being a huge problem for end users. A good and documented example of > where to put everything relative to start of DDR so that nothing should > overlap by default makes things so much easier for everyone. > There is no fits all solution. Boards come with different reserved areas. I even ran into a problem were you simply could not fit the intitrd where Linux was expecting it because there was so much reserved memory in the low 128 MiB. So Linux' EFI stub needed patching. Best regards Heinrich
On Tue, Jul 20, 2021 at 04:50:57PM +0200, Heinrich Schuchardt wrote: > > > On 7/20/21 4:06 PM, Tom Rini wrote: > > On Tue, Jul 20, 2021 at 04:46:23PM +0300, David Abdurachmanov wrote: > > > On Tue, Jul 20, 2021 at 1:06 AM Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Mon, Jul 19, 2021 at 11:52:09PM +0200, Heinrich Schuchardt wrote: > > > > > > > > > > > > > > > On 19.07.21 23:38, Heinrich Schuchardt wrote: > > > > > > Booting Ubuntu Impish showed the following output: > > > > > > > > > > > > relocaddr = 0x00000000fff60000 > > > > > > > > > > > > Loading Ramdisk to fa118000, end fffff19d ... > > > > > > > > > > > > The initrd is overwriting the U-Boot binary. Booting fails. > > > > > > > > > > > > There is no need to copy the initrd from $ramdisk_addr_r. > > > > > > Set init_high = ~0UL to use zero copy. > > > > > > > > > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > > > > > --- > > > > > > Generally copying to another memory location than $ramdisk_addr_r provides > > > > > > no benefit whatsoever. > > > > > > > > > > > > But we still should find out why the initrd relocation fails so badly. > > > > > > --- > > > > > > include/configs/sifive-unmatched.h | 1 + > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > diff --git a/include/configs/sifive-unmatched.h b/include/configs/sifive-unmatched.h > > > > > > index d63a5f62fb..8dcfffedbe 100644 > > > > > > --- a/include/configs/sifive-unmatched.h > > > > > > +++ b/include/configs/sifive-unmatched.h > > > > > > @@ -67,6 +67,7 @@ > > > > > > "scriptaddr=0x88100000\0" \ > > > > > > "pxefile_addr_r=0x88200000\0" \ > > > > > > "ramdisk_addr_r=0x88300000\0" \ > > > > > > + "initrd_high=0xffffffffffffffff\0" \ > > > > > > "kernel_comp_addr_r=0x90000000\0" \ > > > > > > > > > > Tom remarked that kernel_comp_addr_r is poorly chosen as it will overlap > > > > > with initrd if initrd exceeds 125 MiB. Current RISC-V Linux thinks > > > > > initrd should fit into the first 128 MiB of RAM but that is an > > > > > unnecessary restriction in Linux. > > > > > > While I don't expect this to be a major issue we should definitely fix it. > > > When I originally picked the value I wasn't thinking about it and > > > initrd images were small. > > > > Yeah, honestly, I think it should be a high priority thing to fix / > > document. Distro initrds tend to not be small and these overlaps end up > > being a huge problem for end users. A good and documented example of > > where to put everything relative to start of DDR so that nothing should > > overlap by default makes things so much easier for everyone. > > > There is no fits all solution. Boards come with different reserved > areas. I even ran into a problem were you simply could not fit the > intitrd where Linux was expecting it because there was so much reserved > memory in the low 128 MiB. So Linux' EFI stub needed patching. There's certainly "fits most". And documenting restrictions / requirements is important, especially if that's not a bug you're talking about in that specific case. For example, wait, the ramdisk needs to be in the lower 128MiB of memory only for what platforms?
diff --git a/include/configs/sifive-unmatched.h b/include/configs/sifive-unmatched.h index d63a5f62fb..8dcfffedbe 100644 --- a/include/configs/sifive-unmatched.h +++ b/include/configs/sifive-unmatched.h @@ -67,6 +67,7 @@ "scriptaddr=0x88100000\0" \ "pxefile_addr_r=0x88200000\0" \ "ramdisk_addr_r=0x88300000\0" \ + "initrd_high=0xffffffffffffffff\0" \ "kernel_comp_addr_r=0x90000000\0" \ "kernel_comp_size=0x4000000\0" \ "type_guid_gpt_loader1=" TYPE_GUID_LOADER1 "\0" \
Booting Ubuntu Impish showed the following output: relocaddr = 0x00000000fff60000 Loading Ramdisk to fa118000, end fffff19d ... The initrd is overwriting the U-Boot binary. Booting fails. There is no need to copy the initrd from $ramdisk_addr_r. Set init_high = ~0UL to use zero copy. Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> --- Generally copying to another memory location than $ramdisk_addr_r provides no benefit whatsoever. But we still should find out why the initrd relocation fails so badly. --- include/configs/sifive-unmatched.h | 1 + 1 file changed, 1 insertion(+) -- 2.30.2