On 11/15/24 12:20, Rick Edgecombe wrote:
> +struct tdx_td {
> + hpa_t tdr;
> + hpa_t *tdcs;
> +};
This is a step in the right direction because it gives the wrappers some
more type safety.
But an hpa_t is _barely_ better than a u64. If the 'tdr' is a page,
then it needs to be _stored_ as a page:
struct page *tdr_page;
Also, please don't forget to spell these things out:
/* TD root structure: */
struct page *tdr_page;
And the tdcs is an array of pages, right? So it should be:
struct page **tdcs_pages;
Or heck, I _think_ it can theoretically be defined as a variable-length
array:
struct page *tdcs_pages[];
and use the helpers that we have for that.
Putting it all together, you would have this:
struct tdx_td {
/* TD root structure: */
struct page *tdr_page;
int tdcs_nr_pages;
/* TD control structure: */
struct page *tdcs_pages[];
};
That's *MUCH* harder to misuse. It's 100% obvious that you have a
single page, plus a variable-length array of pages. This is all from
just looking at the structure definition.
You know that 'tdr' is not just some random physical address. It's a
whole physical page. It's page-aligned. It was allocated, from the
allocator. It doesn't point to special memory.
Ditto for "hpa_t *tdcs". It's not obvious from the data structure that
it's an array or if it's an array how it got allocated or how large it is.
