[PATCH v7 00/10] TDX host: metadata reading tweaks, bug fix and info dump

Kai Huang posted 10 patches 1 week, 5 days ago
arch/x86/virt/vmx/tdx/tdx.c                 | 178 ++++++++++++--------
arch/x86/virt/vmx/tdx/tdx.h                 |  43 +----
arch/x86/virt/vmx/tdx/tdx_global_metadata.c |  89 ++++++++++
arch/x86/virt/vmx/tdx/tdx_global_metadata.h |  42 +++++
4 files changed, 247 insertions(+), 105 deletions(-)
create mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.c
create mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.h
[PATCH v7 00/10] TDX host: metadata reading tweaks, bug fix and info dump
Posted by Kai Huang 1 week, 5 days ago
This series does necessary tweaks to TDX host "global metadata" reading
code to fix some immediate issues in the TDX module initialization code,
with intention to also provide a flexible code base to support sharing
global metadata to KVM (and other kernel components) for future needs.

This series, and additional patches to initialize TDX when loading KVM
module and read essential metadata fields for KVM TDX can be found at
[1].

Hi Dave (and maintainers),

This series targets x86 tip.  Also add Dan, KVM maintainers and KVM list
so people can also review and comment.

This is a pre-work of the "quite near future" KVM TDX support.  I
appreciate if you can review, comment and take this series if the
patches look good to you.

History:

v6 -> v7:
 - Collect tags from Dan and Nikolay (Thanks!)
 - Address nit comments from Dan in patch 3 changelog.
 - Rebase to tip/x86/tdx

v5 -> v6:
 - Change to use a script [*] to auto-generate metadata reading code.

  - https://lore.kernel.org/kvm/f25673ea-08c5-474b-a841-095656820b67@intel.com/
  - https://lore.kernel.org/kvm/CABgObfYXUxqQV_FoxKjC8U3t5DnyM45nz5DpTxYZv2x_uFK_Kw@mail.gmail.com/

   Per Dave, this patchset doesn't contain a patch to add the script
   to the kernel tree but append it in this cover letter in order to
   minimize the review effort.

 - Change to use auto-generated code to read TDX module version,
   supported features and CMRs in one patch, and made that from and
   signed by Paolo.
 - Couple of new patches due to using the auto-generated code
 - Remove the "reading metadata" part (due to they are auto-generated
   in one patch now) from the consumer patches.

Pervious versions and more background please see:

 - https://lore.kernel.org/kvm/9a06e2cf469cbca2777ac2c4ef70579e6bb934d5.camel@intel.com/T/

[1]: https://github.com/intel/tdx/tree/kvm-tdxinit-host-metadata-v7

[*] The script used to generate the patch 3:

#! /usr/bin/env python3
import json
import sys

# Note: this script does not run as part of the build process.
# It is used to generate structs from the TDX global_metadata.json
# file, and functions to fill in said structs.  Rerun it if
# you need more fields.

TDX_STRUCTS = {
    "version": [
        "BUILD_DATE",
        "BUILD_NUM",
        "MINOR_VERSION",
        "MAJOR_VERSION",
        "UPDATE_VERSION",
        "INTERNAL_VERSION",
    ],
    "features": [
        "TDX_FEATURES0"
    ],
    "tdmr": [
        "MAX_TDMRS",
        "MAX_RESERVED_PER_TDMR",
        "PAMT_4K_ENTRY_SIZE",
        "PAMT_2M_ENTRY_SIZE",
        "PAMT_1G_ENTRY_SIZE",
    ],
    "cmr": [
        "NUM_CMRS", "CMR_BASE", "CMR_SIZE"
    ],
}

STRUCT_PREFIX = "tdx_sys_info"
FUNC_PREFIX = "get_tdx_sys_info"
STRVAR_PREFIX = "sysinfo"

def print_class_struct_field(field_name, element_bytes, num_fields, num_elements, file):
    element_type = "u%s" % (element_bytes * 8)
    element_array = ""
    if num_fields > 1:
        element_array += "[%d]" % (num_fields)
    if num_elements > 1:
        element_array += "[%d]" % (num_elements)
    print("\t%s %s%s;" % (element_type, field_name, element_array), file=file)

def print_class_struct(class_name, fields, file):
    struct_name = "%s_%s" % (STRUCT_PREFIX, class_name)
    print("struct %s {" % (struct_name), file=file)
    for f in fields:
        print_class_struct_field(
            f["Field Name"].lower(),
            int(f["Element Size (Bytes)"]),
            int(f["Num Fields"]),
            int(f["Num Elements"]),
            file=file)
    print("};", file=file)

def print_read_field(field_id, struct_var, struct_member, indent, file):
    print(
        "%sif (!ret && !(ret = read_sys_metadata_field(%s, &val)))\n%s\t%s->%s = val;"
        % (indent, field_id, indent, struct_var, struct_member),
        file=file,
    )

def print_class_function(class_name, fields, file):
    func_name = "%s_%s" % (FUNC_PREFIX, class_name)
    struct_name = "%s_%s" % (STRUCT_PREFIX, class_name)
    struct_var = "%s_%s" % (STRVAR_PREFIX, class_name)

    print("static int %s(struct %s *%s)" % (func_name, struct_name, struct_var), file=file)
    print("{", file=file)
    print("\tint ret = 0;", file=file)
    print("\tu64 val;", file=file)

    has_i = 0
    has_j = 0
    for f in fields:
        num_fields = int(f["Num Fields"])
        num_elements = int(f["Num Elements"])
        if num_fields > 1:
            has_i = 1
        if num_elements > 1:
            has_j = 1

    if has_i == 1 and has_j == 1:
        print("\tint i, j;", file=file)
    elif has_i == 1:
        print("\tint i;", file=file)

    print(file=file)
    for f in fields:
        fname = f["Field Name"]
        field_id = f["Base FIELD_ID (Hex)"]
        num_fields = int(f["Num Fields"])
        num_elements = int(f["Num Elements"])
        struct_member = fname.lower()
        indent = "\t"
        if num_fields > 1:
            if fname == "CMR_BASE" or fname == "CMR_SIZE":
                limit = "%s_%s->num_cmrs" %(STRVAR_PREFIX, "cmr")
            elif fname == "CPUID_CONFIG_LEAVES" or fname == "CPUID_CONFIG_VALUES":
                limit = "%s_%s->num_cpuid_config" %(STRVAR_PREFIX, "td_conf")
            else:
                limit = "%d" %(num_fields)
            print("%sfor (i = 0; i < %s; i++)" % (indent, limit), file=file)
            indent += "\t"
            field_id += " + i"
            struct_member += "[i]"
        if num_elements > 1:
            print("%sfor (j = 0; j < %d; j++)" % (indent, num_elements), file=file)
            indent += "\t"
            field_id += " * 2 + j"
            struct_member += "[j]"

        print_read_field(
            field_id,
            struct_var,
            struct_member,
            indent,
            file=file,
        )

    print(file=file)
    print("\treturn ret;", file=file)
    print("}", file=file)

def print_main_struct(file):
    print("struct tdx_sys_info {", file=file)
    for class_name, field_names in TDX_STRUCTS.items():
        struct_name = "%s_%s" % (STRUCT_PREFIX, class_name)
        struct_var = class_name
        print("\tstruct %s %s;" % (struct_name, struct_var), file=file)
    print("};", file=file)

def print_main_function(file):
    print("static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)", file=file)
    print("{", file=file)
    print("\tint ret = 0;", file=file)
    print(file=file)
    for class_name, field_names in TDX_STRUCTS.items():
        func_name = "%s_%s" % (FUNC_PREFIX, class_name)
        struct_var = class_name
        print("\tret = ret ?: %s(&sysinfo->%s);" % (func_name, struct_var), file=file)
    print(file=file)
    print("\treturn ret;", file=file)
    print("}", file=file)

jsonfile = sys.argv[1]
hfile = sys.argv[2]
cfile = sys.argv[3]
hfileifdef = hfile.replace(".", "_")

with open(jsonfile, "r") as f:
    json_in = json.load(f)
    fields = {x["Field Name"]: x for x in json_in["Fields"]}

