Xen devel - Nov 2009 - [PATCH] Allocate vmcs pages when system booting

Currently the VMCS page is allocated when the CPU is brought-up and identified,
and then spin_debug_enable() is called.

This does not work for cpu hot-add. When hot-added CPU is brought-up and try to
allocate the vmcs page, it will hit check_lock, because irq is disabled still.

This patch allocate the vmcs pages for all possible pages when system booting,
so that we don''t allocate vmcs anymore when secondary CPU is up.

Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>

diff -r 1db7034f4498 xen/arch/x86/hvm/vmx/vmcs.c
--- a/xen/arch/x86/hvm/vmx/vmcs.c	Thu Nov 12 00:05:59 2009 +0800
+++ b/xen/arch/x86/hvm/vmx/vmcs.c	Thu Nov 12 04:08:25 2009 +0800
@@ -267,6 +267,34 @@ static void vmx_free_vmcs(struct vmcs_st
 static void vmx_free_vmcs(struct vmcs_struct *vmcs)
 {
     free_xenheap_page(vmcs);
+}
+
+int vmx_allocate_vmcs_pages(void)
+{
+    int cpu, ret = 0;
+
+    for_each_possible_cpu(cpu)
+    {
+        if (per_cpu(host_vmcs, cpu) == NULL)
+        {
+            per_cpu(host_vmcs, cpu) = vmx_alloc_vmcs();
+            if (per_cpu(host_vmcs, cpu) == NULL)
+            {
+                ret = -1;
+                break;
+            }
+        }
+    }
+
+    if (ret < 0)
+    {
+        for_each_possible_cpu(cpu)
+        {
+            if (per_cpu(host_vmcs, cpu))
+                vmx_free_vmcs(per_cpu(host_vmcs, cpu));
+        }
+    }
+    return ret;
 }
 
 static void __vmx_clear_vmcs(void *info)
diff -r 1db7034f4498 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c	Thu Nov 12 00:05:59 2009 +0800
+++ b/xen/arch/x86/hvm/vmx/vmx.c	Thu Nov 12 04:08:25 2009 +0800
@@ -1450,6 +1450,16 @@ void start_vmx(void)
     if ( !test_bit(X86_FEATURE_VMXE, &boot_cpu_data.x86_capability) )
         return;
 
+    /*
+     * We can''t allocate VMCS pages when start_secondary for hot-added
CPU
+     * because allocate page at that time will trigger check_lock error
+     */
+    if (vmx_allocate_vmcs_pages())
+    {
+        printk("VMX: failed to alloca vmcs pages\n");
+        return;
+    }
+
     set_in_cr4(X86_CR4_VMXE);
 
     if ( !vmx_cpu_up() )
diff -r 1db7034f4498 xen/include/asm-x86/hvm/vmx/vmcs.h
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h	Thu Nov 12 00:05:59 2009 +0800
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h	Thu Nov 12 04:08:25 2009 +0800
@@ -28,6 +28,7 @@ extern void setup_vmcs_dump(void);
 extern void setup_vmcs_dump(void);
 extern int  vmx_cpu_up(void);
 extern void vmx_cpu_down(void);
+extern int vmx_allocate_vmcs_pages(void);
 
 struct vmcs_struct {
     u32 vmcs_revision_id;



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

On 12/11/2009 10:52, "Jiang, Yunhong" <yunhong.jiang@intel.com>
wrote:
> Currently the VMCS page is allocated when the CPU is brought-up and
> identified, and then spin_debug_enable() is called.
> 
> This does not work for cpu hot-add. When hot-added CPU is brought-up and
try
> to allocate the vmcs page, it will hit check_lock, because irq is disabled
> still.
> 
> This patch allocate the vmcs pages for all possible pages when system
booting,
> so that we don''t allocate vmcs anymore when secondary CPU is up.
> 
> Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
A general point: using cpu_possible_map is not a good idea any longer, since
it is going to be all-ones as soon as your physical cpu hotplug patches go
in (I don''t intend to make that a build-time option). Hence we could
potentially massively over-allocate pages with this approach.

The good news is that, when bringing a CPU online, we don''t need to
worry
about acquiring IRQ-unsafe locks with IRQS disabled until the CPU is added
to the cpu_online_map. This is because the race we are trying to avoid with
the spin-debug checks involves rendezvous of CPUs via IPIs, and a CPU not in
cpu_online_map will never get waited on by others.

So, your better fix would be to spin_debug_disable() at the top of
start_secondary(), and spin_debug_enable() just before
cpu_set(...online_map).

Can you try this alternative fix please?

 -- Keir



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

Keir Fraser wrote:
> On 12/11/2009 10:52, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
> 
>> Currently the VMCS page is allocated when the CPU is brought-up and
>> identified, and then spin_debug_enable() is called.
>> 
>> This does not work for cpu hot-add. When hot-added CPU is brought-up
>> and try to allocate the vmcs page, it will hit check_lock, because
>> irq is disabled still. 
>> 
>> This patch allocate the vmcs pages for all possible pages when
>> system booting, so that we don''t allocate vmcs anymore when
>> secondary CPU is up. 
>> 
>> Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
> 
> A general point: using cpu_possible_map is not a good idea any
> longer, since it is going to be all-ones as soon as your physical cpu
> hotplug patches go
> in (I don''t intend to make that a build-time option). Hence we
could
> potentially massively over-allocate pages with this approach.
> 
> The good news is that, when bringing a CPU online, we don''t
> need to worry
> about acquiring IRQ-unsafe locks with IRQS disabled until the
> CPU is added
> to the cpu_online_map. This is because the race we are trying
> to avoid with
> the spin-debug checks involves rendezvous of CPUs via IPIs,
> and a CPU not in
> cpu_online_map will never get waited on by others.
> 
> So, your better fix would be to spin_debug_disable() at the top of
> start_secondary(), and spin_debug_enable() just before
> cpu_set(...online_map). 
> 
> Can you try this alternative fix please?
Yes, spin_debug_disable() is the first idea come into my mind, I change my mind
because the spin_debug is a global variable.  disable spin_debug() before
start_secondary means maybe we can''t catch error happens at that
window, so I choose this method. Previously I thought high-end system support
cpu hotplug may not care about the 64 pages :$

Or you assume this window is quite small, and we can assume safely if a error
happening at this window, it will happen in other timeslot in the end,
sepecially this function itself don''t need the check?

Thanks
Yunhong Jiang


> 
> -- Keir
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 12/11/2009 14:58, "Jiang, Yunhong" <yunhong.jiang@intel.com>
wrote:
>> Can you try this alternative fix please?
> 
> Yes, spin_debug_disable() is the first idea come into my mind, I change my
> mind because the spin_debug is a global variable.  disable spin_debug()
before
> start_secondary means maybe we can''t catch error happens at that
window, so I
> choose this method. Previously I thought high-end system support cpu
hotplug
> may not care about the 64 pages :$
CONFIG_HOTPLUG_CPU is not a user-accessible build option. In fact manually
disabling it in config.h resulted in a broken build, and now I basically
stripped out the non-HOTPLUG_CPU code (in c/s 20431). So this would be an
overhead for absolutely everyone.
> Or you assume this window is quite small, and we can assume safely if a
error
> happening at this window, it will happen in other timeslot in the end,
> sepecially this function itself don''t need the check?
Exactly. The spinlock debug checking can be best effort. I did think about
allowing the checks to be disabled per-cpu, but I don''t think
it''s worth it.

I will check in my suggested fix, then.

 -- Keir



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

Keir Fraser wrote:
> On 12/11/2009 14:58, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
> 
>>> Can you try this alternative fix please?
>> 
>> Yes, spin_debug_disable() is the first idea come into my mind, I
>> change my mind because the spin_debug is a global variable.  disable
>> spin_debug() before start_secondary means maybe we can''t catch
error
>> happens at that window, so I choose this method. Previously I
>> thought high-end system support cpu hotplug may not care about the
>> 64 pages :$ 
> 
> CONFIG_HOTPLUG_CPU is not a user-accessible build option. In
> fact manually
> disabling it in config.h resulted in a broken build, and now I
> basically stripped out the non-HOTPLUG_CPU code (in c/s 20431). So
> this 
> would be an
> overhead for absolutely everyone.
> 
>> Or you assume this window is quite small, and we can assume safely
>> if a error happening at this window, it will happen in other
>> timeslot in the end, sepecially this function itself don''t
need the
>> check? 
> 
> Exactly. The spinlock debug checking can be best effort. I did
> think about
> allowing the checks to be disabled per-cpu, but I don''t think
> it''s worth it.
Agree, it don''t need be per-cpu.
> 
> I will check in my suggested fix, then.
Ok, I will work this way tomorrow.

Thanks
Yunhong Jiang
> 
> -- Keir
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi, Keir, recently a bug is reported for the alloc vmcs page for the new added
CPU. The reason is because when we try to allocate vmcs page, if we need flush
TLB for allocated page, we will hit ASSERT in flush_area_mask in following code.

So my question are:
a) Why do we need make sure the irq is enabeld in flush_area_mask? I assume it
is because this function may takes a long time waiting for all CPU flush TLB, am
I right?
b) Can we still turn to the original patch, i.e. pre-allocate all VMCS pages for
all possible CPU?

