diff mbox series

[v8,11/16] x86/virt/tdx: Designate reserved areas for all TDMRs

Message ID 27dcd2781a450b3f77a2aec833de6a3669bc0fb8.1670566861.git.kai.huang@intel.com (mailing list archive)
State New
Headers show
Series TDX host kernel support | expand

Commit Message

Kai Huang Dec. 9, 2022, 6:52 a.m. UTC
As the last step of constructing TDMRs, populate reserved areas for all
TDMRs.  For each TDMR, put all memory holes within this TDMR to the
reserved areas.  And for all PAMTs which overlap with this TDMR, put
all the overlapping parts to reserved areas too.

Reviewed-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
---

v7 -> v8: (Dave)
 - "set_up" -> "populate" in function name change (Dave).
 - Improved comment suggested by Dave.
 - Other changes due to 'struct tdmr_info_list'.

v6 -> v7:
 - No change.

v5 -> v6:
 - Rebase due to using 'tdx_memblock' instead of memblock.
 - Split tdmr_set_up_rsvd_areas() into two functions to handle memory
   hole and PAMT respectively.
 - Added Isaku's Reviewed-by.

---
 arch/x86/virt/vmx/tdx/tdx.c | 213 ++++++++++++++++++++++++++++++++++--
 1 file changed, 205 insertions(+), 8 deletions(-)

Comments

Dave Hansen Jan. 6, 2023, 10:07 p.m. UTC | #1
On 12/8/22 22:52, Kai Huang wrote:
> +static int tdmr_add_rsvd_area(struct tdmr_info *tdmr, int *p_idx, u64 addr,
> +			      u64 size, u16 max_reserved_per_tdmr)
> +{
> +	struct tdmr_reserved_area *rsvd_areas = tdmr->reserved_areas;
> +	int idx = *p_idx;
> +
> +	/* Reserved area must be 4K aligned in offset and size */
> +	if (WARN_ON(addr & ~PAGE_MASK || size & ~PAGE_MASK))
> +		return -EINVAL;
> +
> +	if (idx >= max_reserved_per_tdmr)
> +		return -E2BIG;
> +
> +	rsvd_areas[idx].offset = addr - tdmr->base;
> +	rsvd_areas[idx].size = size;
> +
> +	*p_idx = idx + 1;
> +
> +	return 0;
> +}

It's probably worth at least a comment here to say:

	/*
	 * Consume one reserved area per call.  Make no effort to
	 * optimize or reduce the number of reserved areas which are
	 * consumed by contiguous reserved areas, for instance.
	 */

I think the -E2BIG is also wrong.  It should be ENOSPC.  I'd also add a
pr_warn() there.  Especially with how lazy this whole thing is, I can
start to see how the reserved areas might be exhausted.  Let's be kind
to our future selves and make the error (and the fix) easier to find.

It's probably also worth noting *somewhere* that there's a balance to be
had between TDMRs and reserved areas.  A system that is running out of
reserved areas in a TDMR could split a TDMR to get more reserved areas.
A system that has run out of TDMRs could relatively easily coalesce two
adjacent TDMRs (before the PAMTs are allocated) and use a reserved area
if there was a gap between them.

I'm *really* close to acking this patch once those are fixed up.
Kai Huang Jan. 10, 2023, 1:19 a.m. UTC | #2
On Fri, 2023-01-06 at 14:07 -0800, Dave Hansen wrote:
> On 12/8/22 22:52, Kai Huang wrote:
> > +static int tdmr_add_rsvd_area(struct tdmr_info *tdmr, int *p_idx, u64 addr,
> > +			      u64 size, u16 max_reserved_per_tdmr)
> > +{
> > +	struct tdmr_reserved_area *rsvd_areas = tdmr->reserved_areas;
> > +	int idx = *p_idx;
> > +
> > +	/* Reserved area must be 4K aligned in offset and size */
> > +	if (WARN_ON(addr & ~PAGE_MASK || size & ~PAGE_MASK))
> > +		return -EINVAL;
> > +
> > +	if (idx >= max_reserved_per_tdmr)
> > +		return -E2BIG;
> > +
> > +	rsvd_areas[idx].offset = addr - tdmr->base;
> > +	rsvd_areas[idx].size = size;
> > +
> > +	*p_idx = idx + 1;
> > +
> > +	return 0;
> > +}
> 
> It's probably worth at least a comment here to say:
> 
> 	/*
> 	 * Consume one reserved area per call.  Make no effort to
> 	 * optimize or reduce the number of reserved areas which are
> 	 * consumed by contiguous reserved areas, for instance.
> 	 */

I'll add this comment before the code to set up rsvd_areas[idx].

> 
> I think the -E2BIG is also wrong.  It should be ENOSPC.  I'd also add a
> pr_warn() there.  Especially with how lazy this whole thing is, I can
> start to see how the reserved areas might be exhausted.  Let's be kind
> to our future selves and make the error (and the fix) easier to find.

Yes agreed.  Will change to -ENOSPC and add pr_warn().

> It's probably also worth noting *somewhere* that there's a balance to be
> had between TDMRs and reserved areas.  A system that is running out of
> reserved areas in a TDMR could split a TDMR to get more reserved areas.
> A system that has run out of TDMRs could relatively easily coalesce two
> adjacent TDMRs (before the PAMTs are allocated) and use a reserved area
> if there was a gap between them.

We can add above to the changelog of this patch, or the patch 09 ("x86/virt/tdx:
Fill out TDMRs to cover all TDX memory regions").  The latter perhaps is better
since that patch is the first place where the balance of TDMRs and reserved
areas is related.

What is your suggestion?

> 
> I'm *really* close to acking this patch once those are fixed up.
Dave Hansen Jan. 10, 2023, 1:22 a.m. UTC | #3
On 1/9/23 17:19, Huang, Kai wrote:
>> It's probably also worth noting *somewhere* that there's a balance to be
>> had between TDMRs and reserved areas.  A system that is running out of
>> reserved areas in a TDMR could split a TDMR to get more reserved areas.
>> A system that has run out of TDMRs could relatively easily coalesce two
>> adjacent TDMRs (before the PAMTs are allocated) and use a reserved area
>> if there was a gap between them.
> We can add above to the changelog of this patch, or the patch 09 ("x86/virt/tdx:
> Fill out TDMRs to cover all TDX memory regions").  The latter perhaps is better
> since that patch is the first place where the balance of TDMRs and reserved
> areas is related.
> 
> What is your suggestion?

Just put it close to the code that actually hits the problem so the
potential solution is close at hand to whoever hits the problem.
Kai Huang Jan. 10, 2023, 11:01 a.m. UTC | #4
On Tue, 2023-01-10 at 01:19 +0000, Huang, Kai wrote:
> > I think the -E2BIG is also wrong.  It should be ENOSPC.  I'd also add a
> > pr_warn() there.  Especially with how lazy this whole thing is, I can
> > start to see how the reserved areas might be exhausted.  Let's be kind
> > to our future selves and make the error (and the fix) easier to find.
> 
> Yes agreed.  Will change to -ENOSPC and add pr_warn().

Btw, in patch ("x86/virt/tdx: Fill out TDMRs to cover all TDX memory regions"),
when there are too many TDMRs, I suppose I should also return -ENOSPC instead of
-E2BIG?
Kai Huang Jan. 10, 2023, 11:01 a.m. UTC | #5
On Mon, 2023-01-09 at 17:22 -0800, Dave Hansen wrote:
> On 1/9/23 17:19, Huang, Kai wrote:
> > > It's probably also worth noting *somewhere* that there's a balance to be
> > > had between TDMRs and reserved areas.  A system that is running out of
> > > reserved areas in a TDMR could split a TDMR to get more reserved areas.
> > > A system that has run out of TDMRs could relatively easily coalesce two
> > > adjacent TDMRs (before the PAMTs are allocated) and use a reserved area
> > > if there was a gap between them.
> > We can add above to the changelog of this patch, or the patch 09 ("x86/virt/tdx:
> > Fill out TDMRs to cover all TDX memory regions").  The latter perhaps is better
> > since that patch is the first place where the balance of TDMRs and reserved
> > areas is related.
> > 
> > What is your suggestion?
> 
> Just put it close to the code that actually hits the problem so the
> potential solution is close at hand to whoever hits the problem.
> 

Sorry to double check: the code which hits the problem is the 'if (idx >=
max_reserved_per_tdmr)' check in tdmr_add_rsvd_area(), so I think I can add
right before this check?
Dave Hansen Jan. 10, 2023, 3:17 p.m. UTC | #6
On 1/10/23 03:01, Huang, Kai wrote:
> On Tue, 2023-01-10 at 01:19 +0000, Huang, Kai wrote:
>>> I think the -E2BIG is also wrong.  It should be ENOSPC.  I'd also add a
>>> pr_warn() there.  Especially with how lazy this whole thing is, I can
>>> start to see how the reserved areas might be exhausted.  Let's be kind
>>> to our future selves and make the error (and the fix) easier to find.
>> Yes agreed.  Will change to -ENOSPC and add pr_warn().
> Btw, in patch ("x86/virt/tdx: Fill out TDMRs to cover all TDX memory regions"),
> when there are too many TDMRs, I suppose I should also return -ENOSPC instead of
> -E2BIG?

Yes.
Dave Hansen Jan. 10, 2023, 3:19 p.m. UTC | #7
On 1/10/23 03:01, Huang, Kai wrote:
> On Mon, 2023-01-09 at 17:22 -0800, Dave Hansen wrote:
>> On 1/9/23 17:19, Huang, Kai wrote:
>>>> It's probably also worth noting *somewhere* that there's a balance to be
>>>> had between TDMRs and reserved areas.  A system that is running out of
>>>> reserved areas in a TDMR could split a TDMR to get more reserved areas.
>>>> A system that has run out of TDMRs could relatively easily coalesce two
>>>> adjacent TDMRs (before the PAMTs are allocated) and use a reserved area
>>>> if there was a gap between them.
>>> We can add above to the changelog of this patch, or the patch 09 ("x86/virt/tdx:
>>> Fill out TDMRs to cover all TDX memory regions").  The latter perhaps is better
>>> since that patch is the first place where the balance of TDMRs and reserved
>>> areas is related.
>>>
>>> What is your suggestion?
>> Just put it close to the code that actually hits the problem so the
>> potential solution is close at hand to whoever hits the problem.
>>
> Sorry to double check: the code which hits the problem is the 'if (idx >=
> max_reserved_per_tdmr)' check in tdmr_add_rsvd_area(), so I think I can add
> right before this check?

Please just hack together how you think it should look and either reply
with an updated patch, or paste the relevant code snippet in your reply.
 That'll keep me from having to go chase this code back down.
Kai Huang Jan. 11, 2023, 10:57 a.m. UTC | #8
On Tue, 2023-01-10 at 07:19 -0800, Dave Hansen wrote:
> On 1/10/23 03:01, Huang, Kai wrote:
> > On Mon, 2023-01-09 at 17:22 -0800, Dave Hansen wrote:
> > > On 1/9/23 17:19, Huang, Kai wrote:
> > > > > It's probably also worth noting *somewhere* that there's a balance to be
> > > > > had between TDMRs and reserved areas.  A system that is running out of
> > > > > reserved areas in a TDMR could split a TDMR to get more reserved areas.
> > > > > A system that has run out of TDMRs could relatively easily coalesce two
> > > > > adjacent TDMRs (before the PAMTs are allocated) and use a reserved area
> > > > > if there was a gap between them.
> > > > We can add above to the changelog of this patch, or the patch 09 ("x86/virt/tdx:
> > > > Fill out TDMRs to cover all TDX memory regions").  The latter perhaps is better
> > > > since that patch is the first place where the balance of TDMRs and reserved
> > > > areas is related.
> > > > 
> > > > What is your suggestion?
> > > Just put it close to the code that actually hits the problem so the
> > > potential solution is close at hand to whoever hits the problem.
> > > 
> > Sorry to double check: the code which hits the problem is the 'if (idx >=
> > max_reserved_per_tdmr)' check in tdmr_add_rsvd_area(), so I think I can add
> > right before this check?
> 
> Please just hack together how you think it should look and either reply
> with an updated patch, or paste the relevant code snippet in your reply.
>  That'll keep me from having to go chase this code back down.
> 

Thanks for the tip.  How about below?

static int tdmr_add_rsvd_area(struct tdmr_info *tdmr, int *p_idx, u64 addr,
                              u64 size, u16 max_reserved_per_tdmr)
{
        struct tdmr_reserved_area *rsvd_areas = tdmr->reserved_areas;
        int idx = *p_idx;

        /* Reserved area must be 4K aligned in offset and size */
        if (WARN_ON(addr & ~PAGE_MASK || size & ~PAGE_MASK))
                return -EINVAL;

        /*
         * The TDX module supports only limited number of TDMRs and
         * limited number of reserved areas for each TDMR.  There's a
         * balance to be had between TDMRs and reserved areas.  A system
         * that is running out of reserved areas in a TDMR could split a
         * TDMR to get more reserved areas.  A system that has run out
         * of TDMRs could relatively easily coalesce two adjacent TDMRs
         * (before the PAMTs are allocated) and use a reserved area if
         * there was a gap between them.
         */
        if (idx >= max_reserved_per_tdmr) {
                pr_warn("too many reserved areas for TDMR [0x%llx, 0x%llx)\n",
                                tdmr->base, tdmr_end(tdmr));
                return -ENOSPC;
        }

        /*
         * Consume one reserved area per call.  Make no effort to
         * optimize or reduce the number of reserved areas which are
         * consumed by contiguous reserved areas, for instance.
         */
        rsvd_areas[idx].offset = addr - tdmr->base;
        rsvd_areas[idx].size = size;

        *p_idx = idx + 1;

        return 0;
}
Dave Hansen Jan. 11, 2023, 4:16 p.m. UTC | #9
On 1/11/23 02:57, Huang, Kai wrote:
> On Tue, 2023-01-10 at 07:19 -0800, Dave Hansen wrote:
>> On 1/10/23 03:01, Huang, Kai wrote:
>>> On Mon, 2023-01-09 at 17:22 -0800, Dave Hansen wrote:
>>>> On 1/9/23 17:19, Huang, Kai wrote:
>>>>>> It's probably also worth noting *somewhere* that there's a balance to be
>>>>>> had between TDMRs and reserved areas.  A system that is running out of
>>>>>> reserved areas in a TDMR could split a TDMR to get more reserved areas.
>>>>>> A system that has run out of TDMRs could relatively easily coalesce two
>>>>>> adjacent TDMRs (before the PAMTs are allocated) and use a reserved area
>>>>>> if there was a gap between them.
>>>>> We can add above to the changelog of this patch, or the patch 09 ("x86/virt/tdx:
>>>>> Fill out TDMRs to cover all TDX memory regions").  The latter perhaps is better
>>>>> since that patch is the first place where the balance of TDMRs and reserved
>>>>> areas is related.
>>>>>
>>>>> What is your suggestion?
>>>> Just put it close to the code that actually hits the problem so the
>>>> potential solution is close at hand to whoever hits the problem.
>>>>
>>> Sorry to double check: the code which hits the problem is the 'if (idx >=
>>> max_reserved_per_tdmr)' check in tdmr_add_rsvd_area(), so I think I can add
>>> right before this check?
>>
>> Please just hack together how you think it should look and either reply
>> with an updated patch, or paste the relevant code snippet in your reply.
>>  That'll keep me from having to go chase this code back down.
>>
> 
> Thanks for the tip.  How about below?
> 
> static int tdmr_add_rsvd_area(struct tdmr_info *tdmr, int *p_idx, u64 addr,
>                               u64 size, u16 max_reserved_per_tdmr)
> {
>         struct tdmr_reserved_area *rsvd_areas = tdmr->reserved_areas;
>         int idx = *p_idx;
> 
>         /* Reserved area must be 4K aligned in offset and size */
>         if (WARN_ON(addr & ~PAGE_MASK || size & ~PAGE_MASK))
>                 return -EINVAL;
> 
>         /*
>          * The TDX module supports only limited number of TDMRs and
>          * limited number of reserved areas for each TDMR.  There's a
>          * balance to be had between TDMRs a2nd reserved areas.  A system
>          * that is running out of reserved areas in a TDMR could split a
>          * TDMR to get more reserved areas.  A system that has run out
>          * of TDMRs could relatively easily coalesce two adjacent TDMRs
>          * (before the PAMTs are allocated) and use a reserved area if
>          * there was a gap between them.
>          */
>         if (idx >= max_reserved_per_tdmr) {
>                 pr_warn("too many reserved areas for TDMR [0x%llx, 0x%llx)\n",
>                                 tdmr->base, tdmr_end(tdmr));
>                 return -ENOSPC;
>         }

This isn't really converging on a solution.  At this point, I just see
my verbatim text being copied and pasted into these functions without
really anything additional.

This comment, for instance, just blathers about what could be done but
doesn't actually explain what it is doing here.

But, again, this isn't converging.  It's just thrashing and not getting
any better.  I guess I'll just fix it up best I can when I apply it.
Kai Huang Jan. 11, 2023, 10:10 p.m. UTC | #10
On Wed, 2023-01-11 at 08:16 -0800, Dave Hansen wrote:
> On 1/11/23 02:57, Huang, Kai wrote:
> > On Tue, 2023-01-10 at 07:19 -0800, Dave Hansen wrote:
> > > On 1/10/23 03:01, Huang, Kai wrote:
> > > > On Mon, 2023-01-09 at 17:22 -0800, Dave Hansen wrote:
> > > > > On 1/9/23 17:19, Huang, Kai wrote:
> > > > > > > It's probably also worth noting *somewhere* that there's a balance to be
> > > > > > > had between TDMRs and reserved areas.  A system that is running out of
> > > > > > > reserved areas in a TDMR could split a TDMR to get more reserved areas.
> > > > > > > A system that has run out of TDMRs could relatively easily coalesce two
> > > > > > > adjacent TDMRs (before the PAMTs are allocated) and use a reserved area
> > > > > > > if there was a gap between them.
> > > > > > We can add above to the changelog of this patch, or the patch 09 ("x86/virt/tdx:
> > > > > > Fill out TDMRs to cover all TDX memory regions").  The latter perhaps is better
> > > > > > since that patch is the first place where the balance of TDMRs and reserved
> > > > > > areas is related.
> > > > > > 
> > > > > > What is your suggestion?
> > > > > Just put it close to the code that actually hits the problem so the
> > > > > potential solution is close at hand to whoever hits the problem.
> > > > > 
> > > > Sorry to double check: the code which hits the problem is the 'if (idx >=
> > > > max_reserved_per_tdmr)' check in tdmr_add_rsvd_area(), so I think I can add
> > > > right before this check?
> > > 
> > > Please just hack together how you think it should look and either reply
> > > with an updated patch, or paste the relevant code snippet in your reply.
> > >  That'll keep me from having to go chase this code back down.
> > > 
> > 
> > Thanks for the tip.  How about below?
> > 
> > static int tdmr_add_rsvd_area(struct tdmr_info *tdmr, int *p_idx, u64 addr,
> >                               u64 size, u16 max_reserved_per_tdmr)
> > {
> >         struct tdmr_reserved_area *rsvd_areas = tdmr->reserved_areas;
> >         int idx = *p_idx;
> > 
> >         /* Reserved area must be 4K aligned in offset and size */
> >         if (WARN_ON(addr & ~PAGE_MASK || size & ~PAGE_MASK))
> >                 return -EINVAL;
> > 
> >         /*
> >          * The TDX module supports only limited number of TDMRs and
> >          * limited number of reserved areas for each TDMR.  There's a
> >          * balance to be had between TDMRs a2nd reserved areas.  A system
> >          * that is running out of reserved areas in a TDMR could split a
> >          * TDMR to get more reserved areas.  A system that has run out
> >          * of TDMRs could relatively easily coalesce two adjacent TDMRs
> >          * (before the PAMTs are allocated) and use a reserved area if
> >          * there was a gap between them.
> >          */
> >         if (idx >= max_reserved_per_tdmr) {
> >                 pr_warn("too many reserved areas for TDMR [0x%llx, 0x%llx)\n",
> >                                 tdmr->base, tdmr_end(tdmr));
> >                 return -ENOSPC;
> >         }
> 
> This isn't really converging on a solution.  At this point, I just see
> my verbatim text being copied and pasted into these functions without
> really anything additional.
> 
> This comment, for instance, just blathers about what could be done but
> doesn't actually explain what it is doing here.
> 
> But, again, this isn't converging.  It's just thrashing and not getting
> any better.  I guess I'll just fix it up best I can when I apply it.

Appreciate your help!
diff mbox series

Patch

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index cf970a783f1f..620b35e2a61b 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -21,6 +21,7 @@ 
 #include <linux/sizes.h>
 #include <linux/pfn.h>
 #include <linux/align.h>
+#include <linux/sort.h>
 #include <asm/pgtable_types.h>
 #include <asm/msr.h>
 #include <asm/tdx.h>
@@ -687,6 +688,202 @@  static unsigned long tdmrs_count_pamt_pages(struct tdmr_info_list *tdmr_list)
 	return pamt_npages;
 }
 
+static int tdmr_add_rsvd_area(struct tdmr_info *tdmr, int *p_idx, u64 addr,
+			      u64 size, u16 max_reserved_per_tdmr)
+{
+	struct tdmr_reserved_area *rsvd_areas = tdmr->reserved_areas;
+	int idx = *p_idx;
+
+	/* Reserved area must be 4K aligned in offset and size */
+	if (WARN_ON(addr & ~PAGE_MASK || size & ~PAGE_MASK))
+		return -EINVAL;
+
+	if (idx >= max_reserved_per_tdmr)
+		return -E2BIG;
+
+	rsvd_areas[idx].offset = addr - tdmr->base;
+	rsvd_areas[idx].size = size;
+
+	*p_idx = idx + 1;
+
+	return 0;
+}
+
+/*
+ * Go through @tmb_list to find holes between memory areas.  If any of
+ * those holes fall within @tdmr, set up a TDMR reserved area to cover
+ * the hole.
+ */
+static int tdmr_populate_rsvd_holes(struct list_head *tmb_list,
+				    struct tdmr_info *tdmr,
+				    int *rsvd_idx,
+				    u16 max_reserved_per_tdmr)
+{
+	struct tdx_memblock *tmb;
+	u64 prev_end;
+	int ret;
+
+	/*
+	 * Start looking for reserved blocks at the
+	 * beginning of the TDMR.
+	 */
+	prev_end = tdmr->base;
+	list_for_each_entry(tmb, tmb_list, list) {
+		u64 start, end;
+
+		start = PFN_PHYS(tmb->start_pfn);
+		end   = PFN_PHYS(tmb->end_pfn);
+
+		/* Break if this region is after the TDMR */
+		if (start >= tdmr_end(tdmr))
+			break;
+
+		/* Exclude regions before this TDMR */
+		if (end < tdmr->base)
+			continue;
+
+		/*
+		 * Skip over memory areas that
+		 * have already been dealt with.
+		 */
+		if (start <= prev_end) {
+			prev_end = end;
+			continue;
+		}
+
+		/* Add the hole before this region */
+		ret = tdmr_add_rsvd_area(tdmr, rsvd_idx, prev_end,
+				start - prev_end,
+				max_reserved_per_tdmr);
+		if (ret)
+			return ret;
+
+		prev_end = end;
+	}
+
+	/* Add the hole after the last region if it exists. */
+	if (prev_end < tdmr_end(tdmr)) {
+		ret = tdmr_add_rsvd_area(tdmr, rsvd_idx, prev_end,
+				tdmr_end(tdmr) - prev_end,
+				max_reserved_per_tdmr);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/*
+ * Go through @tdmr_list to find all PAMTs.  If any of those PAMTs
+ * overlaps with @tdmr, set up a TDMR reserved area to cover the
+ * overlapping part.
+ */
+static int tdmr_populate_rsvd_pamts(struct tdmr_info_list *tdmr_list,
+				    struct tdmr_info *tdmr,
+				    int *rsvd_idx,
+				    u16 max_reserved_per_tdmr)
+{
+	int i, ret;
+
+	for (i = 0; i < tdmr_list->nr_tdmrs; i++) {
+		struct tdmr_info *tmp = tdmr_entry(tdmr_list, i);
+		unsigned long pamt_start_pfn, pamt_npages;
+		u64 pamt_start, pamt_end;
+
+		tdmr_get_pamt(tmp, &pamt_start_pfn, &pamt_npages);
+		/* Each TDMR must already have PAMT allocated */
+		WARN_ON_ONCE(!pamt_npages || !pamt_start_pfn);
+
+		pamt_start = PFN_PHYS(pamt_start_pfn);
+		pamt_end   = PFN_PHYS(pamt_start_pfn + pamt_npages);
+
+		/* Skip PAMTs outside of the given TDMR */
+		if ((pamt_end <= tdmr->base) ||
+				(pamt_start >= tdmr_end(tdmr)))
+			continue;
+
+		/* Only mark the part within the TDMR as reserved */
+		if (pamt_start < tdmr->base)
+			pamt_start = tdmr->base;
+		if (pamt_end > tdmr_end(tdmr))
+			pamt_end = tdmr_end(tdmr);
+
+		ret = tdmr_add_rsvd_area(tdmr, rsvd_idx, pamt_start,
+				pamt_end - pamt_start,
+				max_reserved_per_tdmr);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/* Compare function called by sort() for TDMR reserved areas */
+static int rsvd_area_cmp_func(const void *a, const void *b)
+{
+	struct tdmr_reserved_area *r1 = (struct tdmr_reserved_area *)a;
+	struct tdmr_reserved_area *r2 = (struct tdmr_reserved_area *)b;
+
+	if (r1->offset + r1->size <= r2->offset)
+		return -1;
+	if (r1->offset >= r2->offset + r2->size)
+		return 1;
+
+	/* Reserved areas cannot overlap.  The caller must guarantee. */
+	WARN_ON_ONCE(1);
+	return -1;
+}
+
+/*
+ * Populate reserved areas for the given @tdmr, including memory holes
+ * (via @tmb_list) and PAMTs (via @tdmr_list).
+ */
+static int tdmr_populate_rsvd_areas(struct tdmr_info *tdmr,
+				    struct list_head *tmb_list,
+				    struct tdmr_info_list *tdmr_list,
+				    u16 max_reserved_per_tdmr)
+{
+	int ret, rsvd_idx = 0;
+
+	ret = tdmr_populate_rsvd_holes(tmb_list, tdmr, &rsvd_idx,
+			max_reserved_per_tdmr);
+	if (ret)
+		return ret;
+
+	ret = tdmr_populate_rsvd_pamts(tdmr_list, tdmr, &rsvd_idx,
+			max_reserved_per_tdmr);
+	if (ret)
+		return ret;
+
+	/* TDX requires reserved areas listed in address ascending order */
+	sort(tdmr->reserved_areas, rsvd_idx, sizeof(struct tdmr_reserved_area),
+			rsvd_area_cmp_func, NULL);
+
+	return 0;
+}
+
+/*
+ * Populate reserved areas for all TDMRs in @tdmr_list, including memory
+ * holes (via @tmb_list) and PAMTs.
+ */
+static int tdmrs_populate_rsvd_areas_all(struct tdmr_info_list *tdmr_list,
+					 struct list_head *tmb_list,
+					 u16 max_reserved_per_tdmr)
+{
+	int i;
+
+	for (i = 0; i < tdmr_list->nr_tdmrs; i++) {
+		int ret;
+
+		ret = tdmr_populate_rsvd_areas(tdmr_entry(tdmr_list, i),
+				tmb_list, tdmr_list, max_reserved_per_tdmr);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 /*
  * Construct a list of TDMRs on the preallocated space in @tdmr_list
  * to cover all TDX memory regions in @tmb_list based on the TDX module
@@ -706,14 +903,14 @@  static int construct_tdmrs(struct list_head *tmb_list,
 			sysinfo->pamt_entry_size);
 	if (ret)
 		goto err;
-	/*
-	 * TODO:
-	 *
-	 *  - Designate reserved areas for each TDMR.
-	 *
-	 * Return -EINVAL until constructing TDMRs is done
-	 */
-	ret = -EINVAL;
+
+	ret = tdmrs_populate_rsvd_areas_all(tdmr_list, tmb_list,
+			sysinfo->max_reserved_per_tdmr);
+	if (ret)
+		goto err_free_pamts;
+
+	return 0;
+err_free_pamts:
 	tdmrs_free_pamt_all(tdmr_list);
 err:
 	return ret;