On 07/28/15 09:56, Fan, Jeff wrote: > Yes. I think BootServiceData is ok.
Okay. At the moment I think I agree with the idea that BootServicesData should be sufficient. If that's what you'd like to see, I'll evaluate it in more depth, and try to update the code accordingly, for v2. For some more background: *other* data saved by CpuS3DataDxe into AcpiNVS, and then copied into SMRAM by PiSmmCpuDxeSmm, *must* actually reside in AcpiNVS originally. The point is not data integrity (the runtime OS could easily tamper with that data between normal boot and S3 suspend) -- the point is to inform a well-behaved runtime OS to stay away from that memory range. Namely, at S3 resume, PiSmmCpuDxeSmm restores *other* sensitive data from SMRAM to the original AcpiNVS area *first*, and uses (for whatever purposes) the data from AcpiNVS *second*. So whatever the OS wrote there will be overwritten -- which is good for the integrity of the firmware, but a well-behaved OS should not be corrupted by this (ie. the temporary restoral of data during S3 resume should not overwrite data owned by the OS). Therefore those *other* areas are allocated as AcpiNVS during normal boot, and then the OS knows from the UEFI memmap to stay away. As discussed, this temporary restoral to AcpiNVS at S3 resume does *not* apply to the MTRR settings; those are programmed into the processors directly from SMRAM. Which is why the BootServicesData idea makes sense. (In the Quark distribution the AcpiNVS allocation for this specific data could be an oversight.) Thanks! Laszlo > If OVMF is built without SMRAM support, it's ovmf platform's > responsibility to get it and save it into ACPI NVS. > > Jeff > -----Original Message----- > From: Paolo Bonzini [mailto:[email protected]] On Behalf Of Paolo > Bonzini > Sent: Tuesday, July 28, 2015 3:34 PM > To: Fan, Jeff; Laszlo Ersek; [email protected] > Cc: Chen Fan; Justen, Jordan L > Subject: Re: [edk2] [PATCH 38/58] UefiCpuPkg: CpuDxe: optionally save MTRR > settings to AcpiNVS memory block > > > > On 28/07/2015 09:09, Fan, Jeff wrote: >> I did not receive the patch 42. I have only gotten 38,39,40,41. >> >> OK, If this mtrr setting stored in ACPI NVS is for CpuS3DataDxe to >> store into SMRAM, that's fine. >> >> Then, another question, what's requirement to save MTRR setting into >> ACPI NVS on this case? And need one PCD to switch on/off it? > > Do you mean it could be BootServicesData? I'll let Laszlo answer this. > > The PCD is there to skip this code if OVMF is built without SMRAM support. > > Paolo > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