Thanks
Yunhong Jiang

void flush_area_mask(const cpumask_t *mask, const void *va, unsigned int flags)
{
    ASSERT(local_irq_is_enabled());

    if ( cpu_isset(smp_processor_id(), *mask) )
        flush_area_local(va, flags);

The trace for the error:.
----------------------
(XEN) Xen call trace:
(XEN)    [<ffff82c48016d40b>] flush_area_mask+0x2c/0x140
(XEN)    [<ffff82c480114da7>] alloc_heap_pages+0x44f/0x493
(XEN)    [<ffff82c48011642b>] alloc_domheap_pages+0x115/0x186
(XEN)    [<ffff82c4801164ec>] alloc_xenheap_pages+0x50/0xd8
(XEN)    [<ffff82c4801b3b4f>] vmx_alloc_vmcs+0x18/0x65
(XEN)    [<ffff82c4801b45e9>] vmx_cpu_up+0x4d3/0x72a
(XEN)    [<ffff82c4801b83fc>] start_vmx+0x86/0x144
(XEN)    [<ffff82c480191703>] init_intel+0x2a2/0x2ff
(XEN)    [<ffff82c480191094>] identify_cpu+0xc3/0x23d
(XEN)    [<ffff82c48016d7b4>] smp_store_cpu_info+0x3b/0xc1
(XEN)    [<ffff82c48016d93b>] smp_callin+0x101/0x227
(XEN)    [<ffff82c48016ec85>] start_secondary+0xb3/0x43f
(XEN)    
(XEN) 
(XEN) ****************************************
(XEN) Panic on CPU 56:
(XEN) Assertion ''local_irq_is_enabled()'' failed at smp.c:223


>-----Original Message-----
>From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
>Sent: Thursday, November 12, 2009 7:23 PM
>To: Jiang, Yunhong; xen-devel@lists.xensource.com
>Subject: Re: [PATCH] Allocate vmcs pages when system booting
>
>On 12/11/2009 10:52, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
>
>> Currently the VMCS page is allocated when the CPU is brought-up and
>> identified, and then spin_debug_enable() is called.
>>
>> This does not work for cpu hot-add. When hot-added CPU is brought-up
and try
>> to allocate the vmcs page, it will hit check_lock, because irq is
disabled
>> still.
>>
>> This patch allocate the vmcs pages for all possible pages when system
booting,
>> so that we don''t allocate vmcs anymore when secondary CPU is
up.
>>
>> Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
>
>A general point: using cpu_possible_map is not a good idea any longer, since
>it is going to be all-ones as soon as your physical cpu hotplug patches go
>in (I don''t intend to make that a build-time option). Hence we
could
>potentially massively over-allocate pages with this approach.
>
>The good news is that, when bringing a CPU online, we don''t need to
worry
>about acquiring IRQ-unsafe locks with IRQS disabled until the CPU is added
>to the cpu_online_map. This is because the race we are trying to avoid with
>the spin-debug checks involves rendezvous of CPUs via IPIs, and a CPU not in
>cpu_online_map will never get waited on by others.
>
>So, your better fix would be to spin_debug_disable() at the top of
>start_secondary(), and spin_debug_enable() just before
>cpu_set(...online_map).
>
>Can you try this alternative fix please?
>
> -- Keir
>

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

>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 15.01.10
10:06 >>>
>b) Can we still turn to the original patch, i.e. pre-allocate all VMCS pages
for all possible CPU?
I''m generally opposed to pre-allocations, at least as long as all CPUs
are
considered ''possible''. Building with a relatively high
nr_cpus= setting
and then running on a relatively small system results in (close to)
unacceptable overhead.

