Huang, Kai wrote: [..] > > The unrolled loop is the same amount of work as maintaining @fields. > > Hi Dan, > > Thanks for the feedback. > > AFAICT Dave didn't like this way: > > https://lore.kernel.org/lkml/cover.1699527082.git.kai.huang@xxxxxxxxx/T/#me6f615d7845215c278753b57a0bce1162960209d I agree with Dave that the original was unreadable. However, I also think he glossed over the loss of type-safety and the silliness of defining an array to precisely map fields only to turn around and do a runtime check that the statically defined array was filled out correctly. So I think lets solve the readability problem *and* make the array definition identical in appearance to unrolled type-safe execution, something like (UNTESTED!): diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c index 4e2b2e2ac9f9..a177fb7332ae 100644 --- a/arch/x86/virt/vmx/tdx/tdx.c +++ b/arch/x86/virt/vmx/tdx/tdx.c @@ -270,60 +270,42 @@ static int read_sys_metadata_field(u64 field_id, u64 *data) return 0; } -static int read_sys_metadata_field16(u64 field_id, - int offset, - struct tdx_tdmr_sysinfo *ts) +static int read_sys_metadata_field16(u64 field_id, u16 *val) { - u16 *ts_member = ((void *)ts) + offset; u64 tmp; int ret; - if (WARN_ON_ONCE(MD_FIELD_ID_ELE_SIZE_CODE(field_id) != - MD_FIELD_ID_ELE_SIZE_16BIT)) - return -EINVAL; - ret = read_sys_metadata_field(field_id, &tmp); if (ret) return ret; - *ts_member = tmp; + *val = tmp; return 0; } -struct field_mapping { - u64 field_id; - int offset; -}; - -#define TD_SYSINFO_MAP(_field_id, _offset) \ - { .field_id = MD_FIELD_ID_##_field_id, \ - .offset = offsetof(struct tdx_tdmr_sysinfo, _offset) } - -/* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */ -static const struct field_mapping fields[] = { - TD_SYSINFO_MAP(MAX_TDMRS, max_tdmrs), - TD_SYSINFO_MAP(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr), - TD_SYSINFO_MAP(PAMT_4K_ENTRY_SIZE, pamt_entry_size[TDX_PS_4K]), - TD_SYSINFO_MAP(PAMT_2M_ENTRY_SIZE, pamt_entry_size[TDX_PS_2M]), - TD_SYSINFO_MAP(PAMT_1G_ENTRY_SIZE, pamt_entry_size[TDX_PS_1G]), -}; - -static int get_tdx_tdmr_sysinfo(struct tdx_tdmr_sysinfo *tdmr_sysinfo) +/* + * Assumes locally defined @ret and @ts to convey the error code and the + * 'struct tdx_tdmr_sysinfo' instance to fill out + */ +#define TD_SYSINFO_MAP(_field_id, _offset) \ + ({ \ + if (ret == 0) \ + ret = read_sys_metadata_field16( \ + MD_FIELD_ID_##_field_id, &ts->_offset); \ + }) + +static int get_tdx_tdmr_sysinfo(struct tdx_tdmr_sysinfo *ts) { - int ret; - int i; + int ret = 0; - /* Populate 'tdmr_sysinfo' fields using the mapping structure above: */ - for (i = 0; i < ARRAY_SIZE(fields); i++) { - ret = read_sys_metadata_field16(fields[i].field_id, - fields[i].offset, - tdmr_sysinfo); - if (ret) - return ret; - } + TD_SYSINFO_MAP(MAX_TDMRS, max_tdmrs); + TD_SYSINFO_MAP(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr); + TD_SYSINFO_MAP(PAMT_4K_ENTRY_SIZE, pamt_entry_size[TDX_PS_4K]); + TD_SYSINFO_MAP(PAMT_2M_ENTRY_SIZE, pamt_entry_size[TDX_PS_2M]); + TD_SYSINFO_MAP(PAMT_1G_ENTRY_SIZE, pamt_entry_size[TDX_PS_1G]); - return 0; + return ret; } /* Calculate the actual TDMR size */
