Re: [PATCH v15 09/23] x86/virt/tdx: Get module global metadata for module initialization

Huang, Kai
2023-11-10T02:24:09Z

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.