On 07/28/15 08:51, Paolo Bonzini wrote:
>
>
> On 28/07/2015 08:05, Fan, Jeff wrote:
>> Ersek,
>>
>> I have one comment for PCD PcdCpuSyncMtrrToAcpiNvs.
>>
>> I knew OvmfPkg implemented LockBox based on ACPI NVS. Saving MTRR setting in
>> AcpiNVS is OK for OvmfPkg.
>
> If I understand correctly what you are saying, the AcpiNVS block is only
> used for communication from CpuDxe to CpuS3DataDxe in patch 42.
> CpuS3DataDxe saves the MTRR in SMRAM during SmmReadyToLockEventNotify()
> and PiSmmCpuDxeSmm restores them during S3 resume. So Laszlo's patches
> are doing exactly the "safe" thing, even though they are not using LockBox.
Yes, with minimal tweaks, this is correct.
The minimal tweak is the following: while CpuS3DataDxe collects most
*other* data for PiSmmCpuDxeSmm indeed as a "deep copy", right when it
is notified about the installation of EFI_SMM_CONFIGURATION_PROTOCOL,
the saving of MTRR configuration is delayed, and for that CpuS3DataDxe
only saves the address of the AcpiNVS block -- that CpuDxe maintains --
for PiSmmCpuDxeSmm.
This way other firmware code can continue massaging the MTRR settings.
Normally this happens via
EFI_DXE_SERVICES.SetMemorySpaceAttributes
EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes()
call chains, ie. platform-independent firmware code decides "this memory
range should be uncached", calls gDS->SetMemorySpaceAttributes(), which
is ultimately implemented by the CpuDxe driver, in the form of massaging
MTRR.
Now, whenever this happens, the MTRR settings block in AcpiNVS will be
updated by CpuDxe. When SMM-ready-to-lock is finally installed, then
PiSmmCpuDxeSmm will copy the then-fresh MTRR settings to SMRAM.
This is the reason for the extra indirection here -- in one sentence,
most of the data can be saved by CpuS3DataDxe immediately when
EFI_SMM_CONFIGURATION_PROTOCOL is installed, but the MTRR settings can
validly change after that, so their saving is delayed until
SMM-ready-to-lock.
>
>> But other platform may want to use more safe solution to save MTRR based on
>> in SMM.
>>
>> I think that, for long term, saving MTRR setting by LockBox instead
>> of using ACPI NVS memory directly. Then, different platforms could
>> provide the different LockBox solutions. For short term, still saving
>> MTRR setting in ACPI NVS in CpuDxe, and removing this PCD. That means
>> we could CpuDxe implementation to use the long term solution in the
>> future and needn't to remove one un-used PCD more.
>
> The PCD is consumed in CPUS3DataDxe.
Correct. The dynamic PCD carries the address of the AcpiNVS block
(containing the ever-fresh MTRR settings) from CpuDxe to CpuS3DataDxe,
which CpuS3DataDxe then just passes on to PiSmmCpuDxeSmm, in the
ACPI_CPU_DATA.MtrrTable field.
Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
