On 3/05/2024 12:12 pm, Edgecombe, Rick P wrote:
On Sat, 2024-03-02 at 00:20 +1300, Kai Huang wrote:
+#define TD_SYSINFO_MAP_TDMR_INFO(_field_id, _member) \
+ TD_SYSINFO_MAP(_field_id, struct tdx_tdmr_sysinfo, _member)
static int get_tdx_tdmr_sysinfo(struct tdx_tdmr_sysinfo *tdmr_sysinfo)
{
/* Map TD_SYSINFO fields into 'struct tdx_tdmr_sysinfo': */
const struct field_mapping fields[] = {
- TD_SYSINFO_MAP(MAX_TDMRS, max_tdmrs),
- TD_SYSINFO_MAP(MAX_RESERVED_PER_TDMR, max_reserved_per_tdmr),
- TD_SYSINFO_MAP(PAMT_4K_ENTRY_SIZE,
pamt_entry_size[TDX_PS_4K]),
- TD_SYSINFO_MAP(PAMT_2M_ENTRY_SIZE,
pamt_entry_size[TDX_PS_2M]),
- TD_SYSINFO_MAP(PAMT_1G_ENTRY_SIZE,
pamt_entry_size[TDX_PS_1G]),
+ TD_SYSINFO_MAP_TDMR_INFO(MAX_TDMRS, max_tdmrs),
+ TD_SYSINFO_MAP_TDMR_INFO(MAX_RESERVED_PER_TDMR,
max_reserved_per_tdmr),
+ TD_SYSINFO_MAP_TDMR_INFO(PAMT_4K_ENTRY_SIZE,
pamt_entry_size[TDX_PS_4K]),
+ TD_SYSINFO_MAP_TDMR_INFO(PAMT_2M_ENTRY_SIZE,
pamt_entry_size[TDX_PS_2M]),
+ TD_SYSINFO_MAP_TDMR_INFO(PAMT_1G_ENTRY_SIZE,
pamt_entry_size[TDX_PS_1G]),
The creation of TD_SYSINFO_MAP_TDMR_INFO part is not strictly needed, but makes
sense in the context of the signature change in read_sys_metadata_field16(). It
might be worth justifying it in the log.
I see your point. How about adding below paragraph to the end of this
changelog?
"
The metadata reading code uses the TD_SYSINFO_MAP() macro to describe
the mapping between the metadata fields and the members of the 'struct
tdx_tdmr_sysinfo'. I.e., it hard-codes the 'struct tdx_tdmr_sysinfo'
inside the macro.
As part of unbinding metadata read with 'struct tdx_tdmr_sysinfo', the
TD_SYSINFO_MAP() macro needs to be changed to additionally take the
structure as argument so it can accept any structure. That would make
the current code to read TDMR related metadata fields longer if using
TD_SYSINFO_MAP() directly.
Define a wrapper macro for reading TDMR related metadata fields to make
the code shorter.
"
By typing, it reminds me that I kinda need to learn how to separate the
"high level design" vs "low level implementation details". I think the
latter can be seen easily in the code, and probably can be avoided in
the changelog.
I am not sure whether adding the TD_SYSINFO_MAP_TDMR_INFO() macro belong
to which category, especially when I needed a lot text to justify this
change (thus I wonder whether it is worth to do).
Or any shorter version that you can suggest?
Thanks.
