[Patch 1/2]xen boot_cpu_data initialize

[Patch 1/2]xen boot_cpu_data initialize

[Uma Sharma]

This patch inserts some boot data initilization in xen/arch and
function definition in xen/include.
Signed-off-by: Uma Sharma <uma.sharma523@gmail.com>
---

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index d316212..4f3ce15 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1259,6 +1259,11 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     microcode_grab_module(module_map, mbi, bootstrap_map);
 
     timer_init();
+    /*
+     * Identify the boot CPU, in case the scheduler initialization
+     * needs to know about it (e.g., topology, etc.)
+     */
+    identify_cpu(&boot_cpu_data);
 
     init_idle_domain();
 
@@ -1270,8 +1275,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     arch_init_memory();
 
-    identify_cpu(&boot_cpu_data);
-
     if ( cpu_has_fxsr )
         set_in_cr4(X86_CR4_OSFXSR);
     if ( cpu_has_xmm )
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 7ae561c..b4c4cc2 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -59,7 +59,8 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
 cpumask_t cpu_online_map __read_mostly;
 EXPORT_SYMBOL(cpu_online_map);
 
-struct cpuinfo_x86 cpu_data[NR_CPUS];
+struct cpuinfo_x86 cpu_data[NR_CPUS] =
+        { [0 ... NR_CPUS-1] = { .phys_proc_id=-1, .cpu_core_id=-1 } };
 
 u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly =
 	{ [0 ... NR_CPUS-1] = BAD_APICID };
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 87d80ff..6ec9588 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -214,7 +214,9 @@ extern void detect_extended_topology(struct cpuinfo_x86 *c);
 
 extern void detect_ht(struct cpuinfo_x86 *c);
 
+#define boot_cpu_to_core()   (boot_cpu_data.cpu_core_id)
 #define cpu_to_core(_cpu)   (cpu_data[_cpu].cpu_core_id)
+#define boot_cpu_to_socket() (boot_cpu_data.phys_proc_id)
 #define cpu_to_socket(_cpu) (cpu_data[_cpu].phys_proc_id)
 
 unsigned int apicid_to_socket(unsigned int);

Re: [Patch 1/2]xen boot_cpu_data initialize

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2889 bytes --]

On Mon, 2015-03-09 at 12:48 +0530, Uma Sharma wrote:
> This patch inserts some boot data initilization in xen/arch and
> function definition in xen/include.
> Signed-off-by: Uma Sharma <uma.sharma523@gmail.com>
>
So, this is basically the same as patch 1 from this series of mine:

 http://lists.xen.org/archives/html/xen-devel/2014-08/msg02168.html
 http://lists.xen.org/archives/html/xen-devel/2014-08/msg02166.html

which I still need to rework basing on the comment I got (see the rest
of the thread).

I haven't done this yet, but will soon and, if Uma is ok with it, I'll
add to that the second patch of this series (of course, rataining her
Signed-off-by) when sending it.

I've been sidetracked by a bunch of other stuff, but I've got time now,
and I'll do it right away, is it fine Uma (and others, of course)?

Regards,
Dario

> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index d316212..4f3ce15 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1259,6 +1259,11 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>      microcode_grab_module(module_map, mbi, bootstrap_map);
>  
>      timer_init();
> +    /*
> +     * Identify the boot CPU, in case the scheduler initialization
> +     * needs to know about it (e.g., topology, etc.)
> +     */
> +    identify_cpu(&boot_cpu_data);
>  
>      init_idle_domain();
>  
> @@ -1270,8 +1275,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  
>      arch_init_memory();
>  
> -    identify_cpu(&boot_cpu_data);
> -
>      if ( cpu_has_fxsr )
>          set_in_cr4(X86_CR4_OSFXSR);
>      if ( cpu_has_xmm )
> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
> index 7ae561c..b4c4cc2 100644
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -59,7 +59,8 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
>  cpumask_t cpu_online_map __read_mostly;
>  EXPORT_SYMBOL(cpu_online_map);
>  
> -struct cpuinfo_x86 cpu_data[NR_CPUS];
> +struct cpuinfo_x86 cpu_data[NR_CPUS] =
> +        { [0 ... NR_CPUS-1] = { .phys_proc_id=-1, .cpu_core_id=-1 } };
>  
>  u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly =
>  	{ [0 ... NR_CPUS-1] = BAD_APICID };
> diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
> index 87d80ff..6ec9588 100644
> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -214,7 +214,9 @@ extern void detect_extended_topology(struct cpuinfo_x86 *c);
>  
>  extern void detect_ht(struct cpuinfo_x86 *c);
>  
> +#define boot_cpu_to_core()   (boot_cpu_data.cpu_core_id)
>  #define cpu_to_core(_cpu)   (cpu_data[_cpu].cpu_core_id)
> +#define boot_cpu_to_socket() (boot_cpu_data.phys_proc_id)
>  #define cpu_to_socket(_cpu) (cpu_data[_cpu].phys_proc_id)
>  
>  unsigned int apicid_to_socket(unsigned int);


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Patch 1/2]xen boot_cpu_data initialize

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2085 bytes --]

