On Tue, Dec 02, 2014 at 11:44:39AM +0100, Andrew Jones wrote: > 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. > I don't really think it makes sense to try to guess the guest's page size. You could potentially have non-Linux guests pretending to be Linux for the boot procedure, for example. -Christoffer _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
