Re: [PATCH 2/2] KVM: x86/mmu: Add helper to consolidate huge page promotion

From: Paolo Bonzini <pbonzini@redhat.com>
To: "Sean Christopherson" <sean.j.christopherson@intel.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Adam Borowski <kilobyte@angband.pl>,
	David Hildenbrand <david@redhat.com>,
	Dan Williams <dan.j.williams@intel.com>
Subject: Re: [PATCH 2/2] KVM: x86/mmu: Add helper to consolidate huge page promotion
Date: Wed, 6 Nov 2019 18:22:29 +0100	[thread overview]
Message-ID: <9b3e3cc3-9cd0-1f55-ee19-f33c4cfb7f8a@redhat.com> (raw)
In-Reply-To: <20191106170727.14457-3-sean.j.christopherson@intel.com>

On 06/11/19 18:07, Sean Christopherson wrote:
>  	 */
> -	if (!is_error_noslot_pfn(pfn) && !kvm_is_reserved_pfn(pfn) &&
> -	    !kvm_is_zone_device_pfn(pfn) && level == PT_PAGE_TABLE_LEVEL &&
> -	    PageTransCompoundMap(pfn_to_page(pfn)) &&
> +	if (level == PT_PAGE_TABLE_LEVEL && kvm_is_hugepage_allowed(pfn) &&
>  	    !mmu_gfn_lpage_is_disallowed(vcpu, gfn, PT_DIRECTORY_LEVEL)) {
>  		unsigned long mask;
>  		/*
> @@ -5914,9 +5919,7 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
>  		 * the guest, and the guest page table is using 4K page size
>  		 * mapping if the indirect sp has level = 1.
>  		 */
> -		if (sp->role.direct && !kvm_is_reserved_pfn(pfn) &&
> -		    !kvm_is_zone_device_pfn(pfn) &&
> -		    PageTransCompoundMap(pfn_to_page(pfn))) {
> +		if (sp->role.direct && kvm_is_hugepage_allowed(pfn)) {
>  			pte_list_remove(rmap_head, sptep);

I don't think is_error_noslot_pfn(pfn) makes sense in
kvm_mmu_zap_collapsible_spte, so I'd rather keep it in
transparent_hugepage_adjust.  Actually, it must be is_noslot_pfn only at
this point---error pfns have been sieved earlier in handle_abnormal_pfn,
so perhaps

	if (WARN_ON_ONCE(is_error_pfn(pfn)) || is_noslot_pfn(pfn))
		return;

	if (level == PT_PAGE_TABLE_LEVEL &&
	    kvm_is_hugepage_allowed(pfn) &&
	    !mmu_gfn_lpage_is_disallowed(vcpu, gfn, PT_DIRECTORY_LEVEL))

would be the best option.

Paolo