RE: [Xen-devel] RE: [PATCH 1/2] Vcpu hotplug: Move ACPI processor from \_PR to \_SB

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

Keir Fraser wrote:
> On 12/02/2010 10:07, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> 
>>> Win2k8 has a similar problem?
>>> 
>>>  -- Keir
>> 
>> Yes. Yunhong test Win2k8, and found in Win2k8 data center version,
>> cpu hotplug successed for _SB definition, but failed for _PR
>> definition. (success mean, after we hot-add vcpu, the device manager
>> will shown new processor, but the task manager has no changes, same
>> as
>> http://www.boche.net/blog/index.php/2009/05/10/vsphere-memory-hot-add-cpu-hot-
>> plug/ .Fail mean even the device manager has no changes). 
> 
> Okay, how about the attached alternative patch then? Smaller than
> yours but hoepfully equivalent. It doesn't include your change to the
> _STA method to return 0 instead of 9 in the offline case however --
> there was no explanation for that in the changeset comment?
> 
>  -- Keir

It's fine to me, except it need change offline flag from 0x09 to 0x00.
Yunhong gave a detailed explanation for the reason, see attached.
We have tested the flag value under some hvm guest:
Define cpu by \_SB + 0X00 flag: linux 2.6.30 OK, linux 2.6.32 OK, Win2K8 OK;
Define cpu by \_PR + 0x00 flag: linux 2.6.30 fail, linux 2.6.32 fail, Win2K8 
fail;
Define cpu by \_PR + 0x09 flag: linux 2.6.30 OK (just lucky), linux 2.6.32 
fail, Win2K8 fail;

We didn't test \_SB + 0x09 flag, but per my checking kernel code, .30 will 
lucky wrok but .32 will fail.

Thanks,
Jinsong

--- Begin Message ---

• To: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
• From: "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
• Date: Fri, 12 Feb 2010 10:57:18 +0800
• Accept-language: en-US
• Acceptlanguage: en-US
• Thread-index: AcqrQRdRK42tFnx6Rbij20ZBPcBQRQACAINSAA4+mEA=
• Thread-topic: [PATCH 1/2] Vcpu hotplug: Move ACPI processor from \_PR to \_SB

Per my checking to the code, the Linux has bug to support cpu hotplug for \_PR 
defined processor object.

In kernel, the _PR is defined as ACPI_TYPE_LOCAL_SCOPE type (see 
acpi_gbl_pre_defined_names ) and is skipped without creating corresponding 
device in acpi_bus_scan (checking at acpi_bus_check_add), while _SB is defined 
as ACPI_TYPE_DEVICE.

When DEVICE_CHECK notify received for a processor object, 
acpi_processor_device_add() will be called. When acpi_processor_device_add try 
to get parent device, it will fail for \_PR situation. _SB is ok because it is 
defined as ACPI_TYPE_DEVICE. 

static
int acpi_processor_device_add(acpi_handle handle, struct acpi_device **device)
{ 
.......
        acpi_bus_get_device(phandle, &pdev)) {
        return -ENODEV;
    }

This issue is workaround by lucky in current Xen code for 2.6.30 kernel, mainly 
because of the non-present Processor's status is defined as 0x9 (present, 
functional, not enabled) in Processor object's _STA function. In 2.6.30 kernel, 
this means a device is created by acpi module when kernel booting. Later, when 
the processor hot added, acpi_processor_hotplug_notify() will not call the 
acpi_processor_device_add(), and call the acpi_processor_start() directly to 
start the processor.
However, in 2.6.32, changeset 970b04929a68134acca17878b1d93e115e58c12a remove 
the acpi_processor_start(), remove the acpi_processor_start() at that point, 
assume the acpi_processor_device_add() will call the acpi_processor_start() in 
the end. That means in .32 kernel, s the CPU is not started when hot-added. 
That means the workaround of 0x9 _STA is not working here.

In fact, marked a non-present CPU as 0x9 is really strange. We should of course 
return 0x0 for non-present processor. However, after change the _STA to return 
0x0, it will fail for even 30 kernel for _PR implementation, because then the 
processor device is not crated by acpi module when kernel booting, and the 
acpi_processor_device_add() can't get the parent device, the new processor 
device will not be added when hot-plug.

So in summary, kernel has hotplug bug for Processor defined under _PR, which is 
workaround in xen by lucky for .30 kernel, but failed in .32 kernel.

I also noticed in Win2k8 data center version, cpu hotplug successed for _SB 
definition, but failed for _PR definition. (success mean, after we hot-add 
vcpu, the device manager will shown new processor, but the task manager has no 
changes, same as 
http://www.boche.net/blog/index.php/2009/05/10/vsphere-memory-hot-add-cpu-hot-plug/
 .Fail mean even the device manager has no changes). But I didn't debug to the 
w2k8 issue.

However, according to ACPI spec, the _PR is for ACPI1.0 compatible. We have no 
idea which OS is ACPI 1.0 OS. As HeQing found ACPI 1.0 bugs in Win2K, so we 
assume W2K is ACPI 1.0. We test shows W2K guest is ok with the _SB definition 
in our testing. Maybe Win98/WinMe is ACPI 1.0, but we have no image for these 
OS. But yes, that's a main issue for _SB method and we need more consideration 
here.

In fact, we have internal argue to choose _PR or _SB method before Jinsong's 
initial patch sent out. Later _PR method is chosen because of the ACPI 1.0 
compatible benifit, and kernel 30 version is ok. (IIRC, .32 kernel is not 
released at that time). 

--jyh

>-----Original Message-----
>From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx]
>Sent: Friday, February 12, 2010 2:36 AM
>To: Liu, Jinsong; xen-devel
>Cc: Jiang, Yunhong
>Subject: Re: [PATCH 1/2] Vcpu hotplug: Move ACPI processor from \_PR to \_SB
>
>On 11/02/2010 17:38, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
>
>> Vcpu hotplug: Move ACPI processor from \_PR to \_SB
>>
>> Move processor from \_PR to \_SB. ACPI processor can be defined under
>> \_PR or \_SB. However, recently os like linux 2.6.30/32 support cpu hotplug
>> better for \_SB processor object.
>
>What's 'better' about it? I don't want to mess with that kind of thing right
>now unless there's a good reason. In my experience the majority of systems
>still define a \_PR block -- is Linux having some kind of problem with all
>of them?
>
> -- Keir
>
>> Signed-off-by: Jiang, Yunhong   <yunhong.jiang@xxxxxxxxx>
>>                      Liu, Jinsong     <jinsong.liu@xxxxxxxxx>
>

--- End Message ---

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