Re: issue with MEMBLOCK_NOMAP

[Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Fri, 2016-01-29 at 15:06 +0100, Ard Biesheuvel wrote:
> On 29 January 2016 at 15:00, Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > Hi Ard,
> > 
> > I ran into an issue with your MEMBLOCK_NOMAP changes on a particular
> > firmware. The symptom is the kernel panics at boot time when it hits
> > an unmapped page while unpacking the initramfs. As it turns out, the
> > start of the initramfs shares a 64k kernel page with the UEFI memmap.
> > I can avoid the problem with:
> > 
> > @@ -203,7 +203,7 @@ void __init efi_init(void)
> > 
> >         reserve_regions();
> >         early_memunmap(memmap.map, params.mmap_size);
> > -       memblock_mark_nomap(params.mmap & PAGE_MASK,
> > -                           PAGE_ALIGN(params.mmap_size +
> > -                                      (params.mmap & ~PAGE_MASK)));
> > +       memblock_reserve(params.mmap & PAGE_MASK,
> > +                        PAGE_ALIGN(params.mmap_size +
> > +                                   (params.mmap & ~PAGE_MASK)));
> >  }
> > 
> > 
> > But it makes me worry about the same potential problem with
> > other reserved regions which we nomap. What do you think?
> > 
> 
> So I take it this initramfs allocation is not made by the stub but by
> GRUB? Since the stub rounds all allocations to 64 KB ...
> 
Yes. GRUB.

> In any case, regardless of the underlying cause, if any part of the
> initramfs turns out not to be covered by the linear mapping, we should
> invoke your code to move it. So I think it should be a matter of
> refining the logic in relocate_initrd() to do the right thing in this
> case

That thought had crossed my mind. I think it would be easy enough to
trigger the copy if first or last page of initrd is unmapped. Somewhat
related to this is that I want to rework this old patch to deal with
acpi tables outside mapped ram:

  https://lkml.org/lkml/2015/5/14/357

Basically, we should be able to just do:

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 15e0aad..4ea638c 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -32,7 +32,7 @@
 static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
 					    acpi_size size)
 {
-	if (!page_is_ram(phys >> PAGE_SHIFT))
+	if (!memblock_is_memory(phys))
 		return ioremap(phys, size);
 
 	return ioremap_cache(phys, size);

But this doesn't currently work wrt mem= which works by removing
the end range of memblocks. If I have mem= use the nomap flag
rather than removing the range, the above acpi_os_ioremap change
works, but other issues crop up due to memblock_end_of_DRAM()
returning end of all DRAM regardless of mem=. So we end up with
PFNs and struct pages for memory which will never be in linear
map. Fixing memblock_end_of_DRAM() to look at the flags and
return end of mapped DRAM gets things working but I wonder about
other potential trouble spots with this approach. Any thoughts?


> 
> Your suggested change will break 32-bit ARM, since we use
> ioremap_nocache() to map the UEFI memory map, and ARM does not allow
> that on ranges that are part of the linear mapping.

okay. I'll put together a patch to the initrd relocating code.

Thanks!