On 13.08.21 16:49, Joerg Roedel wrote:

Hi Dave,

On Thu, Aug 12, 2021 at 07:14:20AM -0700, Dave Hansen wrote:

quoted

maybe_accept_page()
{
	unsigned long huge_pfn = page_to_phys(page) / PMD_SIZE;

	/* Test the bit before taking any locks: */
	if (test_bit(huge_pfn, &accepted_bitmap))
		return;

	spin_lock_irq();
	/* Retest inside the lock: */
	if (test_bit(huge_pfn, &accepted_bitmap))
		return;
	tdx_accept_page(page, PMD_SIZE);
	set_bit(huge_pfn, &accepted_bitmap));
	spin_unlock_irq();
}

Yeah, this could work, but the global lock is likely the show-stopper
here. For SNP we also not allowed to double-validate, so we need
something that basically indicates 'validation-is-ongoing' on a per 2MB
basis.

I am not an mm expert, but a page flag probably doesn't work. The flag
would be on the head of the 2MB range and when that page is already used
somewhere else there is no guarantee that the flag will survive. But
correct me if I am wrong here :)

The other options I can come up with are not great either:

	1) using an AVL bit in the direct-mapping PMD of that page. The
	   page-table would only be walked if the bit in the
	   accept_bitmap is clear. But I am not sure that all memory
	   which needs to be validated is in the direct-map.

	2) Use another page-sized bitmap. If the machine has more than
	   64GB of memory the bit index is wrapped around. This
	   shouldn't be a performance problem at runtime, if this page
	   is only consulted when the valid bit is clear in the
	   accept_bitmap.

MM experts could certainly come up with better ideas :)

Not sure if already discussed, but what about making sure that free 
pages are not a mixture (partially unaccepted, partially accepted).

You'd have to expose the pages in that granularity to the buddy 
(__free_pages_core), indicating the state. You'd have to reject merging 
pages of differing acceptance state.

Accepting a page would then be handled outside of the zone lock, 
completely controlled by the state.

So a page in the buddy would either be completely accepted or completely 
unaccepted, signaled e.g., by PageOffline().

Consequently, when allocating a 4KiB page, you'd split an unaccepted 
2MiB page into separate unaccepted pages. You'd grab one of the 
unaccepted 4KiB pages and accept it before initializing it and handing 
it out.

-- 
Thanks,

David / dhildenb