From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=VbjpVB/b; spf=pass (domain: linaro.org, ip: 209.85.128.66, mailfrom: ard.biesheuvel@linaro.org) Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by groups.io with SMTP; Thu, 12 Sep 2019 10:13:42 -0700 Received: by mail-wm1-f66.google.com with SMTP id o184so907104wme.3 for ; Thu, 12 Sep 2019 10:13:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=ys0C5tDaARcNACap0WI1zntWDsf+q6LvIjhwlERed7g=; b=VbjpVB/bxjF2gGkGnDS7yivcywzx/6Ag0kVgSO4g3A2/B82fUT2k/V8gfotzj8wETJ BV2F+XtMvBCCtEU/LLejGlX8wNORIG3t5DtMF6qayAMWmX/sxWym3b6QDl3OGW1SvENd F5x/r5ePMGm7225HRwoJL4dP5vAbnbIMe2zkOLG9WYKDs1vtWD3m5gz70IJWFlS+A6pp NEDODd7kgA4I3ZBWPikkewlaXCk/JjNvQOjwCrGaytqQXrKMGssDuFcEZ/TcQi6bCIiZ C4MyCQsS5alKkBf/gJlQx0Q43ZBQB/aJcK8G+en2SN3hK6URwyVGwOHeRtXWZy8TMY5p zDww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=ys0C5tDaARcNACap0WI1zntWDsf+q6LvIjhwlERed7g=; b=qr9Aa/fVaWV9GiCAQacubCyl8EW0zsG6LfitnshDmdsj8fROkKVKEdOgctT7j7Hfih +837MDQogxDlzQJEbhHC4179Luy2BOlxLut04aK+bgDNBYaTYzj2r6jGp58rjy0nZ3YZ Gx1YMhD1AGruJju/JWMsWllPw4tE58KFQf6MS6Vw93ajoYoppzVacyznkzgHkvIfeA/c AXeTA4wMM4y4pa+VnUz7s1gG8f13DKD6lSykp8xk4TyR7mf0VOw7RCZVv09uWoyACPVv bR/qfGkTPFGyiKI/xkf1FSrOwd4Mg6OYxfq66v9wD4PkYOKyD0jRebhrhO3oqO0TJNXa ckjQ== X-Gm-Message-State: APjAAAWWQXzMxchMy0v6g1gHOOaCgIArhNgZUG/lKw0OUYB7nZ1FnkVA wfP7gXuQdnNvSbvtDnZKhZt+plNJJsPjFqzHafJumw== X-Google-Smtp-Source: APXvYqyXcdmBU2yGcNVC/b+A27Y1EhLXrrVG9VNaDnHHERmexotqrfdDH4x+EYC2xz88K/7lYavrC5AEu7egQrAt1cM= X-Received: by 2002:a7b:cf37:: with SMTP id m23mr879551wmg.53.1568308420126; Thu, 12 Sep 2019 10:13:40 -0700 (PDT) MIME-Version: 1.0 References: <20190911162354.22107-1-ard.biesheuvel@linaro.org> <5141ec32-a51d-3ec6-79ef-ceec43b32c84@redhat.com> In-Reply-To: <5141ec32-a51d-3ec6-79ef-ceec43b32c84@redhat.com> From: "Ard Biesheuvel" Date: Thu, 12 Sep 2019 18:13:18 +0100 Message-ID: Subject: Re: [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds To: =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= Cc: edk2-devel-groups-io , Leif Lindholm , Laszlo Ersek Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 12 Sep 2019 at 18:05, Philippe Mathieu-Daud=C3=A9 wrote: > > On 9/12/19 6:50 PM, Ard Biesheuvel wrote: > > On Thu, 12 Sep 2019 at 16:20, Philippe Mathieu-Daud=C3=A9 wrote: > >> > >> Hi Ard, > >> > >> On 9/11/19 6:23 PM, Ard Biesheuvel wrote: > >>> After upgrading the CI system we use for building the ArmVirtPkg > >>> targets, we started seeing failures due to the NOOPT build running > >>> out of space when using the CLANG38 toolchain definition combined > >>> with clang 7. > >>> > >>> We really don't want to increase the FD/FV sizes in general to > >>> accommodate this, so parameterize the relevant quantities and > >>> increase them by 50% for NOOPT builds. > >>> > >>> Signed-off-by: Ard Biesheuvel > >>> --- > >>> v2: implement suggestions by Laszlo on 1) how to parameterize this fu= rther, > >>> and b) to avoid adding another .inc file > >>> update kernel header field, as pointed out by Philippe > >>> > >>> ArmVirtPkg/ArmVirt.dsc.inc | 28 ++++++++++++++++++++ > >>> ArmVirtPkg/ArmVirtQemu.fdf | 14 +++++++--- > >>> ArmVirtPkg/ArmVirtQemuKernel.fdf | 19 ++++++++++--- > >>> ArmVirtPkg/ArmVirtXen.fdf | 14 +++++++--- > >>> 4 files changed, 66 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > >>> index a4ae25d982a2..d6b58e5c018b 100644 > >>> --- a/ArmVirtPkg/ArmVirt.dsc.inc > >>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc > >>> @@ -10,6 +10,34 @@ > >>> [Defines] > >>> DEFINE DEBUG_PRINT_ERROR_LEVEL =3D 0x8000004F > >>> > >>> + # > >>> + # Flash size selection. Setting FD_SIZE_IN_KB on the command line = directly to > >>> + # one of the supported values, in place of any of the convenience = macros, is > >>> + # permitted. > >>> + # > >>> +!if $(TARGET) =3D=3D NOOPT > >>> + DEFINE FD_SIZE_3MB =3D TRUE > >>> +!endif > >>> + > >>> +!ifdef $(FD_SIZE_2MB) > >>> + DEFINE FD_SIZE_IN_KB =3D 2048 > >>> +!else > >>> +!ifdef $(FD_SIZE_3MB) > >>> + DEFINE FD_SIZE_IN_KB =3D 3072 > >>> +!else > >>> + DEFINE FD_SIZE_IN_KB =3D 2048 > >>> +!endif > >>> +!endif > >>> + > >>> +!if $(FD_SIZE_IN_KB) =3D=3D 2048 > >>> + DEFINE FD_SIZE =3D 0x200000 > >>> + DEFINE FD_NUM_BLOCKS =3D 0x200 > >>> +!endif > >>> +!if $(FD_SIZE_IN_KB) =3D=3D 3072 > >>> + DEFINE FD_SIZE =3D 0x300000 > >>> + DEFINE FD_NUM_BLOCKS =3D 0x300 > >>> +!endif > >>> + > >>> [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DR= IVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_A= PPLICATION] > >>> GCC:*_*_*_DLINK_FLAGS =3D -z common-page-size=3D0x1000 > >>> > >>> diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf > >>> index c2169cb7964b..d3950c8be05e 100644 > >>> --- a/ArmVirtPkg/ArmVirtQemu.fdf > >>> +++ b/ArmVirtPkg/ArmVirtQemu.fdf > >>> @@ -20,14 +20,22 @@ > >>> # > >>> ####################################################################= ############ > >>> > >>> +[Defines] > >>> +!if $(FD_SIZE_IN_KB) =3D=3D 2048 > >>> + DEFINE FVMAIN_COMPACT_SIZE =3D 0x1ff000 > >>> +!endif > >>> +!if $(FD_SIZE_IN_KB) =3D=3D 3072 > >>> + DEFINE FVMAIN_COMPACT_SIZE =3D 0x2ff000 > >>> +!endif > >>> + > >>> [FD.QEMU_EFI] > >>> BaseAddress =3D 0x00000000|gArmTokenSpaceGuid.PcdFdBaseAddress # = QEMU assigns 0 - 0x8000000 for a BootROM > >>> -Size =3D 0x00200000|gArmTokenSpaceGuid.PcdFdSize # = The size in bytes of the FLASH Device > >>> +Size =3D $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # = The size in bytes of the FLASH Device > >>> ErasePolarity =3D 1 > >>> > >>> # This one is tricky, it must be: BlockSize * NumBlocks =3D Size > >>> BlockSize =3D 0x00001000 > >>> -NumBlocks =3D 0x200 > >>> +NumBlocks =3D $(FD_NUM_BLOCKS) > >>> > >>> ####################################################################= ############ > >>> # > >>> @@ -59,7 +67,7 @@ DATA =3D { > >>> !endif > >>> } > >>> > >>> -0x00001000|0x001ff000 > >>> +0x00001000|$(FVMAIN_COMPACT_SIZE) > >>> gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize > >>> FV =3D FVMAIN_COMPACT > >>> > >>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf b/ArmVirtPkg/ArmVirtQem= uKernel.fdf > >>> index f675b6d65ee1..46ec967e1cc0 100644 > >>> --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf > >>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf > >>> @@ -20,14 +20,22 @@ > >>> # > >>> ####################################################################= ############ > >>> > >>> +[Defines] > >>> +!if $(FD_SIZE_IN_KB) =3D=3D 2048 > >>> + DEFINE FVMAIN_COMPACT_SIZE =3D 0x1f8000 > >>> +!endif > >>> +!if $(FD_SIZE_IN_KB) =3D=3D 3072 > >>> + DEFINE FVMAIN_COMPACT_SIZE =3D 0x2f8000 > >>> +!endif > >>> + > >>> [FD.QEMU_EFI] > >>> BaseAddress =3D 0x00000000|gArmTokenSpaceGuid.PcdFdBaseAddress # = QEMU assigns 0 - 0x8000000 for a BootROM > >>> -Size =3D 0x00200000|gArmTokenSpaceGuid.PcdFdSize # = The size in bytes of the FLASH Device > >>> +Size =3D $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # = The size in bytes of the FLASH Device > >>> ErasePolarity =3D 1 > >>> > >>> # This one is tricky, it must be: BlockSize * NumBlocks =3D Size > >>> BlockSize =3D 0x00001000 > >>> -NumBlocks =3D 0x200 > >>> +NumBlocks =3D $(FD_NUM_BLOCKS) > >>> > >>> ####################################################################= ############ > >>> # > >>> @@ -56,7 +64,12 @@ DATA =3D { > >>> 0x01, 0x00, 0x00, 0x10, # code0: adr x1, . > >>> 0xff, 0x1f, 0x00, 0x14, # code1: b 0x8000 > >>> 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, # text_offset: 512= KB > >>> +!if $(FD_SIZE_IN_KB) =3D=3D 2048 > >>> 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, # image_size: 2 MB > >>> +!endif > >>> +!if $(FD_SIZE_IN_KB) =3D=3D 3072 > >>> + 0x00, 0x00, 0x30, 0x00, 0x00, 0x00, 0x00, 0x00, # image_size: 3 MB > >>> +!endif > >> > >> Ah, I was thinking of some inplace endian swapping > >> > > > > If you can make it work, patches welcome :-) > > I was thinking of this: > > 0xf6, 0x1f, 0x00, 0xea, # b 0x8000 > > 0x18, 0x28, 0x6f, 0x01, # magic > > 0x00, 0x00, 0x00, 0x00, # start > > ((($(FD_SIZE)) & 0x000000ff) << 24), # image size > > ((($(FD_SIZE)) & 0x0000ff00) << 8), > > ((($(FD_SIZE)) & 0x00ff0000) >> 8), > > ((($(FD_SIZE)) & 0xff000000) >> 24), > > 0x01, 0x02, 0x03, 0x04 # endiannness flag > > > But now that I look at it, I realize it is uglier to review than your > if/endif... > Does the .fdf parser even support that kind of arithmetic? :-)