In fact it''s really not clear to me why cpu_possible_map must be set to
CPU_MASK_ALL - if Linux has ways to avoid this, Xen should have too.

Jan


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

Another example where a hotplug notifier would be handy. Can you add a VMX
hook in the path bringing the CPU up (e.g., cpu_up, or do_boot_cpu), kind-of
equivalent to what would happen if we had CPU_UP_PREPARE notifiers?
>From there you can allocate memory in a safe (irqs enabled, on a cpu that is
already fully online) context.

This is the right direction for when notifiers get added -- I think I will
sort that out after 4.0 is done.

Btw, I am away rest of today and not back on email until Monday. I can
discuss further then. But we''ll make sure this is sorted one way or
another
before 4.0.0-rc2.

 -- Keir

On 15/01/2010 09:06, "Jiang, Yunhong" <yunhong.jiang@intel.com>
wrote:
> Hi, Keir, recently a bug is reported for the alloc vmcs page for the new
added
> CPU. The reason is because when we try to allocate vmcs page, if we need
flush
> TLB for allocated page, we will hit ASSERT in flush_area_mask in following
> code.
> 
> So my question are:
> a) Why do we need make sure the irq is enabeld in flush_area_mask? I assume
it
> is because this function may takes a long time waiting for all CPU flush
TLB,
> am I right?
> b) Can we still turn to the original patch, i.e. pre-allocate all VMCS
pages
> for all possible CPU?
> 
> Thanks
> Yunhong Jiang
> 
> void flush_area_mask(const cpumask_t *mask, const void *va, unsigned int
> flags)
> {
>     ASSERT(local_irq_is_enabled());
> 
>     if ( cpu_isset(smp_processor_id(), *mask) )
>         flush_area_local(va, flags);
> 
> The trace for the error:.
> ----------------------
> (XEN) Xen call trace:
> (XEN)    [<ffff82c48016d40b>] flush_area_mask+0x2c/0x140
> (XEN)    [<ffff82c480114da7>] alloc_heap_pages+0x44f/0x493
> (XEN)    [<ffff82c48011642b>] alloc_domheap_pages+0x115/0x186
> (XEN)    [<ffff82c4801164ec>] alloc_xenheap_pages+0x50/0xd8
> (XEN)    [<ffff82c4801b3b4f>] vmx_alloc_vmcs+0x18/0x65
> (XEN)    [<ffff82c4801b45e9>] vmx_cpu_up+0x4d3/0x72a
> (XEN)    [<ffff82c4801b83fc>] start_vmx+0x86/0x144
> (XEN)    [<ffff82c480191703>] init_intel+0x2a2/0x2ff
> (XEN)    [<ffff82c480191094>] identify_cpu+0xc3/0x23d
> (XEN)    [<ffff82c48016d7b4>] smp_store_cpu_info+0x3b/0xc1
> (XEN)    [<ffff82c48016d93b>] smp_callin+0x101/0x227
> (XEN)    [<ffff82c48016ec85>] start_secondary+0xb3/0x43f
> (XEN)    
> (XEN) 
> (XEN) ****************************************
> (XEN) Panic on CPU 56:
> (XEN) Assertion ''local_irq_is_enabled()'' failed at
smp.c:223
> 
> 
> 
>> -----Original Message-----
>> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
>> Sent: Thursday, November 12, 2009 7:23 PM
>> To: Jiang, Yunhong; xen-devel@lists.xensource.com
>> Subject: Re: [PATCH] Allocate vmcs pages when system booting
>> 
>> On 12/11/2009 10:52, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
>> 
>>> Currently the VMCS page is allocated when the CPU is brought-up and
>>> identified, and then spin_debug_enable() is called.
>>> 
>>> This does not work for cpu hot-add. When hot-added CPU is
brought-up and try
>>> to allocate the vmcs page, it will hit check_lock, because irq is
disabled
>>> still.
>>> 
>>> This patch allocate the vmcs pages for all possible pages when
system
>>> booting,
>>> so that we don''t allocate vmcs anymore when secondary CPU
is up.
>>> 
>>> Signed-off-by: Jiang, Yunhong <yunhong.jiang@intel.com>
>> 
>> A general point: using cpu_possible_map is not a good idea any longer,
since
>> it is going to be all-ones as soon as your physical cpu hotplug patches
go
>> in (I don''t intend to make that a build-time option). Hence we
could
>> potentially massively over-allocate pages with this approach.
>> 
>> The good news is that, when bringing a CPU online, we don''t
need to worry
>> about acquiring IRQ-unsafe locks with IRQS disabled until the CPU is
added
>> to the cpu_online_map. This is because the race we are trying to avoid
with
>> the spin-debug checks involves rendezvous of CPUs via IPIs, and a CPU
not in
>> cpu_online_map will never get waited on by others.
>> 
>> So, your better fix would be to spin_debug_disable() at the top of
>> start_secondary(), and spin_debug_enable() just before
>> cpu_set(...online_map).
>> 
>> Can you try this alternative fix please?
>> 
>> -- Keir
>> 
> 


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

On 15/01/2010 09:30, "Jan Beulich" <JBeulich@novell.com> wrote:
>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com>
15.01.10 10:06 >>>
>> b) Can we still turn to the original patch, i.e. pre-allocate all VMCS
pages
>> for all possible CPU?
> 
> I''m generally opposed to pre-allocations, at least as long as all
CPUs are
> considered ''possible''. Building with a relatively high
nr_cpus= setting
> and then running on a relatively small system results in (close to)
> unacceptable overhead.
> 
> In fact it''s really not clear to me why cpu_possible_map must be
set to
> CPU_MASK_ALL - if Linux has ways to avoid this, Xen should have too.
Does Linux manage a good reliable job of cutting down cpu_possible_map? That
would save cpu_possible_map in my eyes, if we could do that. Otherwise it is
indeed pretty useless now. Either way, I''d like cpu hotplug notifier
chains
in the 4.1 development window.

 -- Keir



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

