lkml.org 
[lkml]   [2020]   [Feb]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 58/61] KVM: x86/mmu: Configure max page level during hardware setup
Date
Sean Christopherson <sean.j.christopherson@intel.com> writes:

> On Tue, Feb 25, 2020 at 03:43:36PM +0100, Vitaly Kuznetsov wrote:
>> Sean Christopherson <sean.j.christopherson@intel.com> writes:
>>
>> > Configure the max page level during hardware setup to avoid a retpoline
>> > in the page fault handler. Drop ->get_lpage_level() as the page fault
>> > handler was the last user.
>> > @@ -6064,11 +6064,6 @@ static void svm_set_supported_cpuid(struct kvm_cpuid_entry2 *entry)
>> > }
>> > }
>> >
>> > -static int svm_get_lpage_level(void)
>> > -{
>> > - return PT_PDPE_LEVEL;
>> > -}
>>
>> I've probably missed something but before the change, get_lpage_level()
>> on AMD was always returning PT_PDPE_LEVEL, but after the change and when
>> NPT is disabled, we set max_page_level to either PT_PDPE_LEVEL (when
>> boot_cpu_has(X86_FEATURE_GBPAGES)) or PT_DIRECTORY_LEVEL
>> (otherwise). This sounds like a change) unless we think that
>> boot_cpu_has(X86_FEATURE_GBPAGES) is always true on AMD.
>
> It looks like a functional change, but isn't. kvm_mmu_hugepage_adjust()
> caps the page size used by KVM's MMU at the minimum of ->get_lpage_level()
> and the host's mapping level. Barring an egregious bug in the kernel MMU,
> the host page tables will max out at PT_DIRECTORY_LEVEL (2mb) unless
> boot_cpu_has(X86_FEATURE_GBPAGES) is true.
>
> In other words, this is effectively a "documentation" change. I'll figure
> out a way to explain this in the changelog...
>
> max_level = min(max_level, kvm_x86_ops->get_lpage_level());
> for ( ; max_level > PT_PAGE_TABLE_LEVEL; max_level--) {
> linfo = lpage_info_slot(gfn, slot, max_level);
> if (!linfo->disallow_lpage)
> break;
> }
>
> if (max_level == PT_PAGE_TABLE_LEVEL)
> return PT_PAGE_TABLE_LEVEL;
>
> level = host_pfn_mapping_level(vcpu, gfn, pfn, slot);
> if (level == PT_PAGE_TABLE_LEVEL)
> return level;
>
> level = min(level, max_level); <---------

Ok, I see (I believe):

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

It would've helped me a bit if kvm_configure_mmu() was written the
following way:

void kvm_configure_mmu(bool enable_tdp, int tdp_page_level)
{
tdp_enabled = enable_tdp;

if (boot_cpu_has(X86_FEATURE_GBPAGES))
max_page_level = PT_PDPE_LEVEL;
else
max_page_level = PT_DIRECTORY_LEVEL;

if (tdp_enabled)
max_page_level = min(tdp_page_level, max_page_level);
}

(we can't have cpu_has_vmx_ept_1g_page() and not
boot_cpu_has(X86_FEATURE_GBPAGES), right?)

But this is certainly just a personal preference, feel free to ignore)

--
Vitaly

\
 
 \ /
  Last update: 2020-02-26 15:57    [W:0.100 / U:0.276 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site