| Message ID | 6a622271-ee7a-8b00-a3db-36616a46cf73@nokia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] arm: Support initrd with address in boot alias region | expand |
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
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
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
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
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(+)