[kvmarm:master 1/3] arch/arm/kvm/mmu.c:302:14: error: 'S2_PUD_SIZE' undeclared

[Marc Zyngier]

On 04/04/17 11:14, Suzuki K Poulose wrote:
> On 03/04/17 22:15, kbuild test robot wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git master
>> head:   1f1c45c6f66a586ca420ca02cbd93a35690394f9
>> commit: f9d9eb7f7a2c7e388861fe1cdb253f63e63555fe [1/3] kvm: arm/arm64: Fix locking for kvm_free_stage2_pgd
>> config: arm-axm55xx_defconfig (attached as .config)
>> compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
>> reproduce:
>>         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>         chmod +x ~/bin/make.cross
>>         git checkout f9d9eb7f7a2c7e388861fe1cdb253f63e63555fe
>>         # save the attached .config to linux build tree
>>         make.cross ARCH=arm
>>
>> All errors (new ones prefixed by >>):
>>
>>    arch/arm/kvm/mmu.c: In function 'unmap_stage2_range':
>>>> arch/arm/kvm/mmu.c:302:14: error: 'S2_PUD_SIZE' undeclared (first use in this function)
>>       if (size > S2_PUD_SIZE)
>>                  ^~~~~~~~~~~
> 
> Thanks kbuild for catching this one !
> 
>>    arch/arm/kvm/mmu.c:302:14: note: each undeclared identifier is reported only once for each function it appears in
>>
>> vim +/S2_PUD_SIZE +302 arch/arm/kvm/mmu.c
>>
>>    296		pgd = kvm->arch.pgd + stage2_pgd_index(addr);
>>    297		do {
>>    298			/*
>>    299			 * If the range is too large, release the kvm->mmu_lock
>>    300			 * to prevent starvation and lockup detector warnings.
>>    301			 */
>>  > 302			if (size > S2_PUD_SIZE)
>>    303				cond_resched_lock(&kvm->mmu_lock);
>>    304			next = stage2_pgd_addr_end(addr, end);
>>    305			if (!stage2_pgd_none(*pgd))
>>
> 
> 
> Marc, Christoffer,
> 
> Ah! I didn't test this on arm32. We have two options :
> 
> 1) Define S2_P{U,M}_SIZE for arm32 in asm/stage2_pgtable.h
> 
> or,
> 
> 2)  use the following hunk on top of the patch, which changes the lock
> release after we process one PGDIR entry. As for the first time we enter
> the loop we haven't done much with the lock held, hence it may make
> sense to do it after the first round and we have more work to do.
> 
> Let me know what you think
> 
> 
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index db94f3a..582a972 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -295,15 +295,15 @@ static void unmap_stage2_range(struct kvm *kvm, phys_addr_t start, u64 size)
>          assert_spin_locked(&kvm->mmu_lock);
>          pgd = kvm->arch.pgd + stage2_pgd_index(addr);
>          do {
> +               next = stage2_pgd_addr_end(addr, end);
> +               if (!stage2_pgd_none(*pgd))
> +                       unmap_stage2_puds(kvm, pgd, addr, next);
>                  /*
>                   * If the range is too large, release the kvm->mmu_lock
>                   * to prevent starvation and lockup detector warnings.
>                   */
> -               if (size > S2_PUD_SIZE)
> +               if (next != end)
>                          cond_resched_lock(&kvm->mmu_lock);
> -               next = stage2_pgd_addr_end(addr, end);
> -               if (!stage2_pgd_none(*pgd))
> -                       unmap_stage2_puds(kvm, pgd, addr, next);
>          } while (pgd++, addr = next, addr != end);
>   }

Yup, I quite like this last option, as it doesn't rely on a particular
size (or just implicitly that of the PGD). Can you respin this?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...