[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCHv3 05/25] plat: Clean up kernel image symbols

Hello Wei Chen,
This patch is functionally fine but check_patch reports an error. Please find inline where check_patch fails.
On the side note, this patch depends on the previous patch(04) where there is an ongoing discussion on the macro implementation. 

Thanks & Regards
Sharan

On 12/13/18 10:15 AM, Wei Chen wrote:

Use macros instead of access kernel image symbols directly will avoid
undefined behavior of subtracting or comparing pointers that do not
refer to the same array.

Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
---
  plat/kvm/arm/setup.c          |  7 +++----
  plat/kvm/memory.c             | 23 +++++++++++------------
  plat/kvm/x86/setup.c          |  6 +++---
  plat/xen/arm/setup.c          |  6 +++---
  plat/xen/include/xen-arm/mm.h |  3 +--
  plat/xen/include/xen-x86/mm.h | 21 +++++++++++----------
  plat/xen/memory.c             | 22 +++++++++++-----------
  plat/xen/x86/mm.c             | 17 ++++++++---------
  plat/xen/x86/setup.c          |  2 +-
  9 files changed, 52 insertions(+), 55 deletions(-)

diff --git a/plat/kvm/arm/setup.c b/plat/kvm/arm/setup.c
index 09530bb..14efcfd 100644
--- a/plat/kvm/arm/setup.c
+++ b/plat/kvm/arm/setup.c
@@ -19,6 +19,7 @@
   * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
   */
  #include <libfdt.h>
+#include <sections.h>
  #include <kvm/console.h>
  #include <uk/assert.h>
  #include <kvm-arm/mm.h>
