On Tue, Dec 02, 2014 at 10:14:07AM +0000, Richard W.M. Jones wrote: > On Tue, Dec 02, 2014 at 11:09:03AM +0100, Andrew Jones wrote: > > On Mon, Dec 01, 2014 at 03:49:43PM +0000, Richard W.M. Jones wrote: > > > According to Documentation/arm64/booting.txt the dtb can occupy the > > > same page as the initrd. In fact qemu aligns it to 4K. However on a > > > kernel with 64K pages, you can end up freeing part of the device tree > > > when the initrd RAM is freed, causing bugs like this one: > > > > > > https://bugs.launchpad.net/qemu/+bug/1383857 > > > > > > Don't free partial first/last page when freeing initrd. > > > > > > Signed-off-by: Richard W.M. Jones <rjones@xxxxxxxxxx> > > > --- > > > arch/arm64/mm/init.c | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > > > index 494297c..77bdc39 100644 > > > --- a/arch/arm64/mm/init.c > > > +++ b/arch/arm64/mm/init.c > > > @@ -335,11 +335,12 @@ void free_initrd_mem(unsigned long start, unsigned long end) > > > { > > > if (!keep_initrd) { > > > if (start == initrd_start) > > > - start = round_down(start, PAGE_SIZE); > > > + start = round_up(start, PAGE_SIZE); > > > if (end == initrd_end) > > > - end = round_up(end, PAGE_SIZE); > > > + end = round_down(end, PAGE_SIZE); > > > > > > - free_reserved_area((void *)start, (void *)end, 0, "initrd"); > > > + if (start < end) > > > + free_reserved_area((void *)start, (void *)end, 0, "initrd"); > > > } > > > } > > > > What about the equivalent fix for arch/arm/mm/init.c? > > My assumption was that because pages are always 4K on armv7, and > because qemu has always aligned the dtb to 4K, that this couldn't > affect armv7. (It's still an unstated assumption in the boot spec > which just says the dtb must be aligned to 64 bits.) I'm pretty sure > no one has seen this bug on armv7 -- it would be very obvious by now > if it was happening there. I believe we're now fixing two things. The behavior of the kernel wrt to the documentation and the alignment qemu uses when laying out the initrd and dtb. The kernel documentation doesn't say the initrd must be page aligned and sized to a page multiple, and it even recommends laying out the initrd with "A safe location is just above the device tree blob" - afaik there's no guarantee that the dtb will end on a page boundary. So the kernel shouldn't be assuming it can free the initrd's boundary pages. That's what you're fixing here, and I believe we should fix armv7 as well for completeness. Then, as Peter stated, qemu's layout needs to be fixed anyway, in order to deal with kernels that don't have your patch. Also, even with kernel's that do have your patch, we see that less memory would be wasted if the initrd was presented on a page boundary using a page-multiple size. So qemu might as well do that. If we're not happy with just always using 64k, then the problem of determining the guest's page size remains. > > > Should probably > > post to linux-arm-kernel@xxxxxxxxxxxxxxxxxxx and linux-kernel@xxxxxxxxxxxxxxx > > Thanks. > > I'm quite happy for someone to come up with a better way to fix this .. > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > virt-p2v converts physical machines to virtual machines. Boot with a > live CD or over the network (PXE) and turn machines into KVM guests. > http://libguestfs.org/virt-v2v _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
