On September 22, 2021 3:49 PM, Gerd Hoffmann wrote: > Hi, > > > +%ifdef ARCH_X64 > > +; > > +; TDX Metadata offset block > > +; > > +; TdxMetadata.asm is included in ARCH_X64 because Inte TDX is only ; > > +available in ARCH_X64. Below block describes the offset of ; > > +TdxMetadata block in Ovmf image ; ; GUID : > > +e47a6535-984a-4798-865e-4685a7bf8ec2 > > +; > > +tdxMetadataOffsetStart: > > + DD tdxMetadataOffsetStart - TdxMetadataGuid - 16 > > + DW tdxMetadataOffsetEnd - tdxMetadataOffsetStart > > + DB 0x35, 0x65, 0x7a, 0xe4, 0x4a, 0x98, 0x98, 0x47 > > + DB 0x86, 0x5e, 0x46, 0x85, 0xa7, 0xbf, 0x8e, 0xc2 > > +tdxMetadataOffsetEnd: > > + > > +%endif > > This should be switched to common ovmf metadata (see patches 4-7 of the > SEV-SNP series). > > Min: please have a look at these patches. > Hi, Gerd I checked the patches 4-7 of the SEV-SNP series. The common OvmfMetadata is designed for both SEV and TDX, right? If so, then it means the SEV and TDX metadata will be mixed in this OvmfMetadata. I am thinking there will always be different fields for SEV and TDX. For example, SEV has PcdOvmfSecGhcbPageTable but TDX doesn't need that page. If the common OvmfMetadata is consumed by TDX-QEMU, then PcdOvmfSecGhcbPageTableBase will be initialized too. That doesn't make sense. I am thinking that SEV and TDX can keep their own Metadata (in separate files, SevMetadata.asm and TdxMetadata.asm) which are pointed by the SEV or TDX offsets in the GUID-ed chain in ResetVector. In this case, SEV and TDX can design their own metadata flexibly, for example, the attribute, the item structure, add/remove/update the items, etc. And it will be more friendly to the reviewer for the Metadata, at least from the name of the items. > > Brijesh: It might be useful to post the metadata patches as separate series. > > > +; Load the GDT and set the CR0, then jump to Flat 32 protected mode. > > That comment isn't correct, you are already in 32-bit mode. Thanks. It will be updated in the next version. > > > +; Modified: EAX, EBX, CR0, CR4, DS, ES, FS, GS, SS > > CS too ... It will be fixed in the next version. > > > + jmp LINEAR_CODE_SEL:dword > ADDR_OF(jumpToFlat32BitAndLandHere) > > +jumpToFlat32BitAndLandHere: > > ... right here. > > > --- /dev/null > > +++ b/OvmfPkg/ResetVector/Main.asm > > Can you add a separate patch for "copy Main.asm from UefiCpuPkg > unmodified" please? Having the changes for TDX separately is helpful for > review. Sure. It will be separated in the next version. >
Thanks! Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80998): https://edk2.groups.io/g/devel/message/80998 Mute This Topic: https://groups.io/mt/85761661/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
