> > > +#define build_sysmd_read(_size) \ > > +static int __read_sys_metadata_field##_size(u64 field_id, u##_size *val) \ > > +{ \ > > + u64 tmp; \ > > + int ret; \ > > + \ > > + ret = tdh_sys_rd(field_id, &tmp); \ > > + if (ret) \ > > + return ret; \ > > + \ > > + *val = tmp; \ > > + \ > > + return 0; \ > > } > > > > -#define read_sys_metadata_field16(_field_id, _val) \ > > +build_sysmd_read(16) > > nit: Generally the unwritten convention for this kind of macro > definition is to capitalize them and be of the from: > > DEFINE_xxxxx - similar to how event classes are defined. > > perhaps naming this macro: > > DEFINE_TDX_METADATA_READER() ought to be more descriptive, also the > "md" contraction of metadata also seems a bit quirky (at least to me). > > It's not a deal breaker but if there is going to be another posting this > might be something to consider. > Thanks for the comments. I don't have opinion on this. Dan said we can do something like build_mmio_read() macro and this is where build_sysmd_read() came from. Dan used build_tdg_sys_rd() in his patch too: https://lore.kernel.org/all/172618717675.516322.6087817418162288917.stgit@xxxxxxxxxxxxxxxxxxxxxxxxx/ Btw I actually agree that your DEFINE_xx() is more formal thus is better when the macro is used by multiple C files. But here only tdx.c uses it, and IMHO it's also fine to have a informal but shorter name here. Anyway let's see whether other people have anything to say.