Re: [edk2-devel] [PATCH v2 20/25] OvmfPkg: Enable Choosing Memory Protection Profile via QemuCfg

["Taylor Beebe" <taylor.d.beebe@gmail.com>]

On 8/29/23 4:17 AM, Gerd Hoffmann wrote:
>    Hi,
>
>> -  SetDxeMemoryProtectionSettings (&DxeSettings, DxeMemoryProtectionSettingsPcd);
>> -  SetMmMemoryProtectionSettings (&MmSettings, MmMemoryProtectionSettingsPcd);
>> +  Status = QemuFwCfgParseString (DXE_MEMORY_PROTECTION_PROFILE_FWCFG_FILE, &StringSize, String);
>> +  if (!EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_INFO, "Setting DXE Memory Protection Profile: %a\n", String));
>> +    if (AsciiStriCmp (String, "debug") == 0) {
>> +      DxeSettings = DxeMemoryProtectionProfiles[DxeMemoryProtectionSettingsDebug].Settings;
> I'd suggest to just loop over DxeMemoryProtectionProfiles and compare
> String with .Name, so we don't have to touch this in case we add or
> remove profiles.

Sounds good -- will update in v3

>> +    DxeSettings = DxeMemoryProtectionProfiles[DxeMemoryProtectionSettingsDebug].Settings;
> I'd prefer to use DxeMemoryProtectionSettingsPcd by default.

The PCDs are still removed in this patch series. The PCD profile is 
included in the earlier patches of this series to ensure the memory 
protections are consistent as each patch transitions the references to 
use the library interface. I opted to to remove the PCDs for a couple of 
reasons:
1. The PCDs are the legacy interface, and keeping legacy interfaces 
around is sometimes necessary for compatibility but not in this case. 
Keeping the PCDs would disrupt maintainability, clarity, and 
extensibility of memory protections. I am also not confident the legacy 
interface would ever be removed in the future.

2. Removing the PCDs will cause a build failure for platforms which 
reference them. This outcome is desirable in this case because action 
needs to be taken to ensure the platform protection meets expectations 
with this new system. If the PCDs were kept, platform creators may try 
updating the PCDs and be confused when the changes are not reflected in 
the state of the system because the PCD profile is not in use. This 
nuance helps identify a confusing interface.



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