Re: [edk2-devel] SecCore evacuation in PeiCore?

["Marvin Häuser" <mhaeuser@posteo.de>]

On 16/08/2021 18:18, Michael Kubacki wrote:
> Hi Marvin,
>
> Your understanding of SecMigrationPei is correct. It is not ideal as 
> it's an unfamiliar pattern that could give the false impression that 
> it is a universal SEC migration solution, which it is not. But if 
> platforms understand that any additional data published in SecCore 
> must be explicitly migrated (potentially via library extension to 
> SecMigrationPei), it can be used to serve the SEC post-memory 
> migration role.
>
> I assumed it was related to the reset vector due to the 16-bit 
> alignment. I think it would be great to have SecCore aligned properly 
> if possible.

I could probably write a patch, but OVMF does not use this SecCore (and 
still something is misaligned :) ), and I don't have any other platform. 
Maybe I can ask Bret to test it as part of some PE loader validation in 
the future. :)

Would the old solution, which is being removed, be universal? Would it 
be beneficial? I know that the ARM world does not use this SecCore 
either, but I generally don't have a good idea about how their stuff works.

Thanks!

Best regards,
Marvin

>
> Thanks,
> Michael
>
> On 8/14/2021 8:29 AM, Marvin Häuser wrote:
>> Hey Michael,
>>
>> Thank you for your response! It was actually quicker than I imagined. :)
>>
>> I think I understand, but please let me try to get this absolutely 
>> right. Can I think of "SecMigrationPei" as a sort of 
>> "SecCorePostMem", which either is loaded into permanent RAM directly 
>> or is shadowed because it is a PEIM unlike SecCore - and it 
>> republishes all public data, most especially PPIs, such that the 
>> entire PEI stage no longer has any references to the original SecCore 
>> at all, and the SecCore module basically just sits there in the ROM, 
>> and its exposed data is either discarded or orphaned? Is that about 
>> right?
>>
>> I think I hit the alignment issue of SecCore too, but only for X64 
>> builds (likely just because the size happens to be lucky for IA32) of 
>> OVMF. Pretty much sure it's just ResetVector positioning. What would 
>> be the issue with moving the ResetVector into a separate component, 
>> with its fixed position in FD (this is actually how UefiCpuPkg/VTF0 
>> works), and having SecCore aligned correctly? Not specifically to 
>> restore MigrateSecModulesInFv(), but as future-proofing to ensure 
>> expected outputs. In fact, I noticed because my new PE loader code 
>> was upset about the unaligned XIP load address.
>>
>> Also thanks for your patch!
>>
>> Best regards,
>> Marvin
>>
>> On 13/08/2021 18:51, Michael Kubacki wrote:
>>> Hi Marvin,
>>>
>>> I apologize for the delayed response, I missed this message earlier. 
>>> The function was called from EvacuateTempRam() in the initial set of 
>>> patches:
>>> [PATCH 1/6] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore 
>>> (CVE-2019-11098) (groups.io) 
>>> <https://edk2.groups.io/g/devel/message/61823>
>>>
>>> I was not involved in the patch series on the mailing list (job role 
>>> change at the time) but as a comment in that patch notes, there was 
>>> an inconsistency observed in PE32 section alignment in SEC modules. 
>>> I don't see where this was resolved other than the calls being 
>>> removed later in the series. SecCore migration would not occur 
>>> implicitly in the PeiCore flow but there is functionality for SEC 
>>> data migration in UefiCpuPkg/SecMigrationPei.
>>>
>>> Based on what I see now, I'd be happy to send a patch to remove 
>>> MigrateSecModulesInFv().
>>>
>>> Thanks,
>>> Michael
>>>
>>> On 8/7/2021 2:54 PM, Marvin Häuser wrote:
>>>> Good day everyone,
>>>> Good day Michael,
>>>>
>>>> The commit that introduced T-RAM evacuation [1] also introduced the 
>>>> function "MigrateSecModulesInFv()". It also is explicitly mentioned 
>>>> as part of the control flow in the commit message. As far as I can 
>>>> see, since then till today this function has never been called 
>>>> anywhere. Was this some draft function that accidentally made it 
>>>> into the patch, or did the caller get lost somewhere? The 
>>>> description makes sense to me and I'm not experienced enough with 
>>>> the PeiCore control flow to tell whether the PEIM migration somehow 
>>>> covers SecCore implicitly. Also I noticed it only supports SecCore 
>>>> in a PE/COFF section, not a TE section. Is there a rationale for that?
>>>>
>>>> Thank you for your time!
>>>>
>>>> Best regards,
>>>> Marvin
>>>>
>>>>
>>>> [1] 
>>>> https://github.com/tianocore/edk2/commit/9bedaec05b7b8ba9aee248361bb61a85a26726cb
>>>>
>>>>
>>>> 
>>>>
>>>
>