From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EF00BC7618F for ; Fri, 19 Jul 2019 16:48:19 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C5BA72186A for ; Fri, 19 Jul 2019 16:48:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C5BA72186A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:47110 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hoW3O-0001so-Q9 for qemu-devel@archiver.kernel.org; Fri, 19 Jul 2019 12:48:18 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:47055) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hoW3A-0001Tb-9B for qemu-devel@nongnu.org; Fri, 19 Jul 2019 12:48:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hoW2u-0006ZE-I2 for qemu-devel@nongnu.org; Fri, 19 Jul 2019 12:47:53 -0400 Received: from foss.arm.com ([217.140.110.172]:43914) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hoW2n-0006IT-1B; Fri, 19 Jul 2019 12:47:41 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B6096344; Fri, 19 Jul 2019 09:47:36 -0700 (PDT) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 061A23F59C; Fri, 19 Jul 2019 09:47:35 -0700 (PDT) Date: Fri, 19 Jul 2019 17:47:30 +0100 From: Mark Rutland To: Peter Maydell Message-ID: <20190719164729.GA22520@lakrids.cambridge.arm.com> References: <20190516144733.32399-1-peter.maydell@linaro.org> <20190516144733.32399-4-peter.maydell@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190516144733.32399-4-peter.maydell@linaro.org> User-Agent: Mutt/1.11.1+11 (2f07cb52) (2018-12-01) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 217.140.110.172 Subject: Re: [Qemu-devel] [PATCH v2 3/4] hw/arm/boot: Avoid placing the initrd on top of the kernel X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-arm@nongnu.org, Richard Henderson , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Hi Peter, I've just been testing on QEMU v4.1.0-rc1, and found a case where the DTB overlapped the end of the kernel, and I think there's a bug in this patch -- explanation below. On Thu, May 16, 2019 at 03:47:32PM +0100, Peter Maydell wrote: > We currently put the initrd at the smaller of: > * 128MB into RAM > * halfway into the RAM > (with the dtb following it). > > However for large kernels this might mean that the kernel > overlaps the initrd. For some kinds of kernel (self-decompressing > 32-bit kernels, and ELF images with a BSS section at the end) > we don't know the exact size, but even there we have a > minimum size. Put the initrd at least further into RAM than > that. For image formats that can give us an exact kernel size, this > will mean that we definitely avoid overlaying kernel and initrd. > > Signed-off-by: Peter Maydell > --- > hw/arm/boot.c | 34 ++++++++++++++++++++-------------- > 1 file changed, 20 insertions(+), 14 deletions(-) > > diff --git a/hw/arm/boot.c b/hw/arm/boot.c > index 935be3b92a5..e441393fdf5 100644 > --- a/hw/arm/boot.c > +++ b/hw/arm/boot.c > @@ -999,20 +999,6 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu, > if (info->nb_cpus == 0) > info->nb_cpus = 1; > > - /* > - * We want to put the initrd far enough into RAM that when the > - * kernel is uncompressed it will not clobber the initrd. However > - * on boards without much RAM we must ensure that we still leave > - * enough room for a decent sized initrd, and on boards with large > - * amounts of RAM we must avoid the initrd being so far up in RAM > - * that it is outside lowmem and inaccessible to the kernel. > - * So for boards with less than 256MB of RAM we put the initrd > - * halfway into RAM, and for boards with 256MB of RAM or more we put > - * the initrd at 128MB. > - */ > - info->initrd_start = info->loader_start + > - MIN(info->ram_size / 2, 128 * 1024 * 1024); > - > /* Assume that raw images are linux kernels, and ELF images are not. */ > kernel_size = arm_load_elf(info, &elf_entry, &elf_low_addr, > &elf_high_addr, elf_machine, as); > @@ -1064,6 +1050,26 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu, > } > > info->entry = entry; Note: this is the start of the kernel image... > + > + /* > + * We want to put the initrd far enough into RAM that when the > + * kernel is uncompressed it will not clobber the initrd. However > + * on boards without much RAM we must ensure that we still leave > + * enough room for a decent sized initrd, and on boards with large > + * amounts of RAM we must avoid the initrd being so far up in RAM > + * that it is outside lowmem and inaccessible to the kernel. > + * So for boards with less than 256MB of RAM we put the initrd > + * halfway into RAM, and for boards with 256MB of RAM or more we put > + * the initrd at 128MB. > + * We also refuse to put the initrd somewhere that will definitely > + * overlay the kernel we just loaded, though for kernel formats which > + * don't tell us their exact size (eg self-decompressing 32-bit kernels) > + * we might still make a bad choice here. > + */ > + info->initrd_start = info->loader_start + > + MAX(MIN(info->ram_size / 2, 128 * 1024 * 1024), kernel_size); ... but here we add kernel_size to the start of the loader, which is below the kernel. Should that be info->entry? I've seen this trigger a case where: * The kernel's image_size is 0x0a7a8000 * The kernel was loaded at 0x40080000 * The end of the kernel is 0x4A828000 * The DTB was loaded at 0x4a800000 ... and the kernel is unable to find a usable DTB. Thanks, Mark. > + info->initrd_start = TARGET_PAGE_ALIGN(info->initrd_start); > + > if (is_linux) { > uint32_t fixupcontext[FIXUP_MAX]; > > -- > 2.20.1 >