From: christoffer.dall@linaro.org (Christoffer Dall)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/6] ARM: mm: Drop the lowmem watermark check from virt_addr_valid()
Date: Fri, 15 Nov 2013 00:22:32 +0000 [thread overview]
Message-ID: <20131115002232.GE20516@lvm> (raw)
In-Reply-To: <1384457866-16135-4-git-send-email-santosh.shilimkar@ti.com>
On Thu, Nov 14, 2013 at 02:37:43PM -0500, Santosh Shilimkar wrote:
> Slab allocator can allocate memory beyond the lowmem watermark
> which can lead to false failure of virt_addr_valid().
>
> So drop the check. The issue was seen with percpu_alloc()
> in KVM code which was allocating memory beyond lowmem watermark.
>
> Am not completly sure whether this is the right fix and if it could
> impact any other user of virt_addr_valid(). Without this fix as
> pointed out the KVM init was failing in my testing.
>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
> arch/arm/include/asm/memory.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 4dd2145..412da47 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -343,8 +343,7 @@ static inline __deprecated void *bus_to_virt(unsigned long x)
> #define ARCH_PFN_OFFSET PHYS_PFN_OFFSET
>
> #define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
> -#define virt_addr_valid(kaddr) ((unsigned long)(kaddr) >= PAGE_OFFSET && (unsigned long)(kaddr) < (unsigned long)high_memory)
> -
> +#define virt_addr_valid(kaddr) ((unsigned long)(kaddr) >= PAGE_OFFSET)
> #endif
>
> #include <asm-generic/memory_model.h>
> --
> 1.7.9.5
>
This looks wrong to me. Check Documentation/arm/memory.txt, this would
return true for the VMALLOC region, which would cause virt_to_phys to
give you something invalid, which would be bad.
We use the check in create_hyp_mappings to be sure that the physical
address returned by virt_to_phys is valid and that if we're mapping more
than one page that those pages are physically contiguous.
So if you want to get rid of this check, you need to change the mapping
functionality to obtain the physical address by walking the page table
mappings for each page that you are mapping instead. Or limit each call
to a single page in size and take the physical address as input and use
per_cpu_ptr_to_phys at the caller side instead.
Alternatively, we need to get rid of alloc_percpu and use regular
kmallocs instead, unless anyone else knows of an even better way.
-Christoffer
next prev parent reply other threads:[~2013-11-15 0:22 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-14 19:37 [PATCH 0/6] ARM/AMR64: Few patches on kvm and mm code Santosh Shilimkar
2013-11-14 19:37 ` [PATCH 1/6] arm64: mm: Add __virt_to_idmap() to keep kvm build happy Santosh Shilimkar
2013-11-15 13:34 ` Catalin Marinas
2013-11-15 14:55 ` Santosh Shilimkar
2013-11-15 14:58 ` Catalin Marinas
2013-11-15 15:31 ` Santosh Shilimkar
2013-11-15 15:05 ` Marc Zyngier
2013-11-15 15:25 ` Santosh Shilimkar
2013-11-15 15:29 ` Marc Zyngier
2013-11-15 15:31 ` Catalin Marinas
2013-11-15 15:33 ` Santosh Shilimkar
2013-11-14 19:37 ` [PATCH 2/6] ARM: kvm: Use virt_to_idmap instead of virt_to_phys for idmap mappings Santosh Shilimkar
2013-11-15 0:12 ` Christoffer Dall
2013-11-14 19:37 ` [PATCH 3/6] ARM: mm: Drop the lowmem watermark check from virt_addr_valid() Santosh Shilimkar
2013-11-15 0:22 ` Christoffer Dall [this message]
2013-11-15 0:31 ` Santosh Shilimkar
2013-11-15 0:40 ` Christoffer Dall
2013-11-15 11:48 ` Marc Zyngier
2013-11-15 11:43 ` Marc Zyngier
2013-11-15 14:55 ` Santosh Shilimkar
2013-11-15 15:08 ` Marc Zyngier
2013-11-15 15:46 ` Christoffer Dall
2013-11-15 14:20 ` Russell King - ARM Linux
2013-11-15 15:40 ` Santosh Shilimkar
2013-11-14 19:37 ` [PATCH 4/6] arm64: " Santosh Shilimkar
2013-11-15 13:39 ` Catalin Marinas
2013-11-15 14:25 ` Marc Zyngier
2013-11-15 14:57 ` Santosh Shilimkar
2013-11-14 19:37 ` [PATCH 5/6] ARM: kvm: Use phys_addr_t instead of unsigned long in mm code Santosh Shilimkar
2013-11-15 0:09 ` Christoffer Dall
2013-11-15 0:10 ` Santosh Shilimkar
2013-11-15 11:58 ` Marc Zyngier
2013-11-14 19:37 ` [PATCH 6/6] ARM: kvm: TMP: Commit the hyp page tables to main memory Santosh Shilimkar
2013-11-14 22:36 ` Santosh Shilimkar
2013-11-15 0:11 ` Christoffer Dall
2013-11-15 0:15 ` Santosh Shilimkar
2013-11-15 0:27 ` Christoffer Dall
2013-11-15 0:36 ` Santosh Shilimkar
2013-11-15 0:42 ` Christoffer Dall
2013-11-15 1:19 ` Santosh Shilimkar
2013-11-15 1:35 ` Christoffer Dall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131115002232.GE20516@lvm \
--to=christoffer.dall@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).