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

Re: [Xen-devel] [PATCH v3 4/4] xen: use SYMBOL everywhere



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 <stefanos@xxxxxxxxxx>
> > CC: JBeulich@xxxxxxxx
> > CC: andrew.cooper3@xxxxxxxxxx
> > ---
> > 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(&region, xenmap - (void *)_start);
> > +        ret = __apply_alternatives(&region,
> > +                                   (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
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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