On Fri, Jun 16, 2023 at 01:01:08PM -0700, Song Liu wrote:

On Fri, Jun 16, 2023 at 1:51 AM Mike Rapoport [off-list ref] wrote:

quoted

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>

[...]

quoted

 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?

This also fills in jit.text (next patch), so _data doesn't work here :)
And it's not really a default, the defaults are set explicitly for arches
that don't have execmem_arch_params.
 

quoted

+{
+       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;
}

Yes, we can have a single check here.
 

Thanks,
Song

[...]

-- 
Sincerely yours,
Mike.