On July 29, 2021 6:08 PM, Brijesh Singh wrote: > On 7/29/21 1:07 AM, Xu, Min M wrote: > > On July 29, 2021 12:29 PM, Brijesh Singh wrote: > >> On 7/28/21 9:44 PM, Xu, Min M wrote: > >>> Jiewen & Singh > >>> > >>> From the discussion I am thinking we have below rules to follow to > >>> the design the structure of TEE_WORK_AREA: > >>> 1. Design should be flexible but not too complicated 2. Reuse the > >>> current SEV_ES_WORK_AREA (PcdSevEsWorkAreaBase) as > TEE_WORK_AREA 3. > >>> TEE_WORK_AREA should be initialized to all-0 at the beginning of > >>> ResetVecotr 4. Reduce the changes to exiting code if possible > >>> > >>> So I try to make below conclusions below: (Please review) 1. > >>> SEV_ES_WORK_AREA is used as the TEE_WORK_AREA by both TDX and > SEV, > >>> maybe in the future it can be used by other CC technologies. > >>> > >>> 2. In MEMFD, add below initial value. So that TEE_WORK_AREA is > >>> guaranteed to be cleared in legacy guest. In TDX this memory region > >>> is initialized to be all-0 by host VMM. In SEV the memory region is > cleared as well. > >>> 0x00B000|0x001000 > >>> > >> > gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpa > ce > >> Guid.PcdSevEsWorkAreaSize > >>> DATA = { > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 > >>> } > >> Hmm, I thought the contents of the data pages are controlled by the host > VMM. > >> If the backing pages are not zero filled then there is no guarantee > >> that memory will be zero. To verify it: > >> > >> 1. I applied your above change in OvmfPkgX86.fdt. I modified the DATA > >> values from 0x00 -> 0xCC > >> > >> 2. Modified the SecMain.c to dump the SevEsWorkArea on entry > >> > >> And dump does not contain the 0xcc. > >> > >> And to confirm further, I attached to the qemu with the GDB before > >> the booting the OVMF, and modified the SevEsWorkArea with some > >> garbage number and this time the dump printed garbage value I put > through the debugger. > >> > >> In summary, the OVMF to zero the workarea memory on the entry and > we > >> cannot rely on the DATA={0x00, 0x00...} to zero the CCWorkArea. > > So in legacy guest, CCWorkArea is cleared to all-0 without the > DATA={0x00,0x00...}, right? > > Okay, maybe I was not able to communicate it correctly. > > The run I did is for the legacy guest. For the legacy guest, the contents of > the > CCWorkArea may *not* be always zero even when you use the DATA={0x00, > 0x00...}. > > Currently, Qemu uses zero filled backing pages, so we will get a zero filled > CCWorkArea; but nothing says that a backing page *must* be zero. > Another VMM may choose to do things differently. In summary, the OVMF > reset vector code must zero the CCWorkArea before calling SEV or TDX > probes. > Ah, I see. In current CheckSevFeatures, byte[SEV_ES_WORK_AREA] is cleared to0. Then its values is set based on the result of SEV probe.
There is a bug here. CheckTdxFeatures does the similar work and it sets the WORK_AREA to 2. If CheckSevFeatures is called after CheckTdxFeatures, then WORK_AREA is cleared and it is set to 0 because it is not SEV. The value is override. I think there are 2 options: Option 1: Neither CheckTdxFeatures nor CheckSevFeatures should clear WORK_AREA. Instead It should be cleared to 0 outside and before these 2 calls. So in Main16 after TransitionFromReal16To32BitFlat WORK_AREA is cleared to 0. In Tdx guest this WORK_AREA is initialized to 0 by host VMM. Option 2: Another option is to figure out a mechanism that only one CheckXXXFeatures is called. Since there are 2 entry point in Main.asm: Main16 and Main32. In Main16 CheckSevFeatures is called after TransitionFromReal16To32BitFlat. (eax should be saved because it is used in SetCr3ForPageTables64) In Main32 CheckTdxFeatures is called after ReloadFlat32. What's your opinion? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#78367): https://edk2.groups.io/g/devel/message/78367 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] -=-=-=-=-=-=-=-=-=-=-=-