[PATCH v2] arm: Support initrd with address in boot alias region

[PATCH v2] arm: Support initrd with address in boot alias region

[Matija Glavinic Pecotic]

When bootloader passes address of initrd in boot alias region, initrd
will fail on memblock_is_region_memory as memblock with such address
doesn't exist. Problem was uncovered by commit fe7db7570379 ("of/fdt:
Populate phys_initrd_start/phys_initrd_size from FDT") which removed
__virt_to_phys on the initrd physical address. Earlier, __virt_to_phys
on our platform coincidentally sanitized address.

Initrd address in bootalias region is valid to be passed by bootloader,
in our case it was done by kexec:

$ cat /proc/iomem
[cut]
c0000000-d4da1fff : System RAM (boot alias)
840000000-854da1fff : System RAM
  840008000-840bfffff : Kernel code
  840e00000-840ee56fb : Kernel data
[cut]
880000000-8ffffffff : System RAM

arch/arm/kernel/setup.c:
/*
 * Some systems have a special memory alias which is only
 * used for booting.  We need to advertise this region to
 * kexec-tools so they know where bootable RAM is located.
 */
boot_alias_start = phys_to_idmap(start);
if (arm_has_idmap_alias() && boot_alias_start != IDMAP_INVALID_ADDR) {
    res = memblock_alloc(sizeof(*res), SMP_CACHE_BYTES);
    if (!res)
        panic("%s: Failed to allocate %zu bytes\n",
            __func__, sizeof(*res));
    res->name = "System RAM (boot alias)";
    res->start = boot_alias_start;
    res->end = phys_to_idmap(res_end);
    res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
    request_resource(&iomem_resource, res);
}

Fix by trying to sanitize address in case of invalid physical address
for platforms (e.g. Keystone 2) supporting it.

Signed-off-by: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com>
---

v2: Rebased against current tree, update commit message to capture
previous discussion (http://lists.infradead.org/pipermail/linux-arm-kernel/2020-September/605344.html)

 init/initramfs.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/init/initramfs.c b/init/initramfs.c
index 18229cfe8906..27c50910c55f 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -591,6 +591,21 @@ void __init reserve_initrd_mem(void)
 
 	if (!phys_initrd_size)
 		return;
+
+#ifdef CONFIG_ARM
+	/*
+	 * If initrd is not on valid physical address, reason could
+	 * be due to bootloader passing address from bootalias region.
+	 * Try to sanitize such address for platforms supporting boot
+	 * aliases.
+	 */
+	if (!pfn_valid(__phys_to_pfn(phys_initrd_start))) {
+		pr_info("initrd at invalid address, trying to use boot alias\n");
+		if (arm_has_idmap_alias())
+			phys_initrd_start = idmap_to_phys(phys_initrd_start);
+	}
+#endif
+
 	/*
 	 * Round the memory region to page boundaries as per free_initrd_mem()
 	 * This allows us to detect whether the pages overlapping the initrd
-- 
2.26.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] arm: Support initrd with address in boot alias region

[Arnd Bergmann]

On Thu, Aug 18, 2022 at 10:39 AM Matija Glavinic Pecotic
<matija.glavinic-pecotic.ext@nokia.com> wrote:
> +
> +#ifdef CONFIG_ARM
> +       /*
> +        * If initrd is not on valid physical address, reason could
> +        * be due to bootloader passing address from bootalias region.
> +        * Try to sanitize such address for platforms supporting boot
> +        * aliases.
> +        */
> +       if (!pfn_valid(__phys_to_pfn(phys_initrd_start))) {
> +               pr_info("initrd at invalid address, trying to use boot alias\n");
> +               if (arm_has_idmap_alias())
> +                       phys_initrd_start = idmap_to_phys(phys_initrd_start);
> +       }
> +#endif

Hi Matija,

I think the logic looks correct, but I' would not put this into init/initramfs.c
when the problem is specific to Arm. The function is called from
arch/arm/mm/init.c, so you could just put it there.

I'm fairly sure that only keystone2 is affected by this, so it might
even be possible to do this inside of keystone specific code,
e.g. in keystone_pv_fixup(), which is called a little earlier.

        Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] arm: Support initrd with address in boot alias region

[Matija Glavinic Pecotic]

On 18.08.2022 14:03, Arnd Bergmann wrote:
> I think the logic looks correct, but I' would not put this into init/initramfs.c
> when the problem is specific to Arm. The function is called from
> arch/arm/mm/init.c, so you could just put it there.
> 
> I'm fairly sure that only keystone2 is affected by this, so it might
> even be possible to do this inside of keystone specific code,
> e.g. in keystone_pv_fixup(), which is called a little earlier.

Thanks, I agree initramfs was wrong place to put it.

I will find better place, however since idmap is generic (although with
only one known user platform), I'd like to keep it for arm in general.

Let me prepare next patch.

Thanks,

Matija

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] arm: Support initrd with address in boot alias region

[Arnd Bergmann]

On Thu, Aug 18, 2022 at 2:28 PM Matija Glavinic Pecotic
<matija.glavinic-pecotic.ext@nokia.com> wrote:
>
> On 18.08.2022 14:03, Arnd Bergmann wrote:
> > I think the logic looks correct, but I' would not put this into init/initramfs.c
> > when the problem is specific to Arm. The function is called from
> > arch/arm/mm/init.c, so you could just put it there.
> >
> > I'm fairly sure that only keystone2 is affected by this, so it might
> > even be possible to do this inside of keystone specific code,
> > e.g. in keystone_pv_fixup(), which is called a little earlier.
>
> Thanks, I agree initramfs was wrong place to put it.
>
> I will find better place, however since idmap is generic (although with
> only one known user platform), I'd like to keep it for arm in general.

Let's see if Russell has an opinion either way. I think at the time the idmap
code was added, the expectation was that additional platforms might
need it, but now it should be a fairly safe assumption that this won't
be the case. This was really only needed for keystone2 because it
supports a large amount of RAM in 2012, but this was also the peak
point of available RAM. Later chips either went with 64-bit cores, or
they have narrower DDR3 interfaces that only allow at most 2GB
of RAM.

> Let me prepare next patch.

Thanks,

        Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel