Re: [RFC PATCH v2 02/10] x86/virt/tdx: Convert the version metadata reader

[Chao Gao]

>>  /* Read each metadata field listed in @mappings[] into @data. */
>> -static int __maybe_unused __read_sys_metadata_table(const struct field_mapping *mappings,
>> -						    int num_mappings, void *data)
>> +static int __read_sys_metadata_table(const struct field_mapping *mappings,
>> +				     int num_mappings, void *data)
>>  {
>>  	int i, ret;
>>  	u64 val;
>> @@ -431,6 +431,23 @@ static int __maybe_unused __read_sys_metadata_table(const struct field_mapping *
>>  	.size		= sizeof_field(_type, _member),		\
>>  }
>>  
>> +#define TDX_SYSINFO_MAP_VERSION(_field_id, _member) \
>> +	TDX_SYSINFO_MAP(_field_id, struct tdx_sys_info_version, _member)
>> +
>> +static const struct field_mapping version_mappings[] = {
>> +	TDX_SYSINFO_MAP_VERSION(MINOR_VERSION,  minor_version),
>> +	TDX_SYSINFO_MAP_VERSION(MAJOR_VERSION,  major_version),
>> +	TDX_SYSINFO_MAP_VERSION(UPDATE_VERSION, update_version),
>> +};
>> +
>> +#define read_sys_metadata_table(_table, _data) \
>> +	__read_sys_metadata_table(_table, ARRAY_SIZE(_table), _data)
>
>Should this go in the previous patch?

I didn't put it there because the previous patch doesn't strictly need
it; this patch is the first user.

Adding the macro in the previous patch while still tagging
__read_sys_metadata_table() __maybe_unused looks a bit odd:
the macro references the function, yet nothing uses either of them.

That said, I can move it if the oddness doesn't bother you.

>
>> +
>> +static int get_tdx_sys_info_version(struct tdx_sys_info_version *version)
>> +{
>> +	return read_sys_metadata_table(version_mappings, version);
>> +}
>
>I'm not sure we need all of these helpers that are each only called once?

I'd prefer to keep them, for a few reasons:

1. This one is actually called twice: from get_tdx_sys_info() and from
   tdx_module_run_update().

2. The helper name describes what is being read better than a bare
   read_sys_metadata_table() call.

3. Some helpers do more than read a table (see get_tdx_sys_info_td_conf()).
   Without per-class helpers, get_tdx_sys_info() would open-code
   read_sys_metadata_table() for some classes and call a helper for
   others, which looks inconsistent. With one helper per metadata class,
   every class is read the same way.