From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web11.6051.1685440387739394157 for ; Tue, 30 May 2023 02:53:08 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=qgzSAjbK; spf=pass (domain: posteo.de, ip: 185.67.36.65, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id EE021240027 for ; Tue, 30 May 2023 11:53:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1685440386; bh=tktm2QUN9rh77AB9kBggTTSvtQgeDidoc/YQqH4KFI8=; h=From:Message-Id:Mime-Version:Subject:Date:Cc:To:From; b=qgzSAjbK0CjnNOsWWgP2QkY5IqC2CREsazhMee93BCxiWtxzslVAMNlyeT936FW6X dTd/HEH5EgZwgGDhNj3LbWJVxzPg/B6QFRJDPQkfXGTMYtZJUIQVqdUohHji1I5oWp Rirq6GaN/xXdjMUt59ZMxWb84V4AbslVnOBlAe0q5bBMFW25hPDm7pykyfHYOQPz6v ZKwPlMwQADXL0lnPdSnt0UzhEFYMrTCkqMOxzATSO2lJs8279TXQSsuwQWctN8ZdVu YV3YMl/Vyw5W1lVaM1ozOULXwGbgscAWDBFFzpdH5Dyn3X4oRJg+bqT5k7NgV32adM 1bNLmA7dxxLyw== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4QVnl44VZPz9rxK; Tue, 30 May 2023 11:53:04 +0200 (CEST) From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-Id: <8AB1A1CF-9BEA-4F01-8615-A379BC827E64@posteo.de> Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.600.7\)) Subject: Re: [edk2-devel] [RFC PATCH 08/11] MdeModulePkg/DxeIpl: Relocate and remap XIP capable DXE drivers Date: Tue, 30 May 2023 09:52:54 +0000 In-Reply-To: Cc: edk2-devel-groups-io , Ray Ni To: Ard Biesheuvel References: <3425.1685437617401609070@groups.io> Content-Type: multipart/alternative; boundary="Apple-Mail=_EC71C819-F84C-4D63-A1AB-09FC9384E2C8" --Apple-Mail=_EC71C819-F84C-4D63-A1AB-09FC9384E2C8 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On 30. May 2023, at 11:48, Ard Biesheuvel wrote: >=20 > On Tue, 30 May 2023 at 11:47, Ard Biesheuvel > wrote: >>=20 >> On Tue, 30 May 2023 at 11:42, Marvin H=C3=A4user = wrote: >>>=20 >>>=20 >>>> On 30. May 2023, at 11:38, Ard Biesheuvel wrote: >>>>=20 >>>> On Tue, 30 May 2023 at 11:07, Marvin H=C3=A4user = wrote: >>>>>=20 >>>>> Hi Ard, >>>>>=20 >>>>> Native PE toolchains *generally* also generate XIP images (/ALIGN = =3D /FILEALIGN, but with 32 Byte rather than 64 Byte alignment compared = to GCC49+ / CLANGDWARF) [1]. However, because they are underaligned by = default (even for RT images that run in an OS context and MM drivers... = sigh...), platforms manually override SectionAlignment, but not = necessarily FileAlignment [2], breaking XIP. I don't think what you are = doing is perfectly safe for these, as they will have FileAlignment < = SectionAlignment (and by all chances, BaseTools is borked too). In my = opinion check for FileAlignment =3D=3D SectionAlignment. I can't vouch = for how likely FileAlignment has a sane value, the AUDK loader does not = read it at all and instead checks PointerToRawData =3D=3D = VirtualAddress, etc. >>>>>=20 >>>>> BaseTools generally has poor support for non-XIP vs XIP, probably = due to notorious underalignment since the very beginning. For PEIM XIPs = for example, which must ship pre-relocated at least for Intel, GenFv = just relocates the image in-memory and then copies the changes back to = the FFS file [3]. There is no concept of changing the image file size = within the procedure and as such, a non-XIP image cannot be converted to = XIP on demand. This would be useful for a distinction between pre-memory = and post-memory PEIMs, the former of which must be XIP (thus aligned), = while the latter can be loaded and relocated in-RAM (thus can be = underaligned w.r.t. FileAlignment), but alas. >>>>>=20 >>>>=20 >>>> If XIP for PE images with 4k section alignment is an issue, we = could >>>> always explore loading them into a separate allocation from PEI, = just >>>> like we do with DXE core itself. >>>>=20 >>>> This would actually simplify the loader code quite a lot, as we'd = be >>>> able to use the PEI core image loader directly. However, it means = we'd >>>> have to pass this information (array of tuples >>>> describing which images were already loaded by DxeIpl) via a HOB or >>>> some other method. >>>=20 >>> I took a *very brief* look at the entire series now. Is this just to = apply permissions before CpuDxe is loaded >>=20 >> Yes. >>=20 >=20 > Well, actually, the first part of the series gets rid of some > pointless memory copies, which is an improvement in itself. Sorry, I didn't mean the series, I meant the choice to handle things in = DxeIpl over DxeCore in particular. Is there even a non-FOSS producer of the CpuArch protocol? To my = understanding, all (common) Intel platforms use the one in UefiCpuPkg. = ARM and friends feel a lot less proprietary to begin with, but I could = be wrong. We were quite successful so far with just merging CpuArch into = DxeCore and setting it up quite early, but of course not at an = industry-wide scale. :)= --Apple-Mail=_EC71C819-F84C-4D63-A1AB-09FC9384E2C8 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On 30. = May 2023, at 11:48, Ard Biesheuvel <ardb@kernel.org> = wrote:

On Tue, 30 May 2023 at = 11:47, Ard Biesheuvel <ardb@kernel.org> wrote:

On Tue, 30 May 2023 at 11:42, Marvin H=C3=A4us= er <mhaeuser@posteo.de> wrote:


On 30. May 2023, at = 11:38, Ard Biesheuvel <ardb@kernel.org> wrote:

On Tue, 30 = May 2023 at 11:07, Marvin H=C3=A4user <mhaeuser@posteo.de> = wrote:

Hi Ard,

Native PE = toolchains *generally* also generate XIP images (/ALIGN =3D /FILEALIGN, = but with 32 Byte rather than 64 Byte alignment compared to GCC49+ / = CLANGDWARF) [1]. However, because they are underaligned by default (even = for RT images that run in an OS context and MM drivers... sigh...), = platforms manually override SectionAlignment, but not necessarily = FileAlignment [2], breaking XIP. I don't think what you are doing is = perfectly safe for these, as they will have FileAlignment < = SectionAlignment (and by all chances, BaseTools is borked too). In my = opinion check for FileAlignment =3D=3D SectionAlignment. I can't vouch = for how likely FileAlignment has a sane value, the AUDK loader does not = read it at all and instead checks PointerToRawData =3D=3D = VirtualAddress, etc.

BaseTools generally has poor support for = non-XIP vs XIP, probably due to notorious underalignment since the very = beginning. For PEIM XIPs for example, which must ship pre-relocated at = least for Intel, GenFv just relocates the image in-memory and then = copies the changes back to the FFS file [3]. There is no concept of = changing the image file size within the procedure and as such, a non-XIP = image cannot be converted to XIP on demand. This would be useful for a = distinction between pre-memory and post-memory PEIMs, the former of = which must be XIP (thus aligned), while the latter can be loaded and = relocated in-RAM (thus can be underaligned w.r.t. FileAlignment), but = alas.


If XIP for PE images with 4k section = alignment is an issue, we could
always explore loading them into a = separate allocation from PEI, just
like we do with DXE core = itself.

This would actually simplify the loader code quite a lot, = as we'd be
able to use the PEI core image loader directly. However, = it means we'd
have to pass this information (array of <guid, base = address> tuples
describing which images were already loaded by = DxeIpl) via a HOB or
some other method.

I took a = *very brief* look at the entire series now. Is this just to apply = permissions before CpuDxe is = loaded

Yes.


Well, actually, the first part of the = series gets rid of some
pointless memory copies, which is an improvement in = itself.

Sorry, I didn't mean the = series, I meant the choice to handle things in DxeIpl over DxeCore in = particular.

Is there even a non-FOSS producer = of the CpuArch protocol? To my understanding, all (common) Intel = platforms use the one in UefiCpuPkg. ARM and friends feel a lot less = proprietary to begin with, but I could be wrong. We were quite = successful so far with just merging CpuArch into DxeCore and setting it = up quite early, but of course not at an industry-wide scale. = :)
= --Apple-Mail=_EC71C819-F84C-4D63-A1AB-09FC9384E2C8--