Re: [PATCH v2 06/12] mm/execmem: introduce execmem_data_alloc()

Song Liu
2023-06-16T20:01:47Z

On Fri, Jun 16, 2023 at 1:51 AM Mike Rapoport <rppt@kernel.org> wrote:
>
> From: "Mike Rapoport (IBM)" <rppt@kernel.org>
>
> Data related to code allocations, such as module data section, need to
> comply with architecture constraints for its placement and its
> allocation right now was done using execmem_text_alloc().
>
> Create a dedicated API for allocating data related to code allocations
> and allow architectures to define address ranges for data allocations.
>
> Since currently this is only relevant for powerpc variants that use the
> VMALLOC address space for module data allocations, automatically reuse
> address ranges defined for text unless address range for data is
> explicitly defined by an architecture.
>
> With separation of code and data allocations, data sections of the
> modules are now mapped as PAGE_KERNEL rather than PAGE_KERNEL_EXEC which
> was a default on many architectures.
>
> Signed-off-by: Mike Rapoport (IBM) <rppt@kernel.org>
[...]
> static void free_mod_mem(struct module *mod)
> diff --git a/mm/execmem.c b/mm/execmem.c
> index a67acd75ffef..f7bf496ad4c3 100644
> --- a/mm/execmem.c
> +++ b/mm/execmem.c
> @@ -63,6 +63,20 @@ void *execmem_text_alloc(size_t size)
> fallback_start, fallback_end, kasan);
> }
>
> +void *execmem_data_alloc(size_t size)
> +{
> + unsigned long start = execmem_params.modules.data.start;
> + unsigned long end = execmem_params.modules.data.end;
> + pgprot_t pgprot = execmem_params.modules.data.pgprot;
> + unsigned int align = execmem_params.modules.data.alignment;
> + unsigned long fallback_start = execmem_params.modules.data.fallback_start;
> + unsigned long fallback_end = execmem_params.modules.data.fallback_end;
> + bool kasan = execmem_params.modules.flags & EXECMEM_KASAN_SHADOW;
> +
> + return execmem_alloc(size, start, end, align, pgprot,
> + fallback_start, fallback_end, kasan);
> +}
> +
> void execmem_free(void *ptr)
> {
> /*
> @@ -101,6 +115,28 @@ static bool execmem_validate_params(struct execmem_params *p)
> return true;
> }
>
> +static void execmem_init_missing(struct execmem_params *p)

Shall we call this execmem_default_init_data?

> +{
> + struct execmem_modules_range *m = &p->modules;
> +
> + if (!pgprot_val(execmem_params.modules.data.pgprot))
> + execmem_params.modules.data.pgprot = PAGE_KERNEL;

Do we really need to check each of these? IOW, can we do:

if (!pgprot_val(execmem_params.modules.data.pgprot)) {
execmem_params.modules.data.pgprot = PAGE_KERNEL;
execmem_params.modules.data.alignment = m->text.alignment;
execmem_params.modules.data.start = m->text.start;
execmem_params.modules.data.end = m->text.end;
execmem_params.modules.data.fallback_start = m->text.fallback_start;
execmem_params.modules.data.fallback_end = m->text.fallback_end;
}

Thanks,
Song …