Re: [PATCH] arm64/kvm: Fix zapping stage2 page table wrongly

[Auger Eric <eric.auger@redhat.com>]

Hi Marc,

On 9/2/20 1:10 PM, Marc Zyngier wrote:
> On 2020-09-02 11:59, Alexandru Elisei wrote:
>> Hi,
>>
>> On 8/22/20 3:44 AM, Gavin Shan wrote:
>>> Depending on the kernel configuration, PUD_SIZE could be equal to
>>> PMD_SIZE. For example, both of them are 512MB with the following
>>> kernel configuration. In this case, both PUD and PMD are folded
>>> to PGD.
>>>
>>>    CONFIG_ARM64_64K_PAGES   y
>>>    CONFIG_ARM64_VA_BITS     42
>>>    CONFIG_PGTABLE_LEVELS    2
>>>
>>> With the above configuration, the stage2 PUD is used to backup the
>>> 512MB huge page when the stage2 mapping is built. During the mapping,
>>> the PUD and its subordinate levels of page table entries are unmapped
>>> if the PUD is present and not huge page sensitive in
>>> stage2_set_pud_huge().
>>> Unfornately, the @addr isn't aligned to S2_PUD_SIZE and wrong page table
>>> entries are zapped. It eventually leads to PUD's present bit can't be
>>> cleared successfully and infinite loop in stage2_set_pud_huge().
>>>
>>> This fixes the issue by checking with S2_{PUD, PMD}_SIZE instead of
>>> {PUD, PMD}_SIZE to determine if stage2 PUD or PMD is used to back the
>>> huge page. For this particular case, the stage2 PMD entry should be
>>> used to backup the 512MB huge page with stage2_set_pmd_huge().
>>
>> I can reproduce this on my rockpro64 using kvmtool.
>>
>> I see two issues here: first, PUD_SIZE = 512MB, but S2_PUD_SIZE = 4TB
>> (checked
>> using printk), and second, stage2_set_pud_huge() hangs. I'm working on
>> debugging them.
> 
> I have this as an immediate fix for the set_pud_huge hang, tested
> on Seattle with 64k/42bits.
> 
> I can't wait to see the back of this code...
> 
>         M.
> 
> From 2a345a826a47f9061bb37045a1d89ea54b51fb80 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Wed, 2 Sep 2020 11:18:29 +0100
> Subject: [PATCH] KVM: arm64: Do not try to map PUDs when they are folded
> into
>  PMD
> 
> For the obscure cases where PMD and PUD are the same size
> (64kB pages with 42bit VA, for example, which results in only
> two levels of page tables), we can't map anything as a PUD,
> because there is... erm... no PUD to speak of. Everything is
> either a PMD or a PTE.
> 
> So let's only try and map a PUD when its size is different from
> that of a PMD.
> 
> Cc: stable@vger.kernel.org
> Fixes: b8e0ba7c8bea ("KVM: arm64: Add support for creating PUD hugepages
> at stage 2")
> Reported-by: Gavin Shan <gshan@redhat.com>
> Reported-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index ba00bcc0c884..c3a92fa537fd 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1970,7 +1970,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
> phys_addr_t fault_ipa,
>          (fault_status == FSC_PERM &&
>           stage2_is_exec(mmu, fault_ipa, vma_pagesize));
> 
> -    if (vma_pagesize == PUD_SIZE) {
> +    if (PUD_SIZE != PMD_SIZE && vma_pagesize == PUD_SIZE) {
>          pud_t new_pud = kvm_pfn_pud(pfn, mem_type);
> 
>          new_pud = kvm_pud_mkhuge(new_pud);
also works for me

Tested-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm