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
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:
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-v6
[*] 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"
],
# "td_ctrl": [
# "TDR_BASE_SIZE",
# "TDCS_BASE_SIZE",
# "TDVPS_BASE_SIZE",
# ],
# "td_conf": [
# "ATTRIBUTES_FIXED0",
# "ATTRIBUTES_FIXED1",
# "XFAM_FIXED0",
# "XFAM_FIXED1",
# "NUM_CPUID_CONFIG",
# "MAX_VCPUS_PER_TD",
# "CPUID_CONFIG_LEAVES",
# "CPUID_CONFIG_VALUES",
# ],
}
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 = "tdx_sys_info_%s" % (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 = "get_tdx_sys_info_%s" % (class_name)
struct_name = "tdx_sys_info_%s" % (class_name)
struct_var = "sysinfo_%s" % (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 = "sysinfo_cmr->num_cmrs"
elif fname == "CPUID_CONFIG_LEAVES" or fname == "CPUID_CONFIG_VALUES":
limit = "sysinfo_td_conf->num_cpuid_config"
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 = "tdx_sys_info_%s" % (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 = "get_tdx_sys_info_" + 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: 21f0d4005e7eb71b95cf6b55041fd525bdb11c1f
--
2.46.2
On 10/28/24 13:41, 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.
Kai/Dave/Rick,
the v6 of this series messes up the TDX patches for KVM, which do not
apply anymore. I can work on a rebase myself for the sake of putting
this series in kvm-coco-queue; but please help me a little bit by
including in the generated data all the fields that KVM needs.
Are you able to send quickly a v7 that includes these fields, and that
also checks in the script that generates the files?
Emphasis on "quickly". No internal review processes of any kind, please.
Thanks,
Paolo
> 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:
>
> 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-v6
>
> [*] 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"
> ],
> # "td_ctrl": [
> # "TDR_BASE_SIZE",
> # "TDCS_BASE_SIZE",
> # "TDVPS_BASE_SIZE",
> # ],
> # "td_conf": [
> # "ATTRIBUTES_FIXED0",
> # "ATTRIBUTES_FIXED1",
> # "XFAM_FIXED0",
> # "XFAM_FIXED1",
> # "NUM_CPUID_CONFIG",
> # "MAX_VCPUS_PER_TD",
> # "CPUID_CONFIG_LEAVES",
> # "CPUID_CONFIG_VALUES",
> # ],
> }
>
> 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 = "tdx_sys_info_%s" % (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 = "get_tdx_sys_info_%s" % (class_name)
> struct_name = "tdx_sys_info_%s" % (class_name)
> struct_var = "sysinfo_%s" % (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 = "sysinfo_cmr->num_cmrs"
> elif fname == "CPUID_CONFIG_LEAVES" or fname == "CPUID_CONFIG_VALUES":
> limit = "sysinfo_td_conf->num_cpuid_config"
> 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 = "tdx_sys_info_%s" % (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 = "get_tdx_sys_info_" + 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: 21f0d4005e7eb71b95cf6b55041fd525bdb11c1f
On 29/10/2024 7:35 am, Paolo Bonzini wrote: > On 10/28/24 13:41, 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. > > Kai/Dave/Rick, > > the v6 of this series messes up the TDX patches for KVM, which do not > apply anymore. I can work on a rebase myself for the sake of putting > this series in kvm-coco-queue; but please help me a little bit by > including in the generated data all the fields that KVM needs. I have already rebased the impacted patches and pushed to here: https://github.com/intel/tdx/commits/kvm-tdxinit-host-metadata-v6/ .. which includes: 1) this series 2) TDX module init in KVM patches 3) 3 additional patches to reading more metadata and share to KVM. Besides the above, one minor update is needed to the KVM TDX patch KVM: TDX: Get system-wide info about TDX module on initialization .. because now the cpuid_config_value is now a 2-dimensional array: The updated patch can be found here: https://github.com/intel/tdx/commit/fd7947118b76f6d4256bc4228e03e73262e67ba2 > > Are you able to send quickly a v7 that includes these fields, and that > also checks in the script that generates the files? Yeah I can do. But for KVM to use those fields, we will also need export those metadata. Do you want me to just include all the 3 patches that are mentioned in the above item 3) to v7? Hi Dave, Could you also comment whether this is OK to you? > > Emphasis on "quickly". No internal review processes of any kind, please. Yeah understood.
>> >> Are you able to send quickly a v7 that includes these fields, and that >> also checks in the script that generates the files? > > Yeah I can do. But for KVM to use those fields, we will also need > export those metadata. Do you want me to just include all the 3 patches > that are mentioned in the above item 3) to v7? Sorry I just realize I cannot just move all the 3 patches in item 3) to this series since one of them depends on TDX module init KVM patch. So yeah I can just move the patch which reads more fields for KVM to this series but leave the rest to future patchset. But for kvm-coco-queue purpose as mentioned in the previous reply I have rebased those patches and pushed to github. So perhaps we can leave them to the future patchset for the sake of keeping this series simple? Adding the patch which adds the script to this series is another topic. I can certainly do if Dave is fine.
On Tue, Oct 29, 2024 at 1:24 AM Huang, Kai <kai.huang@intel.com> wrote: > >> Are you able to send quickly a v7 that includes these fields, and that > >> also checks in the script that generates the files? > > > > Yeah I can do. But for KVM to use those fields, we will also need > > export those metadata. Do you want me to just include all the 3 patches > > that are mentioned in the above item 3) to v7? > > for kvm-coco-queue purpose as mentioned in the previous reply I have > rebased those patches and pushed to github. So perhaps we can leave > them to the future patchset for the sake of keeping this series simple? Yes, I have now pushed a new kvm-coco-queue. > Adding the patch which adds the script to this series is another topic. > I can certainly do if Dave is fine. It's better since future patches will almost certainly regenerate the file. Can you post a followup patch to this thread,, like "9/8", that adds the script? Then maintainers can decide whether to pick it. Thanks, Paolo
On Wed, 2024-10-30 at 15:48 +0100, Paolo Bonzini wrote: > On Tue, Oct 29, 2024 at 1:24 AM Huang, Kai <kai.huang@intel.com> wrote: > > > > Are you able to send quickly a v7 that includes these fields, and that > > > > also checks in the script that generates the files? > > > > > > Yeah I can do. But for KVM to use those fields, we will also need > > > export those metadata. Do you want me to just include all the 3 patches > > > that are mentioned in the above item 3) to v7? > > > > for kvm-coco-queue purpose as mentioned in the previous reply I have > > rebased those patches and pushed to github. So perhaps we can leave > > them to the future patchset for the sake of keeping this series simple? > > Yes, I have now pushed a new kvm-coco-queue. Thanks. Btw Rick made some comments and I updated them a little bit. Now they have been sent out in Rick's "v2 TDX vCPU/VM creation". We should use them in the kvm-coco-queue. > > > Adding the patch which adds the script to this series is another topic. > > I can certainly do if Dave is fine. > > It's better since future patches will almost certainly regenerate the file. > > Can you post a followup patch to this thread,, like "9/8", that adds > the script? Then maintainers can decide whether to pick it. OK will make a patch and send out. Thanks!
On Tue, 2024-10-29 at 01:41 +1300, 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. > > 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. > Hi Dave, Could you help to take a look?
On 10/28/24 13:41, Kai Huang wrote:
> 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.
I think Dave did want to check it in, but not tie it to the build (so
that you don't need to have global_metadata.json).
You can add an eleventh patch (or a v7 just for patch 3) that adds it in
scripts/. Maybe also add a
print("/* Generated from global_metadata.json by
scripts/tdx_parse_metadata.py */", file=f);
line to the script, for both hfile and cfile?
> - 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.
> 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 = "sysinfo_cmr->num_cmrs"
> elif fname == "CPUID_CONFIG_LEAVES" or fname == "CPUID_CONFIG_VALUES":
> limit = "sysinfo_td_conf->num_cpuid_config"
Thanks Intel for not telling the whole story in the "Num Fields" value
of global_metadata.json. :)
Paolo
On 29/10/2024 6:59 am, Paolo Bonzini wrote:
> On 10/28/24 13:41, Kai Huang wrote:
>
>> 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.
>
> I think Dave did want to check it in, but not tie it to the build (so
> that you don't need to have global_metadata.json).
> > You can add an eleventh patch (or a v7 just for patch 3) that adds
it in
> scripts/. Maybe also add a
>
> print("/* Generated from global_metadata.json by scripts/
> tdx_parse_metadata.py */", file=f);
>
> line to the script, for both hfile and cfile?
Sure I can do. But IIUC Dave wanted to keep this series simple and we
can start with appending the script to the coverletter (we had as short
chat internally).
Hi Dave, what's your preference?
>
>> - 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.
>
>> 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 = "sysinfo_cmr->num_cmrs"
>> elif fname == "CPUID_CONFIG_LEAVES" or fname ==
>> "CPUID_CONFIG_VALUES":
>> limit = "sysinfo_td_conf->num_cpuid_config"
>
> Thanks Intel for not telling the whole story in the "Num Fields" value
> of global_metadata.json. :)
Yeah I feel bad about this too. I asked whether we can use the value
reported in "Num Fields" as the limit to loop, but I was told we should
use the value reported in "NUM_CMRS". :-(
© 2016 - 2024 Red Hat, Inc.