Re: [edk2-devel] [PATCH v4 20/28] MdeModulePkg: Add Additional Profiles to SetMemoryProtectionsLib

["Laszlo Ersek" <lersek@redhat.com>]

On 10/4/23 18:31, Taylor Beebe wrote:
> 
> On 10/4/23 1:46 AM, Gerd Hoffmann wrote:
>> On Fri, Sep 29, 2023 at 12:52:35PM -0700, Taylor Beebe wrote:
>>> I can also update ArmVirtPkg to disable execution protection
>>> for EfiLoaderData by default until fw_cfg parsing
>>> support is added to ArmVirtPkg. Let me know if you think
>>> this is necessary.
>> With MemoryProtectionConfigLib adding fw_cfg parsing support to
>> ArmVirtPkg should be easy, so maybe just do that?
> 
> From what I see, the QemuFwCfgLib instance compatible with Arm requires
> UefiBootServicesTableLib so fw_cfg cannot be parsed early enough to set
> the memory protection policy on ArmVirt.

QemuFwCfgLibMmio is DXE_DRIVER and UEFI_DRIVER only; it depends on (a)
the FDT client protocol, (b) MMIO accesses (that is, page tables).

On x86, PEI can use IO ports (no page tables), but that's not available
on ARM. I don't recall off-hand where / when exactly page tables are set
up during ArmVirtQemu boot.

You'd probably have to do something similar to
"ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c" and
"ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c"
-- parse the FDT on every fw_cfg access for the MMIO registers, then use
those registers to fetch the profile name, and do all that early enough
to configure the page protections -- so possibly *before*, or as a part
of, creating the page tables in the first place.

> An Arm compatible PEIM instance of QemuFwCfgLib will need to be created.
> I'm happy to look into it, but I don't want to hang up this patch series on
> that addition. Instead, I'll set the protection policy for ArmVirtPkg to
> the equivalent of the new GrubCompat profile in this series.

Can you base the default policy (i.e., the one that takes effect in the
absence of fw_cfg) on a PCD?

Such a PCD could be fixed-at-build, but I think it might even be
DynamicHII (compare commit 7e5f1b673870,
"ArmVirtPkg/PlatformHasAcpiDtDxe: allow guest level ACPI disable
override", 2017-03-31). That would have the benefit that people could
set a default at build time, with the --pcd build option. With
DynamicHII, the default could be stored in a non-volatile UEFI variable,
and ArmVirtQemu does have PEI-time (read-only) variable access.

Well, one argument against DynamicHII is that you'd want to prevent
later phases of the firmware from overwriting that variable, so you'd
have to get into variable policy / locking whatnot. So I'd suggest
picking the default profile from a fixed-at-build PCD (impossible to
overwrite "from the inside", and still enables the build-time --pcd
switch, for setting the platform default), *plus* fw-cfg for dynamic
configuration.

(Sorry I don't know anything about your series; it's possible you
already set the platform default via PCDs.)

I think platform builders should have the choice of picking a default
profile at build time; neither the Release (?) nor the GrubCompat
profile will fit everyone. I agree the *DEC* default should be the
strictest, but --pcd should be able to override that at build time, even
if -fw_cfg will allow for totally dynamic configuration too.

Laszlo

> Please let me know if I'm missing an obvious route to PEI fw_cfg
> parsing on Arm.
> 
> 
> -Taylor
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109344): https://edk2.groups.io/g/devel/message/109344
Mute This Topic: https://groups.io/mt/101469960/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-