Hi Mike,
Drive by review comments below...
On 23/10/2024 17:27, Mike Rapoport wrote:
> From: "Mike Rapoport (Microsoft)" <rppt@xxxxxxxxxx>
>
> Using large pages to map text areas reduces iTLB pressure and improves
> performance.
>
> Extend execmem_alloc() with an ability to use huge pages with ROX
> permissions as a cache for smaller allocations.
>
> To populate the cache, a writable large page is allocated from vmalloc with
> VM_ALLOW_HUGE_VMAP, filled with invalid instructions and then remapped as
> ROX.
>
> The direct map alias of that large page is exculded from the direct map.
>
> Portions of that large page are handed out to execmem_alloc() callers
> without any changes to the permissions.
>
> When the memory is freed with execmem_free() it is invalidated again so
> that it won't contain stale instructions.
>
> An architecture has to implement execmem_fill_trapping_insns() callback
> and select ARCH_HAS_EXECMEM_ROX configuration option to be able to use
> the ROX cache.
>
> The cache is enabled on per-range basis when an architecture sets
> EXECMEM_ROX_CACHE flag in definition of an execmem_range.
>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> Reviewed-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> Tested-by: kdevops <kdevops@xxxxxxxxxxxxxxx>
> ---
[...]
> +
> +static int execmem_cache_populate(struct execmem_range *range, size_t size)
> +{
> + unsigned long vm_flags = VM_ALLOW_HUGE_VMAP;
> + unsigned long start, end;
> + struct vm_struct *vm;
> + size_t alloc_size;
> + int err = -ENOMEM;
> + void *p;
> +
> + alloc_size = round_up(size, PMD_SIZE);
> + p = execmem_vmalloc(range, alloc_size, PAGE_KERNEL, vm_flags);
Shouldn't this be passing PAGE_KERNEL_ROX? Otherwise I don't see how the
allocated memory is ROX? I don't see any call below where you change the permission.
Given the range has the pgprot in it, you could just drop passing the pgprot
explicitly here and have execmem_vmalloc() use range->pgprot directly?
Thanks,
Ryan
> + if (!p)
> + return err;
> +
> + vm = find_vm_area(p);
> + if (!vm)
> + goto err_free_mem;
> +
> + /* fill memory with instructions that will trap */
> + execmem_fill_trapping_insns(p, alloc_size, /* writable = */ true);
> +
> + start = (unsigned long)p;
> + end = start + alloc_size;
> +
> + vunmap_range(start, end);
> +
> + err = execmem_set_direct_map_valid(vm, false);
> + if (err)
> + goto err_free_mem;
> +
> + err = vmap_pages_range_noflush(start, end, range->pgprot, vm->pages,
> + PMD_SHIFT);
> + if (err)
> + goto err_free_mem;
> +
> + err = execmem_cache_add(p, alloc_size);
> + if (err)
> + goto err_free_mem;
> +
> + return 0;
> +
> +err_free_mem:
> + vfree(p);
> + return err;
> +}
[...]
