Re: [RFC PATCH v2 02/10] x86/virt/tdx: Convert the version metadata reader
From: Dave Hansen
Date: Mon Sep 28 2026 - 10:49:05 EST
On 9/28/26 04:32, Chao Gao wrote:
> Then I would go one step further and move the mapping table into
> get_tdx_sys_info() too, so it sits next to its only use. Using the
> "features" metadata class as an example:
>
> static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
> {
> static const struct field_mapping feature_mappings[] __initconst = {
> TDX_SYSINFO_MAP_FEATURES(TDX_FIELD_TDX_FEATURES0, tdx_features0),
> };
> int ret = 0;
>
> ret = ret ?: read_sys_metadata_table(version_mappings, &sysinfo->version);
> ...
> ret = ret ?: read_sys_metadata_table(feature_mappings, &sysinfo->features);
>
>
> version_mappings[] will be the exception and stay at file scope because
> tdx_module_run_update() reads it as well.
>
> Let me know if you disagree.
Two concerns: One, the structures are wide and the extra indentation
might makes them too wide. Two, this separates the code from the
function header. That might hurt function readability.
The value of making a static variable is also lower for something that's
const because you don't have writers.
So I don't hate it especially for 3 lines, but I do think it generally
looks more natural to have these at global scope.