with open(hfile, "w") as f:
    print("/* SPDX-License-Identifier: GPL-2.0 */", file=f)
    print("/* Automatically generated TDX global metadata structures. */", file=f)
    print("#ifndef _X86_VIRT_TDX_AUTO_GENERATED_" + hfileifdef.upper(), file=f)
    print("#define _X86_VIRT_TDX_AUTO_GENERATED_" + hfileifdef.upper(), file=f)
    print(file=f)
    print("#include <linux/types.h>", file=f)
    print(file=f)
    for class_name, field_names in TDX_STRUCTS.items():
        print_class_struct(class_name, [fields[x] for x in field_names], file=f)
        print(file=f)
    print_main_struct(file=f)
    print(file=f)
    print("#endif", file=f)

with open(cfile, "w") as f:
    print("// SPDX-License-Identifier: GPL-2.0", file=f)
    print("/*", file=f)
    print(" * Automatically generated functions to read TDX global metadata.", file=f)
    print(" *", file=f)
    print(" * This file doesn't compile on its own as it lacks of inclusion", file=f)
    print(" * of SEAMCALL wrapper primitive which reads global metadata.", file=f)
    print(" * Include this file to other C file instead.", file=f)
    print(" */", file=f)
    for class_name, field_names in TDX_STRUCTS.items():
        print(file=f)
        print_class_function(class_name, [fields[x] for x in field_names], file=f)
    print(file=f)
    print_main_function(file=f)





Kai Huang (9):
  x86/virt/tdx: Rename 'struct tdx_tdmr_sysinfo' to reflect the spec
    better
  x86/virt/tdx: Start to track all global metadata in one structure
  x86/virt/tdx: Use dedicated struct members for PAMT entry sizes
  x86/virt/tdx: Add missing header file inclusion to local tdx.h
  x86/virt/tdx: Switch to use auto-generated global metadata reading
    code
  x86/virt/tdx: Trim away tail null CMRs
  x86/virt/tdx: Reduce TDMR's reserved areas by using CMRs to find
    memory holes
  x86/virt/tdx: Require the module to assert it has the NO_RBP_MOD
    mitigation
  x86/virt/tdx: Print TDX module version

Paolo Bonzini (1):
  x86/virt/tdx: Use auto-generated code to read global metadata

 arch/x86/virt/vmx/tdx/tdx.c                 | 178 ++++++++++++--------
 arch/x86/virt/vmx/tdx/tdx.h                 |  43 +----
 arch/x86/virt/vmx/tdx/tdx_global_metadata.c |  89 ++++++++++
 arch/x86/virt/vmx/tdx/tdx_global_metadata.h |  42 +++++
 4 files changed, 247 insertions(+), 105 deletions(-)
 create mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.c
 create mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.h


base-commit: 7ae15e2f69bad06527668b478dff7c099ad2e6ae
-- 
2.46.2
Re: [PATCH v7 00/10] TDX host: metadata reading tweaks, bug fix and info dump
Posted by Dave Hansen 1 week, 4 days ago
On 11/11/24 02:39, Kai Huang wrote:
> This series does necessary tweaks to TDX host "global metadata" reading
> code to fix some immediate issues in the TDX module initialization code,
> with intention to also provide a flexible code base to support sharing
> global metadata to KVM (and other kernel components) for future needs.

Could we please just limit this to the bug fix and the new TD metadata
infrastructure?  Let's not mix it all up with the debugging printk()s.
Re: [PATCH v7 00/10] TDX host: metadata reading tweaks, bug fix and info dump
Posted by Huang, Kai 1 week, 4 days ago
On Mon, 2024-11-11 at 12:33 -0800, Dave Hansen wrote:
> On 11/11/24 02:39, Kai Huang wrote:
> > This series does necessary tweaks to TDX host "global metadata" reading
> > code to fix some immediate issues in the TDX module initialization code,
> > with intention to also provide a flexible code base to support sharing
> > global metadata to KVM (and other kernel components) for future needs.
> 
> Could we please just limit this to the bug fix and the new TD metadata
> infrastructure?  Let's not mix it all up with the debugging printk()s.

