Xen devel - Jul 2013 - [PATCH] xen/arm: Clean up identify processor call for secondary cpus

The smp_processor_id() is set at the beginning of start_secondary. We
don''t
need to compute ourself the offset of the cpu data.
---
 xen/arch/arm/smpboot.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 5895178..872cc25 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -137,15 +137,12 @@ void __cpuinit start_secondary(unsigned long
boot_phys_offset,
                                unsigned long fdt_paddr,
                                unsigned long cpuid)
 {
-    struct cpuinfo_arm *c = cpu_data + cpuid;
-
     memset(get_cpu_info(), 0, sizeof (struct cpu_info));
 
     /* TODO: handle boards where CPUIDs are not contiguous */
     set_processor_id(cpuid);
 
-    *c = boot_cpu_data;
-    identify_cpu(c);
+    identify_cpu(&current_cpu_data);
 
     init_traps();
 
-- 
1.7.10.4

On 07/25/2013 04:29 PM, Julien Grall wrote:
> The smp_processor_id() is set at the beginning of start_secondary. We
don''t
> need to compute ourself the offset of the cpu data.
I forgot the signed-off-by.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/smpboot.c |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 5895178..872cc25 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -137,15 +137,12 @@ void __cpuinit start_secondary(unsigned long
boot_phys_offset,
>                                 unsigned long fdt_paddr,
>                                 unsigned long cpuid)
>  {
> -    struct cpuinfo_arm *c = cpu_data + cpuid;
> -
>      memset(get_cpu_info(), 0, sizeof (struct cpu_info));
>  
>      /* TODO: handle boards where CPUIDs are not contiguous */
>      set_processor_id(cpuid);
>  
> -    *c = boot_cpu_data;
> -    identify_cpu(c);
> +    identify_cpu(&current_cpu_data);
>  
>      init_traps();
>  
>

On Thu, 2013-07-25 at 16:29 +0100, Julien Grall wrote:
> The smp_processor_id() is set at the beginning of start_secondary. We
don''t
> need to compute ourself the offset of the cpu data.
> ---
>  xen/arch/arm/smpboot.c |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 5895178..872cc25 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -137,15 +137,12 @@ void __cpuinit start_secondary(unsigned long
boot_phys_offset,
>                                 unsigned long fdt_paddr,
>                                 unsigned long cpuid)
>  {
> -    struct cpuinfo_arm *c = cpu_data + cpuid;
> -
>      memset(get_cpu_info(), 0, sizeof (struct cpu_info));
>  
>      /* TODO: handle boards where CPUIDs are not contiguous */
>      set_processor_id(cpuid);
>  
> -    *c = boot_cpu_data;
This step defaults everything to the boot cpu''s settings. You could
argue that this isn''t necessary iff identify_cpu always reinitialises
everything but it doesn''t belong in this change IMHO.

I think the defaulting behaviour is fine BTW, even if we don''t
currently
make use of it today.

Ian.
> -    identify_cpu(c);
> +    identify_cpu(&current_cpu_data);
>  
>      init_traps();
>

On 07/29/2013 09:57 AM, Ian Campbell wrote:
> On Thu, 2013-07-25 at 16:29 +0100, Julien Grall wrote:
>> The smp_processor_id() is set at the beginning of start_secondary. We
don''t
>> need to compute ourself the offset of the cpu data.
>> ---
>>  xen/arch/arm/smpboot.c |    5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
>> index 5895178..872cc25 100644
>> --- a/xen/arch/arm/smpboot.c
>> +++ b/xen/arch/arm/smpboot.c
>> @@ -137,15 +137,12 @@ void __cpuinit start_secondary(unsigned long
boot_phys_offset,
>>                                 unsigned long fdt_paddr,
>>                                 unsigned long cpuid)
>>  {
>> -    struct cpuinfo_arm *c = cpu_data + cpuid;
>> -
>>      memset(get_cpu_info(), 0, sizeof (struct cpu_info));
>>  
>>      /* TODO: handle boards where CPUIDs are not contiguous */
>>      set_processor_id(cpuid);
>>  
>> -    *c = boot_cpu_data;
> 
> This step defaults everything to the boot cpu''s settings. You
could
> argue that this isn''t necessary iff identify_cpu always
reinitialises
> everything but it doesn''t belong in this change IMHO.
> 
> I think the defaulting behaviour is fine BTW, even if we don''t
currently
> make use of it today.
I will remove this line from the patch.

-- 
Julien