For size field, please refer to PE/COFF specification https://docs.microsoft.com/en-us/windows/win32/debug/pe-format
The "Section Table (Section Headers)" defines two fields: ======================= VirtualSize - The total size of the section when loaded into memory. If this value is greater than SizeOfRawData, the section is zero-padded. This field is valid only for executable images and should be set to zero for object files. SizeOfRawData - The size of the section (for object files) or the size of the initialized data on disk (for image files). For executable images, this must be a multiple of FileAlignment from the optional header. If this is less than VirtualSize, the remainder of the section is zero-filled. Because the SizeOfRawData field is rounded but the VirtualSize field is not, it is possible for SizeOfRawData to be greater than VirtualSize as well. When a section contains only uninitialized data, this field should be zero. ======================= We took similar concept here. RawDataSize == size in the file. MemoryDataSize == size in the memory. They are totally different concept. For example, you can have 0xC81 RawDataSize, but the MemoryDataSize is 0x1000. If one project enforces RawDataSize == MemoryDataSize, then only one size is needed. But if one project wants to RawDataSize <= MemoryDataSize, then we need two size fields. Thank you Yao Jiewen > -----Original Message----- > From: kra...@redhat.com <kra...@redhat.com> > Sent: Monday, September 27, 2021 10:59 PM > To: Yao, Jiewen <jiewen....@intel.com> > Cc: devel@edk2.groups.io; Xu, Min M <min.m...@intel.com>; Brijesh Singh > <brijesh.si...@amd.com>; Ard Biesheuvel <ardb+tianoc...@kernel.org>; > Justen, Jordan L <jordan.l.jus...@intel.com>; Erdem Aktas > <erdemak...@google.com>; James Bottomley <j...@linux.ibm.com>; Tom > Lendacky <thomas.lenda...@amd.com> > Subject: Re: [edk2-devel] [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector > > Hi, > > > > struct { > > > uint64_t load_address; > > > uint32_t file_offset; > > > uint32_t section_size; > > > uint32_t section_type; > > > uint32_t section_flags; > > > }; > > > > [Jiewen] This data structure does not work in a special use case - A > > TD may want to have a fixed memory size. It is TD that tells the VMM > > how many DRAM should be allocated by using metadata table. Not the > > case that a VMM tells the TD how many DRAM is allocated by using HOB. > > > > In that special case, the TD_HOB is NOT required. The VMM parses the > > metadata to allocate the DRAM (AUG page). > > Hmm. Not covered in tdx-virtual-firmware-design-guide-rev-1.pdf > > > The runtime section size must be UINT64, otherwise we cannot support > 4G > memory section. > > The build time section size can be UINT32. We don't expect to create a >4G > binary in near future. > > And we need both. > > That still doesn't explain why you need two sizes. Instead of depending > on zero-fill in case MemoryDataSize > RawDataSize you can just use two > entries, simliar to ELF binaries which have separate '.data' and '.bss' > sections too. > > > I can understand why you think there is no needed fields, based upon > > what you see in EDKII/TDVF project. However, the usage in current > > TDVF is just a subset, but not all usages. > > So you are doing stuff behind closed doors ... > > The TDX metadata structure is carefully designed to support those > > variants. Also, leaving some room for future is a common practice. > > Besides EDKII/TDVF, we are doing other TDX related projects reusing > > the same metadata structure. (But sorry, I cannot tell more at this > > time.) > > ... and don't want tell details. Even the fact that you are doing that > is disclosed only after poking for a while because the patches submitted > leave a bunch of questions open. > > This is NOT how Open Source Development works. > > If the patches can't speak for themself in cases like this the very > minimum requirement is proper documentation. It is not acceptable > that I have to ask five times to figure that the format is supposed > to cover use cases beyond TDVF. > > > The benefit is that the KVM or cloud hypervisor can have a common > > logic to handle "TDX boot", instead of using different table in > > different use cases. > > The benefit of a unified table for tdx and sev is that the VMM can > have common logic to find page ranges which need special > initialization. > > But I suspect at that point we are going to trade code sharing at one > place for code sharing at another place. > > > I think it is OK, if SEV wants to reuse the existing TDX metadata > > table. (We need SEV people agree.) Then we can have one metadata > > table. > > So, when submitting the next revision of this series, please ... > > (1) Move the tdx metadata changes to a separate patch. > (2) Add *complete* documentation for the TDX metadata format > > ... so the SEV people can make up their mind whenever they want use > that or not. > > Please do also clarify what the process to allocate section type numbers > (or reserve a number range) for SEV would be. > > > [Jiewen] We don't fork OVMF in config-B. > > > > Instead of we will create new Tdvf/Tdvf.dsc and Tdvf/Tdvf.fdf in > > config-B, similar to > > https://github.com/tianocore/edk2/tree/master/OvmfPkg/AmdSev or > > https://github.com/tianocore/edk2/tree/master/OvmfPkg/Bhyve That is > > why I treat it as different platform. > > The differences between Ovmf and AmdSev are very small. > > Bhyve has more differences, but it's a different hypervisor > so it isn't surprising it has its own PlatformPei. > > How does Tdvf handle the platform setup? It must be done in SEC > somehow, so I suspect you have a (possibly stripped-down) version > of the PlatformPei adapted for SEC? That is exactly the kind of > code duplication I want avoid. > > > I reluctant to merge it back to Ovmf.dsc/fdf. > > I don't worry that much about Ovmf.dsc/fdf files. Whenever we add a > compile-time option (-D ENABLE_TDX) to Ovmf.dsc/fdf or whenever we add a > separate Tdvf.dsc/fdf doesn't make that much of a difference. > > I'm more worried about the code duplication and the completely different > (PEI-less) initialization workflow. When touching the platform setup > code both cases (with/without PEI) must be considered, which increases > development and testing and maintenance effort long-term. > > I want less variants, not more. Ideally I'd like to also get rid of the > OvmfPkgIa32X64.dsc/fdf for example. It seems some features have a > dependency on PEI running in 32-bit mode though. > > > The reason is the main Ovmf supports some features (such as S3, TPM) > > which may depend on PEI modules, but it is NOT needed in TDVF. > > So using PEI in OVMF isn't that over-engineered, isn't it? > > And I suspect SMM support can be added to the list of features which > depend on the PEI phase (at least when we want reuse the existing common > code in UefiCpuPkg, MdePkg and elsewhere). > > > We need re-evaluate the effort to enable those features in non-PEI > > configuration in OVMF. - That is totally unnecessary in TDVF enabling > > task. > > Well, it's surely additional upfront work. But I expect it will pay off > long-term. Less maintenance work, less testing work, lower risk of > adding regressions due to SEC and PEI init code paths variants running > out of sync. So "totally unnecessary" only when you ignore the work > needed after the initial merge. > > take care, > Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#81186): https://edk2.groups.io/g/devel/message/81186 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] -=-=-=-=-=-=-=-=-=-=-=-