Re: [PATCH 1/5] mm: Add support for unaccepted memory

From: David Hildenbrand <david@redhat.com>
To: Joerg Roedel <jroedel@suse.de>, Dave Hansen <dave.hansen@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
	Sean Christopherson <seanjc@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Kuppuswamy Sathyanarayanan
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	David Rientjes <rientjes@google.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Ingo Molnar <mingo@redhat.com>,
	Varad Gautam <varad.gautam@suse.com>,
	Dario Faggioli <dfaggioli@suse.com>,
	x86@kernel.org, linux-mm@kvack.org, linux-coco@lists.linux.dev,
	linux-kernel@vger.kernel.org,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH 1/5] mm: Add support for unaccepted memory
Date: Tue, 17 Aug 2021 17:00:55 +0200	[thread overview]
Message-ID: <b0228048-d2c2-6081-cfae-147e17acf785@redhat.com> (raw)
In-Reply-To: <YRaGch+D5HPtUk9l@suse.de>

On 13.08.21 16:49, Joerg Roedel wrote:
> Hi Dave,
> 
> On Thu, Aug 12, 2021 at 07:14:20AM -0700, Dave Hansen wrote:
>> 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