On Fri, 9 Nov 2018, Julien Grall wrote:
> Hi Stefano,
>
> On 06/11/2018 22:05, Stefano Stabellini wrote:
> > Use SYMBOL everywhere _stext, _etext, etc. are used. Technically, it
> > is required when comparing and subtracting pointers [1], but use it
> > everywhere to avoid confusion.
> >
> > M3CM: Rule-18.2: Subtraction between pointers shall only be applied to
> > pointers that address elements of the same array
> >
> > [1]
> > https://wiki.sei.cmu.edu/confluence/display/c/ARR36-C.+Do+not+subtract+or+compare+two+pointers+that+do+not+refer+to+the+same+array
> >
> > QAVerify: 2761 (and many others)
> > Signed-off-by: Stefano Stabellini <[email protected]>
> > CC: [email protected]
> > CC: [email protected]
> > ---
> > Changes in v3:
> > - improve commit message
> > - no hard tabs
> > - rename __symbol to SYMBOL
> > - fix __end_vpci_array and __start_vpci_array
> > - avoid all comparisons between pointers: including (void *) casted
> > returns from SYMBOL()
> > - remove useless casts to (unsigned long)
> >
> > Changes in v2:
> > - cast return of SYMBOL to char* when required
> > - define __pa as unsigned long in is_kernel* functions
> > ---
> > xen/arch/arm/alternative.c | 7 ++--
> > xen/arch/arm/arm32/livepatch.c | 2 +-
> > xen/arch/arm/arm64/livepatch.c | 2 +-
> > xen/arch/arm/domain_build.c | 2 +-
> > xen/arch/arm/livepatch.c | 6 +--
> > xen/arch/arm/mm.c | 17 ++++----
> > xen/arch/arm/setup.c | 8 ++--
> > xen/arch/x86/setup.c | 79
> > +++++++++++++++++++------------------
> > xen/arch/x86/tboot.c | 12 +++---
> > xen/arch/x86/x86_64/machine_kexec.c | 4 +-
> > xen/drivers/vpci/vpci.c | 7 +++-
> > xen/include/asm-arm/grant_table.h | 3 +-
> > xen/include/asm-arm/mm.h | 4 +-
> > xen/include/asm-x86/mm.h | 4 +-
> > xen/include/xen/kernel.h | 24 +++++------
> > 15 files changed, 97 insertions(+), 84 deletions(-)
> >
> > diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> > index 52ed7ed..2efa9ca 100644
> > --- a/xen/arch/arm/alternative.c
> > +++ b/xen/arch/arm/alternative.c
> > @@ -187,8 +187,8 @@ static int __apply_alternatives_multi_stop(void *unused)
> > {
> > int ret;
> > struct alt_region region;
> > - mfn_t xen_mfn = virt_to_mfn(_start);
> > - paddr_t xen_size = _end - _start;
> > + mfn_t xen_mfn = virt_to_mfn(SYMBOL(_start));
> > + paddr_t xen_size = SYMBOL(_end) - SYMBOL(_start);
> > unsigned int xen_order = get_order_from_bytes(xen_size);
> > void *xenmap;
> > @@ -206,7 +206,8 @@ static int __apply_alternatives_multi_stop(void
> > *unused)
> > region.begin = __alt_instructions;
> > region.end = __alt_instructions_end;
> > - ret = __apply_alternatives(®ion, xenmap - (void *)_start);
> > + ret = __apply_alternatives(®ion,
> > + (unsigned long)xenmap - SYMBOL(_start));
> > /* The patching is not expected to fail during boot. */
> > BUG_ON(ret != 0);
> > diff --git a/xen/arch/arm/arm32/livepatch.c
> > b/xen/arch/arm/arm32/livepatch.c
> > index 41378a5..6bf9132 100644
> > --- a/xen/arch/arm/arm32/livepatch.c
> > +++ b/xen/arch/arm/arm32/livepatch.c
> > @@ -56,7 +56,7 @@ void arch_livepatch_apply(struct livepatch_func *func)
> > else
> > insn = 0xe1a00000; /* mov r0, r0 */
> > - new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
> > + new_ptr = func->old_addr - (void *)SYMBOL(_start) + vmap_of_xen_text;
>
> You cast again to (void *) and old_addr is a pointer as well. How is it safe?
It is not. I caught it after sending this version of the series to the
list. Now I am using the following:
new_ptr = (uint32_t *)((unsigned long)func->old_addr - SYMBOL(_start) +
(unsigned long)vmap_of_xen_text);
There are a couple of other instances exactly like this.
> > len = len / sizeof(uint32_t);
> > /* PATCH! */
> > diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
> > index 2247b92..fb1477d 100644
> > --- a/xen/arch/arm/arm64/livepatch.c
> > +++ b/xen/arch/arm/arm64/livepatch.c
> > @@ -43,7 +43,7 @@ void arch_livepatch_apply(struct livepatch_func *func)
> > /* Verified in livepatch_verify_distance. */
> > ASSERT(insn != AARCH64_BREAK_FAULT);
> > - new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
> > + new_ptr = func->old_addr - SYMBOL(_start) + vmap_of_xen_text;
>
> Here vmap_of_xen_text is a pointer and old_addr is a pointer too. How is it
> safe?
Yes, same here as above.
> > len = len / sizeof(uint32_t);
> > /* PATCH! */
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index f552154..6c03996 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -2090,7 +2090,7 @@ static void __init find_gnttab_region(struct domain
> > *d,
> > * Only use the text section as it's always present and will contain
> > * enough space for a large grant table
> > */
> > - kinfo->gnttab_start = __pa(_stext);
> > + kinfo->gnttab_start = __pa(SYMBOL(_stext));
>
> Would it make sense to introduce __pa_symbol here?
Following Jan's suggestion, all the __pa(SYMBOL(_stext)) will disappear
in the next version. I have already made the change. That is because
this is not a required change (no points comparisons or subtractions.)
> > kinfo->gnttab_size = gnttab_dom0_frames() << PAGE_SHIFT;
> > #ifdef CONFIG_ARM_32
> > diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
> > index 279d52c..609ab35 100644
> > --- a/xen/arch/arm/livepatch.c
> > +++ b/xen/arch/arm/livepatch.c
> > @@ -26,8 +26,8 @@ int arch_livepatch_quiesce(void)
> > if ( vmap_of_xen_text )
> > return -EINVAL;
> > - text_mfn = virt_to_mfn(_start);
> > - text_order = get_order_from_bytes(_end - _start);
> > + text_mfn = virt_to_mfn(SYMBOL(_start));
> > + text_order = get_order_from_bytes(SYMBOL(_end) - SYMBOL(_start));
> > /*
> > * The text section is read-only. So re-map Xen to be able to patch
> > @@ -78,7 +78,7 @@ void arch_livepatch_revert(const struct livepatch_func
> > *func)
> > uint32_t *new_ptr;
> > unsigned int len;
> > - new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
> > + new_ptr = func->old_addr - SYMBOL(_start) + vmap_of_xen_text;
>
> Same question as the previous old_addr.
Yes, I fixed it.
> > len = livepatch_insn_len(func);
> > memcpy(new_ptr, func->opaque, len);
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index 7a06a33..16a8afc 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -620,7 +620,7 @@ void __init setup_pagetables(unsigned long
> > boot_phys_offset, paddr_t xen_paddr)
> > int i;
> > /* Calculate virt-to-phys offset for the new location */
> > - phys_offset = xen_paddr - (unsigned long) _start;
> > + phys_offset = xen_paddr - SYMBOL(_start);
> > #ifdef CONFIG_ARM_64
> > p = (void *) xen_pgtable;
> > @@ -681,7 +681,8 @@ void __init setup_pagetables(unsigned long
> > boot_phys_offset, paddr_t xen_paddr)
> > ttbr = (uintptr_t) cpu0_pgtable + phys_offset;
> > #endif
> > - relocate_xen(ttbr, _start, (void*)dest_va, _end - _start);
> > + relocate_xen(ttbr, (void*)SYMBOL(_start), (void*)dest_va,
> > + SYMBOL(_end) - SYMBOL(_start));
> > /* Clear the copy of the boot pagetables. Each secondary CPU
> > * rebuilds these itself (see head.S) */
> > @@ -1089,7 +1090,7 @@ int modify_xen_mappings(unsigned long s, unsigned long
> > e, unsigned int flags)
> > }
> > enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
> > -static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg
> > mg)
> > +static void set_pte_flags_on_range(const unsigned long p, unsigned long l,
> > enum mg mg)
> > {
> > lpae_t pte;
> > int i;
> > @@ -1100,8 +1101,8 @@ static void set_pte_flags_on_range(const char *p,
> > unsigned long l, enum mg mg)
> > ASSERT(!((unsigned long) p & ~PAGE_MASK));
> > ASSERT(!(l & ~PAGE_MASK));
> > - for ( i = (p - _start) / PAGE_SIZE;
> > - i < (p + l - _start) / PAGE_SIZE;
> > + for ( i = (p - SYMBOL(_start)) / PAGE_SIZE;
> > + i < (p + l - SYMBOL(_start)) / PAGE_SIZE;
> > i++ )
> > {
> > pte = xen_xenmap[i];
> > @@ -1138,12 +1139,12 @@ static void set_pte_flags_on_range(const char *p,
> > unsigned long l, enum mg mg)
> > void free_init_memory(void)
> > {
> > paddr_t pa = virt_to_maddr(__init_begin);
> > - unsigned long len = __init_end - __init_begin;
> > + unsigned long len = SYMBOL(__init_end) - SYMBOL(__init_begin);
> > uint32_t insn;
> > unsigned int i, nr = len / sizeof(insn);
> > uint32_t *p;
> > - set_pte_flags_on_range(__init_begin, len, mg_rw);
> > + set_pte_flags_on_range(SYMBOL(__init_begin), len, mg_rw);
> > #ifdef CONFIG_ARM_32
> > /* udf instruction i.e (see A8.8.247 in ARM DDI 0406C.c) */
> > insn = 0xe7f000f0;
> > @@ -1154,7 +1155,7 @@ void free_init_memory(void)
> > for ( i = 0; i < nr; i++ )
> > *(p + i) = insn;
> > - set_pte_flags_on_range(__init_begin, len, mg_clear);
> > + set_pte_flags_on_range(SYMBOL(__init_begin), len, mg_clear);
> > init_domheap_pages(pa, pa + len);
> > printk("Freed %ldkB init memory.\n",
> > (long)(__init_end-__init_begin)>>10);
> > }
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index ea2495a..e3adddf 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -394,7 +394,8 @@ static paddr_t __init get_xen_paddr(void)
> > paddr_t paddr = 0;
> > int i;
> > - min_size = (_end - _start + (XEN_PADDR_ALIGN-1)) &
> > ~(XEN_PADDR_ALIGN-1);
> > + min_size = (SYMBOL(_end) - SYMBOL(_start) +
> > + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1);
> > /* Find the highest bank with enough space. */
> > for ( i = 0; i < mi->nr_banks; i++ )
> > @@ -727,8 +728,9 @@ void __init start_xen(unsigned long boot_phys_offset,
> > /* Register Xen's load address as a boot module. */
> > xen_bootmodule = add_boot_module(BOOTMOD_XEN,
> > - (paddr_t)(uintptr_t)(_start +
> > boot_phys_offset),
> > - (paddr_t)(uintptr_t)(_end - _start + 1),
> > NULL);
> > + (paddr_t)(uintptr_t)(SYMBOL(_start) +
> > boot_phys_offset),
> > + (paddr_t)(uintptr_t)(SYMBOL(_end) -
> > + SYMBOL(_start) + 1),
> > NULL);
>
> Are the casts necessary when you use SYMBOL?
I can remove the cast to (uintptr_t) that is unnecessary
> > BUG_ON(!xen_bootmodule);
> > xen_paddr = get_xen_paddr();
> > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> > index 93b79c7..1a02b30 100644
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -578,13 +578,13 @@ static void __init kexec_reserve_area(struct e820map
> > *e820)
> > static inline bool using_2M_mapping(void)
> > {
> > - return !l1_table_offset((unsigned long)__2M_text_end) &&
> > - !l1_table_offset((unsigned long)__2M_rodata_start) &&
> > - !l1_table_offset((unsigned long)__2M_rodata_end) &&
> > - !l1_table_offset((unsigned long)__2M_init_start) &&
> > - !l1_table_offset((unsigned long)__2M_init_end) &&
> > - !l1_table_offset((unsigned long)__2M_rwdata_start) &&
> > - !l1_table_offset((unsigned long)__2M_rwdata_end);
> > + return !l1_table_offset(SYMBOL(__2M_text_end)) &&
> > + !l1_table_offset(SYMBOL(__2M_rodata_start)) &&
> > + !l1_table_offset(SYMBOL(__2M_rodata_end)) &&
> > + !l1_table_offset(SYMBOL(__2M_init_start)) &&
> > + !l1_table_offset(SYMBOL(__2M_init_end)) &&
> > + !l1_table_offset(SYMBOL(__2M_rwdata_start)) &&
> > + !l1_table_offset(SYMBOL(__2M_rwdata_end));
> > }
> > static void noinline init_done(void)
> > @@ -606,13 +606,13 @@ static void noinline init_done(void)
> > /* Destroy Xen's mappings, and reuse the pages. */
> > if ( using_2M_mapping() )
> > {
> > - start = (unsigned long)&__2M_init_start,
> > - end = (unsigned long)&__2M_init_end;
> > + start = SYMBOL(&__2M_init_start),
> > + end = SYMBOL(&__2M_init_end);
> > }
> > else
> > {
> > - start = (unsigned long)&__init_begin;
> > - end = (unsigned long)&__init_end;
> > + start = SYMBOL(&__init_begin);
> > + end = SYMBOL(&__init_end);
> > }
> > destroy_xen_mappings(start, end);
> > @@ -966,8 +966,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> > * This needs to remain in sync with xen_in_range() and the
> > * respective reserve_e820_ram() invocation below.
> > */
> > - mod[mbi->mods_count].mod_start = virt_to_mfn(_stext);
> > - mod[mbi->mods_count].mod_end = __2M_rwdata_end - _stext;
> > + mod[mbi->mods_count].mod_start = virt_to_mfn(SYMBOL(_stext));
> > + mod[mbi->mods_count].mod_end = SYMBOL(__2M_rwdata_end) -
> > SYMBOL(_stext);
> > }
> > modules_headroom = bzimage_headroom(bootstrap_map(mod),
> > mod->mod_end);
> > @@ -1018,7 +1018,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> > 1UL << (PAGE_SHIFT + 32)) )
> > e = min(HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START,
> > 1UL << (PAGE_SHIFT + 32));
> > -#define reloc_size ((__pa(__2M_rwdata_end) + mask) & ~mask)
> > +#define reloc_size ((__pa(SYMBOL(__2M_rwdata_end)) + mask) & ~mask)
> > /* Is the region suitable for relocating Xen? */
> > if ( !xen_phys_start && e <= limit )
> > {
> > @@ -1034,7 +1034,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> > * Is the region size greater than zero and does it begin
> > * at or above the end of current Xen image placement?
> > */
> > - if ( (end > s) && (end - reloc_size + XEN_IMG_OFFSET >= __pa(_end))
> > )
> > + if ( (end > s) && (end - reloc_size + XEN_IMG_OFFSET >=
> > + __pa(SYMBOL(_end))) )
> > {
> > l4_pgentry_t *pl4e;
> > l3_pgentry_t *pl3e;
> > @@ -1062,7 +1063,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> > * data until after we have switched to the relocated
> > pagetables!
> > */
> > barrier();
> > - move_memory(e + XEN_IMG_OFFSET, XEN_IMG_OFFSET, _end - _start,
> > 1);
> > + move_memory(e + XEN_IMG_OFFSET, XEN_IMG_OFFSET,
> > + SYMBOL(_end) - SYMBOL(_start), 1);
> > /* Walk initial pagetables, relocating page directory
> > entries. */
> > pl4e = __va(__pa(idle_pg_table));
> > @@ -1103,8 +1105,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> > * is contained in this PTE.
> > */
> > BUG_ON(using_2M_mapping() &&
> > - l2_table_offset((unsigned long)_erodata) ==
> > - l2_table_offset((unsigned long)_stext));
> > + l2_table_offset(SYMBOL(_erodata)) ==
> > + l2_table_offset(SYMBOL(_stext)));
> > *pl2e++ = l2e_from_pfn(xen_phys_start >> PAGE_SHIFT,
> > PAGE_HYPERVISOR_RX | _PAGE_PSE);
> > for ( i = 1; i < L2_PAGETABLE_ENTRIES; i++, pl2e++ )
> > @@ -1122,22 +1124,22 @@ void __init noreturn __start_xen(unsigned long
> > mbi_p)
> > continue;
> > }
> > - if ( i < l2_table_offset((unsigned long)&__2M_text_end) )
> > + if ( i < l2_table_offset((unsigned
> > long)SYMBOL(&__2M_text_end)) )
> > {
> > flags = PAGE_HYPERVISOR_RX | _PAGE_PSE;
> > }
> > - else if ( i >= l2_table_offset((unsigned
> > long)&__2M_rodata_start) &&
> > - i < l2_table_offset((unsigned
> > long)&__2M_rodata_end) )
> > + else if ( i >= l2_table_offset(SYMBOL(&__2M_rodata_start))
> > &&
> > + i < l2_table_offset(SYMBOL(&__2M_rodata_end)) )
> > {
> > flags = PAGE_HYPERVISOR_RO | _PAGE_PSE;
> > }
> > - else if ( i >= l2_table_offset((unsigned
> > long)&__2M_init_start) &&
> > - i < l2_table_offset((unsigned
> > long)&__2M_init_end) )
> > + else if ( i >= l2_table_offset(SYMBOL(&__2M_init_start)) &&
> > + i < l2_table_offset(SYMBOL(&__2M_init_end)) )
> > {
> > flags = PAGE_HYPERVISOR_RWX | _PAGE_PSE;
> > }
> > - else if ( (i >= l2_table_offset((unsigned
> > long)&__2M_rwdata_start) &&
> > - i < l2_table_offset((unsigned
> > long)&__2M_rwdata_end)) )
> > + else if ( (i >= l2_table_offset(SYMBOL(&__2M_rwdata_start))
> > &&
> > + i < l2_table_offset(SYMBOL(&__2M_rwdata_end)))
> > )
> > {
> > flags = PAGE_HYPERVISOR_RW | _PAGE_PSE;
> > }
> > @@ -1234,7 +1236,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> > panic("Not enough memory to relocate Xen\n");
> > /* This needs to remain in sync with xen_in_range(). */
> > - reserve_e820_ram(&boot_e820, __pa(_stext), __pa(__2M_rwdata_end));
> > + reserve_e820_ram(&boot_e820, __pa(SYMBOL(_stext)),
> > __pa(SYMBOL(__2M_rwdata_end)));
> > /* Late kexec reservation (dynamic start address). */
> > kexec_reserve_area(&boot_e820);
> > @@ -1377,7 +1379,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> > }
> > #endif
> > - xen_virt_end = ((unsigned long)_end + (1UL << L2_PAGETABLE_SHIFT) -
> > 1) &
> > + xen_virt_end = (SYMBOL(_end) +
> > + (1UL << L2_PAGETABLE_SHIFT) - 1) &
> > ~((1UL << L2_PAGETABLE_SHIFT) - 1);
> > destroy_xen_mappings(xen_virt_end, XEN_VIRT_START +
> > BOOTSTRAP_MAP_BASE);
> > @@ -1390,22 +1393,22 @@ void __init noreturn __start_xen(unsigned long
> > mbi_p)
> > {
> > /* Mark .text as RX (avoiding the first 2M superpage). */
> > modify_xen_mappings(XEN_VIRT_START + MB(2),
> > - (unsigned long)&__2M_text_end,
> > + SYMBOL(&__2M_text_end),
> > PAGE_HYPERVISOR_RX);
> > /* Mark .rodata as RO. */
> > - modify_xen_mappings((unsigned long)&__2M_rodata_start,
> > - (unsigned long)&__2M_rodata_end,
> > + modify_xen_mappings(SYMBOL(&__2M_rodata_start),
> > + SYMBOL(&__2M_rodata_end),
> > PAGE_HYPERVISOR_RO);
> > /* Mark .data and .bss as RW. */
> > - modify_xen_mappings((unsigned long)&__2M_rwdata_start,
> > - (unsigned long)&__2M_rwdata_end,
> > + modify_xen_mappings(SYMBOL(&__2M_rwdata_start),
> > + SYMBOL(&__2M_rwdata_end),
> > PAGE_HYPERVISOR_RW);
> > /* Drop the remaining mappings in the shattered superpage. */
> > - destroy_xen_mappings((unsigned long)&__2M_rwdata_end,
> > - ROUNDUP((unsigned long)&__2M_rwdata_end,
> > MB(2)));
> > + destroy_xen_mappings(SYMBOL(&__2M_rwdata_end),
> > + ROUNDUP(SYMBOL(&__2M_rwdata_end), MB(2)));
> > }
> > nr_pages = 0;
> > @@ -1860,11 +1863,11 @@ int __hwdom_init xen_in_range(unsigned long mfn)
> > */
> > /* hypervisor .text + .rodata */
> > - xen_regions[region_ro].s = __pa(&_stext);
> > - xen_regions[region_ro].e = __pa(&__2M_rodata_end);
> > + xen_regions[region_ro].s = __pa(SYMBOL(&_stext));
> > + xen_regions[region_ro].e = __pa(SYMBOL(&__2M_rodata_end));
> > /* hypervisor .data + .bss */
> > - xen_regions[region_rw].s = __pa(&__2M_rwdata_start);
> > - xen_regions[region_rw].e = __pa(&__2M_rwdata_end);
> > + xen_regions[region_rw].s = __pa(SYMBOL(&__2M_rwdata_start));
> > + xen_regions[region_rw].e = __pa(SYMBOL(&__2M_rwdata_end));
> > }
> > start = (paddr_t)mfn << PAGE_SHIFT;
> > diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c
> > index f3fdee4..e355232 100644
> > --- a/xen/arch/x86/tboot.c
> > +++ b/xen/arch/x86/tboot.c
> > @@ -373,13 +373,13 @@ void tboot_shutdown(uint32_t shutdown_type)
> > g_tboot_shared->mac_regions[0].size = bootsym_phys(trampoline_end)
> > -
> >
> > bootsym_phys(trampoline_start);
> > /* hypervisor .text + .rodata */
> > - g_tboot_shared->mac_regions[1].start = (uint64_t)__pa(&_stext);
> > - g_tboot_shared->mac_regions[1].size = __pa(&__2M_rodata_end) -
> > - __pa(&_stext);
> > + g_tboot_shared->mac_regions[1].start =
> > (uint64_t)__pa(SYMBOL(&_stext));
> > + g_tboot_shared->mac_regions[1].size =
> > __pa(SYMBOL(&__2M_rodata_end)) -
> > + __pa(SYMBOL(&_stext));
> > /* hypervisor .data + .bss */
> > - g_tboot_shared->mac_regions[2].start =
> > (uint64_t)__pa(&__2M_rwdata_start);
> > - g_tboot_shared->mac_regions[2].size = __pa(&__2M_rwdata_end) -
> > - __pa(&__2M_rwdata_start);
> > + g_tboot_shared->mac_regions[2].start =
> > (uint64_t)__pa(SYMBOL(&__2M_rwdata_start));
> > + g_tboot_shared->mac_regions[2].size =
> > __pa(SYMBOL(&__2M_rwdata_end)) -
> > +
> > __pa(SYMBOL(&__2M_rwdata_start));
> > /*
> > * MAC domains and other Xen memory
> > diff --git a/xen/arch/x86/x86_64/machine_kexec.c
> > b/xen/arch/x86/x86_64/machine_kexec.c
> > index f4a005c..91936db 100644
> > --- a/xen/arch/x86/x86_64/machine_kexec.c
> > +++ b/xen/arch/x86/x86_64/machine_kexec.c
> > @@ -13,8 +13,8 @@
> > int machine_kexec_get_xen(xen_kexec_range_t *range)
> > {
> > - range->start = virt_to_maddr(_start);
> > - range->size = virt_to_maddr(_end) - (unsigned long)range->start;
> > + range->start = virt_to_maddr(SYMBOL(_start));
> > + range->size = virt_to_maddr(SYMBOL(_end)) - (unsigned
> > long)range->start;
> > return 0;
> > }
> > diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> > index 82607bd..df2ca47 100644
> > --- a/xen/drivers/vpci/vpci.c
> > +++ b/xen/drivers/vpci/vpci.c
> > @@ -33,7 +33,7 @@ struct vpci_register {
> > #ifdef __XEN__
> > extern vpci_register_init_t *const __start_vpci_array[];
> > extern vpci_register_init_t *const __end_vpci_array[];
> > -#define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array)
> > +#define NUM_VPCI_INIT (SYMBOL(__end_vpci_array) -
> > SYMBOL(__start_vpci_array))
> > void vpci_remove_device(struct pci_dev *pdev)
> > {
> > @@ -71,6 +71,11 @@ int __hwdom_init vpci_add_handlers(struct pci_dev *pdev)
> > for ( i = 0; i < NUM_VPCI_INIT; i++ )
> > {
> > + /*
> > + * We should use SYMBOL here, but it would make the code awkward
> > + * and it is not required due because there are no pointers
> > + * comparison. Leave it as is.
> > + */
> > rc = __start_vpci_array[i](pdev);
> > if ( rc )
> > break;
> > diff --git a/xen/include/asm-arm/grant_table.h
> > b/xen/include/asm-arm/grant_table.h
> > index 37415b7..6c5edcc 100644
> > --- a/xen/include/asm-arm/grant_table.h
> > +++ b/xen/include/asm-arm/grant_table.h
> > @@ -31,7 +31,8 @@ void gnttab_mark_dirty(struct domain *d, mfn_t mfn);
> > * enough space for a large grant table
> > */
> > #define gnttab_dom0_frames() \
> > - min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext - _stext))
> > + min_t(unsigned int, opt_max_grant_frames, \
> > + PFN_DOWN(SYMBOL(_etext) - SYMBOL(_stext)))
> > #define gnttab_init_arch(gt)
> > \
> > ({ \
> > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> > index 940b74b..61983f6 100644
> > --- a/xen/include/asm-arm/mm.h
> > +++ b/xen/include/asm-arm/mm.h
> > @@ -151,8 +151,8 @@ extern vaddr_t xenheap_virt_start;
> > #endif
> > #define is_xen_fixed_mfn(mfn) \
> > - ((pfn_to_paddr(mfn) >= virt_to_maddr(&_start)) && \
> > - (pfn_to_paddr(mfn) <= virt_to_maddr(&_end)))
> > + ((pfn_to_paddr(mfn) >= virt_to_maddr(SYMBOL(&_start))) && \
> > + (pfn_to_paddr(mfn) <= virt_to_maddr(SYMBOL(&_end))))
>
> __pa_symbol would help here.
I have removed these changes.
> > #define page_get_owner(_p) (_p)->v.inuse.domain
> > #define page_set_owner(_p,_d) ((_p)->v.inuse.domain = (_d))
> > diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
> > index 6e45651..82018b2 100644
> > --- a/xen/include/asm-x86/mm.h
> > +++ b/xen/include/asm-x86/mm.h
> > @@ -273,8 +273,8 @@ struct page_info
> > #define is_xen_heap_mfn(mfn) \
> > (__mfn_valid(mfn) && is_xen_heap_page(mfn_to_page(_mfn(mfn))))
> > #define is_xen_fixed_mfn(mfn) \
> > - ((((mfn) << PAGE_SHIFT) >= __pa(&_stext)) && \
> > - (((mfn) << PAGE_SHIFT) <= __pa(&__2M_rwdata_end)))
> > + ((((mfn) << PAGE_SHIFT) >= __pa(SYMBOL(&_stext))) && \
> > + (((mfn) << PAGE_SHIFT) <= __pa(SYMBOL(&__2M_rwdata_end))))
>
> Same here.
These changes have also been removed
> > #define PRtype_info "016lx"/* should only be used for printk's */
> > diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
> > index 548b64d..cd27030 100644
> > --- a/xen/include/xen/kernel.h
> > +++ b/xen/include/xen/kernel.h
> > @@ -66,27 +66,27 @@
> > })
> > extern char _start[], _end[], start[];
> > -#define is_kernel(p) ({ \
> > - char *__p = (char *)(unsigned long)(p); \
> > - (__p >= _start) && (__p < _end); \
> > +#define is_kernel(p) ({ \
> > + const unsigned long __p = (unsigned long)(p); \
> > + (__p >= SYMBOL(_start)) && (__p < SYMBOL(_end)); \
> > })
> > extern char _stext[], _etext[];
> > -#define is_kernel_text(p) ({ \
> > - char *__p = (char *)(unsigned long)(p); \
> > - (__p >= _stext) && (__p < _etext); \
> > +#define is_kernel_text(p) ({ \
> > + const unsigned long __p = (unsigned long)(p); \
> > + (__p >= SYMBOL(_stext)) && (__p < SYMBOL(_etext)); \
> > })
> > extern const char _srodata[], _erodata[];
> > -#define is_kernel_rodata(p) ({ \
> > - const char *__p = (const char *)(unsigned long)(p); \
> > - (__p >= _srodata) && (__p < _erodata); \
> > +#define is_kernel_rodata(p) ({ \
> > + const unsigned long __p = (unsigned long)(p); \
> > + (__p >= SYMBOL(_srodata)) && (__p < SYMBOL(_erodata)); \
> > })
> > extern char _sinittext[], _einittext[];
> > -#define is_kernel_inittext(p) ({ \
> > - char *__p = (char *)(unsigned long)(p); \
> > - (__p >= _sinittext) && (__p < _einittext); \
> > +#define is_kernel_inittext(p) ({ \
> > + const unsigned long __p = (unsigned long)(p); \
> > + (__p >= SYMBOL(_sinittext)) && (__p < SYMBOL(_einittext)); \
> > })
> > extern enum system_state {
> >
>
> Cheers,
>
> --
> Julien Grall
>
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