In Linux side, it required some changes on ACPI. See information from
Documentation/x86/x86_64/cpu-hotplug-spec:

Linux/x86-64 supports CPU hotplug now. For various reasons Linux wants to
know in advance of boot time the maximum number of CPUs that could be plugged
into the system. ACPI 3.0 currently has no official way to supply
this information from the firmware to the operating system.

But this requirement is not added to ACPI 4.0, so when working on CPU hotplug, I
didn''t try to use the same method.

After reading the code again, I think maybe I missed one possible place to
achieve the same purpose. We can simply allocate the VMCS pag from cpu_hotadd in
XENPF_cpu_hotadd, where the CPU changed from possible to present. Later, when we
bring-up the CPU, we don''t need allocate VMCS page anymore. That is, we
will allocate VMCS pages still, but only for present cpus, not possible CPUs.
When CPU is changed from logical online to offline, this VMCS page will not be
freed.

Do you agree this method?

Thanks
Yunhong Jiang
>-----Original Message-----
>From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
>Sent: Friday, January 15, 2010 6:32 PM
>To: Jan Beulich; Jiang, Yunhong
>Cc: xen-devel@lists.xensource.com
>Subject: Re: [Xen-devel] RE: [PATCH] Allocate vmcs pages when system booting
>
>On 15/01/2010 09:30, "Jan Beulich" <JBeulich@novell.com>
wrote:
>
>>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com>
15.01.10 10:06 >>>
>>> b) Can we still turn to the original patch, i.e. pre-allocate all
VMCS pages
>>> for all possible CPU?
>>
>> I''m generally opposed to pre-allocations, at least as long as
all CPUs are
>> considered ''possible''. Building with a relatively
high nr_cpus= setting
>> and then running on a relatively small system results in (close to)
>> unacceptable overhead.
>>
>> In fact it''s really not clear to me why cpu_possible_map must
be set to
>> CPU_MASK_ALL - if Linux has ways to avoid this, Xen should have too.
>
>Does Linux manage a good reliable job of cutting down cpu_possible_map? That
>would save cpu_possible_map in my eyes, if we could do that. Otherwise it is
>indeed pretty useless now. Either way, I''d like cpu hotplug
notifier chains
>in the 4.1 development window.
>
> -- Keir
>

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

