Re: [PATCH] arm/arm64: KVM: Fix hyp mappings of vmalloc regions

From: Marc Zyngier <marc.zyngier@arm.com>
To: Christoffer Dall <christoffer.dall@linaro.org>
Cc: <kvmarm@lists.cs.columbia.edu>, <kvm@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>, <patches@linaro.org>
Subject: Re: [PATCH] arm/arm64: KVM: Fix hyp mappings of vmalloc regions
Date: Sat, 16 Nov 2013 10:01:01 +0000	[thread overview]
Message-ID: <87846f3195cadcae32824120f78e3c9d@www.loen.fr> (raw)
In-Reply-To: <1384557880-14107-1-git-send-email-christoffer.dall@linaro.org>

On 2013-11-15 23:24, Christoffer Dall wrote:
> Using virt_to_phys on percpu mappings is horribly wrong as it may be
> backed by vmalloc.  Introduce kvm_kaddr_to_phys which translates both
> types of valid kernel addresses to the corresponding physical 
> address.
>
> At the same time resolves a typing issue where we were storing the
> physical address as a 32 bit unsigned long (on arm), truncating the
> physical address for addresses above the 4GB limit.  This caused
> breakage on Keystone.
>
> Reported-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>
> This patch is loosely based on Marc's previous patch from today but
> instead of introducing another Hyp mapping function, it fixes the
> existing one to deal with both kinds of kernel addresses.

Looks good to me! This should probably be merged quickly (after testing 
by Santosh), and possibly Cc-ed to stable.

Thanks,

         M.

>
>  arch/arm/kvm/mmu.c |   34 ++++++++++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index 3719583..5809069 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -334,6 +334,17 @@ out:
>  	return err;
>  }
>
> +static phys_addr_t kvm_kaddr_to_phys(void *kaddr)
> +{
> +	if (!is_vmalloc_addr(kaddr)) {
> +		BUG_ON(!virt_addr_valid(kaddr));
> +		return __pa(kaddr);
> +	} else {
> +		return page_to_phys(vmalloc_to_page(kaddr)) +
> +		       offset_in_page(kaddr);
> +	}
> +}
> +
>  /**
>   * create_hyp_mappings - duplicate a kernel virtual address range in
> Hyp mode
>   * @from:	The virtual kernel start address of the range
> @@ -345,16 +356,27 @@ out:
>   */
>  int create_hyp_mappings(void *from, void *to)
>  {
> -	unsigned long phys_addr = virt_to_phys(from);
> +	phys_addr_t phys_addr;
> +	unsigned long virt_addr;
>  	unsigned long start = KERN_TO_HYP((unsigned long)from);
>  	unsigned long end = KERN_TO_HYP((unsigned long)to);
>
> -	/* Check for a valid kernel memory mapping */
> -	if (!virt_addr_valid(from) || !virt_addr_valid(to - 1))
> -		return -EINVAL;
> +	start = start & PAGE_MASK;
> +	end = PAGE_ALIGN(end);
>
> -	return __create_hyp_mappings(hyp_pgd, start, end,
> -				     __phys_to_pfn(phys_addr), PAGE_HYP);
> +	for (virt_addr = start; virt_addr < end; virt_addr += PAGE_SIZE) {
> +		int err;
> +
> +		phys_addr = kvm_kaddr_to_phys(from + virt_addr - start);
> +		err = __create_hyp_mappings(hyp_pgd, virt_addr,
> +					    virt_addr + PAGE_SIZE,
> +					    __phys_to_pfn(phys_addr),
> +					    PAGE_HYP);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
>  }
>
>  /**

-- 
Fast, cheap, reliable. Pick two.