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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