It also has a patch to fail module initialization when NO_MOD_BBP feature is not
support.

Just want to confirm, do you want to remove the code to:

 - print CMRs;
 - print TDX module versoin;

?

Then I will need to:

 - remove "printing CMRs" in patch 7 ("x86/virt/tdx: Trim away tail null CMRs").
 - remove patch 10 ("x86/virt/tdx: Print TDX module version").
Re: [PATCH v7 00/10] TDX host: metadata reading tweaks, bug fix and info dump
Posted by Dave Hansen 1 week, 4 days ago
On 11/11/24 12:49, Huang, Kai wrote:
> It also has a patch to fail module initialization when NO_MOD_BBP feature is not
> support.
> 
> Just want to confirm, do you want to remove the code to:
> 
>  - print CMRs;
>  - print TDX module versoin;

What is your goal?  What is the bare minimum amount of code to get there?
Re: [PATCH v7 00/10] TDX host: metadata reading tweaks, bug fix and info dump
Posted by Huang, Kai 1 week, 4 days ago
On Mon, 2024-11-11 at 13:00 -0800, Hansen, Dave wrote:
> On 11/11/24 12:49, Huang, Kai wrote:
> > It also has a patch to fail module initialization when NO_MOD_BBP feature is not
> > support.
> > 
> > Just want to confirm, do you want to remove the code to:
> > 
> >  - print CMRs;
> >  - print TDX module versoin;
> 
> What is your goal?  What is the bare minimum amount of code to get there?

The goal is to get everything that KVM TDX needs merged, plus the bug fix.

KVM TDX needs the new metadata infrastructure and the NO_MOD_BRP patch, so yeah
only printing CMRs and TDX module version are not needed.

I'll remove them in the next version.
Re: [PATCH v7 00/10] TDX host: metadata reading tweaks, bug fix and info dump
Posted by Huang, Kai 1 week, 3 days ago
On Mon, 2024-11-11 at 21:28 +0000, Huang, Kai wrote:
> On Mon, 2024-11-11 at 13:00 -0800, Hansen, Dave wrote:
> > On 11/11/24 12:49, Huang, Kai wrote:
> > > It also has a patch to fail module initialization when NO_MOD_BBP feature is not
> > > support.
> > > 
> > > Just want to confirm, do you want to remove the code to:
> > > 
> > >  - print CMRs;
> > >  - print TDX module versoin;
> > 
> > What is your goal?  What is the bare minimum amount of code to get there?
> 
> The goal is to get everything that KVM TDX needs merged, plus the bug fix.
> 
> KVM TDX needs the new metadata infrastructure and the NO_MOD_BRP patch, so yeah
> only printing CMRs and TDX module version are not needed.
> 
> I'll remove them in the next version.

I removed the "version" part in the 'tdx_global_metadata.py' script in order to
remove the code which reads TDX module version from the auto-generated code. 
For the sake of having a lore link of the script that I used in the new version,
I attached the updated script here.  It just got "version" part removed thus is
not interesting to read.

And Sorry I didn't provide enough info about the "goal" in my previous reply:

The goal of this series is to provide a new TDX global metadata infrastructure
to:

1) address two issues in the current TDX module initialization code, and
2) have an extendable infrastructure which is super easy to read more metadata
and share with KVM for KVM TDX support (and other kernel components for TDX
Connect in the future).

And the reason that we need a new global metadata infrastructure is the current
one can only read TDMR related metadata fields and it is not extendable to read
more metadata fields, which is required to address both 1) and 2) above.

Specifically, below two issues in the current module initialization code need to
be addressed:

1) Module initialization may fail on some large systems (e.g., with 4 or more
sockets).
2) Some old modules can clobber host's RBP when existing from the TDX guest, and
currently they can be initialized successfully.  We don't want to use such
modules thus we should just fail to initialize them to avoid memory/cpu cycle
cost of initializing TDX module.

The minimal code to achieve this goal is to remove the code which prints TDX
module version and CMR info in this series.  After removing them, the fist 6
patches in this series introduce the new metadata infrastructure, and the rest
patches address the two above issues.