Re: [edk2-devel] [PATCH V6 1/1] OvmfPkg: Enable TDX in ResetVector

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

On September 14, 2021 7:25 PM, Brijesh Singh wrote:
> 
> Hi Min,
> 
> A quick question below.
> 
> On 9/14/21 3:50 AM, Min Xu wrote:
> > RFC：
> >
> > 1. Definition of BFV & CFV
> > Tdx Virtual Firmware (TDVF) includes one Firmware Volume (FV) known as
> > the Boot Firmware Volume (BFV). The FV format is defined in the UEFI
> > Platform Initialization (PI) spec. BFV includes all TDVF components
> > required during boot.
> >
> > TDVF also include a configuration firmware volume (CFV) that is
> > separated from the BFV. The reason is because the CFV is measured in
> > RTMR, while the BFV is measured in MRTD.
> >
> > In practice BFV is the code part of Ovmf image (OVMF_CODE.fd). CFV is
> > the vars part of Ovmf image (OVMF_VARS.fd).
> >
> > 2. PcdOvmfImageSizeInKb
> > PcdOvmfImageSizeInKb indicates the size of Ovmf image. It is used to
> > calculate the offset of TdxMetadata in ResetVectorVtf0.asm.
> 
> In SEV-SNP v7 series, I implemented the metadata support. I did not see a
> need for the PcdOvmfImageSizeInKB. Why do you need it? I think your
> calculation below will not work if someone is using the OVMF_CODE.fd
> instead of OVMF.fd. Have you tried booting with OVMF_CODE.fd ?
In the original PoC, TDVF is required to be launched with OVMF.fd (OVMF_CODE.fd and OVMF_VARS.fd is not supported)  because of the TDX-QEMU limitation. So PcdOvmfImageSizeInKb is used to calculate the offset of Metadata (The offset is from fourGigabytes).
But you're right. PcdOvmfImageSizeInKB is not needed. The offset should be from the TDX Metadata offset block in GUIDed chain. 
TDX-QEMU team is aware of the limitation that OVMF_CODE.fd&OVMF_VARS.fd should be supported too, otherwise the SecureBoot does not work with libvirt. They are working on this limitation.

I will remove PcdOvmfImageSizeInKB and update the Metadata offset like below:
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:

Thanks!
Min