Re: [PATCH 8/9] x86/virt/tdx: Exclude memory region hole within CMR as TDMR's reserved area

Chao Gao
2024-06-17T10:54:55Z

>+/* Return whether a given region [start, end) is a sub-region of any CMR */
>+static bool is_cmr_subregion(struct tdx_sysinfo_cmr_info *cmr_info, u64 start,
>+ u64 end)
>+{
>+ int i;
>+
>+ for (i = 0; i < cmr_info->num_cmrs; i++) {
>+ u64 cmr_base = cmr_info->cmr_base[i];
>+ u64 cmr_size = cmr_info->cmr_size[i];
>+
>+ if (start >= cmr_base && end <= (cmr_base + cmr_size))
>+ return true;
>+ }
>+
>+ return false;
>+}
>+
> /*
> * Go through @tmb_list to find holes between memory areas. If any of

The logic here is:
1. go through @tmb_list to find holes
2. skip a hole if it is in CMRs

I am wondering if the kernel can traverse CMRs directly to find holes. This
way, the new is_cmr_subregion() can be removed. And @tmb_list can be dropped
from a few functions e.g., tdmr_populate_rsvd_holes/areas/areas_all(). So, this
will simplify those functions a bit.

> * those holes fall within @tdmr, set up a TDMR reserved area to cover
>@@ -835,7 +932,8 @@ static int tdmr_add_rsvd_area(struct tdmr_info *tdmr, int *p_idx, u64 addr,
> static int tdmr_populate_rsvd_holes(struct list_head *tmb_list,
> struct tdmr_info *tdmr,
> int *rsvd_idx,
>- u16 max_reserved_per_tdmr)
>+ u16 max_reserved_per_tdmr,
>+ struct tdx_sysinfo_cmr_info *cmr_info)

Maybe this function can accept a pointer to tdx_sysinfo and remove
@max_reserved_per_tdmr and @cmr_info because they are both TDX metadata and
have only one possible combination for a given TDX module. Anyway, I don't have
a strong opinion on this.

> {
> struct tdx_memblock *tmb;
> u64 prev_end;
>@@ -864,10 +962,16 @@ static int tdmr_populate_rsvd_holes(struct list_head *tmb_list,
> * Skip over memory areas that
> * have already been dealt with.
> */
>- if (start <= prev_end) {
>- prev_end = end;
>- continue;
>- }
>+ if (start <= prev_end)
>+ goto next_tmb;
>+
>+ /*
>+ * Found the hole [prev_end, start) before this region.
>+ * Skip the hole if it is within any CMR to reduce the
>+ * consumption of reserved areas.
>+ */
>+ if (is_cmr_subregion(cmr_info, prev_end, start))
>+ goto next_tmb;
>
> /* Add the hole before this region */
> ret = tdmr_add_rsvd_area(tdmr, rsvd_idx, prev_end,
>@@ -876,11 +980,16 @@ static int tdmr_populate_rsvd_holes(struct list_head *tmb_list,
> if (ret)
> return ret;
>
>+next_tmb:
> prev_end = end;
> }
>
>- /* Add the hole after the last region if it exists. */
>- if (prev_end < tdmr_end(tdmr)) {
>+ /*
>+ * Add the hole after the last region if it exists, but skip
>+ * if it is within any CMR.
>+ */
>+ if (prev_end < tdmr_end(tdmr) &&
>+ !is_cmr_subregion(cmr_info, prev_end, tdmr_end(tdmr))) {
> ret = tdmr_add_rsvd_area(tdmr, rsvd_idx, prev_end,
> tdmr_end(tdmr) - prev_end,
> max_reserved_per_tdmr);