On 4/2/2025 8:08 PM, Daniel P. Berrangé wrote:
On Tue, Apr 01, 2025 at 09:01:27AM -0400, Xiaoyao Li wrote:
From: Isaku Yamahata <isaku.yamah...@intel.com>

TDX VM needs to boot with its specialized firmware, Trusted Domain
Virtual Firmware (TDVF). QEMU needs to parse TDVF and map it in TD
guest memory prior to running the TDX VM.

A TDVF Metadata in TDVF image describes the structure of firmware.
QEMU refers to it to setup memory for TDVF. Introduce function
tdvf_parse_metadata() to parse the metadata from TDVF image and store
the info of each TDVF section.

TDX metadata is located by a TDX metadata offset block, which is a
GUID-ed structure. The data portion of the GUID structure contains
only an 4-byte field that is the offset of TDX metadata to the end
of firmware file.

Select X86_FW_OVMF when TDX is enable to leverage existing functions
to parse and search OVMF's GUID-ed structures.

Signed-off-by: Isaku Yamahata <isaku.yamah...@intel.com>
Co-developed-by: Xiaoyao Li <xiaoyao...@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao...@intel.com>
Acked-by: Gerd Hoffmann <kra...@redhat.com>
---
Changes in v8:
  - Drop the failure handling of memcpy() since it cannot fail;

Changes in v7:
  - Update license info to only use SPDX tag;
  - use g_autofree to avoid manually free;

Changes in v6:
  - Drop the the data endianness change for metadata->Length;

Changes in v1:
  - rename tdvf_parse_section_entry() to
    tdvf_parse_and_check_section_entry()

Changes in RFC v4:
  - rename TDX_METADATA_GUID to TDX_METADATA_OFFSET_GUID
---
  hw/i386/Kconfig        |   1 +
  hw/i386/meson.build    |   1 +
  hw/i386/tdvf.c         | 183 +++++++++++++++++++++++++++++++++++++++++
  include/hw/i386/tdvf.h |  38 +++++++++
  4 files changed, 223 insertions(+)
  create mode 100644 hw/i386/tdvf.c
  create mode 100644 include/hw/i386/tdvf.h

diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index cce9521ba934..eb65bda6e071 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -12,6 +12,7 @@ config SGX
config TDX
      bool
+    select X86_FW_OVMF
      depends on KVM
config PC
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index 10bdfde27c69..3bc1da2b6eb4 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -32,6 +32,7 @@ i386_ss.add(when: 'CONFIG_PC', if_true: files(
    'port92.c'))
  i386_ss.add(when: 'CONFIG_X86_FW_OVMF', if_true: files('pc_sysfw_ovmf.c'),
                                          if_false: 
files('pc_sysfw_ovmf-stubs.c'))
+i386_ss.add(when: 'CONFIG_TDX', if_true: files('tdvf.c'))
subdir('kvm')
  subdir('xen')
diff --git a/hw/i386/tdvf.c b/hw/i386/tdvf.c
new file mode 100644
index 000000000000..328d1b7ffdf8
--- /dev/null
+++ b/hw/i386/tdvf.c
@@ -0,0 +1,183 @@
+/*
+ * Copyright (c) 2025 Intel Corporation
+ * Author: Isaku Yamahata <isaku.yamahata at gmail.com>
+ *                        <isaku.yamahata at intel.com>
+ *         Xiaoyao Li <xiaoyao...@intel.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+
+#include "hw/i386/pc.h"
+#include "hw/i386/tdvf.h"
+#include "system/kvm.h"
+
+#define TDX_METADATA_OFFSET_GUID    "e47a6535-984a-4798-865e-4685a7bf8ec2"
+#define TDX_METADATA_VERSION        1
+#define TDVF_SIGNATURE              0x46564454 /* TDVF as little endian */
+
+typedef struct {
+    uint32_t DataOffset;
+    uint32_t RawDataSize;
+    uint64_t MemoryAddress;
+    uint64_t MemoryDataSize;
+    uint32_t Type;
+    uint32_t Attributes;
+} TdvfSectionEntry;
+
+typedef struct {
+    uint32_t Signature;
+    uint32_t Length;
+    uint32_t Version;
+    uint32_t NumberOfSectionEntries;
+    TdvfSectionEntry SectionEntries[];
+} TdvfMetadata;

struct field names starting with an initial capital is
not the usual QEMU code style. Can this be all initial
lowercase, with capital just for word separation.

I think Isaku used such names because they are defined in TDVF Design Guide spec[1].

QEMU's internal data structure for metadata are below, which follows QEMU coding style.

        typedef struct TdxFirmwareEntry {
            uint32_t data_offset;
            uint32_t data_len;
            uint64_t address;
            uint64_t size;
            uint32_t type;
            uint32_t attributes;
        } TdxFirmwareEntry;

        typedef struct TdxFirmware {
            uint32_t nr_entries;
            TdxFirmwareEntry *entries;
        } TdxFirmware;

So if no strong preference, I would keep it as-is that the raw struct read from TDVF keeps the name convention of TDVF Design Guide spec and internal data struct uses QEMU's convention.

[1] https://cdrdv2.intel.com/v1/dl/getContent/733585


+
+struct tdx_metadata_offset {
+    uint32_t offset;
+};
+

With regards,
Daniel


Reply via email to