Re: [edk2-devel] [PATCH V9 4/4] OvmfPkg: Enable TDX in ResetVector

["Min Xu" <min.m.xu@intel.com>]

On October 12, 2021 3:43 PM, Gerd Hoffmann wrote:
>   Hi,
> 
> > +; Load the GDT and set the CR0.
> > +;
> > +; Modified:  EAX, EBX, CR0, CR4, DS, ES, FS, GS, SS, CS ;
> > +ReloadFlat32:
> > +
> > +    cli
> > +    mov     ebx, ADDR_OF(gdtr)
> > +    lgdt    [ebx]
> 
> No need to modify ebx here, eax should do fine.
You're right. It will be updated in next version.
> 
> > +    mov     eax, SEC_DEFAULT_CR0
> > +    mov     cr0, eax
> > +
> > +    jmp     LINEAR_CODE_SEL:dword ADDR_OF(jumpToFlat32BitAndLandHere)
> > +
> > +jumpToFlat32BitAndLandHere:
> 
> Strictly speaking this is not correct, you are already in Flat32 mode, so this only
> loads cs.
TDX: https://software.intel.com/content/dam/develop/external/us/en/documents/tdx-module-1.0-public-spec-v0.931.pdf
In [TDX] Section 10.1.3 CR0 and CR4 are initialized when Tdx guest is created. So initialization of CR0 and CR4 are not needed.
It will be updated in next version.
> 
> > +InitTdx:
> > +    ;
> > +    ; Save EBX in EBP because EBX will be changed in ReloadFlat32
> > +    ;
> > +    mov     ebp, ebx
> 
> See above, there is no need to modify ebx in ReloadFlat32.
> Also: seems ebx is never restored ...
In [TDX] Section 10.1.2 EBX[5:0] contains the GPAW. Since EBX is not changed in ReloadFlat32, *mov ebp, ebx* is not needed.
It will be removed in next version.
> 

Thanks!
Min