Re: [PATCH v3 3/8] x86/virt/tdx: Prepare to support reading other global metadata fields

["Huang, Kai" <kai.huang@intel.com>]

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?