Re: [Xen-devel] [PATCH v6 2/4] xen/arm: use SYMBOL when required

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.01.19 at 00:42, <sstabellini@xxxxxxxxxx> wrote:
> @@ -1138,9 +1138,10 @@ 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);
> +    printk("Freed %ldkB init memory.\n",
> +           (long)(SYMBOL(__init_end)-SYMBOL(__init_begin))>>10);

I've noticed this only here, but I can't exclude I've overlooked other
instances: I think it would be really nice if you corrected formatting
at the same time (here: add the missing blanks).

> --- a/xen/arch/arm/percpu.c
> +++ b/xen/arch/arm/percpu.c
> @@ -6,7 +6,7 @@
>  
>  unsigned long __per_cpu_offset[NR_CPUS];
>  #define INVALID_PERCPU_AREA (-(long)__per_cpu_start)
> -#define PERCPU_ORDER 
> (get_order_from_bytes(__per_cpu_data_end-__per_cpu_start))
> +#define PERCPU_ORDER (get_order_from_bytes(SYMBOL(__per_cpu_data_end) - 
> SYMBOL(__per_cpu_start)))

Long line.

> @@ -37,7 +37,7 @@ static void _free_percpu_area(struct rcu_head *head)
>  {
>      struct free_info *info = container_of(head, struct free_info, rcu);
>      unsigned int cpu = info->cpu;
> -    char *p = __per_cpu_start + __per_cpu_offset[cpu];
> +    char *p = SYMBOL(__per_cpu_start) + __per_cpu_offset[cpu];
>      free_xenheap_pages(p, PERCPU_ORDER);
>      __per_cpu_offset[cpu] = INVALID_PERCPU_AREA;
>  }

As per above, to add the missing blank line would be quite nice at
this occasion.

> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -772,8 +772,10 @@ 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), false);
> +                             (paddr_t)(uintptr_t)(SYMBOL(_start) +
> +                                                  boot_phys_offset),
> +                             (paddr_t)(uintptr_t)(SYMBOL(_end) -
> +                                                  SYMBOL(_start) + 1), 
> false);

Why you need the double casts, i.e. why does (uintptr_t) alone not
suffice?

Jan



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