Re: [RFC PATCH 4/4] KVM: SVM: Add Page modification logging support

["Huang, Kai" <kai.huang@intel.com>]

On Thu, 2025-08-28 at 11:09 +0000, Huang, Kai wrote:
> On Thu, 2025-08-28 at 16:13 +0530, Nikunj A. Dadhania wrote:
> > On 8/28/2025 4:03 PM, Huang, Kai wrote:
> > > On Thu, 2025-08-28 at 12:07 +0530, Nikunj A. Dadhania wrote:
> > > > 
> > > > On 8/28/2025 5:14 AM, Huang, Kai wrote:
> > > > > On Mon, 2025-08-25 at 15:20 +0000, Nikunj A Dadhania wrote:
> > > > > > +	if (pml) {
> > > > > > +		svm->pml_page = snp_safe_alloc_page();
> > > > > > +		if (!svm->pml_page)
> > > > > > +			goto error_free_vmsa_page;
> > > > > > +	}
> > > > > 
> > > > > I didn't see this yesterday.  Is it mandatory for AMD PML to use
> > > > > snp_safe_alloc_page() to allocate the PML buffer, or we can also use
> > > > > normal page allocation API?
> > > > 
> > > > As it is dependent on HvInUseWrAllowed, I need to use snp_safe_alloc_page().
> > > 
> > > So the patch 2 is actually a dependent for PML?
> > 
> > Not really, if the patch 2 is not there, the 2MB alignment workaround will be
> > applied to PML page allocation.
> 
> Sounds they are related, at least.
> 
> I don't have intention to judge whether patch 2 should be in this series
> or not, nor whether snp_safe_alloc_page_node() is the right place to
> workaround the 2MB alignment for PML buffer.
> 
> I just think it's good to see some text explaining why patch 2 is needed
> for PML if eventually you decide to keep it in this series.

Btw, there's one big comment in snp_safe_alloc_page_node():

        /*
         * Allocate an SNP-safe page to workaround the SNP erratum where 
         * the CPU will incorrectly signal an RMP violation #PF if a
         * hugepage (2MB or 1GB) collides with the RMP entry of a
         * 2MB-aligned VMCB, VMSA, or AVIC backing page.  
	 * ...
	 */

You might want to include the PML buffer to the list too.