Hi Uma,

First of all, nice to see the patch here on xen-devel... good job! ;-P

A few things:

On Mon, 2015-03-09 at 12:48 +0530, Uma Sharma wrote:
> This patch inserts some boot data initilization in xen/arch and
> function definition in xen/include.
>
About this description. The fact that it adds a function in a file, is
pretty evident from the code, so it's not really important to say it
here.

OTOH, what you could do here in the changelog, is explaining why the
change introduced is good/necessary.

In the case of this patch, you can see here:

http://lists.xen.org/archives/html/xen-devel/2014-08/msg02166.html

how I tried to do right that (sorry for 'citing' myself, but it's too
easy an example in this case! :-P)

Also, when one needs to send more than one patch (we call it a "patch
series"), what we usually do is to provide some sort of 'patch 0
message', also known as 'cover letter', in which you introduce the
various patches, explaining what they do, why you've done the work, and
other things... For example, in your case, that would be the perfect
place where to put a summary of the experiments you performed and of
they're results, so people can see what the patches are useful for
(performance improvement, in this case).

Also from my old series, here's the cover letter:

http://lists.xen.org/archives/html/xen-devel/2014-08/msg02168.html

And you'll fine a lot of other examples on the mailing list. :-)

About Cc-ing people: you did good Cc-ing George, which is the maintainer
of the scheduler, and me, which am also active in that area, and am
familiar with your work. However, this patches touches arch/x86, so you
should Cc the maintainers of the x86 architecture too (you can easily
figure that out from the MAINTAINERS file).

Last but not least, threading: the cover letter should start a new
thread, and all the patches should be replies to it. I don't know what
you use to generate and send te patches, but whatever tool it is, there
usually are many ways to achieve the above.

Regards,
Dario

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Patch 1/2]xen boot_cpu_data initialize

[Uma Sharma]

Thank you :-)

Sure I have no problem, you can add second patch of this series.

I will work on the comments and will again resend the patch. Thank you.


Regards,
Uma Sharma

On Mon, Mar 9, 2015 at 3:57 PM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> Hi Uma,
>
> First of all, nice to see the patch here on xen-devel... good job! ;-P
>
> A few things:
>
> On Mon, 2015-03-09 at 12:48 +0530, Uma Sharma wrote:
>> This patch inserts some boot data initilization in xen/arch and
>> function definition in xen/include.
>>
> About this description. The fact that it adds a function in a file, is
> pretty evident from the code, so it's not really important to say it
> here.
>
> OTOH, what you could do here in the changelog, is explaining why the
> change introduced is good/necessary.
>
> In the case of this patch, you can see here:
>
> http://lists.xen.org/archives/html/xen-devel/2014-08/msg02166.html
>
> how I tried to do right that (sorry for 'citing' myself, but it's too
> easy an example in this case! :-P)
>
> Also, when one needs to send more than one patch (we call it a "patch
> series"), what we usually do is to provide some sort of 'patch 0
> message', also known as 'cover letter', in which you introduce the
> various patches, explaining what they do, why you've done the work, and
> other things... For example, in your case, that would be the perfect
> place where to put a summary of the experiments you performed and of
> they're results, so people can see what the patches are useful for
> (performance improvement, in this case).
>
> Also from my old series, here's the cover letter:
>
> http://lists.xen.org/archives/html/xen-devel/2014-08/msg02168.html
>
> And you'll fine a lot of other examples on the mailing list. :-)
>
> About Cc-ing people: you did good Cc-ing George, which is the maintainer
> of the scheduler, and me, which am also active in that area, and am
> familiar with your work. However, this patches touches arch/x86, so you
> should Cc the maintainers of the x86 architecture too (you can easily
> figure that out from the MAINTAINERS file).
>
> Last but not least, threading: the cover letter should start a new
> thread, and all the patches should be replies to it. I don't know what
> you use to generate and send te patches, but whatever tool it is, there
> usually are many ways to achieve the above.
>
> Regards,
> Dario



-- 
Regards,
Uma Sharma
http://about.me/umasharma