<div dir="ltr"><div>Hi, Kazu</div>Sorry for the late reply.<br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, May 11, 2023 at 12:35 PM HAGIO KAZUHITO(萩尾 一仁) <<a href="mailto:k-hagio-ab@nec.com" target="_blank">k-hagio-ab@nec.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Supported:<br>
- startup without "--no_modules"<br>
- "mod" command (without options)<br>
<br>
Signed-off-by: Kazuhito Hagio <<a href="mailto:k-hagio-ab@nec.com" target="_blank">k-hagio-ab@nec.com</a>><br>
---<br>
 defs.h    |  36 ++++<br>
 kernel.c  |  26 ++-<br>
 symbols.c | 482 ++++++++++++++++++++++++++++++++++++++++++++++++++++--<br>
 3 files changed, 527 insertions(+), 17 deletions(-)<br>
<br>
diff --git a/defs.h b/defs.h<br>
index 12ad6aaa0998..655cd2add163 100644<br>
--- a/defs.h<br>
+++ b/defs.h<br>
@@ -675,6 +675,7 @@ struct new_utsname {<br>
 #define IRQ_DESC_TREE_RADIX        (0x40ULL)<br>
 #define IRQ_DESC_TREE_XARRAY       (0x80ULL)<br>
 #define KMOD_PAX                  (0x100ULL)<br>
+#define KMOD_MEMORY               (0x200ULL)<br>
<br></blockquote><div><br></div><div>Similar to the KMOD_PAX, we should print the KMOD_MEMORY in the dump_kernel_table().</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
 #define XEN()       (kt->flags & ARCH_XEN)<br>
 #define OPENVZ()    (kt->flags & ARCH_OPENVZ)<br>
@@ -682,6 +683,7 @@ struct new_utsname {<br>
 #define PVOPS_XEN() (kt->flags & ARCH_PVOPS_XEN)<br>
<br>
 #define PAX_MODULE_SPLIT() (kt->flags2 & KMOD_PAX)<br>
+#define MODULE_MEMORY()    (kt->flags2 & KMOD_MEMORY)<br>
<br>
 #define XEN_MACHINE_TO_MFN(m)    ((ulonglong)(m) >> PAGESHIFT())<br>
 #define XEN_PFN_TO_PSEUDO(p)     ((ulonglong)(p) << PAGESHIFT())<br>
@@ -2216,6 +2218,9 @@ struct offset_table {                    /* stash of commonly-used offsets */<br>
        long in6_addr_in6_u;<br>
        long kset_kobj;<br>
        long subsys_private_subsys;<br>
+       long module_mem;<br>
+       long module_memory_base;<br>
+       long module_memory_size;<br>
 };<br>
<br>
 struct size_table {         /* stash of commonly-used sizes */<br>
@@ -2389,6 +2394,7 @@ struct size_table {         /* stash of commonly-used sizes */<br>
        long percpu_counter;<br>
        long maple_tree;<br>
        long maple_node;<br>
+       long module_memory;<br>
 };<br>
<br>
 struct array_table {<br>
@@ -2921,6 +2927,25 @@ struct mod_section_data {<br>
         int flags;<br>
 };<br>
<br>
+/* This is unlikely to change, so imported from kernel for now. */<br></blockquote><div><br></div><div>Here, It may be good to add the source file: include/linux/module.h to code comment, which is convenient to track the future changes, no need to search for them with grep next time.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+enum mod_mem_type {<br>
+       MOD_TEXT = 0,<br>
+       MOD_DATA,<br>
+       MOD_RODATA,<br>
+       MOD_RO_AFTER_INIT,<br>
+       MOD_INIT_TEXT,<br>
+       MOD_INIT_DATA,<br>
+       MOD_INIT_RODATA,<br>
+<br>
+       MOD_MEM_NUM_TYPES,<br>
+       MOD_INVALID = -1,<br>
+};<br>
+<br>
+struct module_memory {<br>
+       ulong base;<br>
+       uint size;<br>
+};<br>
+<br>
 struct load_module {<br>
         ulong mod_base;<br>
        ulong module_struct;<br>
@@ -2954,13 +2979,23 @@ struct load_module {<br>
        ulong mod_percpu;<br>
        ulong mod_percpu_size;<br>
        struct objfile *loaded_objfile;<br>
+<br>
+       /* For 6.4 module_memory */<br>
+       struct module_memory mem[MOD_MEM_NUM_TYPES];<br>
+       struct syment *symtable[MOD_MEM_NUM_TYPES];<br>
+       struct syment *symend[MOD_MEM_NUM_TYPES];<br>
 };<br>
<br>
+#define IN_MODULE(A,L)         (_in_module(A, L, MOD_TEXT))<br>
+#define IN_MODULE_INIT(A,L)    (_in_module(A, L, MOD_INIT_TEXT))<br>
+<br>
+/*<br>
 #define IN_MODULE(A,L) \<br>
  (((ulong)(A) >= (L)->mod_base) && ((ulong)(A) < ((L)->mod_base+(L)->mod_size)))<br>
<br>
 #define IN_MODULE_INIT(A,L) \<br>
  (((ulong)(A) >= (L)->mod_init_module_ptr) && ((ulong)(A) < ((L)->mod_init_module_ptr+(L)->mod_init_size)))<br>
+*/<br></blockquote><div><br></div><div>The above two macro definitions can be removed, they have been replaced with the _in_module() implementation, and for now the _in_module() is only called via IN_MODULE() and IN_MODULE_INIT(), so we need to check the MOD_TEXT and MOD_INIT_TEXT in _in_module() for non MODULE_MEMORY() cases. (will comment on the _in_module() later)</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
 #define IN_MODULE_PERCPU(A,L) \<br>
  (((ulong)(A) >= (L)->mod_percpu) && ((ulong)(A) < ((L)->mod_percpu+(L)->mod_percpu_size)))<br>
@@ -5581,6 +5616,7 @@ void dump_struct_member(char *, ulong, unsigned);<br>
 void dump_union(char *, ulong, unsigned);<br>
 void store_module_symbols_v1(ulong, int);<br>
 void store_module_symbols_v2(ulong, int);<br>
+void store_module_symbols_v3(ulong, int);<br>
 int is_datatype_command(void);<br>
 int is_typedef(char *);<br>
 int arg_to_datatype(char *, struct datatype_member *, ulong);<br>
diff --git a/kernel.c b/kernel.c<br>
index 6e98f5f6f6b1..62afff25bca8 100644<br>
--- a/kernel.c<br>
+++ b/kernel.c<br>
@@ -3571,7 +3571,18 @@ module_init(void)<br>
                MEMBER_OFFSET_INIT(module_num_gpl_syms, "module", <br>
                        "num_gpl_syms");<br>
<br>
-               if (MEMBER_EXISTS("module", "module_core")) {<br>
+               if (MEMBER_EXISTS("module", "mem")) {   /* 6.4 and later */<br></blockquote><div><br></div><div>Add the debug info for the KMOD_MEMORY flag2 as below:</div><div><br></div>                        if (CRASHDEBUG(1))<br><div>                               error(INFO, "The module memory detected.\n");</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+                       kt->flags2 |= KMOD_MEMORY;      /* MODULE_MEMORY() can be used. */<br>
+<br>
+                       MEMBER_OFFSET_INIT(module_mem, "module", "mem");<br>
+                       MEMBER_OFFSET_INIT(module_memory_base, "module_memory", "base");<br>
+                       MEMBER_OFFSET_INIT(module_memory_size, "module_memory", "size");<br>
+                       STRUCT_SIZE_INIT(module_memory, "module_memory");<br>
+<br>
+                       if (get_array_length("module.mem", NULL, 0) != MOD_MEM_NUM_TYPES)<br>
+                               error(WARNING, "module memory types have changed!\n");<br>
+<br>
+               } else if (MEMBER_EXISTS("module", "module_core")) {<br>
                        MEMBER_OFFSET_INIT(module_core_size, "module",<br>
                                           "core_size");<br>
                        MEMBER_OFFSET_INIT(module_init_size, "module",<br>
@@ -3757,6 +3768,8 @@ module_init(void)<br>
                total += nsyms;<br>
                total += 2;  /* store the module's start/ending addresses */<br>
                total += 2;  /* and the init start/ending addresses */<br>
+               if (MODULE_MEMORY()) /* 7 regions at most -> 14 */<br></blockquote><div><br></div><div>This should reuse the above 4 symbols spaces, so it is 4 plus 10.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+                       total += 10;<br>
<br>
                /*<br>
                 *  If the module has kallsyms, set up to grab them as well.<br>
@@ -3784,7 +3797,11 @@ module_init(void)<br>
                case KALLSYMS_V2:<br>
                        if (THIS_KERNEL_VERSION >= LINUX(2,6,27)) {<br>
                                numksyms = UINT(modbuf + OFFSET(module_num_symtab));<br>
-                               size = UINT(modbuf + MODULE_OFFSET2(module_core_size, rx));<br>
+                               if (MODULE_MEMORY())<br>
+                                       /* check mem[MOD_TEXT].size only */<br>
+                                       size = UINT(modbuf + OFFSET(module_mem) + OFFSET(module_memory_size));<br>
+                               else<br>
+                                       size = UINT(modbuf + MODULE_OFFSET2(module_core_size, rx));<br>
                        } else {<br>
                                numksyms = ULONG(modbuf + OFFSET(module_num_symtab));<br>
                                size = ULONG(modbuf + MODULE_OFFSET2(module_core_size, rx));<br>
@@ -3822,7 +3839,10 @@ module_init(void)<br>
                store_module_symbols_v1(total, kt->mods_installed);<br>
                break;<br>
        case KMOD_V2:<br>
-               store_module_symbols_v2(total, kt->mods_installed);<br>
+               if (MODULE_MEMORY())<br>
+                       store_module_symbols_v3(total, kt->mods_installed);<br></blockquote><div><br></div><div>The function name "store_module_symbols_v3" is confusing to me, which makes me consider it as KMOD_V3, and it is put into the case KMOD_V2 code block. But it is really not KMOD_V3, just like adding the split module layout(see the crash commit 5a2645623eeb ("Basic support for PaX's split module layout")), for now we are adding the similar module memory.</div><div><br></div><div>Given that, I would suggest changing the above function name to the store_module_symbols_v6_4() with the kernel version suffix. That can differentiate them and avoid confusion.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+               else<br>
+                       store_module_symbols_v2(total, kt->mods_installed);<br>
                break;<br>
        }<br>
<br>
diff --git a/symbols.c b/symbols.c<br>
index f0721023816d..88849833bada 100644<br>
--- a/symbols.c<br>
+++ b/symbols.c<br>
@@ -103,6 +103,8 @@ static void free_structure(struct struct_elem *);<br>
 static unsigned char is_right_brace(const char *);<br>
 static struct struct_elem *find_node(struct struct_elem *, char *);<br>
 static void dump_node(struct struct_elem *, char *, unsigned char, unsigned char);<br>
+static int _in_module(ulong, struct load_module *, int);<br>
+static ulong module_mem_end(ulong, struct load_module *);<br>
<br>
<br>
 /*<br>
@@ -1788,6 +1790,387 @@ modsym_value(ulong syms, union kernel_symbol *modsym, int i)<br>
        return 0;<br>
 }<br>
<br>
+static const char *module_start_tags[] = {<br>
+       "_MODULE_TEXT_START_",<br>
+       "_MODULE_DATA_START_",<br>
+       "_MODULE_RODATA_START_",<br>
+       "_MODULE_RO_AFTER_INIT_START_",<br>
+       "_MODULE_INIT_TEXT_START_",<br>
+       "_MODULE_INIT_DATA_START_",<br>
+       "_MODULE_INIT_RODATA_START_"<br>
+};<br>
+static const char *module_end_tags[] = {<br>
+       "_MODULE_TEXT_END_",<br>
+       "_MODULE_DATA_END_",<br>
+       "_MODULE_RODATA_END_",<br>
+       "_MODULE_RO_AFTER_INIT_END_",<br>
+       "_MODULE_INIT_TEXT_END_",<br>
+       "_MODULE_INIT_DATA_END_",<br>
+       "_MODULE_INIT_RODATA_END_"<br>
+};<br></blockquote><div><br></div><div>For the above two definitions, I noticed that they should be in pairs, and associated with another definition mod_mem_type. In addition, this looks like hard code.  How about the following changes?</div><div><br></div>#define NAME(s)                                  #s<br>#define MODULE_TAG(TYPE, SE)    NAME(_MODULE_ ## TYPE ## _## SE ##_)<br><br>struct mod_node {<br>        char *s;<br>        char *e;<br>};<br><br>static const struct mod_node module_tags[] = {<br>    {MODULE_TAG(TEXT, START), MODULE_TAG(TEXT, END)},<br>    {MODULE_TAG(DATA, START), MODULE_TAG(DATA, END)},<br>    {MODULE_TAG(RODATA, START), MODULE_TAG(RODATA, END)},<br>    {MODULE_TAG(RO_AFTER_INIT, START), MODULE_TAG(RO_AFTER_INIT, END)},<br>    {MODULE_TAG(INIT_TEXT, START), MODULE_TAG(INIT_TEXT, END)},<br>    {MODULE_TAG(INIT_DATA, START), MODULE_TAG(INIT_DATA, END)},<br>    {MODULE_TAG(INIT_RODATA, START), MODULE_TAG(INIT_RODATA, END)},<br><br>    {NULL, NULL}<br>};<br><div><br></div><div>And it can be easy to use as below:</div><div><br></div><div>+               for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) {<br></div><div>...</div><div>+                       sprintf(buf1, "%s%s", module_tags[i].s, lm->mod_name);<br>+                       sprintf(buf2, "%s%s", module_tags[i].e, lm->mod_name);<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+/*<br>
+ * Linux 6.4 introduced module.mem memory layout<br>
+ */<br>
+void<br>
+store_module_symbols_v3(ulong total, int mods_installed)<br>
+{<br>
+       int i, m;<br>
+       ulong mod, mod_next;<br>
+       char *mod_name;<br>
+       uint nsyms, ngplsyms;<br>
+       ulong syms, gpl_syms;<br>
+       ulong nksyms;<br>
+       long strbuflen;<br>
+       ulong size;<br>
+       int mcnt, lm_mcnt;<br>
+       union kernel_symbol *modsym;<br>
+       size_t kernel_symbol_size;<br>
+       struct load_module *lm;<br>
+       char buf1[BUFSIZE];<br>
+       char buf2[BUFSIZE];<br>
+       char *strbuf, *modbuf, *modsymbuf;<br></blockquote><div><br></div><div>Here the strbuf is initialized to a NULL pointer, and later we can drop the first else statement(see comment later).</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+       struct syment *sp;<br>
+       ulong first, last;<br>
+<br>
+       st->mods_installed = mods_installed;<br>
+<br>
+       if (!st->mods_installed) {<br>
+               st->flags &= ~MODULE_SYMS;<br>
+               return;<br>
+       }<br>
+<br>
+       /*<br>
+        *  If we've been here before, free up everything and start over.<br>
+        */<br>
+       if (st->flags & MODULE_SYMS)<br>
+               error(FATAL, "re-initialization of module symbols not implemented yet!\n");<br>
+<br>
+       kernel_symbol_size = kernel_symbol_type_init();<br>
+<br>
+       if ((st->ext_module_symtable = (struct syment *)<br>
+           calloc(total, sizeof(struct syment))) == NULL)<br>
+               error(FATAL, "v2 module syment space malloc (%ld symbols): %s\n",<br>
+                       total, strerror(errno));<br>
+<br>
+       if (!namespace_ctl(NAMESPACE_INIT, &st->ext_module_namespace,<br>
+           (void *)total, NULL))<br>
+               error(FATAL, "module namespace malloc: %s\n", strerror(errno));<br>
+<br>
+       if ((st->load_modules = (struct load_module *)calloc<br>
+           (st->mods_installed, sizeof(struct load_module))) == NULL)<br>
+               error(FATAL, "load_module array malloc: %s\n", strerror(errno));<br>
+<br>
+       modbuf = GETBUF(SIZE(module));<br>
+       modsymbuf = NULL;<br>
+       m = mcnt = mod_next = 0;<br>
+<br>
+       for (mod = kt->module_list; mod != kt->kernel_module; mod = mod_next) {<br>
+<br>
+               readmem(mod, KVADDR, modbuf, SIZE(module),<br>
+                       "module buffer", FAULT_ON_ERROR);<br>
+<br>
+               syms = ULONG(modbuf + OFFSET(module_syms));<br>
+               gpl_syms = ULONG(modbuf + OFFSET(module_gpl_syms));<br>
+               nsyms = UINT(modbuf + OFFSET(module_num_syms));<br>
+               ngplsyms = UINT(modbuf + OFFSET(module_num_gpl_syms));<br>
+<br>
+               nksyms = UINT(modbuf + OFFSET(module_num_symtab));<br>
+<br>
+               mod_name = modbuf + OFFSET(module_name);<br>
+<br>
+               lm = &st->load_modules[m++];<br>
+               BZERO(lm, sizeof(struct load_module));<br>
+<br>
+               size = 0;<br>
+               for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) {<br>
+                       lm->mem[i].base = ULONG(modbuf + OFFSET(module_mem) +<br>
+                                               SIZE(module_memory) * i + OFFSET(module_memory_base));<br>
+                       lm->mem[i].size = UINT(modbuf + OFFSET(module_mem) +<br>
+                                               SIZE(module_memory) * i + OFFSET(module_memory_size));<br>
+                       if (i < MOD_INIT_TEXT)<br>
+                               size += lm->mem[i].size;<br>
+               }<br>
+               /* mem[MOD_TEXT].base for now */<br>
+               lm->mod_base = lm->mem[MOD_TEXT].base;<br>
+               /* The entire module size */<br>
+               lm->mod_size = size;<br>
+               lm->module_struct = mod;<br>
+<br>
+               if (strlen(mod_name) < MAX_MOD_NAME)<br>
+                       strcpy(lm->mod_name, mod_name);<br>
+               else {<br>
+                       error(INFO, "module name greater than MAX_MOD_NAME: %s\n", mod_name);<br>
+                       strncpy(lm->mod_name, mod_name, MAX_MOD_NAME-1);<br>
+               }<br>
+               if (CRASHDEBUG(3))<br>
+                       fprintf(fp, "%lx (%lx): %s syms: %d gplsyms: %d ksyms: %ld\n",<br>
+                               mod, lm->mod_base, lm->mod_name, nsyms, ngplsyms, nksyms);<br>
+<br>
+               lm->mod_flags = MOD_EXT_SYMS;<br>
+               lm->mod_ext_symcnt = mcnt;<br>
+<br>
+               lm->mod_text_start = lm->mod_base;<br>
+               lm->mod_etext_guess = lm->mod_base + lm->mem[MOD_TEXT].size;<br>
+<br>
+               lm->mod_init_module_ptr = lm->mem[MOD_INIT_TEXT].base;<br>
+               lm->mod_init_size = lm->mem[MOD_INIT_TEXT].size; /* tentative.. */<br>
+               lm->mod_init_text_size = lm->mem[MOD_INIT_TEXT].size;<br>
+<br>
+               if (VALID_MEMBER(module_percpu))<br>
+                       lm->mod_percpu = ULONG(modbuf + OFFSET(module_percpu));<br>
+<br>
+               lm_mcnt = mcnt;<br>
+               for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) {<br>
+                       if (!lm->mem[i].base)<br>
+                               continue;<br>
+<br>
+                       st->ext_module_symtable[mcnt].value = lm->mem[i].base;<br>
+                       st->ext_module_symtable[mcnt].type = 'm';<br>
+                       st->ext_module_symtable[mcnt].flags |= MODULE_SYMBOL;<br>
+                       sprintf(buf2, "%s%s", module_start_tags[i], mod_name);<br>
+                       namespace_ctl(NAMESPACE_INSTALL, &st->ext_module_namespace,<br>
+                               &st->ext_module_symtable[mcnt], buf2);<br>
+                       lm_mcnt = mcnt;<br>
+                       mcnt++;<br>
+<br>
+                       if (i >= MOD_INIT_TEXT)<br>
+                               lm->mod_flags |= MOD_INIT;<br>
+               }<br>
+<br>
+               if (nsyms && !IN_MODULE(syms, lm)) {<br>
+                       error(WARNING,<br>
+                           "[%s] module.syms outside of module " "address space (%lx)\n\n",<br>
+                               lm->mod_name, syms);<br>
+                       nsyms = 0;<br>
+               }<br>
+<br>
+               if (nsyms) {<br>
+                       modsymbuf = GETBUF(kernel_symbol_size*nsyms);<br>
+                       readmem((ulong)syms, KVADDR, modsymbuf,<br>
+                               nsyms * kernel_symbol_size,<br>
+                               "module symbols", FAULT_ON_ERROR);<br>
+               }<br>
+<br>
+               for (i = first = last = 0; i < nsyms; i++) {<br>
+                       modsym = (union kernel_symbol *)<br>
+                           (modsymbuf + (i * kernel_symbol_size));<br>
+                       if (!first<br>
+                           || first > modsym_name(syms, modsym, i))<br>
+                               first = modsym_name(syms, modsym, i);<br>
+                       if (modsym_name(syms, modsym, i) > last)<br>
+                               last = modsym_name(syms, modsym, i);<br>
+               }<br>
+<br>
+               if (last > first) {<br>
+                       /* The buffer should not go over the block. */<br>
+                       ulong end = module_mem_end(first, lm);<br>
+<br>
+                       strbuflen = (last-first) + BUFSIZE;<br>
+                       if ((first + strbuflen) >= end) {<br>
+                               strbuflen = end - first;<br>
+<br>
+                       }<br>
+                       strbuf = GETBUF(strbuflen);<br>
+<br>
+                       if (!readmem(first, KVADDR, strbuf, strbuflen,<br>
+                           "module symbol strings", RETURN_ON_ERROR)) {<br>
+                               FREEBUF(strbuf);<br>
+                               strbuf = NULL;<br>
+                       }<br></blockquote><div><br></div><div>The else code block can be dropped.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+               } else<br>
+                       strbuf = NULL;<br></blockquote><div><br></div><div>The strbuf can be initialized at the beginning of this function. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+<br>
+               for (i = 0; i < nsyms; i++) {<br>
+                       modsym = (union kernel_symbol *)(modsymbuf + (i * kernel_symbol_size));<br>
+<br>
+                       BZERO(buf1, BUFSIZE);<br>
+<br>
+                       if (strbuf)<br>
+                               strcpy(buf1, &strbuf[modsym_name(syms, modsym, i) - first]);<br>
+                       else<br>
+                               read_string(modsym_name(syms, modsym, i), buf1, BUFSIZE-1);<br>
+<br>
+                       if (strlen(buf1)) {<br>
+                               st->ext_module_symtable[mcnt].value =<br>
+                                       modsym_value(syms, modsym, i);<br>
+                               st->ext_module_symtable[mcnt].type = '?';<br>
+                               st->ext_module_symtable[mcnt].flags |= MODULE_SYMBOL;<br>
+                               strip_module_symbol_end(buf1);<br>
+                               strip_symbol_end(buf1, NULL);<br>
+                               namespace_ctl(NAMESPACE_INSTALL,<br>
+                                   &st->ext_module_namespace,<br>
+                                   &st->ext_module_symtable[mcnt], buf1);<br>
+<br>
+                               mcnt++;<br>
+                       }<br>
+               }<br>
+<br>
+               if (modsymbuf) {<br>
+                       FREEBUF(modsymbuf);<br>
+                       modsymbuf = NULL;<br>
+               }<br>
+<br>
+               if (strbuf)<br>
+                       FREEBUF(strbuf);<br>
+<br>
+               if (ngplsyms) {<br>
+                       modsymbuf = GETBUF(kernel_symbol_size * ngplsyms);<br>
+                       readmem((ulong)gpl_syms, KVADDR, modsymbuf,<br>
+                               ngplsyms * kernel_symbol_size,<br>
+                               "module gpl symbols", FAULT_ON_ERROR);<br>
+               }<br>
+<br>
+               for (i = first = last = 0; i < ngplsyms; i++) {<br>
+                       modsym = (union kernel_symbol *)<br>
+                           (modsymbuf + (i * kernel_symbol_size));<br>
+                       if (!first<br>
+                           || first > modsym_name(gpl_syms, modsym, i))<br>
+                               first = modsym_name(gpl_syms, modsym, i);<br>
+                       if (modsym_name(gpl_syms, modsym, i) > last)<br>
+                               last = modsym_name(gpl_syms, modsym, i);<br>
+               }<br>
+<br>
+               if (last > first) {<br>
+                       ulong end = module_mem_end(first, lm);<br>
+<br>
+                       strbuflen = (last-first) + BUFSIZE;<br>
+                       if ((first + strbuflen) >= end) {<br>
+                               strbuflen = end - first;<br>
+<br>
+                       }<br>
+                       strbuf = GETBUF(strbuflen);<br>
+<br>
+                       if (!readmem(first, KVADDR, strbuf, strbuflen,<br>
+                           "module gpl symbol strings", RETURN_ON_ERROR)) {<br>
+                               FREEBUF(strbuf);<br>
+                               strbuf = NULL;<br>
+                       }<br>
+               } else<br>
+                       strbuf = NULL;<br>
+<br>
+               for (i = 0; i < ngplsyms; i++) {<br>
+                       modsym = (union kernel_symbol *) (modsymbuf + (i * kernel_symbol_size));<br>
+<br>
+                       BZERO(buf1, BUFSIZE);<br>
+<br>
+                       if (strbuf)<br>
+                               strcpy(buf1, &strbuf[modsym_name(gpl_syms, modsym, i) - first]);<br>
+                       else<br>
+                               read_string(modsym_name(gpl_syms, modsym, i), buf1, BUFSIZE-1);<br>
+<br>
+                       if (strlen(buf1)) {<br>
+                               st->ext_module_symtable[mcnt].value =<br>
+                                       modsym_value(gpl_syms, modsym, i);<br>
+                               st->ext_module_symtable[mcnt].type = '?';<br>
+                               st->ext_module_symtable[mcnt].flags |= MODULE_SYMBOL;<br>
+                               strip_module_symbol_end(buf1);<br>
+                               strip_symbol_end(buf1, NULL);<br>
+                               namespace_ctl(NAMESPACE_INSTALL,<br>
+                                   &st->ext_module_namespace,<br>
+                                   &st->ext_module_symtable[mcnt], buf1);<br>
+<br>
+                               mcnt++;<br>
+                       }<br>
+               }<br>
+<br>
+               if (modsymbuf) {<br>
+                       FREEBUF(modsymbuf);<br>
+                       modsymbuf = NULL;<br>
+               }<br>
+<br>
+               if (strbuf)<br>
+                       FREEBUF(strbuf);<br>
+<br>
+               /*<br>
+                *  If the module was compiled with kallsyms, add them in.<br>
+                */<br>
+               switch (kt->flags & (KALLSYMS_V1|KALLSYMS_V2))<br>
+               {<br>
+               case KALLSYMS_V1:  /* impossible, I hope... */<br>
+                       mcnt += store_module_kallsyms_v1(lm, lm_mcnt, mcnt, modbuf);<br>
+                       break;<br>
+               case KALLSYMS_V2:<br>
+                       mcnt += store_module_kallsyms_v2(lm, lm_mcnt, mcnt, modbuf);<br>
+                       break;<br>
+               }<br>
+<br>
+               for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) {<br>
+                       if (!lm->mem[i].base)<br>
+                               continue;<br>
+<br>
+                       st->ext_module_symtable[mcnt].value = lm->mem[i].base + lm->mem[i].size;<br>
+                       st->ext_module_symtable[mcnt].type = 'm';<br>
+                       st->ext_module_symtable[mcnt].flags |= MODULE_SYMBOL;<br>
+                       sprintf(buf2, "%s%s", module_end_tags[i], mod_name);<br>
+                       namespace_ctl(NAMESPACE_INSTALL,<br>
+                               &st->ext_module_namespace,<br>
+                               &st->ext_module_symtable[mcnt], buf2);<br>
+                       mcnt++;<br>
+               }<br>
+<br>
+               lm->mod_ext_symcnt = mcnt - lm->mod_ext_symcnt;<br>
+<br>
+               if (!lm->mod_etext_guess)<br>
+                       find_mod_etext(lm);<br>
+<br>
+               NEXT_MODULE(mod_next, modbuf);<br>
+       }<br>
+<br>
+       FREEBUF(modbuf);<br>
+<br>
+       st->ext_module_symcnt = mcnt;<br>
+       st->ext_module_symend = &st->ext_module_symtable[mcnt];<br>
+<br>
+       namespace_ctl(NAMESPACE_COMPLETE, &st->ext_module_namespace,<br>
+               st->ext_module_symtable, st->ext_module_symend);<br>
+<br>
+       qsort(st->ext_module_symtable, mcnt, sizeof(struct syment),<br>
+               compare_syms);<br>
+<br>
+       /* not sure whether this sort is meaningful anymore. */<br></blockquote><div><br></div><div>I tried to remove it and tested again, seems that the sort result is not expected.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+       qsort(st->load_modules, m, sizeof(struct load_module), compare_mods);<br>
+<br>
+       for (m = 0; m < st->mods_installed; m++) {<br>
+               lm = &st->load_modules[m];<br>
+<br>
+               /* TODO: make more efficient */<br>
+               for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) {<br>
+                       if (!lm->mem[i].base)<br>
+                               continue;<br>
+<br>
+                       sprintf(buf1, "%s%s", module_start_tags[i], lm->mod_name);<br>
+                       sprintf(buf2, "%s%s", module_end_tags[i], lm->mod_name);<br>
+<br>
+                       for (sp = st->ext_module_symtable; sp < st->ext_module_symend; sp++) {<br>
+                               if (STREQ(sp->name, buf1)) {<br>
+                                       lm->symtable[i] = sp;<br>
+                                       break;<br>
+                               }<br>
+                       }<br>
+                       for ( ; sp < st->ext_module_symend; sp++) {<br>
+                               if (STREQ(sp->name, buf2)) {<br>
+                                       lm->symend[i] = sp;<br>
+                                       break;<br>
+                               }<br>
+                       }<br>
+<br>
+                       if (lm->symtable[i] && lm->symend[i])<br>
+                               mod_symtable_hash_install_range(lm->symtable[i], lm->symend[i]);<br>
+               }<br>
+       }<br>
+<br>
+       st->flags |= MODULE_SYMS;<br>
+<br>
+       /* probably not needed anymore */<br></blockquote><div> </div><div>Could you please explain the details why this is not needed anymore?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+       if (symbol_query("__insmod_", NULL, NULL))<br>
+               st->flags |= INSMOD_BUILTIN;<br>
+<br>
+       if (mcnt > total)<br>
+               error(FATAL, "store_module_symbols_v3: total: %ld mcnt: %d\n", total, mcnt);<br>
+}<br>
+<br>
 void<br>
 store_module_symbols_v2(ulong total, int mods_installed)<br>
 {<br>
@@ -2384,6 +2767,7 @@ store_module_kallsyms_v2(struct load_module *lm, int start, int curr,<br>
         int mcnt;<br>
         int mcnt_idx;<br>
        char *module_buf_init = NULL;<br>
+       ulong base, base_init, size, size_init;<br>
<br>
        if (!(kt->flags & KALLSYMS_V2))<br>
                return 0;<br>
@@ -2394,9 +2778,22 @@ store_module_kallsyms_v2(struct load_module *lm, int start, int curr,<br>
         ns = &st->ext_module_namespace;<br>
        ec = &elf_common;<br>
<br>
-        module_buf = GETBUF(lm->mod_size);<br>
+       /* kallsyms data looks to be in MOD_DATA region. */<br>
+       if (MODULE_MEMORY()) {<br>
+               base = lm->mem[MOD_DATA].base;<br>
+               size = lm->mem[MOD_DATA].size;<br>
+               base_init = lm->mem[MOD_INIT_DATA].base;<br>
+               size_init = lm->mem[MOD_INIT_DATA].size;<br>
+       } else {<br>
+               base = lm->mod_base;<br>
+               size = lm->mod_size;<br>
+               base_init = lm->mod_init_module_ptr;<br>
+               size_init = lm->mod_init_size;<br>
+       }<br>
+<br>
+       module_buf = GETBUF(size);<br>
<br>
-        if (!readmem(lm->mod_base, KVADDR, module_buf, lm->mod_size,<br>
+        if (!readmem(base, KVADDR, module_buf, size,<br>
             "module (kallsyms)", RETURN_ON_ERROR|QUIET)) {<br>
                 error(WARNING,"cannot access module kallsyms\n");<br>
                 FREEBUF(module_buf);<br>
@@ -2404,10 +2801,10 @@ store_module_kallsyms_v2(struct load_module *lm, int start, int curr,<br>
         }<br>
<br>
        if (lm->mod_init_size > 0) {<br>
-               module_buf_init = GETBUF(lm->mod_init_size);<br>
+               module_buf_init = GETBUF(size_init);<br>
<br>
-               if (!readmem(lm->mod_init_module_ptr, KVADDR, module_buf_init, lm->mod_init_size,<br>
-                                       "module init (kallsyms)", RETURN_ON_ERROR|QUIET)) {<br>
+               if (!readmem(base_init, KVADDR, module_buf_init, size_init,<br>
+                               "module init (kallsyms)", RETURN_ON_ERROR|QUIET)) {<br>
                        error(WARNING,"cannot access module init kallsyms\n");<br>
                        FREEBUF(module_buf_init);<br>
                }<br>
@@ -2429,9 +2826,9 @@ store_module_kallsyms_v2(struct load_module *lm, int start, int curr,<br>
                return 0;<br>
        } <br>
        if (IN_MODULE(ksymtab, lm))<br>
-               locsymtab = module_buf + (ksymtab - lm->mod_base);<br>
+               locsymtab = module_buf + (ksymtab - base);<br>
        else<br>
-               locsymtab = module_buf_init + (ksymtab - lm->mod_init_module_ptr);<br>
+               locsymtab = module_buf_init + (ksymtab - base_init);<br>
<br>
        kstrtab = ULONG(modbuf + OFFSET(module_strtab));<br>
        if (!IN_MODULE(kstrtab, lm) && !IN_MODULE_INIT(kstrtab, lm)) {<br>
@@ -2444,9 +2841,9 @@ store_module_kallsyms_v2(struct load_module *lm, int start, int curr,<br>
                return 0;<br>
        }<br>
        if (IN_MODULE(kstrtab, lm))<br>
-               locstrtab = module_buf + (kstrtab - lm->mod_base);<br>
+               locstrtab = module_buf + (kstrtab - base);<br>
        else<br>
-               locstrtab = module_buf_init + (kstrtab - lm->mod_init_module_ptr);<br>
+               locstrtab = module_buf_init + (kstrtab - base_init);<br>
<br>
        for (i = 1; i < nksyms; i++) {  /* ELF starts real symbols at 1 */<br>
                switch (BITS())<br>
@@ -2461,11 +2858,8 @@ store_module_kallsyms_v2(struct load_module *lm, int start, int curr,<br>
                        break;<br>
                }<br>
<br>
-               if (((ec->st_value < lm->mod_base) ||<br>
-                   (ec->st_value >  (lm->mod_base + lm->mod_size))) &&<br>
-                   ((ec->st_value < lm->mod_init_module_ptr) ||<br>
-                   (ec->st_value > (lm->mod_init_module_ptr + lm->mod_init_size))))<br>
-                               continue;<br>
+               if (!IN_MODULE(ec->st_value, lm) && !IN_MODULE_INIT(ec->st_value, lm))<br>
+                       continue;<br>
<br>
                if (ec->st_shndx == SHN_UNDEF)<br>
                         continue;<br>
@@ -3531,6 +3925,13 @@ dump_symbol_table(void)<br>
                        (ulong)lm->mod_section_data,<br>
                        lm->mod_section_data ? "" : "(not allocated)");<br>
<br>
+               if (MODULE_MEMORY()) {<br>
+                       int j;<br>
+                       for (j = MOD_TEXT; j < MOD_MEM_NUM_TYPES; j++) {<br>
+                               fprintf(fp, "                mem[%d]: %lx (%x)\n",<br>
+                                       j, lm->mem[j].base, lm->mem[j].size);<br>
+                       }<br>
+               }<br>
<br>
                for (s = 0; s < lm->mod_sections; s++) {<br>
                        fprintf(fp, <br>
@@ -9228,6 +9629,9 @@ dump_offset_table(char *spec, ulong makestruct)<br>
                OFFSET(module_strtab));<br>
        fprintf(fp, "                 module_percpu: %ld\n",<br>
                OFFSET(module_percpu));<br>
+       fprintf(fp, "                    module_mem: %ld\n", OFFSET(module_mem));<br>
+       fprintf(fp, "            module_memory_base: %ld\n", OFFSET(module_memory_base));<br>
+       fprintf(fp, "            module_memory_size: %ld\n", OFFSET(module_memory_size));<br>
<br>
        fprintf(fp, "             module_sect_attrs: %ld\n",<br>
                OFFSET(module_sect_attrs));<br>
@@ -10851,6 +11255,7 @@ dump_offset_table(char *spec, ulong makestruct)<br>
                SIZE(super_block)); <br>
        fprintf(fp, "                       irqdesc: %ld\n", SIZE(irqdesc));<br>
        fprintf(fp, "                        module: %ld\n", SIZE(module));<br>
+       fprintf(fp, "                 module_memory: %ld\n", SIZE(module_memory));<br>
        fprintf(fp, "              module_sect_attr: %ld\n", SIZE(module_sect_attr));<br>
        fprintf(fp, "                     list_head: %ld\n", SIZE(list_head));<br>
        fprintf(fp, "                    hlist_head: %ld\n", SIZE(hlist_head));<br>
@@ -13520,3 +13925,52 @@ symbol_complete_match(const char *match, struct syment *sp_last)<br>
<br>
        return NULL;<br>
 }<br>
+<br>
+static int<br>
+_in_module(ulong addr, struct load_module *lm, int type)<br>
+{<br>
+       ulong base, size;<br>
+       int i, last;<br>
+<br>
+       if (MODULE_MEMORY()) {<br>
+               if (type == MOD_TEXT)<br>
+                       last = MOD_RO_AFTER_INIT;<br>
+               else<br>
+                       last = MOD_INIT_RODATA;<br></blockquote><div> </div><div>The above if-else code block is to speed up the searching performance? Or are there overlapped address spaces in different module types?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+               for (i = type ; i <= last; i++) {<br>
+                       base = lm->mem[i].base;<br>
+                       size = lm->mem[i].size;<br>
+                       if (!base)<br>
+                               continue;<br>
+                       if ((addr >= base) && (addr < (base + size)))<br>
+                               return TRUE;<br>
+               }<br>
+               return FALSE;<br>
+       }<br>
+<br>
+       if (type == MOD_TEXT) {<br>
+               base = lm->mod_base;<br>
+               size = lm->mod_size;<br>
+       } else {<br>
+               base = lm->mod_init_module_ptr;<br>
+               size = lm->mod_init_size;<br>
+       }<br></blockquote><div><br></div><div>For the old module layout(non module memory), only deal with two cases according to the IN_MODULE() and IN_MODULE_INIT(): MOD_TEXT and MOD_INIT_TEXT. Otherwise explicitly says that it is not supported. For example:</div><div> </div><div>switch (type)</div><div>{</div><div>case MOD_TEXT:</div><div>    base = lm->mod_base;</div><div>    size = lm->mod_size;</div><div>    break;</div><div>case MOD_INIT_TEXT:</div><div>    base = lm->mod_init_module_ptr;</div><div>    size = lm->mod_init_size;</div><div>    break;</div><div>default:</div><div>    error(FATAL, "unsupported module layout type!");</div><div>} </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+       return ((addr >= base) && (addr < (base + size)));<br>
+}<br>
+<br>
+/* Returns the end address of the module memory region. */<br>
+static ulong<br>
+module_mem_end(ulong addr, struct load_module *lm)<br>
+{<br>
+       ulong base, end;<br>
+       int i;<br></blockquote><div><br></div><div>This function only handles the module memory caes, so need to add a check as below:</div><div><br></div><div>if (!MODULE_MEMORY()) </div><div>    return 0;</div><div><br></div><div>Although the module_mem_end() is only invoked in the store_module_symbols_v3(), we can not guarantee that the module_mem_end() won't be called in other functions in future.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+       for (i = MOD_TEXT; i < MOD_MEM_NUM_TYPES; i++) {<br>
+               base = lm->mem[i].base;<br>
+               if (!base)<br>
+                       continue;<br>
+               end = base + lm->mem[i].size;<br>
+               if ((addr >= base) && (addr < end))<br>
+                       return end;<br>
+       }<br>
+       return 0;<br>
+}<br></blockquote><div><br></div><div>BTW: I noticed that they have the similar code between the _in_module() and module_mem_end(), if we can improve the _in_module() and return the end address of a module memory region from _in_module(), instead of only returning TRUE or FALSE, maybe the module_mem_end() can be removed.</div><div><br></div><div>I will continue to comment on the remaining patches next week.</div><div><br></div><div>Thanks.</div><div>Lianbo</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
-- <br>
2.31.1<br>
<br>
</blockquote></div></div>