[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCHv6 04/23] plat: Clean up kernel image symbols
Hello Jia He,
Please find a comment inline.
Thanks & Regards
Sharan
On 2/22/19 7:21 AM, 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>
---
v5->v6: fix compilation error on x86 xen plat.
---
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..1862118 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)
Redundant type conversion.
+ if (mem_base > (unsigned long)__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 */
--
2.17.1
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
