Re: [PATCH 4/6] x86/virt/tdx: Add extra memory to TDX module for the extensions

[Xu Yilun]

> > > +	page = alloc_contig_pages(required_pages, GFP_KERNEL, numa_mem_id(),
> > > +				  &node_online_map);
> > 
> > Why contiguous? TDH.EXT.MEM.ADD takes a list of page addresses and the loop
> > below writes every one of them out separately.
> > 
> > alloc_pages_bulk() fits the chunking that is already here, and a short
> > return can be handled per chunk. alloc_contig_pages() isolates and migrates
> > to get its range and fails TDX init outright when it cannot find one. PAMT
> 
> Yeah, this is not the ABI requirement, but the kernel's consideration. A
> brief reasoning in the commit log: avoiding permanent memory fragmentation
> and buddy allocator efficiency loss.
> 
> Also there is some discussion:
> 
> https://lore.kernel.org/all/167d9540-2d9a-4367-bc68-b96494bc4044@xxxxxxxxx/
> 
> TL;DR
>   - The memory will never return to the kernel.
>   - There is chance that this tens of megabytes will fragment tens of
>     gigabytes of memory forever.
>   - The chance of fragmentation is actually low since at boot up, but
>     let the buddy allocator take care of these never-returned memory
>     is not necessary and lowers its efficiency.

Hi Kiryl & David:

I see there is another suggestion that the whole memory adding process
could be a little simpler if we allocate & add pages 4k by 4k [1],
rather than one-time pre-allocation. The concern of this alternative is,
as said above, memory fragmentation.

[1] https://lore.kernel.org/lkml/f48b83feb2ee1d3c88b5a1627cf35b4b282d3f90.camel@xxxxxxxxx/

And I've realized the memory fragmentation discussion is not actually
closed in previous thread [2]. We need more input.

[2] https://lore.kernel.org/all/167d9540-2d9a-4367-bc68-b96494bc4044@xxxxxxxxx/

Let me give a brief overview of the problem:

Intel TDX (Trust Domain Extensions) is a feature for confidential
computing. A secure firmware called "TDX module" runs in an isolated
environment to provide services about security.

In Linux, the host initializes TDX module at boot up time
(subsys_initcall()). During the initialization, the host must donate
tens of mega bytes physical memory (35M ~ 110M in the forseeable
future) to the TDX module. These memory will *never be revoked* cause
the TDX Module initialization is a one way path.

The TDX Module doesn't require this memory be physically contiguous. But
the kernel side concern is if we do PAGE_SIZE allocation, it may
permanently fragment memory regions, stop them from allocating 2M huge
pages. In worst case, ~50G (110M * 512) memory regions affacted.

So is the physically contiguous allocation really a better choice here?
We appreciate inputs from mm folks. Thanks!

Yilun

> 
> > needs it because the TDMR ABI describes each PAMT as base+size. This does
> > not.
> > 
> > > +	if (!page) {
> > > +		ret = -ENOMEM;
> > > +		goto out_free_hpa_list;
> > > +	}
> > > +
> > > +	added_pages = 0;
> > > +	while (added_pages < required_pages) {
> > > +		unsigned int chunk_pages = min(required_pages - added_pages,
> > > +					       TDX_HPA_LIST_MAX_NR_PAGES);
> > > +		struct page *chunk = page + added_pages;
> > > +		unsigned int i;
> > > +
> > > +		for (i = 0; i < chunk_pages; i++)
> > > +			hpa_list->phys[i] = page_to_phys(chunk + i);
> > > +
> > > +		ret = tdx_ext_mem_add(hpa_list, chunk_pages);
> > > +		if (ret) {