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

["Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>]

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>
>


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