Huang, Kai wrote:
>
>
> On 27/09/2024 10:26 am, Hansen, Dave wrote:
> > On 9/26/24 15:22, Huang, Kai wrote:
> >> But Dan commented using typeless 'void *' and 'size' is kinda a step
> >> backwards and we should do something similar to build_mmio_read():
> >
> > Well, void* is typeless, but at least it knows the size in this case.
> > It's not completely aimless. I was thinking of how things like
> > get_user() work.
>
> get_user(x,ptr) only works with simple types:
>
> * @ptr must have pointer-to-simple-variable type, and the result of
> * dereferencing @ptr must be assignable to @x without a cast.
>
> The compiler knows the type of both @x and @(*ptr), so it knows
> type-safety and size to copy.
>
> I think we can eliminate the __read_sys_metadata_field() by implementing
> it as a macro directly and get rid of 'void *' and 'size':
>
> static int tdh_sys_rd(u64 field_id, u64 *val) {}
>
> /* @_valptr must be pointer to u8/u16/u32/u64 */
> #define read_sys_metadata_field(_field_id, _valptr) \
> ({ \
> u64 ___tmp; \
> int ___ret; \
> \
> BUILD_BUG_ON(MD_FIELD_ELE_SIZE(_field_id) != \
> sizeof(*_valptr)); \
> \
> ___ret = tdh_sys_rd(_field_id, &___tmp); \
> \
> *_valptr = ___tmp; \
> ___ret;
> })
>
> It sets *_valptr unconditionally but we can also only do it when ___ret
> is 0.
>
> The caller will need to do:
>
> static int get_tdx_metadata_X_which_is_32bit(...)
> {
> u32 metadata_X;
> int ret;
>
> ret = read_sys_metadata_field(MD_FIELD_ID_X, &metadata_X);
>
> return ret;
> }
>
> I haven't compiled and tested but it seems feasible.
>
> Any comments?
If it works this approach addresses all the concerns I had with getting
the compiler to validate field sizes.
Should be straightforward to put this in a shared location so that it
can optionally use tdg_sys_rd internally.
