Re: [edk2-devel] [PATCH v6 06/29] OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase

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

On September 6, 2021 8:17 PM, Gerd Hoffman wrote:
>   Hi,
> 
> > > >  sevSnpBootBlockStart:
> > > > +    DD      SNP_HV_VALIDATED_START
> > > > +    DD      SNP_HV_VALIDATED_END
> 
> > We pack all the Tdx information into  a blob (TdxMetadata). These tdx
> > information Includes the BFV(i.e. OVMF_CODE.fd), the CFV(i.e.
> OVMF_VARS.fd), TdMailbox, etc.
> > The offset to the TdxMetadata is in the GUIDed chain in
> ResetVectorVtf0.asm.
> 
> > In the future new metadata can be added into the TdxMetadata without
> > changes in ResetVectorVtf0.asm.
> 
> [ Looking at https://www.mail-
> archive.com/devel@edk2.groups.io/msg33605.html ]
> 
> So, there isn't much tdx-specific in tdx-metadata.  Most ranges are
> TDX_METADATA_SECTION_TYPE_TEMP_MEM which I think basically means
> these ranges should be accepted by the hypervisor, which is pretty much the
> same issue snp tries to solve with this pre-validation range.  Then there are
> the ranges for code (aka bfv), for vars (aka cfv) and td_hob.
> 
> td_hob is the only tdx-specific item there, and even that concept (pass
> memory ranges as hob list from hypervisor to guest) might be useful outside
> tdx.
Mailbox is tdx-specific too. But Stack/Heap/OvmfWorkarea/OvmfPageTable are
common. BFV/CFV are common too.
> 
> So, can we settle on one approach for this please?  I think the tdx-metadata
> style approach is more flexible and future-proof.  It can easily be extended
> without changing data structures, we only need new section types.  I expect
> this will work better long-term when it comes to backward-compatibility.
> 
> I'd suggest we generalize the tdx-metadata idea and define both generic and
> vmm-specific section types:
> 
> enum {
>   OVMF_SECTION_TYPE_UNDEFINED = 0;
> 
>   /* generic */
>   OVMF_SECTION_TYPE_CODE = 0x100,
>   OVMF_SECTION_TYPE_VARS
>   OVMF_SECTION_TYPE_SEC_MEM  /* vmm should accept/validate this */
> 
>   /* sev */
>   OVMF_SECTION_TYPE_SEV_SECRETS = 0x200,
>   OVMF_SECTION_TYPE_SEV_CPUID /* or move to generic? */
> 
>   /* tdx */
>   OVMV_SECTION_TYPE_TDX_TD_HOB = 0x300,
> };
> 
> Comments?
TDX has similar section type.
But I am not sure if SEV can use this metadata mechanism.
Need SEV's comments.
> 
> Looking at tdx-metadata I have a few questions:
> 
> +_Bfv:
> +  DD TDX_BFV_RAW_DATA_OFFSET
> +  DD TDX_BFV_RAW_DATA_SIZE
> 
> What is this and why is it needed?
Host VMM need to measure the code part (BFV) to MRTD register
(which is similar to TPM PCRs).
> 
> +  DQ TDX_BFV_MEMORY_BASE
> +  DQ TDX_BFV_MEMORY_SIZE
> 
> Why "DQ"?  TDX is defined to start in 32bit mode, so you can hardly have
> addresses here which do not fit into "DD", correct?
Those are the memory. TDX is running in long mode. So it is DQ.
> 
> +  DD TDX_METADATA_SECTION_TYPE_BFV
> +  DD TDX_METADATA_ATTRIBUTES_EXTENDMR
> 
> What does this attribute mean?
It indicates if the host VMM should use TDCALL[TDH.MR.EXTEND] for this section.
https://software.intel.com/content/dam/develop/external/us/en/documents/tdx-virtual-firmware-design-guide-rev-1.pdf
Please refer to Section 11 TDVF Metadata for more detailed information.
Note: this Section will be updated according to the comments from the community.
> 

Thanks!
Min