[edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

[Brijesh Singh via groups.io]

On 7/27/21 6:51 AM, Xu, Min M wrote:
> On July 27, 2021 6:57 PM, Brijesh Singh wrote:
>> Hi Min,
>>
>> This refactoring is already done by the SNP patch series.
>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F77336%3Fp%3D%2C%2C%2C20%2C0%2C0%2C0%3A%3ACreated%2C%2Cpost&data=04%7C01%7Cbrijesh.singh%40amd.com%7C22b61f2ff5bb48348b0608d950f4d7c5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637629834792320372%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=tMGpR4a2uZTTR%2FsciTN0oeca2mZ32GfX3K78lA5BWas%3D&reserved=0
>> erid%3A5969970,20,2,20,83891510
>>
>> It appears that you are also pulling in some of TDX logic inside the
>> AMDSev.asm such as
>>
>> ;
>> +PostJump64BitAndLandHereSev:
>> +
>> +    ;
>> +    ; If it is Tdx guest, jump to exit point directly.
>> +    ; This is because following code may access the memory region which has
>> +    ; not been accepted. It is not allowed in Tdx guests.
>> +    ;
>> +    mov     eax, dword[TDX_WORK_AREA]
>> +    cmp     eax, 0x47584454             ; 'TDXG'
>> +    jz      GoodCompare
>>
>> Why we are referring the TDX workarea inside the AmdSev.asm ?
> See my explanation in the above comments. In Tdx guests memory region cannot
> be accessed unless it is accepted by guest or initialized by the host VMM. In 
> PostJump64BitAndLandHereSev there is access to dword[SEV_ES_WORK_AREA_RDRAND]
> which is not initialized by host VMM. If this code will not be executed in 
> Tdx guest, then the above check is not needed. I need your help to confirm it.
>
> There are similar Tdx check in my patch of AmdSev.asm. For example in CheckSevFeatures
> byte[SEV_ES_WORK_AREA] is used to record the SEV-ES flag. This memory region is
> not initialized by host VMM either. So in Tdx it will trigger error.
>
> Another solution is that the memory region used by SEV in ResetVector are added
> Into Tdx metadata so that host VMM will initialize those memory region when 
> It creates the Td guest. What's your opinion?

I am not full versed on TDX yet and sorry I am not able to follow you
question completely to provide any advice. With SEV and SEV-ES, a guest
can access the memory without going through the validation process, but
with the SEV-SNP, the page need to be validated (aka accepted) before
the access. In SNP series, we ensure that the data pages used in the
reset vector are pre-validated during the VM creation time -- this
allows us to access the pages without going through accept process. If I
follow you correctly on your metadata comment then it is similar to
saying is pre-validate these range of pages used in the reset vector
code (that include GHCB page, Page table pages etc), right ? 

For SEV-SNP, see this patch

https://edk2.groups.io/g/devel/message/77342?p=,,,20,0,0,0::Created,,posterid%3A5969970,20,2,20,83891520

A VMM (qemu) looks for the range of page it need to prevalidate before
the boot, the range is provided through the GUID (SevSnpBootBlock).

>> I will take out my refactoring patch outside of the SNP series and submit it so
>> that you can build on top of. This will simplify review process.
>>
> Thank you very much for the refactoring.  I will refine my patch based on it. 
>> thanks
>>
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78222): https://edk2.groups.io/g/devel/message/78222
Mute This Topic: https://groups.io/mt/84476064/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-