On Fri, 2024-09-06 at 14:34 -0700, Dan Williams wrote:
> Kai Huang wrote:
> > Future changes will need to read more metadata fields with different
> > element sizes. To make the code short, extend the helper to take a
> > 'void *' buffer and the buffer size so it can work with all element
> > sizes.
>
> The whole point was to have compile time type safety for global struct
> member and the read routine. So, no, using 'void *' is a step backwards.
>
> Take some inspiration from build_mmio_read() and just have a macro that
> takes the size as an argument to the macro, not the runtime routine.
>
AFAICT build_mmio_read() macro defines the body of the assembly to read
requested size from a port. But instead of a single macro which can be used
universally for all port reading, the kernel uses build_mmio_read() to define
multiple read functions:
build_mmio_read(readb, "b", unsigned char, "=q", :"memory")
build_mmio_read(readw, "w", unsigned short, "=r", :"memory")
build_mmio_read(readl, "l", unsigned int, "=r", :"memory")
...
So just to understand correctly, do you mean something like below?
#define build_sysmd_read(_bits) \
static __maybe_unused int \
__read_sys_metadata_field##_bits(u64 field_id, u##_bits *val) \
{ \
u64 tmp; \
int ret; \
\
ret = tdh_sys_rd(field_id, &tmp); \
if (ret) \
return ret; \
*val = tmp; \
return ret; \
}
build_sysmd_read(8)
build_sysmd_read(16)
build_sysmd_read(32)
build_sysmd_read(64)
> The
> macro statically selects the right routine to call and does not need to
> grow a size parameter itself.
Sorry for my ignorance. Do you mean using switch-case statement to select right
routine?
