On Thu, 2023-11-09 at 15:29 -0800, Dave Hansen wrote: > On 11/9/23 03:55, Kai Huang wrote: > ...> + ret = read_sys_metadata_field16(MD_FIELD_ID_MAX_TDMRS, > > + &tdmr_sysinfo->max_tdmrs); > > + if (ret) > > + return ret; > > + > > + ret = read_sys_metadata_field16(MD_FIELD_ID_MAX_RESERVED_PER_TDMR, > > + &tdmr_sysinfo->max_reserved_per_tdmr); > > + if (ret) > > + return ret; > > + > > + ret = read_sys_metadata_field16(MD_FIELD_ID_PAMT_4K_ENTRY_SIZE, > > + &tdmr_sysinfo->pamt_entry_size[TDX_PS_4K]); > > + if (ret) > > + return ret; > > + > > + ret = read_sys_metadata_field16(MD_FIELD_ID_PAMT_2M_ENTRY_SIZE, > > + &tdmr_sysinfo->pamt_entry_size[TDX_PS_2M]); > > + if (ret) > > + return ret; > > + > > + return read_sys_metadata_field16(MD_FIELD_ID_PAMT_1G_ENTRY_SIZE, > > + &tdmr_sysinfo->pamt_entry_size[TDX_PS_1G]); > > +} > > I kinda despise how this looks. It's impossible to read. > > I'd much rather do something like the attached where you just map the > field number to a structure member. Note that this kind of structure > could also be converted to leverage the bulk metadata query in the future. > > Any objections to doing something more like the attached completely > untested patch? Hi Dave, No objection and thanks! I've just tested with your diff I can successfully initialize the TDX module.