Re: [PATCH v3 07/10] x86/virt/tdx: Convert the td_conf metadata reader

[Nikolay Borisov]

On 1.10.26 г. 2:54 ч., Edgecombe, Rick P wrote:
> On Tue, 2026-09-29 at 22:38 -0700, Chao Gao wrote:
> > +	/*
> > +	 * TDX_FIELD_CPUID_CONFIG_LEAVES and TDX_FIELD_CPUID_CONFIG_VALUES
> > +	 * give the field ID of each array's first element. The remaining
> > +	 * elements follow consecutively, in the order they appear in the
> > +	 * structure.
> > +	 */
> > +	for (i = 0; i < td_conf->num_cpuid_config; i++) {
> > +		ret = read_sys_metadata_field(TDX_FIELD_CPUID_CONFIG_LEAVES + i,
> > +					      &td_conf->cpuid_config_leaves[i]);
> > +		if (ret)
> > +			return ret;
> > +
> > +		/* Each config has two u64s of CPUID values. */
> > +		for (j = 0; j < 2; j++) {
> > +			ret = read_sys_metadata_field(
> > +				TDX_FIELD_CPUID_CONFIG_VALUES + i * 2 + j,
> > +				&td_conf->cpuid_config_values[i][j]);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
>
> The comments are not strictly required, but probably fit into turning this thing
> into "hand coding". I think it's a nice addition. I wonder if we should make the
> "i * 2 + j" a bit clearer though?
>
> I'm not against leaving it as is for now. It helps that it doesn't morph too
> much from the code getting moved. But I think the above falls a bit short of
> hand coded quality.

Perhaps this code warrants open coding? The code will end up looking 
like config_values[i][0] ... config_values[i][1] ....