lkml.org 
[lkml]   [2019]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] kvm: arm64: Relax the restriction on using stage2 PUD huge mapping
From
Date
Marc,

On 30/01/2019 10:21, Marc Zyngier wrote:
> On 29/01/2019 19:12, Suzuki K Poulose wrote:
>> We restrict mapping the PUD huge pages in stage2 to only when the
>> stage2 has 4 level page table, leaving the feature unused with
>> the default IPA size. But we could use it even with a 3
>> level page table, i.e, when the PUD level is folded into PGD,
>> just like the stage1. Relax the condition to allow using the
>> PUD huge page mappings at stage2 when it is possible.
>>
>> Cc: Christoffer Dall <christoffer.dall@arm.com>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> virt/kvm/arm/mmu.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index fbdf3ac..30251e2 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -1695,11 +1695,14 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>
>> vma_pagesize = vma_kernel_pagesize(vma);
>> /*
>> - * PUD level may not exist for a VM but PMD is guaranteed to
>> - * exist.
>> + * The stage2 has a minimum of 2 level table (For arm64 see
>> + * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can
>> + * use PMD_SIZE huge mappings (even when the PMD is folded into PGD).
>> + * As for PUD huge maps, we must make sure that we have at least
>> + * 3 levels, i.e, PMD is not folded.
>> */
>> if ((vma_pagesize == PMD_SIZE ||
>> - (vma_pagesize == PUD_SIZE && kvm_stage2_has_pud(kvm))) &&
>> + (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm))) &&
>> !force_pte) {
>> gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT;
>> }
>>
>
> For the record, it took a 10 minute discussion with Suzuki to
> understand that the above is actually correct, even if massively
> confusing. Why is it correct:
>
> - In a 4 level stage-2, each of PGD/PUD/PMD exists on its own, and start
> level is 0
> - In a 3 level stage-2, PGD and PUD are fused as the start level is 1,
> and PMD exists on its own
> - In a 2 level stage-2, both PGD, PUD and PMD are fused, and start level
> is 2.
>
> From the above, we can extract that you can always have a block mapping
> at the PUD level if you have a standalone PMD, and that's the logic this
> patch is using.

Thanks for writing it up ! :-)

>
> Now, the *real* reason is that you can map a given size in your S2PT,
> not that some level is fused or not. What we want is probably a helper
> that says:
>
> bool kvm_stage2_can_map_block_size(struct kvm *kvm, size_t sz)
> {
> return sz >= PMD_SIZE && stage2_pgdir_size(kvm) >= sz);
> }
>
> and the above becomes:
>
> if (kvm_stage2_can_map_block_size(kvm, vma_pagesize)) && !force_pte)
>
> which I find much nicer.
>
> I guess I can still take the above as a fix if Christoffer is happy with
> it, but if you think my above hack is correct, I'd like things to be
> cleaned-up in the near future.

Sure, as we discussed that makes it much simpler and generic. I could
address that.

Suzuki

\
 
 \ /
  Last update: 2019-01-30 11:40    [W:0.038 / U:3.992 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site