On 17/01/2010 02:02, "Jiang, Yunhong" <yunhong.jiang@intel.com>
wrote:
> After reading the code again, I think maybe I missed one possible place to
> achieve the same purpose. We can simply allocate the VMCS pag from
cpu_hotadd
> in XENPF_cpu_hotadd, where the CPU changed from possible to present. Later,
> when we bring-up the CPU, we don''t need allocate VMCS page
anymore. That is,
> we will allocate VMCS pages still, but only for present cpus, not possible
> CPUs. When CPU is changed from logical online to offline, this VMCS page
will
> not be freed.
> 
> Do you agree this method?
I will come up with a patch tomorrow for you to try.

 -- Keir



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

On 17/01/2010 21:35, "Keir Fraser" <keir.fraser@eu.citrix.com>
wrote:
> On 17/01/2010 02:02, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
> 
>> Do you agree this method?
> 
> I will come up with a patch tomorrow for you to try.
Does the attached patch work okay for you?

 -- Keir



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

Really thanks and I will have a try.

Thanks
--jyh
>-----Original Message-----
>From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
>Sent: Monday, January 18, 2010 4:12 PM
>To: Jiang, Yunhong; Jan Beulich
>Cc: xen-devel@lists.xensource.com
>Subject: Re: [Xen-devel] RE: [PATCH] Allocate vmcs pages when system booting
>
>On 17/01/2010 21:35, "Keir Fraser"
<keir.fraser@eu.citrix.com> wrote:
>
>> On 17/01/2010 02:02, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
>>
>>> Do you agree this method?
>>
>> I will come up with a patch tomorrow for you to try.
>
>Does the attached patch work okay for you?
>
> -- Keir

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

>>> Keir Fraser <keir.fraser@eu.citrix.com> 15.01.10 11:31
>>>
>On 15/01/2010 09:30, "Jan Beulich" <JBeulich@novell.com>
wrote:
>
>>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com>
15.01.10 10:06 >>>
>>> b) Can we still turn to the original patch, i.e. pre-allocate all
VMCS pages
>>> for all possible CPU?
>> 
>> I''m generally opposed to pre-allocations, at least as long as
all CPUs are
>> considered ''possible''. Building with a relatively
high nr_cpus= setting
>> and then running on a relatively small system results in (close to)
>> unacceptable overhead.
>> 
>> In fact it''s really not clear to me why cpu_possible_map must
be set to
>> CPU_MASK_ALL - if Linux has ways to avoid this, Xen should have too.
>
>Does Linux manage a good reliable job of cutting down cpu_possible_map? That
>would save cpu_possible_map in my eyes, if we could do that. Otherwise it is
>indeed pretty useless now. Either way, I''d like cpu hotplug
notifier chains
>in the 4.1 development window.
Only now got to look into this: Linux simply counts the disabled CPUs
along with the present ones, plus it allows a command line override to
the number of CPU slots reserved for hotplug. I just put together
something similar, partly copying code over from there. Will submit
post-4.0.

Jan


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

