[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCHv7 04/23] plat: Clean up kernel image symbols



Hello Jia He,

This patch seems fine.

Reviewed-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>

Thanks & Regards
Sharan

On 3/1/19 3:54 PM, Jia He wrote:
From: Wei Chen <wei.chen@xxxxxxx>
There is a potential undefined behaviour for pointer comparision.
How to avoid this gcc optimization is under discussing. Since it
is not a block issue. I will follow it up in the future.
Provide macro definitions for text,bss,data... is the precondition
for future solution.

Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
Signed-off-by: Jia He <justin.he@xxxxxxx>
Reviewed-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
---
  plat/common/include/sections.h | 14 ++++++++++++++
  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 +-
  10 files changed, 66 insertions(+), 55 deletions(-)

diff --git a/plat/common/include/sections.h b/plat/common/include/sections.h
index 42f41d2..1701b00 100644
--- a/plat/common/include/sections.h
+++ b/plat/common/include/sections.h
@@ -62,4 +62,18 @@ extern char __bss_start[];
  /* _end: end of kernel image */
  extern char _end[];
+#define __uk_image_symbol(addr) ((unsigned long)(addr))
+
+#define __DTB          __uk_image_symbol(_dtb)
+#define __TEXT         __uk_image_symbol(_text)
+#define __ETEXT                __uk_image_symbol(_etext)
+#define __RODATA       __uk_image_symbol(_rodata)
+#define __ERODATA      __uk_image_symbol(_erodata)
+#define __DATA         __uk_image_symbol(_data)
+#define __EDATA                __uk_image_symbol(_edata)
+#define __CTORS                __uk_image_symbol(_ctors)
+#define __ECTORS       __uk_image_symbol(_ectors)
+#define __BSS_START    __uk_image_symbol(__bss_start)
+#define __END          __uk_image_symbol(_end)
+
  #endif /* __PLAT_CMN_SECTIONS_H__ */
diff --git a/plat/kvm/arm/setup.c b/plat/kvm/arm/setup.c
index 09530bb..c4a1f78 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);
        _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 c17a7dd..4ccd947 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/cpu.h>
  #include <x86/traps.h>
  #include <kvm/console.h>
@@ -79,7 +80,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;
@@ -103,9 +103,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 b91c0bd..56f1564 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..32d876a 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                 ((unsigned long)(__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 1f55887..b63f11b 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..2f23855 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 3c5631d..14b2c26 100644
--- a/plat/xen/x86/setup.c
+++ b/plat/xen/x86/setup.c
@@ -141,7 +141,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

 


Rackspace

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