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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 17/06/2024 10:54 pm, Chao Gao wrote:
+/* 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.

Traversing CMRs to find reserved areas for a given TDMR sounds good to me logically. The whole construct_tdmrs() assumes all TDX memory blocks are fully covered by CMRs anyway.

I'll try this out (validation is a bit tricky because we cannot reproduce this issue internally).


  * 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.

There are pros/cons of the two options. Passing the @cmr_info and @max_reserved_per_tdmr makes this function more clear that it _exactly_ requires these two. Since passing a single @tdx_sysinfo doesn't reduce the function arguments a lot (only one argument, and you have to get the two inside the function anyway), I would keep the current way unless I hear from something different from others.




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux