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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 17 Jun 2024 18:54:38 +0800
From: Chao Gao <chao.gao@...el.com>
To: Kai Huang <kai.huang@...el.com>
CC: <linux-kernel@...r.kernel.org>, <x86@...nel.org>, <dave.hansen@...el.com>,
	<dan.j.williams@...el.com>, <kirill.shutemov@...ux.intel.com>,
	<rick.p.edgecombe@...el.com>, <peterz@...radead.org>, <tglx@...utronix.de>,
	<bp@...en8.de>, <mingo@...hat.com>, <hpa@...or.com>, <seanjc@...gle.com>,
	<pbonzini@...hat.com>, <kvm@...r.kernel.org>, <isaku.yamahata@...el.com>,
	<binbin.wu@...ux.intel.com>
Subject: Re: [PATCH 8/9] x86/virt/tdx: Exclude memory region hole within CMR
 as TDMR's reserved area

>+/* 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);