On 18/3/10 10:45, "Jan Beulich" <JBeulich@novell.com> wrote:
>>> In fact it''s really not clear to me why cpu_possible_map
must be set to
>>> CPU_MASK_ALL - if Linux has ways to avoid this, Xen should have
too.
>> 
>> Does Linux manage a good reliable job of cutting down cpu_possible_map?
That
>> would save cpu_possible_map in my eyes, if we could do that. Otherwise
it is
>> indeed pretty useless now. Either way, I''d like cpu hotplug
notifier chains
>> in the 4.1 development window.
> 
> Only now got to look into this: Linux simply counts the disabled CPUs
> along with the present ones, plus it allows a command line override to
> the number of CPU slots reserved for hotplug. I just put together
> something similar, partly copying code over from there. Will submit
> post-4.0.
Intel are going to submit patches for cpu hotplug notifiers post-4.0, which
is the best solution. I think we can get rid of all possible_map uses in the
end.

 -- Keir



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

>-----Original Message-----
>From: Jan Beulich [mailto:JBeulich@novell.com]
>Sent: Thursday, March 18, 2010 6:45 PM
>To: Keir Fraser; Jiang, Yunhong
>Cc: xen-devel@lists.xensource.com
>Subject: Re: [Xen-devel] RE: [PATCH] Allocate vmcs pages when system booting
>
>>>> Keir Fraser <keir.fraser@eu.citrix.com> 15.01.10 11:31
>>>
>>On 15/01/2010 09:30, "Jan Beulich" <JBeulich@novell.com>
wrote:
>>
>>>>>> "Jiang, Yunhong"
<yunhong.jiang@intel.com> 15.01.10 10:06 >>>
>>>> b) Can we still turn to the original patch, i.e. pre-allocate
all VMCS pages
>>>> for all possible CPU?
>>>
>>> I''m generally opposed to pre-allocations, at least as long
as all CPUs are
>>> considered ''possible''. Building with a relatively
high nr_cpus= setting
>>> and then running on a relatively small system results in (close to)
>>> unacceptable overhead.
>>>
>>> In fact it''s really not clear to me why cpu_possible_map
must be set to
>>> CPU_MASK_ALL - if Linux has ways to avoid this, Xen should have
too.
>>
>>Does Linux manage a good reliable job of cutting down cpu_possible_map?
That
>>would save cpu_possible_map in my eyes, if we could do that. Otherwise
it is
>>indeed pretty useless now. Either way, I''d like cpu hotplug
notifier chains
>>in the 4.1 development window.
>
>Only now got to look into this: Linux simply counts the disabled CPUs
>along with the present ones, plus it allows a command line override to
>the number of CPU slots reserved for hotplug. I just put together
>something similar, partly copying code over from there. Will submit
>post-4.0.
>
>Jan
Jan, thanks for your look on this. Yes, that''s explained in
kernel''s
"linux-2.6/Documentation/x86/x86_64/cpu-hotplug-spec" . I
didn''t take the assumption that MADT wil always list hot-plugable CPUs.
The reason I take this is because
a) at that time, I thought linux is not the only possible dom0, other OS like
Solaris or FreeBSD can work as dom0 also in theory, so we''d better work
as the spec''s statement, not according to kernel''s
implementation, after all, xen is HYPERVISOR :-)
b) This statement talks about ACPI 3.0, and AFAIK, the ACPI 4.0 didn''t
change for this requirement still.
c) I didn''t realize this VMCS page issue when I working on the patch :$

Of course, I''m not strong against the kernel''s method, and
your patch is ok for me.

Below are kernel''s doc:
"Linux/x86-64 supports CPU hotplug now. For various reasons Linux wants to
know in advance of boot time the maximum number of CPUs that could be plugged
into the system. ACPI 3.0 currently has no official way to supply
this information from the firmware to the operating system.
......
For CPU hotplug Linux/x86-64 expects now that any possible future hotpluggable
CPU is already available in the MADT. If the CPU is not available yet
it should have its LAPIC Enabled bit set to 0. Linux will use the number
of disabled LAPICs to compute the maximum number of future CPUs.

.. 
In the worst case the user can overwrite this choice using a command line
option (additional_cpus=...), but it is recommended to supply the correct
number (or a reasonable approximation of it, with erring towards more not less)
in the MADT to avoid manual configuration."

--jyh


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