@@ -95,8 +96,6 @@ enomethod:
static void _init_dtb_mem(void) 
  {
-       extern char _text[];
-       extern char _end[];
        int fdt_mem, prop_len = 0, prop_min_len;
        int naddr, nsize;
        const uint64_t *regs;
@@ -142,11 +141,11 @@ static void _init_dtb_mem(void)
mem_base = fdt64_to_cpu(regs[0]); 
        mem_size = fdt64_to_cpu(regs[1]);
-       if (mem_base > (uint64_t)&_text)
+       if (mem_base > __TEXT)
                UK_CRASH("Fatal: Image outside of RAM\n");
max_addr = mem_base + mem_size; 
-       _libkvmplat_pagetable =(void *) ALIGN_DOWN((size_t)&_end, __PAGE_SIZE);
+       _libkvmplat_pagetable =(void *) ALIGN_DOWN((size_t)__END, __PAGE_SIZE);

Check patch reports an error. Should be
_libkvmplat_pagetable = (void *) ALIGN_DOWN((size_t)__END, __PAGE_SIZE);


        _libkvmplat_heap_start = _libkvmplat_pagetable + PAGE_TABLE_SIZE;
        _libkvmplat_mem_end = (void *) max_addr;
diff --git a/plat/kvm/memory.c b/plat/kvm/memory.c 
index 11c993d..a7b4d5e 100644
--- a/plat/kvm/memory.c
+++ b/plat/kvm/memory.c
@@ -19,6 +19,7 @@
   * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
   */
+#include <sections.h> 
  #include <sys/types.h>
  #include <uk/plat/memory.h>
  #include <uk/assert.h>
@@ -37,16 +38,14 @@ int ukplat_memregion_count(void)
int ukplat_memregion_get(int i, struct ukplat_memregion_desc *m) 
  {
-       extern char _text, _etext, _data, _edata, _rodata, _erodata,
-                   _ctors, _ectors, __bss_start, _end;
        int ret;
UK_ASSERT(m); switch (i) { 
        case 0: /* text */
-               m->base  = &_text;
-               m->len   = (size_t) &_etext - (size_t) &_text;
+               m->base  = (void *) __TEXT;
+               m->len   = (size_t) __ETEXT - (size_t) __TEXT;
                m->flags = (UKPLAT_MEMRF_RESERVED
                            | UKPLAT_MEMRF_READABLE);
  #if CONFIG_UKPLAT_MEMRNAME
@@ -55,8 +54,8 @@ int ukplat_memregion_get(int i, struct ukplat_memregion_desc 
*m)
                ret = 0;
                break;
        case 1: /* rodata */
-               m->base  = &_rodata;
-               m->len   = (size_t) &_erodata - (size_t) &_rodata;
+               m->base  = (void *) __RODATA;
+               m->len   = (size_t) __ERODATA - (size_t) __RODATA;
                m->flags = (UKPLAT_MEMRF_RESERVED
                            | UKPLAT_MEMRF_READABLE);
  #if CONFIG_UKPLAT_MEMRNAME
@@ -65,8 +64,8 @@ int ukplat_memregion_get(int i, struct ukplat_memregion_desc 
*m)
                ret = 0;
                break;
        case 2: /* ctors */
-               m->base  = &_ctors;
-               m->len   = (size_t) &_ectors - (size_t) &_ctors;
+               m->base  = (void *) __CTORS;
+               m->len   = (size_t) __ECTORS - (size_t) __CTORS;
                m->flags = (UKPLAT_MEMRF_RESERVED
                            | UKPLAT_MEMRF_READABLE);
  #if CONFIG_UKPLAT_MEMRNAME
@@ -75,8 +74,8 @@ int ukplat_memregion_get(int i, struct ukplat_memregion_desc 
*m)
                ret = 0;
                break;
        case 3: /* data */
-               m->base  = &_data;
-               m->len   = (size_t) &_edata - (size_t) &_data;
+               m->base  = (void *) __DATA;
+               m->len   = (size_t) __EDATA - (size_t) __DATA;
                m->flags = (UKPLAT_MEMRF_RESERVED
                            | UKPLAT_MEMRF_READABLE
                            | UKPLAT_MEMRF_WRITABLE);
@@ -86,8 +85,8 @@ int ukplat_memregion_get(int i, struct ukplat_memregion_desc 
*m)
                ret = 0;
                break;
        case 4: /* bss */
-               m->base  = &__bss_start;
-               m->len   = (size_t) &_end - (size_t) &__bss_start;
+               m->base  = (void *) __BSS_START;
+               m->len   = (size_t) __END - (size_t) __BSS_START;
                m->flags = (UKPLAT_MEMRF_RESERVED
                            | UKPLAT_MEMRF_READABLE
                            | UKPLAT_MEMRF_WRITABLE);
diff --git a/plat/kvm/x86/setup.c b/plat/kvm/x86/setup.c
index e02886d..a6f31dd 100644
--- a/plat/kvm/x86/setup.c
+++ b/plat/kvm/x86/setup.c
@@ -27,6 +27,7 @@
   */
#include <string.h> 
+#include <sections.h>
  #include <x86/traps.h>
  #include <kvm/console.h>
  #include <kvm/intctrl.h>
@@ -78,7 +79,6 @@ static inline void _mb_get_cmdline(struct multiboot_info *mi, 
char *cmdline,
static inline void _mb_init_mem(struct multiboot_info *mi) 
  {
-       extern char _end;
        multiboot_memory_map_t *m;
        size_t offset, max_addr;
@@ -102,9 +102,9 @@ static inline void _mb_init_mem(struct multiboot_info *mi) 
        max_addr = m->addr + m->len;
        if (max_addr > PLATFORM_MAX_MEM_ADDR)
                max_addr = PLATFORM_MAX_MEM_ADDR;
-       UK_ASSERT((size_t)&_end <= max_addr);
+       UK_ASSERT((size_t)__END <= max_addr);
- _libkvmplat_heap_start = (void *) ALIGN_UP((size_t)&_end, __PAGE_SIZE); 
+       _libkvmplat_heap_start = (void *) ALIGN_UP((size_t)__END, __PAGE_SIZE);
        _libkvmplat_mem_end    = (void *) max_addr;
        _libkvmplat_stack_top  = (void *) (max_addr - __STACK_SIZE);
  }
diff --git a/plat/xen/arm/setup.c b/plat/xen/arm/setup.c
index 4b9efe5..d854fa0 100644
--- a/plat/xen/arm/setup.c
+++ b/plat/xen/arm/setup.c
@@ -25,7 +25,7 @@
  /* Ported from Mini-OS */
#include <string.h> 
-
+#include <sections.h>
  #include <xen-arm/os.h>
  #include <xen-arm/mm.h>
  #include <xen/xen.h>
@@ -142,10 +142,10 @@ static inline void _dtb_init_mem(uint32_t physical_offset)
        if (regs == NULL && prop_len < 16)
                UK_CRASH("Bad 'reg' property: %p %d\n", regs, prop_len);
- end = (uintptr_t) &_end; 
+       end = (uintptr_t) __END;
        mem_base = fdt64_to_cpu(regs[0]);
        mem_size = fdt64_to_cpu(regs[1]);
-       if (to_virt(mem_base) > (void *)&_text)
+       if (to_virt(mem_base) > (void *)__TEXT)
                UK_CRASH("Fatal: Image outside of RAM\n");
start_pfn_p = PFN_UP(to_phys(end)); 
diff --git a/plat/xen/include/xen-arm/mm.h b/plat/xen/include/xen-arm/mm.h
index 9b8ea85..0f5c8f5 100644
--- a/plat/xen/include/xen-arm/mm.h
+++ b/plat/xen/include/xen-arm/mm.h
@@ -28,11 +28,10 @@
  #define _ARCH_MM_H_
#include <stdint.h> 
+#include <sections.h>
  #include <uk/arch/limits.h>
typedef uint64_t paddr_t; 
-
-extern char _text, _etext, _data, _edata, _rodata, _erodata, _end, __bss_start;
  extern int _boot_stack[];
  extern int _boot_stack_end[];
  /* Add this to a virtual address to get the physical address (wraps at 4GB) */
diff --git a/plat/xen/include/xen-x86/mm.h b/plat/xen/include/xen-x86/mm.h
index 0e59796..dc36b59 100644
--- a/plat/xen/include/xen-x86/mm.h
+++ b/plat/xen/include/xen-x86/mm.h
@@ -9,22 +9,23 @@
   * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
   * sell copies of the Software, and to permit persons to whom the Software is
   * furnished to do so, subject to the following conditions:
- *
+ *
   * The above copyright notice and this permission notice shall be included in
   * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
   * DEALINGS IN THE SOFTWARE.
   */
#ifndef _ARCH_MM_H_ 
  #define _ARCH_MM_H_
+#include <sections.h> 
  #ifndef __ASSEMBLY__
  #include <xen/xen.h>
  #if defined(__i386__)
@@ -221,7 +222,7 @@ extern unsigned long *phys_to_machine_mapping;
  #else
  extern pgentry_t page_table_base[];
  #endif
-extern char _text, _etext, _erodata, _edata, _end;
+
  extern unsigned long mfn_zero;
  static __inline__ maddr_t phys_to_machine(paddr_t phys)
  {
@@ -237,7 +238,7 @@ static __inline__ paddr_t machine_to_phys(maddr_t machine)
        return phys;
  }
-#define VIRT_START ((unsigned long)&_text) 
+#define VIRT_START                 (__TEXT)
#define to_phys(x) ((unsigned long)(x)-VIRT_START) 
  #define to_virt(x)                 ((void *)((unsigned long)(x)+VIRT_START))
diff --git a/plat/xen/memory.c b/plat/xen/memory.c
index f84dca7..cb8e3b8 100644
--- a/plat/xen/memory.c
+++ b/plat/xen/memory.c
@@ -34,6 +34,7 @@
   */
#include <string.h> 
+#include <sections.h>
#include <common/gnttab.h> 
  #if (defined __X86_32__) || (defined __X86_64__)
@@ -51,14 +52,13 @@ int ukplat_memregion_count(void)
int ukplat_memregion_get(int i, struct ukplat_memregion_desc *m) 
  {
-       extern char _text, _etext, _data, _edata, _rodata, _erodata, _ctors, 
_ectors, _end, __bss_start;
UK_ASSERT(m); switch(i) { 
        case 0: /* text */
-               m->base     = &_text;
-               m->len   = (size_t) &_etext - (size_t) &_text;
+               m->base  = (void *) __TEXT;
+               m->len   = (size_t) __ETEXT - (size_t) __TEXT;
                m->flags = (UKPLAT_MEMRF_RESERVED
                            | UKPLAT_MEMRF_READABLE);
  #if CONFIG_UKPLAT_MEMRNAME
@@ -66,8 +66,8 @@ int ukplat_memregion_get(int i, struct ukplat_memregion_desc 
*m)
  #endif
                break;
        case 1: /* ro data */
-               m->base  = &_rodata;
-               m->len   = (size_t) &_erodata - (size_t) &_rodata;
+               m->base  = (void *) __RODATA;
+               m->len   = (size_t) __ERODATA - (size_t) __RODATA;
                m->flags = (UKPLAT_MEMRF_RESERVED
                               | UKPLAT_MEMRF_READABLE);
  #if CONFIG_UKPLAT_MEMRNAME
@@ -75,8 +75,8 @@ int ukplat_memregion_get(int i, struct ukplat_memregion_desc 
*m)
  #endif
                break;
        case 2: /* ctors */
-               m->base  = &_ctors;
-               m->len   = (size_t) &_ectors - (size_t) &_ctors;
+               m->base  = (void *) __CTORS;
+               m->len   = (size_t) __ECTORS - (size_t) __CTORS;
                m->flags = (UKPLAT_MEMRF_RESERVED
                            | UKPLAT_MEMRF_READABLE);
  #if CONFIG_UKPLAT_MEMRNAME
@@ -84,8 +84,8 @@ int ukplat_memregion_get(int i, struct ukplat_memregion_desc 
*m)
  #endif
                break;
        case 3: /* data */
-               m->base  = &_data;
-               m->len   = (size_t) &_edata - (size_t) &_data;
+               m->base  = (void *) __DATA;
+               m->len   = (size_t) __EDATA - (size_t) __DATA;
                m->flags = (UKPLAT_MEMRF_RESERVED
                            | UKPLAT_MEMRF_READABLE
                            | UKPLAT_MEMRF_WRITABLE);
@@ -94,8 +94,8 @@ int ukplat_memregion_get(int i, struct ukplat_memregion_desc 
*m)
  #endif
                break;
        case 4: /* bss */
-               m->base  = &__bss_start;
-               m->len   = (size_t) &_end - (size_t) &__bss_start;
+               m->base  = (void *) __BSS_START;
+               m->len   = (size_t) __END - (size_t) __BSS_START;
                m->flags = (UKPLAT_MEMRF_RESERVED
                            | UKPLAT_MEMRF_READABLE
                            | UKPLAT_MEMRF_WRITABLE);
diff --git a/plat/xen/x86/mm.c b/plat/xen/x86/mm.c
index b89384f..cadca8a 100644
--- a/plat/xen/x86/mm.c
+++ b/plat/xen/x86/mm.c
@@ -36,6 +36,7 @@
   */
#include <string.h> 
+#include <sections.h>
  #include <errno.h>
  #include <uk/alloc.h>
  #include <uk/plat/config.h>
@@ -142,12 +143,10 @@ void _init_mem_build_pagetable(unsigned long *start_pfn, 
unsigned long *max_pfn)
      {
            uk_pr_warn("Trying to use Xen virtual space. "
                       "Truncating memory from %luMB to ",
-                      ((unsigned long)pfn_to_virt(*max_pfn) -
-                       (unsigned long)&_text)>>20);
+                      ((unsigned long)pfn_to_virt(*max_pfn) - __TEXT)>>20);
            *max_pfn = virt_to_pfn(HYPERVISOR_VIRT_START - PAGE_SIZE);
            uk_pr_warn("%luMB\n",
-                      ((unsigned long)pfn_to_virt(*max_pfn) -
-                       (unsigned long)&_text)>>20);
+                      ((unsigned long)pfn_to_virt(*max_pfn) - __TEXT)>>20);
      }
  #else
      /* Round up to next 2MB boundary as we are using 2MB pages on HVMlite. */
@@ -670,18 +669,18 @@ void _init_mem_clear_bootstrap(void)
      pgentry_t *pgt;
  #endif
- uk_pr_debug("Clear bootstrapping memory: %p\n", &_text); 
+    uk_pr_debug("Clear bootstrapping memory: %p\n", (void *)__TEXT);
/* Use first page as the CoW zero page */ 
-    memset(&_text, 0, PAGE_SIZE);
-    mfn_zero = virt_to_mfn((unsigned long) &_text);
+    memset((void *)__TEXT, 0, PAGE_SIZE);
+    mfn_zero = virt_to_mfn(__TEXT);
  #ifdef CONFIG_PARAVIRT
      if ( (rc = HYPERVISOR_update_va_mapping(0, nullpte, UVMF_INVLPG)) )
            uk_pr_err("Unable to unmap NULL page. rc=%d\n", rc);
  #else
-    pgt = get_pgt((unsigned long)&_text);
+    pgt = get_pgt(__TEXT);
      *pgt = 0;
-    invlpg((unsigned long)&_text);
+    invlpg(__TEXT);
  #endif
  }
diff --git a/plat/xen/x86/setup.c b/plat/xen/x86/setup.c 
index 35fdd35..80927fa 100644
--- a/plat/xen/x86/setup.c
+++ b/plat/xen/x86/setup.c
@@ -154,7 +154,7 @@ static inline void _init_mem(void)
_init_mem_build_pagetable(&start_pfn, &max_pfn); 
        _init_mem_clear_bootstrap();
-       _init_mem_set_readonly(&_text, &_erodata);
+       _init_mem_set_readonly((void *)__TEXT, (void *)__ERODATA);
/* Fill out mrd array */ 
        /* heap */



_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.

Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.