On 2016/1/18 19:56, Mark Rutland wrote:

On Wed, Jan 13, 2016 at 05:10:31PM +0100, Ard Biesheuvel wrote:

quoted

On 13 January 2016 at 15:03, Ard Biesheuvel [off-list ref] wrote:

quoted

On 12 January 2016 at 22:46, Laura Abbott [off-list ref] wrote:

quoted

The range of set_memory_* is currently restricted to the module address
range because of difficulties in breaking down larger block sizes.
vmalloc maps PAGE_SIZE pages so it is safe to use as well. Update the
function ranges and add a comment explaining why the range is restricted
the way it is.

Signed-off-by: Laura Abbott <redacted>
---
This should let the protections for the eBPF work as expected, I don't
know if there is some sort of self test for thatL.

This is going to conflict with my KASLR implementation, since it puts
the kernel image right in the middle of the vmalloc area, and the
kernel is obviously mapped with block mappings. In fact, I am
proposing enabling huge-vmap for arm64 as well, since it seems an
improvement generally, but also specifically allows me to unmap the
__init section using the generic vunmap code (remove_vm_area). But in
general, I think the assumption that the whole vmalloc area is mapped
using pages is not tenable.

AFAICT, vmalloc still use pages exclusively even with huge-vmap (but
ioremap does not). So perhaps it would make sense to check for the
VM_ALLOC bit in the VMA flags (which I will not set for the kernel
regions either)

Something along these lines, perhaps?

diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 3571c7309c5e..bda0a776c58e 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c

@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/vmalloc.h>
 #include <linux/sched.h>

 #include <asm/pgtable.h>

@@ -44,6 +45,7 @@ static int change_memory_common(unsigned long addr
        unsigned long end = start + size;
        int ret;
        struct page_change_data data;
+       struct vm_struct *area;

        if (!PAGE_ALIGNED(addr)) {
                start &= PAGE_MASK;

@@ -51,10 +53,14 @@ static int change_memory_common(unsigned long addr,
                WARN_ON_ONCE(1);
        }

-       if (start < MODULES_VADDR || start >= MODULES_END)
-               return -EINVAL;
-
-       if (end < MODULES_VADDR || end >= MODULES_END)
+       /*
+        * Check whether the [addr, addr + size) interval is entirely
+        * covered by precisely one VM area that has the VM_ALLOC flag set
+        */
+       area = find_vm_area((void *)addr);
+       if (!area ||
+           end > (unsigned long)area->addr + area->size ||
+           !(area->flags & VM_ALLOC))
                return -EINVAL;

        data.set_mask = set_mask;

Neat. That fixes the fencepost bug too.

Looks good to me, though as Laura suggested we should have a comment as
to why we limit changes to such regions. Fancy taking her wording below
and spinning this as a patch?

quoted

quoted

quoted

+       /*
+        * This check explicitly excludes most kernel memory. Most kernel
+        * memory is mapped with a larger page size and breaking down the
+        * larger page size without causing TLB conflicts is very difficult.
+        *
+        * If you need to call set_memory_* on a range, the recommendation is
+        * to use vmalloc since that range is mapped with pages.
+        */

Thanks,
Mark.

Hi Mark,

After change the flag, it calls only flush_tlb_kernel_range(), so why not use 
cpu_replace_ttbr1(swapper_pg_dir)? 

One more question, does TLB conflict only affect kernel page talbe?
There is no problem when spliting the transparent hugepage, right?

Thanks,
Xishi Qiu

.