On 25 June 2015 at 12:23, Laszlo Ersek <ler...@redhat.com> wrote: > On 06/25/15 10:27, Ard Biesheuvel wrote: >> On 25 June 2015 at 10:08, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: >>> On 25 June 2015 at 09:58, Laszlo Ersek <ler...@redhat.com> wrote: >>>> On 06/25/15 01:31, Yao, Jiewen wrote: >>>>> Hi Ard >>>>> You are right that END_OF_DXE must be signal in Bds. >>>>> >>>>> My thought is that this event should be signal in "platform BDS" >>>>> instead of "generic BDS". >>>>> >>>>> The reason is that only platform know when to exit platform >>>>> manufacture auth state, and platform may have some special step >>>>> before that. >>>>> >>>>> Signing end of dxe in generic BDS may break existing platform. >>>> >>>> Ard, I think we have the justification we needed for going with >>>> platform-specific End-of-Dxe signalling. Please update your original >>>> patch with a link to Jiewen's message: >>>> >>>> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16088/focus=16109 >>>> >>>> And I'm going to ACK it like that for ArmVirtPkg. >>>> >>> >>> OK, fair enough. >>> >>>> ( >>>> Note, this is actually option #2 from my email >>>> <http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16088/focus=16095>: >>>> >>>>> - Or, OVMF will have to signal End-of-Dxe within the >>>>> SaveS3BootScript() function (see again the patch), right before >>>>> installing "gEfiDxeSmmReadyToLockProtocolGuid". Only that would >>>>> place the new step (2) between existent steps (1) and (3). >>>>> >>>>> In parallel, ArmVirtPkg, which doesn't implement S3 or SMM, could >>>>> signal End-of-Dxe wherever it wanted to. Meaning, your patch is >>>>> correct (assuming it doesn't break anything in practice), *if* we >>>>> prefer this option. >>>> ) >>>> >>>> Jiewen, >>>> >>>>> Would you mind to let me know which platform forget to signal this, >>>> >>>> That's every single platform in the open source edk2 tree that uses >>>> Intel BDS. >>>> >>>>> and can we fix it in platform BDS? >>>> >>>> Yes, we can do that; Ard already posted a patch with such an approach >>>> for ArmVirtPkg, and I intend to do the same for OvmfPkg. Our main doubt >>>> was whether it was *right* (by design) to do this separately for each >>>> platform (in the respective PlatformBdsLib instance), as opposed to >>>> doing it centrally in the BDS driver. >>>> >>>> You just lifted that doubt, so we know the way forward. >>>> >>> >>> Indeed. FYI, I noticed that S3Save () does the right thing if it is >>> invoked more than once. >>> That means you may be able to get away with calling it both in >>> PlatformBdsInit() (before end-of-DXE) *and* keep the original call >>> site in GenericBdsLib intact, and still end up with something that >>> works. It does rely on an implementation detail of AcpiS3Save though >> >> ... which you are probably doing already, I guess ... :-) > > Not sure if I understand right, but if you mean that OVMF's platform BDS > already calls S3Save directly -- that's not the case. We currently rely > on the call inside the BDS driver. >
OK, guess I was too confused to pick up on that :-) Anyway, I guess we should add such a call in the correct place before removing it from BdsBoot.c then -- Ard. ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
